Skip to content

Commit ba35a57

Browse files
instagibbsinstagibbs
committed
CheckEphemeralSpends: return boolean, and set child state and txid outparams
1 parent cf0cee1 commit ba35a57

File tree

5 files changed

+101
-41
lines changed

5 files changed

+101
-41
lines changed

‎src/bench/mempool_ephemeral_spends.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,12 @@ static void MempoolCheckEphemeralSpends(benchmark::Bench& bench)
7373

7474
uint32_t iteration{0};
7575

76+
TxValidationState dummy_state;
77+
Txid dummy_txid;
78+
7679
bench.run([&]() NO_THREAD_SAFETY_ANALYSIS {
7780

78-
CheckEphemeralSpends({tx2_r}, /*dust_relay_rate=*/CFeeRate(iteration * COIN / 10), pool);
81+
CheckEphemeralSpends({tx2_r}, /*dust_relay_rate=*/CFeeRate(iteration * COIN / 10), pool, dummy_state, dummy_txid);
7982
iteration++;
8083
});
8184
}

‎src/policy/ephemeral_policy.cpp‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,11 @@ bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmou
3030
return true;
3131
}
3232

33-
std::optional<Txid> CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool)
33+
bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Txid& out_child_txid)
3434
{
3535
if (!Assume(std::ranges::all_of(package, [](const auto& tx){return tx != nullptr;}))) {
3636
// Bail out of spend checks if caller gave us an invalid package
37-
return std::nullopt;
37+
return true;
3838
}
3939

4040
std::map<Txid, CTransactionRef> map_txid_ref;
@@ -83,9 +83,12 @@ std::optional<Txid> CheckEphemeralSpends(const Package& package, CFeeRate dust_r
8383
}
8484

8585
if (!unspent_parent_dust.empty()) {
86-
return tx->GetHash();
86+
out_child_txid = tx->GetHash();
87+
out_child_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends",
88+
strprintf("tx %s did not spend parent's ephemeral dust", out_child_txid.ToString()));
89+
return false;
8790
}
8891
}
8992

90-
return std::nullopt;
93+
return true;
9194
}

‎src/policy/ephemeral_policy.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,9 @@ bool PreCheckEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate, CAmou
5050
/** Must be called for each transaction(package) if any dust is in the package.
5151
* Checks that each transaction's parents have their dust spent by the child,
5252
* where parents are either in the mempool or in the package itself.
53-
* @returns std::nullopt if all dust is properly spent, or the txid of the violating child spend.
53+
* Sets out_child_state and out_child_txid on failure.
54+
* @returns true if all dust is properly spent.
5455
*/
55-
std::optional<Txid> CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool);
56+
bool CheckEphemeralSpends(const Package& package, CFeeRate dust_relay_rate, const CTxMemPool& tx_pool, TxValidationState& out_child_state, Txid& out_child_txid);
5657

5758
#endif // BITCOIN_POLICY_EPHEMERAL_POLICY_H

‎src/test/txvalidation_tests.cpp‎

Lines changed: 82 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup)
117117
TestMemPoolEntryHelper entry;
118118
CTxMemPool::setEntries empty_ancestors;
119119

120+
TxValidationState child_state;
121+
Txid child_txid;
122+
120123
// Arbitrary non-0 feerate for these tests
121124
CFeeRate dustrelay(DUST_RELAY_TX_FEE);
122125

@@ -130,88 +133,143 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup)
130133
// We first start with nothing "in the mempool", using package checks
131134

132135
// Trivial single transaction with no dust
133-
BOOST_CHECK(!CheckEphemeralSpends({dust_spend}, dustrelay, pool));
136+
BOOST_CHECK(CheckEphemeralSpends({dust_spend}, dustrelay, pool, child_state, child_txid));
137+
BOOST_CHECK(child_state.IsValid());
138+
BOOST_CHECK_EQUAL(child_txid, Txid());
134139

135140
// Now with dust, ok because the tx has no dusty parents
136-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool));
141+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_txid));
142+
BOOST_CHECK(child_state.IsValid());
143+
BOOST_CHECK_EQUAL(child_txid, Txid());
137144

138145
// Dust checks pass
139-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool));
140-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend}, dustrelay, pool));
146+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend}, CFeeRate(0), pool, child_state, child_txid));
147+
BOOST_CHECK(child_state.IsValid());
148+
BOOST_CHECK_EQUAL(child_txid, Txid());
149+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, dust_spend}, dustrelay, pool, child_state, child_txid));
150+
BOOST_CHECK(child_state.IsValid());
151+
BOOST_CHECK_EQUAL(child_txid, Txid());
141152

