Skip to content

Commit 32a775f

Browse files
instagibbsinstagibbs
committed
CheckEphemeralSpends: return boolean, and set child state and txid outparams
1 parent 827ab83 commit 32a775f

File tree

5 files changed

+101
-39
lines changed

5 files changed

+101
-39
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 PreCheckValidEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate,
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& child_state, Txid& 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+
child_txid = tx->GetHash();
87+
child_state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "missing-ephemeral-spends",
88+
strprintf("tx %s did not spend parent's ephemeral dust", 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 PreCheckValidEphemeralTx(const CTransaction& tx, CFeeRate dust_relay_rate,
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+
* child_state and child_txid set 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& child_state, Txid& child_txid);
5657

5758
#endif // BITCOIN_POLICY_EPHEMERAL_POLICY_H

‎src/test/txvalidation_tests.cpp

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

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

@@ -131,88 +134,145 @@ BOOST_FIXTURE_TEST_CASE(ephemeral_tests, RegTestingSetup)
131134
// We first start with nothing "in the mempool", using package checks
132135

133136
// Trivial single transaction with no dust
134-
BOOST_CHECK(!CheckEphemeralSpends({dust_spend}, dustrelay, pool));
137+
BOOST_CHECK(CheckEphemeralSpends({dust_spend}, dustrelay, pool, child_state, child_txid));
138+
BOOST_CHECK(child_state.IsValid());
139+
BOOST_CHECK_EQUAL(child_txid, Txid());
135140

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

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

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

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

153178
auto grandparent_tx_2 = make_ephemeral_tx(random_outpoints(1), /*version=*/2);
154179
const auto dust_txid_2 = grandparent_tx_2->GetHash();
155180

156181
// Spend dust from one but not another is ok, as long as second grandparent has no child
157-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, dustrelay, pool));
182+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend}, dustrelay, pool, child_state, child_txid));
183+
BOOST_CHECK(child_state.IsValid());
184+
BOOST_CHECK_EQUAL(child_txid, Txid());
158185

159186
auto dust_non_spend_both_parents = make_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index - 1}}, /*version=*/2);
160187
// But if we spend from the parent, it must spend dust
161-
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());
188+
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid));
189+
BOOST_CHECK(!child_state.IsValid());
190+
BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash());
191+
child_state = TxValidationState();
192+
child_txid = Txid();
162193

163194
auto dust_spend_both_parents = make_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index}}, /*version=*/2);
164-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool));
195+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_both_parents}, dustrelay, pool, child_state, child_txid));
196+
BOOST_CHECK(child_state.IsValid());
197+
BOOST_CHECK_EQUAL(child_txid, Txid());
165198

166199
// Spending other outputs is also correct, as long as the dusty one is spent
167200
const std::vector<COutPoint> all_outpoints{COutPoint(dust_txid, 0), COutPoint(dust_txid, 1), COutPoint(dust_txid, 2),
168201
COutPoint(dust_txid_2, 0), COutPoint(dust_txid_2, 1), COutPoint(dust_txid_2, 2)};
169202
auto dust_spend_all_outpoints = make_tx(all_outpoints, /*version=*/2);
170-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, dustrelay, pool));
203+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, dust_spend_all_outpoints}, dustrelay, pool, child_state, child_txid));
204+
BOOST_CHECK(child_state.IsValid());
205+
BOOST_CHECK_EQUAL(child_txid, Txid());
171206

172207
// 2 grandparents with dust <- 1 dust-spending parent with dust <- child with no dust
173208
auto parent_with_dust = make_ephemeral_tx({COutPoint{dust_txid, dust_index}, COutPoint{dust_txid_2, dust_index}}, /*version=*/2);
174209
// Ok for parent to have dust
175-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool));
210+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid));
211+
BOOST_CHECK(child_state.IsValid());
212+
BOOST_CHECK_EQUAL(child_txid, Txid());
176213
auto child_no_dust = make_tx({COutPoint{parent_with_dust->GetHash(), dust_index}}, /*version=*/2);
177-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool));
214+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_no_dust}, dustrelay, pool, child_state, child_txid));
215+
BOOST_CHECK(child_state.IsValid());
216+
BOOST_CHECK_EQUAL(child_txid, Txid());
178217

