diff --git a/src/bench/mempool_eviction.cpp b/src/bench/mempool_eviction.cpp index 84f873d18394f..40bd56f46d853 100644 --- a/src/bench/mempool_eviction.cpp +++ b/src/bench/mempool_eviction.cpp @@ -28,7 +28,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& nFee, CTxMemPool& po LockPoints lp; pool.addUnchecked(CTxMemPoolEntry( tx, nFee, nTime, nHeight, sequence, - spendsCoinbase, sigOpCost, lp)); + spendsCoinbase, /*dust_indexes=*/{}, sigOpCost, lp)); } // Right now this is only testing eviction performance in an extremely small diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp index 67f689e4eaf4b..210ca9ffa85ab 100644 --- a/src/bench/mempool_stress.cpp +++ b/src/bench/mempool_stress.cpp @@ -28,7 +28,7 @@ static void AddTx(const CTransactionRef& tx, CTxMemPool& pool) EXCLUSIVE_LOCKS_R bool spendsCoinbase = false; unsigned int sigOpCost = 4; LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, 1000, nTime, nHeight, sequence, spendsCoinbase, sigOpCost, lp)); + pool.addUnchecked(CTxMemPoolEntry(tx, 1000, nTime, nHeight, sequence, spendsCoinbase, /*dust_indexes=*/{}, sigOpCost, lp)); } struct Available { diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp index 6e8757bbd5f89..660feb939fb27 100644 --- a/src/bench/rpc_mempool.cpp +++ b/src/bench/rpc_mempool.cpp @@ -21,7 +21,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, pool.cs) { LockPoints lp; - pool.addUnchecked(CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); + pool.addUnchecked(CTxMemPoolEntry(tx, fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, /*spends_coinbase=*/false, /*dust_indexes=*/{}, /*sigops_cost=*/4, lp)); } static void RpcMempool(benchmark::Bench& bench) diff --git a/src/kernel/mempool_entry.h b/src/kernel/mempool_entry.h index 2adeaea652df8..f174de1adf8c0 100644 --- a/src/kernel/mempool_entry.h +++ b/src/kernel/mempool_entry.h @@ -88,6 +88,7 @@ class CTxMemPoolEntry const bool spendsCoinbase; //!< keep track of transactions that spend a coinbase const int64_t sigOpCost; //!< Total sigop cost CAmount m_modified_fee; //!< Used for determining the priority of the transaction for mining in a block + const std::vector m_dust_indexes; //!< Used to track ephemeral dust this transaction may create mutable LockPoints lockPoints; //!< Track the height and time at which tx was final // Information about descendants of this transaction that are in the @@ -109,6 +110,7 @@ class CTxMemPoolEntry CTxMemPoolEntry(const CTransactionRef& tx, CAmount fee, int64_t time, unsigned int entry_height, uint64_t entry_sequence, bool spends_coinbase, + std::vector dust_indexes, int64_t sigops_cost, LockPoints lp) : tx{tx}, nFee{fee}, @@ -120,6 +122,7 @@ class CTxMemPoolEntry spendsCoinbase{spends_coinbase}, sigOpCost{sigops_cost}, m_modified_fee{nFee}, + m_dust_indexes{dust_indexes}, lockPoints{lp}, nSizeWithDescendants{GetTxSize()}, nModFeesWithDescendants{nFee}, @@ -145,6 +148,7 @@ class CTxMemPoolEntry std::chrono::seconds GetTime() const { return std::chrono::seconds{nTime}; } unsigned int GetHeight() const { return entryHeight; } uint64_t GetSequence() const { return entry_sequence; } + std::vector GetDustIndexes() const { return m_dust_indexes; } int64_t GetSigOpCost() const { return sigOpCost; } CAmount GetModifiedFee() const { return m_modified_fee; } size_t DynamicMemoryUsage() const { return nUsageSize; } diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 54b986c926afd..390c28e3e76cd 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -726,7 +726,7 @@ class ChainImpl : public Chain { if (!m_node.mempool) return {}; LockPoints lp; - CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, 0, lp); + CTxMemPoolEntry entry(tx, 0, 0, 0, 0, false, {}, 0, lp); LOCK(m_node.mempool->cs); return m_node.mempool->CheckPackageLimits({tx}, entry.GetTxSize()); } diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index 4bb32f41b69d8..df8089fba8b33 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -5,30 +5,34 @@ #include #include -bool CheckValidEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_fee, CAmount base_fee, CAmount mod_fee, TxValidationState& state) +bool CheckValidEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_fee, CAmount base_fee, CAmount mod_fee, std::vector& dust_indexes, TxValidationState& state) { + dust_indexes.clear(); + for (size_t i = 0; i < tx->vout.size(); ++i) { + const auto& output = tx->vout[i]; + if (IsDust(output, dust_relay_fee)) dust_indexes.push_back(i); + } + // We never want to give incentives to mine this transaction alone if ((base_fee != 0 || mod_fee != 0) && - std::any_of(tx->vout.cbegin(), tx->vout.cend(), [&](const auto& output) { return IsDust(output, dust_relay_fee); })) { + !dust_indexes.empty()) { return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "dust", "tx with dust output must be 0-fee"); } return true; } -std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool) +std::optional CheckEphemeralSpends(const std::vector& package_entries, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool) { - if (!Assume(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;}))) { - // Bail out of spend checks if caller gave us an invalid package - return std::nullopt; + std::map map_txid_ref; + for (const auto& entry : package_entries) { + const auto& tx = entry->GetSharedTx(); + map_txid_ref[tx->GetHash()] = entry; } - std::map map_txid_ref; - for (const auto& tx : package) { - map_txid_ref[tx->GetHash()] = tx; - } + for (const auto& entry : package_entries) { + const auto& tx = entry->GetSharedTx(); - for (const auto& tx : package) { Txid txid = tx->GetHash(); std::unordered_set processed_parent_set; std::unordered_set unspent_parent_dust; @@ -39,21 +43,13 @@ std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_r if (processed_parent_set.contains(parent_txid)) continue; // We look for an in-package or in-mempool dependency - CTransactionRef parent_ref = nullptr; - if (map_txid_ref.contains(parent_txid)) { - parent_ref = map_txid_ref[parent_txid]; - } else { - parent_ref = tx_pool.get(parent_txid); - } + const CTxMemPoolEntry* parent_entry = map_txid_ref.contains(parent_txid) ? map_txid_ref[parent_txid] : tx_pool.GetEntry(parent_txid); - // Check for dust on parents - if (parent_ref) { - for (uint32_t out_index = 0; out_index < parent_ref->vout.size(); out_index++) { - const auto& tx_output = parent_ref->vout[out_index]; - if (IsDust(tx_output, dust_relay_rate)) { - unspent_parent_dust.insert(COutPoint(parent_txid, out_index)); - } - } + // Accumulate dust from parents + if (parent_entry) { + for (const auto& dust_index : parent_entry->GetDustIndexes()) { + unspent_parent_dust.insert(COutPoint(parent_txid, dust_index)); + } } processed_parent_set.insert(parent_txid); diff --git a/src/policy/ephemeral_policy.h b/src/policy/ephemeral_policy.h index a81dde592e479..a8af2571cf85a 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -38,15 +38,16 @@ /** Must be called for each transaction once transaction fees are known. * Does context-less checks about a single transaction. + * dust_indexes is cleared and populated with dust output indexes, if any. * Returns false if the fee is non-zero and dust exists, populating state. True otherwise. */ -bool CheckValidEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_fee, CAmount base_fee, CAmount mod_fee, TxValidationState& state); +bool CheckValidEphemeralTx(const CTransactionRef& tx, CFeeRate dust_relay_fee, CAmount base_fee, CAmount mod_fee, std::vector& dust_indexes, TxValidationState& state); /** Must be called for each transaction(package) if any dust is in the package. * Checks that each transaction's parents have their dust spent by the child, * where parents are either in the mempool or in the package itself. * The function returns std::nullopt if all dust is properly spent, or the txid of the violating child spend. */ -std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool); +std::optional CheckEphemeralSpends(const std::vector& package_entries, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool); #endif // BITCOIN_POLICY_EPHEMERAL_POLICY_H diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index b44e463513b12..52cd0db00ebf8 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -107,108 +107,6 @@ static inline CTransactionRef make_ephemeral_tx(const std::vector& in return MakeTransactionRef(mtx); } -BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) -{ - CTxMemPool& pool = *Assert(m_node.mempool); - LOCK2(cs_main, pool.cs); - TestMemPoolEntryHelper entry; - CTxMemPool::setEntries empty_ancestors; - - CFeeRate minrelay(1000); - - // Basic transaction with dust - auto grandparent_tx_1 = make_ephemeral_tx(random_outpoints(1), /*version=*/2); - const auto dust_txid = grandparent_tx_1->GetHash(); - - uint32_t dust_index = 2; - - // Child transaction spending dust - auto dust_spend = make_tx({COutPoint{dust_txid, dust_index}}, /*version=*/2); - - // We first start with nothing "in the mempool", using package checks - - // Trivial single transaction with no dust - BOOST_CHECK(!CheckEphemeralSpends({dust_spend}, minrelay, pool).has_value()); - - // Now with dust, ok because the tx has no dusty parents - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, minrelay, pool).has_value()); - - // Dust checks pass - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool).has_value()); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, minrelay, pool).has_value()); - - auto dust_non_spend = make_tx({COutPoint{dust_txid, dust_index - 1}}, /*version=*/2); - - // Child spending non-dust only from parent should be disallowed even if dust otherwise spent - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, minrelay, pool).has_value()); - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, minrelay, pool).has_value()); - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, minrelay, pool).has_value()); - - auto grandparent_tx_2 = make_ephemeral_tx(random_outpoints(1), /*version=*/2); - const auto dust_txid_2 = grandparent_tx_2->GetHash(); - - // Spend dust from one but not another is ok, as long as second grandparent has no child - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, minrelay, pool).has_value()); - - auto dust_non_spend_both_parents = make_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index - 1}}, /*version=*/2); - // But if we spend from the parent, it must spend dust - BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, minrelay, pool).has_value()); - - auto dust_spend_both_parents = make_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, minrelay, pool).has_value()); - - // Spending other outputs is also correct, as long as the dusty one is spent - const std::vector all_outpoints{COutPoint(dust_txid, 0), COutPoint(dust_txid, 1), COutPoint(dust_txid, 2), - COutPoint(dust_txid_2, 0), COutPoint(dust_txid_2, 1), COutPoint(dust_txid_2, 2)}; - auto dust_spend_all_outpoints = make_tx(all_outpoints, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, minrelay, pool).has_value()); - - // 2 grandparents with dust <- 1 dust-spending parent with dust <- child with no dust - auto parent_with_dust = make_ephemeral_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index}}, /*version=*/2); - // Ok for parent to have dust - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, minrelay, pool).has_value()); - auto child_no_dust = make_tx({COutPoint{parent_with_dust->GetHash(), dust_index}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, minrelay, pool).has_value()); - - // 2 grandparents with dust <- 1 dust-spending parent with dust <- child with dust - auto child_with_dust = make_ephemeral_tx({COutPoint{parent_with_dust->GetHash(), dust_index}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, minrelay, pool).has_value()); - - // Tests with parents in mempool - - // Empty ancestors means this should never fail since it only checks parents' dust - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, minrelay, pool).has_value()); - - // Add first grandparent to mempool and fetch entry - pool.addUnchecked(entry.FromTx(grandparent_tx_1)); - - // Ignores ancestors that aren't direct parents - BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, minrelay, pool).has_value()); - - // Valid spend of dust with grandparent in mempool - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, minrelay, pool).has_value()); - - // Second grandparent in same package - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, minrelay, pool).has_value()); - // Order in package doesn't matter - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, minrelay, pool).has_value()); - - // Add second grandparent to mempool - pool.addUnchecked(entry.FromTx(grandparent_tx_2)); - - // Only spends single dust out of two direct parents - BOOST_CHECK(CheckEphemeralSpends({dust_non_spend_both_parents}, minrelay, pool).has_value()); - - // Spends both parents' dust - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, minrelay, pool).has_value()); - - // Now add dusty parent to mempool - pool.addUnchecked(entry.FromTx(parent_with_dust)); - - // Passes dust checks even with non-parent ancestors - BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, minrelay, pool).has_value()); -} - BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) { // Test TRUC policy helper functions diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 11bc5e2e8bb96..73283665a2856 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -560,7 +560,7 @@ std::vector TestChain100Setup::PopulateMempool(FastRandomContex LockPoints lp; m_node.mempool->addUnchecked(CTxMemPoolEntry(ptx, /*fee=*/(total_in - num_outputs * amount_per_output), /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, - /*spends_coinbase=*/false, /*sigops_cost=*/4, lp)); + /*spends_coinbase=*/false, /*dust_indexes=*/{}, /*sigops_cost=*/4, lp)); } --num_transactions; } @@ -590,7 +590,7 @@ void TestChain100Setup::MockMempoolMinFee(const CFeeRate& target_feerate) m_node.mempool->m_opts.incremental_relay_feerate.GetFee(GetVirtualTransactionSize(*tx)); m_node.mempool->addUnchecked(CTxMemPoolEntry(tx, /*fee=*/tx_fee, /*time=*/0, /*entry_height=*/1, /*entry_sequence=*/0, - /*spends_coinbase=*/true, /*sigops_cost=*/1, lp)); + /*spends_coinbase=*/true, /*dust_indexes=*/{}, /*sigops_cost=*/1, lp)); m_node.mempool->TrimToSize(0); assert(m_node.mempool->GetMinFee() == target_feerate); } diff --git a/src/test/util/txmempool.cpp b/src/test/util/txmempool.cpp index ee811cfda53b7..9d8d591777b41 100644 --- a/src/test/util/txmempool.cpp +++ b/src/test/util/txmempool.cpp @@ -37,7 +37,7 @@ CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CMutableTransaction& tx) co CTxMemPoolEntry TestMemPoolEntryHelper::FromTx(const CTransactionRef& tx) const { - return CTxMemPoolEntry{tx, nFee, TicksSinceEpoch(time), nHeight, m_sequence, spendsCoinbase, sigOpCost, lp}; + return CTxMemPoolEntry{tx, nFee, TicksSinceEpoch(time), nHeight, m_sequence, spendsCoinbase, m_dust_indexes, sigOpCost, lp}; } std::optional CheckPackageMempoolAcceptResult(const Package& txns, diff --git a/src/test/util/txmempool.h b/src/test/util/txmempool.h index dbbd8e7665a28..77818afc80d9b 100644 --- a/src/test/util/txmempool.h +++ b/src/test/util/txmempool.h @@ -24,6 +24,7 @@ struct TestMemPoolEntryHelper { uint64_t m_sequence{0}; bool spendsCoinbase{false}; unsigned int sigOpCost{4}; + std::vector m_dust_indexes; LockPoints lp; CTxMemPoolEntry FromTx(const CMutableTransaction& tx) const; diff --git a/src/validation.cpp b/src/validation.cpp index 446dc1951c570..9b2abf1b1cf7a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -928,20 +928,21 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws) } } - // Set entry_sequence to 0 when bypass_limits is used; this allows txs from a block - // reorg to be marked earlier than any child txs that were already in the mempool. - const uint64_t entry_sequence = bypass_limits ? 0 : m_pool.GetSequence(); - entry.reset(new CTxMemPoolEntry(ptx, ws.m_base_fees, nAcceptTime, m_active_chainstate.m_chain.Height(), entry_sequence, - fSpendsCoinbase, nSigOpsCost, lock_points.value())); - ws.m_vsize = entry->GetTxSize(); - // Enforces 0-fee for dust transactions, no incentive to be mined alone + std::vector dust_indexes; if (m_pool.m_opts.require_standard) { - if (!CheckValidEphemeralTx(ptx, m_pool.m_opts.dust_relay_feerate, ws.m_base_fees, ws.m_modified_fees, state)) { + if (!CheckValidEphemeralTx(ptx, m_pool.m_opts.dust_relay_feerate, ws.m_base_fees, ws.m_modified_fees, dust_indexes, state)) { return false; // state filled in by CheckValidEphemeralTx } } + // Set entry_sequence to 0 when bypass_limits is used; this allows txs from a block + // reorg to be marked earlier than any child txs that were already in the mempool. + const uint64_t entry_sequence = bypass_limits ? 0 : m_pool.GetSequence(); + entry.reset(new CTxMemPoolEntry(ptx, ws.m_base_fees, nAcceptTime, m_active_chainstate.m_chain.Height(), entry_sequence, + fSpendsCoinbase, dust_indexes, nSigOpsCost, lock_points.value())); + ws.m_vsize = entry->GetTxSize(); + if (nSigOpsCost > MAX_STANDARD_TX_SIGOPS_COST) return state.Invalid(TxValidationResult::TX_NOT_STANDARD, "bad-txns-too-many-sigops", strprintf("%d", nSigOpsCost)); @@ -1463,7 +1464,9 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef } if (m_pool.m_opts.require_standard) { - if (const auto ephemeral_violation{CheckEphemeralSpends(/*package=*/{ptx}, m_pool.m_opts.dust_relay_feerate, m_pool)}) { + std::vector package_entries; + package_entries.push_back(ws.m_entry.get()); + if (const auto ephemeral_violation{CheckEphemeralSpends(package_entries, m_pool.m_opts.dust_relay_feerate, m_pool)}) { const Txid& txid = ephemeral_violation.value(); Assume(txid == ptx->GetHash()); ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends", @@ -1612,7 +1615,10 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: // Now that we've bounded the resulting possible ancestry count, check parents for dust spends if (m_pool.m_opts.require_standard) { - if (const auto ephemeral_violation{CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate, m_pool)}) { + std::vector package_entries; + for (const auto& ws : workspaces) package_entries.push_back(ws.m_entry.get()); + + if (const auto ephemeral_violation{CheckEphemeralSpends(package_entries, m_pool.m_opts.dust_relay_feerate, m_pool)}) { const Txid& parent_txid = ephemeral_violation.value(); TxValidationState child_state; child_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends",