diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index fd18d4c96d7..12d59690a3a 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -974,6 +974,8 @@ static RPCHelpMan testmempoolaccept() continue; } const auto& tx_result = it->second; + // Package testmempoolaccept doesn't allow transactions to already be in the mempool. + CHECK_NONFATAL(tx_result.m_result_type != MempoolAcceptResult::ResultType::MEMPOOL_ENTRY); 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 diff --git a/src/validation.cpp b/src/validation.cpp index 692c18983fe..dbcb99a6aa3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -916,6 +916,10 @@ bool MemPoolAccept::PackageMempoolChecks(const std::vector& txn AssertLockHeld(cs_main); AssertLockHeld(m_pool.cs); + // CheckPackageLimits expects the package transactions to not already be in the mempool. + assert(std::all_of(txns.cbegin(), txns.cend(), [this](const auto& tx) + { return !m_pool.exists(GenTxid::Txid(tx->GetHash()));})); + std::string err_string; if (!m_pool.CheckPackageLimits(txns, m_limit_ancestors, m_limit_ancestor_size, m_limit_descendants, m_limit_descendant_size, err_string)) { @@ -1238,7 +1242,49 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptPackage(const Package& package, m_view.SetBackend(m_dummy); LOCK(m_pool.cs); - return AcceptMultipleTransactions(package, args); + std::map results; + // As node operators are free to set their mempool policies however they please, it's possible + // for package transaction(s) to already be in the mempool, and we don't want to reject the + // entire package in that case (as that could be a censorship vector). Filter the transactions + // that are already in mempool and add their information to results, since we already have them. + std::vector txns_new; + for (const auto& tx : package) { + const auto& wtxid = tx->GetWitnessHash(); + const auto& txid = tx->GetHash(); + // There are 3 possibilities: already in mempool, same-txid-diff-wtxid already in mempool, + // or not in mempool. An already confirmed tx is treated as one not in mempool, because all + // we know is that the inputs aren't available. + if (m_pool.exists(GenTxid::Wtxid(wtxid))) { + // Exact transaction already exists in the mempool. + auto iter = m_pool.GetIter(wtxid); + assert(iter != std::nullopt); + results.emplace(wtxid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee())); + } else if (m_pool.exists(GenTxid::Txid(txid))) { + // Transaction with the same non-witness data but different witness (same txid, + // different wtxid) already exists in the mempool. + // + // We don't allow replacement transactions right now, so just swap the package + // transaction for the mempool one. Note that we are ignoring the validity of the + // package transaction passed in. + // TODO: allow witness replacement in packages. + auto iter = m_pool.GetIter(wtxid); + assert(iter != std::nullopt); + results.emplace(txid, MempoolAcceptResult::MempoolTx(iter.value()->GetTxSize(), iter.value()->GetFee())); + } else { + // Transaction does not already exist in the mempool. + txns_new.push_back(tx); + } + } + + // Nothing to do if the entire package has already been submitted. + if (txns_new.empty()) return PackageMempoolAcceptResult(package_state, std::move(results)); + // Validate the (deduplicated) transactions as a package. + auto submission_result = AcceptMultipleTransactions(txns_new, args); + // Include already-in-mempool transaction results in the final result. + for (const auto& [wtxid, mempoolaccept_res] : results) { + submission_result.m_tx_results.emplace(wtxid, mempoolaccept_res); + } + return submission_result; } } // anon namespace diff --git a/src/validation.h b/src/validation.h index e3ad5f2e9f7..e53ef53b071 100644 --- a/src/validation.h +++ b/src/validation.h @@ -161,17 +161,19 @@ struct MempoolAcceptResult { enum class ResultType { VALID, //!> Fully validated, valid. INVALID, //!> Invalid. + MEMPOOL_ENTRY, //!> Valid, transaction was already in the mempool. }; const ResultType m_result_type; const TxValidationState m_state; - // The following fields are only present when m_result_type = ResultType::VALID + // The following fields are only present when m_result_type = ResultType::VALID or MEMPOOL_ENTRY /** 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); } @@ -180,6 +182,10 @@ struct MempoolAcceptResult { return MempoolAcceptResult(std::move(replaced_txns), vsize, fees); } + static MempoolAcceptResult MempoolTx(int64_t vsize, CAmount fees) { + return MempoolAcceptResult(vsize, fees); + } + // Private constructors. Use static methods MempoolAcceptResult::Success, etc. to construct. private: /** Constructor for failure case */ @@ -192,6 +198,10 @@ private: explicit MempoolAcceptResult(std::list&& replaced_txns, int64_t vsize, CAmount fees) : m_result_type(ResultType::VALID), m_replaced_transactions(std::move(replaced_txns)), m_vsize{vsize}, m_base_fees(fees) {} + + /** Constructor for already-in-mempool case. It wouldn't replace any transactions. */ + explicit MempoolAcceptResult(int64_t vsize, CAmount fees) + : m_result_type(ResultType::MEMPOOL_ENTRY), m_vsize{vsize}, m_base_fees(fees) {} }; /** @@ -201,7 +211,7 @@ struct PackageMempoolAcceptResult { const PackageValidationState m_state; /** - * Map from wtxid to finished MempoolAcceptResults. The client is responsible + * Map from (w)txid to finished MempoolAcceptResults. The client is responsible * for keeping track of the transaction objects themselves. If a result is not * present, it means validation was unfinished for that transaction. If there * was a package-wide error (see result in m_state), m_tx_results will be empty.