From 36a8441912bf84b4da9c74826dcd42533d8abaaa Mon Sep 17 00:00:00 2001 From: glozow Date: Thu, 2 Sep 2021 13:09:11 +0100 Subject: [PATCH] [validation/rpc] cache + use vsize calculated in PreChecks This is not only cleaner but also helps make sure we are always using the virtual size measure that includes the sigop weight heuristic (which is the vsize the mempool would return). --- src/rpc/rawtransaction.cpp | 2 +- src/validation.cpp | 21 +++++++++++++-------- src/validation.h | 10 ++++++---- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index 483717aa7a6..fd18d4c96d7 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -977,7 +977,7 @@ static RPCHelpMan testmempoolaccept() if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) { const CAmount fee = tx_result.m_base_fees.value(); // Check that fee does not exceed maximum fee - const int64_t virtual_size = GetVirtualTransactionSize(*tx); + const int64_t virtual_size = tx_result.m_vsize.value(); const CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size); if (max_raw_tx_fee && fee > max_raw_tx_fee) { result_inner.pushKV("allowed", false); diff --git a/src/validation.cpp b/src/validation.cpp index c32309b2149..266a0ac9e60 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -503,6 +503,9 @@ private: std::unique_ptr m_entry; std::list m_replaced_transactions; + /** Virtual size of the transaction as used by the mempool, calculated using serialized size + * of the transaction and sigops. */ + int64_t m_vsize; CAmount m_base_fees; CAmount m_modified_fees; /** Total modified fees of all transactions being replaced. */ @@ -732,7 +735,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) entry.reset(new CTxMemPoolEntry(ptx, ws.m_base_fees, nAcceptTime, m_active_chainstate.m_chain.Height(), fSpendsCoinbase, nSigOpsCost, lp)); - unsigned int nSize = entry->GetTxSize(); + ws.m_vsize = entry->GetTxSize(); if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-too-many-sigops", @@ -740,7 +743,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // No transactions are allowed below minRelayTxFee except from disconnected // blocks - if (!bypass_limits && !CheckFeeRate(nSize, nModifiedFees, state)) return false; + if (!bypass_limits && !CheckFeeRate(ws.m_vsize, nModifiedFees, state)) return false; const CTxMemPool::setEntries setIterConflicting = m_pool.GetIterSet(setConflicts); // Calculate in-mempool ancestors, up to a limit. @@ -795,7 +798,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) // to be secure by simply only having two immediately-spendable // outputs - one for each counterparty. For more info on the uses for // this, see https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2018-November/016518.html - if (nSize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || + if (ws.m_vsize > EXTRA_DESCENDANT_TX_SIZE_LIMIT || !m_pool.CalculateMemPoolAncestors(*entry, setAncestors, 2, m_limit_ancestor_size, m_limit_descendants + 1, m_limit_descendant_size + EXTRA_DESCENDANT_TX_SIZE_LIMIT, dummy_err_string)) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "too-long-mempool-chain", errString); } @@ -813,7 +816,7 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) m_rbf = !setConflicts.empty(); if (m_rbf) { - CFeeRate newFeeRate(nModifiedFees, nSize); + CFeeRate newFeeRate(nModifiedFees, ws.m_vsize); // It's possible that the replacement pays more fees than its direct conflicts but not more // than all conflicts (i.e. the direct conflicts have high-fee descendants). However, if the // replacement doesn't pay more fees than its direct conflicts, then we can be sure it's not @@ -841,7 +844,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) nConflictingFees += it->GetModifiedFee(); nConflictingSize += it->GetTxSize(); } - if (const auto err_string{PaysForRBF(nConflictingFees, nModifiedFees, nSize, ::incrementalRelayFee, hash)}) { + if (const auto err_string{PaysForRBF(nConflictingFees, nModifiedFees, ws.m_vsize, + ::incrementalRelayFee, hash)}) { return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "insufficient fee", *err_string); } } @@ -967,14 +971,14 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef // Tx was accepted, but not added if (args.m_test_accept) { - return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_base_fees); + return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees); } if (!Finalize(args, ws)) return MempoolAcceptResult::Failure(ws.m_state); GetMainSignals().TransactionAddedToMempool(ptx, m_pool.GetAndIncrementSequence()); - return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_base_fees); + return MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_vsize, ws.m_base_fees); } PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::vector& txns, ATMPArgs& args) @@ -1033,7 +1037,8 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: // When test_accept=true, transactions that pass PolicyScriptChecks are valid because there are // no further mempool checks (passing PolicyScriptChecks implies passing ConsensusScriptChecks). results.emplace(ws.m_ptx->GetWitnessHash(), - MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), ws.m_base_fees)); + MempoolAcceptResult::Success(std::move(ws.m_replaced_transactions), + ws.m_vsize, ws.m_base_fees)); } } diff --git a/src/validation.h b/src/validation.h index 4da8ec8d24b..256981224ac 100644 --- a/src/validation.h +++ b/src/validation.h @@ -158,14 +158,16 @@ struct MempoolAcceptResult { // The following fields are only present when m_result_type = ResultType::VALID /** Mempool transactions replaced by the tx per BIP 125 rules. */ const std::optional> m_replaced_transactions; + /** Virtual size as used by the mempool, calculated using serialized size and sigops. */ + const std::optional m_vsize; /** Raw base fees in satoshis. */ const std::optional m_base_fees; static MempoolAcceptResult Failure(TxValidationState state) { return MempoolAcceptResult(state); } - static MempoolAcceptResult Success(std::list&& replaced_txns, CAmount fees) { - return MempoolAcceptResult(std::move(replaced_txns), fees); + static MempoolAcceptResult Success(std::list&& replaced_txns, int64_t vsize, CAmount fees) { + return MempoolAcceptResult(std::move(replaced_txns), vsize, fees); } // Private constructors. Use static methods MempoolAcceptResult::Success, etc. to construct. @@ -177,9 +179,9 @@ private: } /** Constructor for success case */ - explicit MempoolAcceptResult(std::list&& replaced_txns, CAmount fees) + explicit MempoolAcceptResult(std::list&& replaced_txns, int64_t vsize, CAmount fees) : m_result_type(ResultType::VALID), - m_replaced_transactions(std::move(replaced_txns)), m_base_fees(fees) {} + m_replaced_transactions(std::move(replaced_txns)), m_vsize{vsize}, m_base_fees(fees) {} }; /**