142153
auto dust_non_spend = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX - 1}}, /*version=*/2);
143154

144155
// Child spending non-dust only from parent should be disallowed even if dust otherwise spent
145156
const auto dust_non_spend_txid{dust_non_spend->GetHash()};
146-
const Txid null_txid;
147-
assert(dust_non_spend_txid != null_txid);
148-
BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid);
149-
BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid);
150-
BOOST_CHECK_EQUAL(CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, dustrelay, pool).value_or(null_txid), dust_non_spend_txid);
157+
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend, dust_spend}, dustrelay, pool, child_state, child_txid));
158+
BOOST_CHECK(!child_state.IsValid());
159+
BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid);
160+
child_state = TxValidationState();
161+
child_txid = Txid();
162+
163+
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_spend, dust_non_spend}, dustrelay, pool, child_state, child_txid));
164+
BOOST_CHECK(!child_state.IsValid());
165+
BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid);
166+
child_state = TxValidationState();
167+
child_txid = Txid();
168+
169+
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, dust_non_spend}, dustrelay, pool, child_state, child_txid));
170+
BOOST_CHECK(!child_state.IsValid());
171+
BOOST_CHECK_EQUAL(child_txid, dust_non_spend_txid);
172+
child_state = TxValidationState();
173+
child_txid = Txid();
151174

152175
auto grandparent_tx_2 = make_ephemeral_tx(random_outpoints(1), /*version=*/2);
153176
const auto dust_txid_2 = grandparent_tx_2->GetHash();
154177

155178
// Spend dust from one but not another is ok, as long as second grandparent has no child
156-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, dustrelay, pool));
179+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, dustrelay, pool, child_state, child_txid));
180+
BOOST_CHECK(child_state.IsValid());
181+
BOOST_CHECK_EQUAL(child_txid, Txid());
157182

158183
auto dust_non_spend_both_parents = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}, COutPoint{dust_txid_2, EPHEMERAL_DUST_INDEX - 1}}, /*version=*/2);
159184
// But if we spend from the parent, it must spend dust
160-
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());
185+
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid));
186+
BOOST_CHECK(!child_state.IsValid());
187+
BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash());
188+
child_state = TxValidationState();
189+
child_txid = Txid();
161190

162191
auto dust_spend_both_parents = make_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}, COutPoint{dust_txid_2, EPHEMERAL_DUST_INDEX}}, /*version=*/2);
163-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool));
192+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool, child_state, child_txid));
193+
BOOST_CHECK(child_state.IsValid());
194+
BOOST_CHECK_EQUAL(child_txid, Txid());
164195

165196
// Spending other outputs is also correct, as long as the dusty one is spent
166197
const std::vector<COutPoint> all_outpoints{COutPoint(dust_txid, 0), COutPoint(dust_txid, 1), COutPoint(dust_txid, 2),
167198
COutPoint(dust_txid_2, 0), COutPoint(dust_txid_2, 1), COutPoint(dust_txid_2, 2)};
168199
auto dust_spend_all_outpoints = make_tx(all_outpoints, /*version=*/2);
169-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, dustrelay, pool));
200+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, dustrelay, pool, child_state, child_txid));
201+
BOOST_CHECK(child_state.IsValid());
202+
BOOST_CHECK_EQUAL(child_txid, Txid());
170203

171204
// 2 grandparents with dust <- 1 dust-spending parent with dust <- child with no dust
172205
auto parent_with_dust = make_ephemeral_tx({COutPoint{dust_txid, EPHEMERAL_DUST_INDEX}, COutPoint{dust_txid_2, EPHEMERAL_DUST_INDEX}}, /*version=*/2);
173206
// Ok for parent to have dust
174-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool));
207+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid));
208+
BOOST_CHECK(child_state.IsValid());
209+
BOOST_CHECK_EQUAL(child_txid, Txid());
175210
auto child_no_dust = make_tx({COutPoint{parent_with_dust->GetHash(), EPHEMERAL_DUST_INDEX}}, /*version=*/2);
176-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool));
211+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool, child_state, child_txid));
212+
BOOST_CHECK(child_state.IsValid());
213+
BOOST_CHECK_EQUAL(child_txid, Txid());
177214

178215
// 2 grandparents with dust <- 1 dust-spending parent with dust <- child with dust
179216
auto child_with_dust = make_ephemeral_tx({COutPoint{parent_with_dust->GetHash(), EPHEMERAL_DUST_INDEX}}, /*version=*/2);
180-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool));
217+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool, child_state, child_txid));
218+
BOOST_CHECK(child_state.IsValid());
219+
BOOST_CHECK_EQUAL(child_txid, Txid());
181220

