Merge #11309: Minor cleanups for AcceptToMemoryPool
bf64c3cb3
Ignore transactions added to mempool during a reorg for fee estimation purposes. (Alex Morcos)04f78ab5b
Do not reject based on mempool min fee when bypass_limits is set. (Alex Morcos)fd849e1b0
Change AcceptToMemoryPool function signature (Alex Morcos) Pull request description: First commit just removes default arguments from `AcceptToMemoryPool` and consolidates two arguments, it does not change behavior. Second commit finally fixes the fact that we're not meant to reject based on mempool min fee when adding a transaction from a disconnected block during a reorg as mentioned [here](https://github.com/bitcoin/bitcoin/pull/9602#issue-202197849) Third commit makes fee estimation ignore transactions added from a disconnected block during a reorg. I think this was another source of fee estimates returning estimates below 1000 sat/kB as in #11303. Tree-SHA512: 30925ca8b341915bb214f1d2590b36b7931f2e125b7660150e38ae70338f00db5aa7f1608546dddb181446924177eb7cf62ea8bd2583068acc074d6c3f86bc0c
This commit is contained in:
commit
93d20a734d
6 changed files with 34 additions and 27 deletions
|
@ -1788,7 +1788,8 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
|||
|
||||
std::list<CTransactionRef> lRemovedTxn;
|
||||
|
||||
if (!AlreadyHave(inv) && AcceptToMemoryPool(mempool, state, ptx, true, &fMissingInputs, &lRemovedTxn)) {
|
||||
if (!AlreadyHave(inv) &&
|
||||
AcceptToMemoryPool(mempool, state, ptx, &fMissingInputs, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
|
||||
mempool.check(pcoinsTip);
|
||||
RelayTransaction(tx, connman);
|
||||
for (unsigned int i = 0; i < tx.vout.size(); i++) {
|
||||
|
@ -1826,7 +1827,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr
|
|||
|
||||
if (setMisbehaving.count(fromPeer))
|
||||
continue;
|
||||
if (AcceptToMemoryPool(mempool, stateDummy, porphanTx, true, &fMissingInputs2, &lRemovedTxn)) {
|
||||
if (AcceptToMemoryPool(mempool, stateDummy, porphanTx, &fMissingInputs2, &lRemovedTxn, false /* bypass_limits */, 0 /* nAbsurdFee */)) {
|
||||
LogPrint(BCLog::MEMPOOL, " accepted orphan tx %s\n", orphanHash.ToString());
|
||||
RelayTransaction(orphanTx, connman);
|
||||
for (unsigned int i = 0; i < orphanTx.vout.size(); i++) {
|
||||
|
|
|
@ -942,8 +942,8 @@ UniValue sendrawtransaction(const JSONRPCRequest& request)
|
|||
// push to local node and sync with wallets
|
||||
CValidationState state;
|
||||
bool fMissingInputs;
|
||||
bool fLimitFree = true;
|
||||
if (!AcceptToMemoryPool(mempool, state, std::move(tx), fLimitFree, &fMissingInputs, nullptr, false, nMaxRawTxFee)) {
|
||||
if (!AcceptToMemoryPool(mempool, state, std::move(tx), &fMissingInputs,
|
||||
nullptr /* plTxnReplaced */, false /* bypass_limits */, nMaxRawTxFee)) {
|
||||
if (state.IsInvalid()) {
|
||||
throw JSONRPCError(RPC_TRANSACTION_REJECTED, strprintf("%i: %s", state.GetRejectCode(), state.GetRejectReason()));
|
||||
} else {
|
||||
|
|
|
@ -29,7 +29,8 @@ ToMemPool(CMutableTransaction& tx)
|
|||
LOCK(cs_main);
|
||||
|
||||
CValidationState state;
|
||||
return AcceptToMemoryPool(mempool, state, MakeTransactionRef(tx), false, nullptr, nullptr, true, 0);
|
||||
return AcceptToMemoryPool(mempool, state, MakeTransactionRef(tx), nullptr /* pfMissingInputs */,
|
||||
nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */);
|
||||
}
|
||||
|
||||
BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup)
|
||||
|
|
|
@ -384,7 +384,9 @@ void UpdateMempoolForReorg(DisconnectedBlockTransactions &disconnectpool, bool f
|
|||
while (it != disconnectpool.queuedTx.get<insertion_order>().rend()) {
|
||||
// ignore validation errors in resurrected transactions
|
||||
CValidationState stateDummy;
|
||||
if (!fAddToMempool || (*it)->IsCoinBase() || !AcceptToMemoryPool(mempool, stateDummy, *it, false, nullptr, nullptr, true)) {
|
||||
if (!fAddToMempool || (*it)->IsCoinBase() ||
|
||||
!AcceptToMemoryPool(mempool, stateDummy, *it, nullptr /* pfMissingInputs */,
|
||||
nullptr /* plTxnReplaced */, true /* bypass_limits */, 0 /* nAbsurdFee */)) {
|
||||
// If the transaction doesn't make it in to the mempool, remove any
|
||||
// transactions that depend on it (which would now be orphans).
|
||||
mempool.removeRecursive(**it, MemPoolRemovalReason::REORG);
|
||||
|
@ -443,9 +445,9 @@ static bool CheckInputsFromMempoolAndCache(const CTransaction& tx, CValidationSt
|
|||
return CheckInputs(tx, state, view, true, flags, cacheSigStore, true, txdata);
|
||||
}
|
||||
|
||||
static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx, bool fLimitFree,
|
||||
static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool& pool, CValidationState& state, const CTransactionRef& ptx,
|
||||
bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced,
|
||||
bool fOverrideMempoolLimit, const CAmount& nAbsurdFee, std::vector<COutPoint>& coins_to_uncache)
|
||||
bool bypass_limits, const CAmount& nAbsurdFee, std::vector<COutPoint>& coins_to_uncache)
|
||||
{
|
||||
const CTransaction& tx = *ptx;
|
||||
const uint256 hash = tx.GetHash();
|
||||
|
@ -618,12 +620,12 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
|
|||
strprintf("%d", nSigOpsCost));
|
||||
|
||||
CAmount mempoolRejectFee = pool.GetMinFee(gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000).GetFee(nSize);
|
||||
if (mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) {
|
||||
if (!bypass_limits && mempoolRejectFee > 0 && nModifiedFees < mempoolRejectFee) {
|
||||
return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool min fee not met", false, strprintf("%d < %d", nFees, mempoolRejectFee));
|
||||
}
|
||||
|
||||
// No transactions are allowed below minRelayTxFee except from disconnected blocks
|
||||
if (fLimitFree && nModifiedFees < ::minRelayTxFee.GetFee(nSize)) {
|
||||
if (!bypass_limits && nModifiedFees < ::minRelayTxFee.GetFee(nSize)) {
|
||||
return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "min relay fee not met");
|
||||
}
|
||||
|
||||
|
@ -855,17 +857,18 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
|
|||
}
|
||||
pool.RemoveStaged(allConflicting, false, MemPoolRemovalReason::REPLACED);
|
||||
|
||||
// This transaction should only count for fee estimation if it isn't a
|
||||
// BIP 125 replacement transaction (may not be widely supported), the
|
||||
// node is not behind, and the transaction is not dependent on any other
|
||||
// transactions in the mempool.
|
||||
bool validForFeeEstimation = !fReplacementTransaction && IsCurrentForFeeEstimation() && pool.HasNoInputsOf(tx);
|
||||
// This transaction should only count for fee estimation if:
|
||||
// - it isn't a BIP 125 replacement transaction (may not be widely supported)
|
||||
// - it's not being readded during a reorg which bypasses typical mempool fee limits
|
||||
// - the node is not behind
|
||||
// - the transaction is not dependent on any other transactions in the mempool
|
||||
bool validForFeeEstimation = !fReplacementTransaction && !bypass_limits && IsCurrentForFeeEstimation() && pool.HasNoInputsOf(tx);
|
||||
|
||||
// Store transaction in memory
|
||||
pool.addUnchecked(hash, entry, setAncestors, validForFeeEstimation);
|
||||
|
||||
// trim mempool and check if tx was trimmed
|
||||
if (!fOverrideMempoolLimit) {
|
||||
if (!bypass_limits) {
|
||||
LimitMempoolSize(pool, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000, gArgs.GetArg("-mempoolexpiry", DEFAULT_MEMPOOL_EXPIRY) * 60 * 60);
|
||||
if (!pool.exists(hash))
|
||||
return state.DoS(0, false, REJECT_INSUFFICIENTFEE, "mempool full");
|
||||
|
@ -878,12 +881,12 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
|
|||
}
|
||||
|
||||
/** (try to) add transaction to memory pool with a specified acceptance time **/
|
||||
static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool fLimitFree,
|
||||
static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx,
|
||||
bool* pfMissingInputs, int64_t nAcceptTime, std::list<CTransactionRef>* plTxnReplaced,
|
||||
bool fOverrideMempoolLimit, const CAmount nAbsurdFee)
|
||||
bool bypass_limits, const CAmount nAbsurdFee)
|
||||
{
|
||||
std::vector<COutPoint> coins_to_uncache;
|
||||
bool res = AcceptToMemoryPoolWorker(chainparams, pool, state, tx, fLimitFree, pfMissingInputs, nAcceptTime, plTxnReplaced, fOverrideMempoolLimit, nAbsurdFee, coins_to_uncache);
|
||||
bool res = AcceptToMemoryPoolWorker(chainparams, pool, state, tx, pfMissingInputs, nAcceptTime, plTxnReplaced, bypass_limits, nAbsurdFee, coins_to_uncache);
|
||||
if (!res) {
|
||||
for (const COutPoint& hashTx : coins_to_uncache)
|
||||
pcoinsTip->Uncache(hashTx);
|
||||
|
@ -894,12 +897,12 @@ static bool AcceptToMemoryPoolWithTime(const CChainParams& chainparams, CTxMemPo
|
|||
return res;
|
||||
}
|
||||
|
||||
bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool fLimitFree,
|
||||
bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx,
|
||||
bool* pfMissingInputs, std::list<CTransactionRef>* plTxnReplaced,
|
||||
bool fOverrideMempoolLimit, const CAmount nAbsurdFee)
|
||||
bool bypass_limits, const CAmount nAbsurdFee)
|
||||
{
|
||||
const CChainParams& chainparams = Params();
|
||||
return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, fLimitFree, pfMissingInputs, GetTime(), plTxnReplaced, fOverrideMempoolLimit, nAbsurdFee);
|
||||
return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, pfMissingInputs, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee);
|
||||
}
|
||||
|
||||
/** Return transaction in txOut, and if it was found inside a block, its hash is placed in hashBlock */
|
||||
|
@ -4306,7 +4309,8 @@ bool LoadMempool(void)
|
|||
CValidationState state;
|
||||
if (nTime + nExpiryTimeout > nNow) {
|
||||
LOCK(cs_main);
|
||||
AcceptToMemoryPoolWithTime(chainparams, mempool, state, tx, true, nullptr, nTime, nullptr, false, 0);
|
||||
AcceptToMemoryPoolWithTime(chainparams, mempool, state, tx, nullptr /* pfMissingInputs */, nTime,
|
||||
nullptr /* plTxnReplaced */, false /* bypass_limits */, 0 /* nAbsurdFee */);
|
||||
if (state.IsValid()) {
|
||||
++count;
|
||||
} else {
|
||||
|
|
|
@ -301,9 +301,9 @@ void PruneBlockFilesManual(int nManualPruneHeight);
|
|||
|
||||
/** (try to) add transaction to memory pool
|
||||
* plTxnReplaced will be appended to with all transactions replaced from mempool **/
|
||||
bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx, bool fLimitFree,
|
||||
bool* pfMissingInputs, std::list<CTransactionRef>* plTxnReplaced = nullptr,
|
||||
bool fOverrideMempoolLimit=false, const CAmount nAbsurdFee=0);
|
||||
bool AcceptToMemoryPool(CTxMemPool& pool, CValidationState &state, const CTransactionRef &tx,
|
||||
bool* pfMissingInputs, std::list<CTransactionRef>* plTxnReplaced,
|
||||
bool bypass_limits, const CAmount nAbsurdFee);
|
||||
|
||||
/** Convert CValidationState to a human-readable message for logging */
|
||||
std::string FormatStateMessage(const CValidationState &state);
|
||||
|
|
|
@ -4047,5 +4047,6 @@ int CMerkleTx::GetBlocksToMaturity() const
|
|||
|
||||
bool CMerkleTx::AcceptToMemoryPool(const CAmount& nAbsurdFee, CValidationState& state)
|
||||
{
|
||||
return ::AcceptToMemoryPool(mempool, state, tx, true, nullptr, nullptr, false, nAbsurdFee);
|
||||
return ::AcceptToMemoryPool(mempool, state, tx, nullptr /* pfMissingInputs */,
|
||||
nullptr /* plTxnReplaced */, false /* bypass_limits */, nAbsurdFee);
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue