diff --git a/src/bench/mempool_ephemeral_spends.cpp b/src/bench/mempool_ephemeral_spends.cpp index 78f869b9cde42..600eb6d7cb1df 100644 --- a/src/bench/mempool_ephemeral_spends.cpp +++ b/src/bench/mempool_ephemeral_spends.cpp @@ -73,9 +73,12 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench) uint32_t iteration{0}; + TxValidationState dummy_state; + Txid dummy_txid; + bench.run([&]() NO_THREAD_SAFETY_ANALYSIS { - CheckEphemeralSpends({tx2_r}, /*dust_relay_rate=*/CFeeRate(iteration * COIN / 10), pool); + CheckEphemeralSpends({tx2_r}, /*dust_relay_rate=*/CFeeRate(iteration * COIN / 10), pool, dummy_state, dummy_txid); iteration++; }); } diff --git a/src/policy/ephemeral_policy.cpp b/src/policy/ephemeral_policy.cpp index 84863edd594e5..b7d254dd63045 100644 --- a/src/policy/ephemeral_policy.cpp +++ b/src/policy/ephemeral_policy.cpp @@ -30,11 +30,11 @@ bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmou return true; } -std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool) +bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Txid& out_child_txid) { if (!Assume(std::ranges::all_of(package, [](const auto& tx){return tx != nullptr;}))) { // Bail out of spend checks if caller gave us an invalid package - return std::nullopt; + return true; } std::map map_txid_ref; @@ -83,9 +83,12 @@ std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_r } if (!unspent_parent_dust.empty()) { - return tx->GetHash(); + out_child_txid = tx->GetHash(); + out_child_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends", + strprintf("tx %s did not spend parent's ephemeral dust", out_child_txid.ToString())); + return false; } } - return std::nullopt; + return true; } diff --git a/src/policy/ephemeral_policy.h b/src/policy/ephemeral_policy.h index 015c0770c12cf..f9c6e4363e733 100644 --- a/src/policy/ephemeral_policy.h +++ b/src/policy/ephemeral_policy.h @@ -50,8 +50,9 @@ bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmou /** 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. - * @returns std::nullopt if all dust is properly spent, or the txid of the violating child spend. + * Sets out_child_state and out_child_txid on failure. + * @returns true if all dust is properly spent. */ -std::optional CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool); +bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Txid& out_child_txid); #endif // BITCOIN_POLICY_EPHEMERAL_POLICY_H diff --git a/src/test/txvalidation_tests.cpp b/src/test/txvalidation_tests.cpp index 63876616d3df3..1e036aa508731 100644 --- a/src/test/txvalidation_tests.cpp +++ b/src/test/txvalidation_tests.cpp @@ -117,6 +117,9 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) TestMemPoolEntryHelper entry; CTxMemPool::setEntries empty_ancestors; + TxValidationState child_state; + Txid child_txid; + // Arbitrary non-0 feerate for these tests CFeeRate dustrelay(DUST_RELAY_TX_FEE); @@ -130,88 +133,143 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup) // We first start with nothing "in the mempool", using package checks // Trivial single transaction with no dust - BOOST_CHECK(!CheckEphemeralSpends({dust_spend}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Now with dust, ok because the tx has no dusty parents - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Dust checks pass - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool)); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); auto dust_non_spend = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX - 1}}, /*version=*/2); // Child spending non-dust only from parent should be disallowed even if dust otherwise spent const auto dust_non_spend_txid{dust_non_spend->GetHash()}; - const Txid null_txid; - assert(dust_non_spend_txid != null_txid); - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid); - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid); - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid); + child_state = TxValidationState(); + child_txid = Txid(); + + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid); + child_state = TxValidationState(); + child_txid = Txid(); + + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid); + child_state = TxValidationState(); + child_txid = Txid(); 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}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); auto dust_non_spend_both_parents = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}, COutPoint{dust_txid_2, EPHEMERAL_DUST_INDEX - 1}}, /*version=*/2); // But if we spend from the parent, it must spend dust - BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, dustrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); + BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash()); + child_state = TxValidationState(); + child_txid = Txid(); auto dust_spend_both_parents = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}, COutPoint{dust_txid_2, EPHEMERAL_DUST_INDEX}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // 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}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // 2 grandparents with dust <- 1 dust-spending parent with dust <- child with no dust auto parent_with_dust = make_ephemeral_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}, COutPoint{dust_txid_2, EPHEMERAL_DUST_INDEX}}, /*version=*/2); // Ok for parent to have dust - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); auto child_no_dust = make_tx({COutPoint{parent_with_dust->GetHash(), EPHEMERAL_DUST_INDEX}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // 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(), EPHEMERAL_DUST_INDEX}}, /*version=*/2); - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Tests with parents in mempool // Nothing in mempool, this should pass for any transaction - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // 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}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Valid spend of dust with grandparent in mempool - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Second grandparent in same package - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); + // Order in package doesn't matter - BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // Add second grandparent to mempool pool.addUnchecked(entry.FromTx(grandparent_tx_2)); // Only spends single dust out of two direct parents - BOOST_CHECK_EQUAL(CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash()); + BOOST_CHECK(!CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(!child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash()); + child_state = TxValidationState(); + child_txid = Txid(); // Spends both parents' dust - BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); // 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}, dustrelay, pool)); + BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid)); + BOOST_CHECK(child_state.IsValid()); + BOOST_CHECK_EQUAL(child_txid, Txid()); } BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup) diff --git a/src/validation.cpp b/src/validation.cpp index 91a9d02611ed8..7ab857e58a53a 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1441,11 +1441,8 @@ 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)}) { - const Txid& txid = ephemeral_violation.value(); - Assume(txid == ptx->GetHash()); - ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends", - strprintf("tx %s did not spend parent's ephemeral dust", txid.ToString())); + Txid dummy_txid; + if (!CheckEphemeralSpends(/*package=*/{ptx}, m_pool.m_opts.dust_relay_feerate, m_pool, ws.m_state, dummy_txid)) { return MempoolAcceptResult::Failure(ws.m_state); } } @@ -1590,11 +1587,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std:: // Now that we've bounded the resulting possible ancestry count, check package 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)}) { - const Txid& child_txid = ephemeral_violation.value(); - TxValidationState child_state; - child_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends", - strprintf("tx %s did not spend parent's ephemeral dust", child_txid.ToString())); + TxValidationState child_state; + Txid child_txid; + if (!CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate, m_pool, child_state, child_txid)) { package_state.Invalid(PackageValidationResult::PCKG_TX, "unspent-dust"); results.emplace(child_txid, MempoolAcceptResult::Failure(child_state)); return PackageMempoolAcceptResult(package_state, std::move(results));