182221
// Tests with parents in mempool
183222

184223
// Nothing in mempool, this should pass for any transaction
185-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool));
224+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_txid));
225+
BOOST_CHECK(child_state.IsValid());
226+
BOOST_CHECK_EQUAL(child_txid, Txid());
186227

187228
// Add first grandparent to mempool and fetch entry
188229
pool.addUnchecked(entry.FromTx(grandparent_tx_1));
189230

190231
// Ignores ancestors that aren't direct parents
191-
BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, dustrelay, pool));
232+
BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid));
233+
BOOST_CHECK(child_state.IsValid());
234+
BOOST_CHECK_EQUAL(child_txid, Txid());
192235

193236
// Valid spend of dust with grandparent in mempool
194-
BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool));
237+
BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid));
238+
BOOST_CHECK(child_state.IsValid());
239+
BOOST_CHECK_EQUAL(child_txid, Txid());
195240

196241
// Second grandparent in same package
197-
BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool));
242+
BOOST_CHECK(CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool, child_state, child_txid));
243+
BOOST_CHECK(child_state.IsValid());
244+
BOOST_CHECK_EQUAL(child_txid, Txid());
245+
198246
// Order in package doesn't matter
199-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool));
247+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid));
248+
BOOST_CHECK(child_state.IsValid());
249+
BOOST_CHECK_EQUAL(child_txid, Txid());
200250

201251
// Add second grandparent to mempool
202252
pool.addUnchecked(entry.FromTx(grandparent_tx_2));
203253

204254
// Only spends single dust out of two direct parents
205-
BOOST_CHECK_EQUAL(CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash());
255+
BOOST_CHECK(!CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid));
256+
BOOST_CHECK(!child_state.IsValid());
257+
BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash());
258+
child_state = TxValidationState();
259+
child_txid = Txid();
206260

207261
// Spends both parents' dust
208-
BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool));
262+
BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid));
263+
BOOST_CHECK(child_state.IsValid());
264+
BOOST_CHECK_EQUAL(child_txid, Txid());
209265

210266
// Now add dusty parent to mempool
211267
pool.addUnchecked(entry.FromTx(parent_with_dust));
212268

213269
// Passes dust checks even with non-parent ancestors
214-
BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, dustrelay, pool));
270+
BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid));
271+
BOOST_CHECK(child_state.IsValid());
272+
BOOST_CHECK_EQUAL(child_txid, Txid());
215273
}
216274

217275
BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)

‎src/validation.cpp‎

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1441,11 +1441,8 @@ MempoolAcceptResult MemPoolAccept::AcceptSingleTransaction(const CTransactionRef
14411441
}
14421442

14431443
if (m_pool.m_opts.require_standard) {
1444-
if (const auto ephemeral_violation{CheckEphemeralSpends(/*package=*/{ptx}, m_pool.m_opts.dust_relay_feerate, m_pool)}) {
1445-
const Txid& txid = ephemeral_violation.value();
1446-
Assume(txid == ptx->GetHash());
1447-
ws.m_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends",
1448-
strprintf("tx %s did not spend parent's ephemeral dust", txid.ToString()));
1444+
Txid dummy_txid;
1445+
if (!CheckEphemeralSpends(/*package=*/{ptx}, m_pool.m_opts.dust_relay_feerate, m_pool, ws.m_state, dummy_txid)) {
14491446
return MempoolAcceptResult::Failure(ws.m_state);
14501447
}
14511448
}
@@ -1590,11 +1587,9 @@ PackageMempoolAcceptResult MemPoolAccept::AcceptMultipleTransactions(const std::
15901587

15911588
// Now that we've bounded the resulting possible ancestry count, check package for dust spends
15921589
if (m_pool.m_opts.require_standard) {
1593-
if (const auto ephemeral_violation{CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate, m_pool)}) {
1594-
const Txid& child_txid = ephemeral_violation.value();
1595-
TxValidationState child_state;
1596-
child_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends",
1597-
strprintf("tx %s did not spend parent's ephemeral dust", child_txid.ToString()));
1590+
TxValidationState child_state;
1591+
Txid child_txid;
1592+
if (!CheckEphemeralSpends(txns, m_pool.m_opts.dust_relay_feerate, m_pool, child_state, child_txid)) {
15981593
package_state.Invalid(PackageValidationResult::PCKG_TX, "unspent-dust");
15991594
results.emplace(child_txid, MempoolAcceptResult::Failure(child_state));
16001595
return PackageMempoolAcceptResult(package_state, std::move(results));

0 commit comments

Comments
 (0)