179218
// 2 grandparents with dust <- 1 dust-spending parent with dust <- child with dust
180219
auto child_with_dust = make_ephemeral_tx({COutPoint{parent_with_dust->GetHash(), dust_index}}, /*version=*/2);
181-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool));
220+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1, grandparent_tx_2, parent_with_dust, child_with_dust}, dustrelay, pool, child_state, child_txid));
221+
BOOST_CHECK(child_state.IsValid());
222+
BOOST_CHECK_EQUAL(child_txid, Txid());
182223

183224
// Tests with parents in mempool
184225

185226
// Nothing in mempool, this should pass for any transaction
186-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool));
227+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_1}, dustrelay, pool, child_state, child_txid));
228+
BOOST_CHECK(child_state.IsValid());
229+
BOOST_CHECK_EQUAL(child_txid, Txid());
187230

188231
// Add first grandparent to mempool and fetch entry
189232
pool.addUnchecked(entry.FromTx(grandparent_tx_1));
190233

191234
// Ignores ancestors that aren't direct parents
192-
BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, dustrelay, pool));
235+
BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid));
236+
BOOST_CHECK(child_state.IsValid());
237+
BOOST_CHECK_EQUAL(child_txid, Txid());
193238

194239
// Valid spend of dust with grandparent in mempool
195-
BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool));
240+
BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid));
241+
BOOST_CHECK(child_state.IsValid());
242+
BOOST_CHECK_EQUAL(child_txid, Txid());
196243

197244
// Second grandparent in same package
198-
BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool));
245+
BOOST_CHECK(CheckEphemeralSpends({parent_with_dust, grandparent_tx_2}, dustrelay, pool, child_state, child_txid));
246+
BOOST_CHECK(child_state.IsValid());
247+
BOOST_CHECK_EQUAL(child_txid, Txid());
248+
199249
// Order in package doesn't matter
200-
BOOST_CHECK(!CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool));
250+
BOOST_CHECK(CheckEphemeralSpends({grandparent_tx_2, parent_with_dust}, dustrelay, pool, child_state, child_txid));
251+
BOOST_CHECK(child_state.IsValid());
252+
BOOST_CHECK_EQUAL(child_txid, Txid());
201253

202254
// Add second grandparent to mempool
203255
pool.addUnchecked(entry.FromTx(grandparent_tx_2));
204256

205257
// Only spends single dust out of two direct parents
206-
BOOST_CHECK_EQUAL(CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool).value_or(null_txid), dust_non_spend_both_parents->GetHash());
258+
BOOST_CHECK(!CheckEphemeralSpends({dust_non_spend_both_parents}, dustrelay, pool, child_state, child_txid));
259+
BOOST_CHECK(!child_state.IsValid());
260+
BOOST_CHECK_EQUAL(child_txid, dust_non_spend_both_parents->GetHash());
261+
child_state = TxValidationState();
262+
child_txid = Txid();
207263

208264
// Spends both parents' dust
209-
BOOST_CHECK(!CheckEphemeralSpends({parent_with_dust}, dustrelay, pool));
265+
BOOST_CHECK(CheckEphemeralSpends({parent_with_dust}, dustrelay, pool, child_state, child_txid));
266+
BOOST_CHECK(child_state.IsValid());
267+
BOOST_CHECK_EQUAL(child_txid, Txid());
210268

211269
// Now add dusty parent to mempool
212270
pool.addUnchecked(entry.FromTx(parent_with_dust));
213271

214272
// Passes dust checks even with non-parent ancestors
215-
BOOST_CHECK(!CheckEphemeralSpends({child_no_dust}, dustrelay, pool));
273+
BOOST_CHECK(CheckEphemeralSpends({child_no_dust}, dustrelay, pool, child_state, child_txid));
274+
BOOST_CHECK(child_state.IsValid());
275+
BOOST_CHECK_EQUAL(child_txid, Txid());
216276
}
217277

218278
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)