Skip to content

Commit b5d15f7

Browse files
glozowglozow
committed
[refactor] return pair from SingleV3Checks
1 parent baed5ed commit b5d15f7

File tree

4 files changed

+47
-31
lines changed

4 files changed

+47
-31
lines changed

‎src/policy/v3_policy.cpp‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -158,21 +158,23 @@ std::optional<std::string> PackageV3Checks(const CTransactionRef& ptx, int64_t v
158158
return std::nullopt;
159159
}
160160

161-
std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
161+
std::optional<std::pair<std::string, CTransactionRef>> SingleV3Checks(const CTransactionRef& ptx,
162162
const CTxMemPool::setEntries& mempool_ancestors,
163163
const std::set<Txid>& direct_conflicts,
164164
int64_t vsize)
165165
{
166166
// Check v3 and non-v3 inheritance.
167167
for (const auto& entry : mempool_ancestors) {
168168
if (ptx->nVersion != 3 && entry->GetTx().nVersion == 3) {
169-
return strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
169+
return std::make_pair(strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
170170
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
171-
entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString());
171+
entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()),
172+
nullptr);
172173
} else if (ptx->nVersion == 3 && entry->GetTx().nVersion != 3) {
173-
return strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
174+
return std::make_pair(strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
174175
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(),
175-
entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString());
176+
entry->GetSharedTx()->GetHash().ToString(), entry->GetSharedTx()->GetWitnessHash().ToString()),
177+
nullptr);
176178
}
177179
}
178180

@@ -185,16 +187,18 @@ std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
185187

186188
// Check that V3_ANCESTOR_LIMIT would not be violated.
187189
if (mempool_ancestors.size() + 1 > V3_ANCESTOR_LIMIT) {
188-
return strprintf("tx %s (wtxid=%s) would have too many ancestors",
189-
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString());
190+
return std::make_pair(strprintf("tx %s (wtxid=%s) would have too many ancestors",
191+
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString()),
192+
nullptr);
190193
}
191194

192195
// Remaining checks only pertain to transactions with unconfirmed ancestors.
193196
if (mempool_ancestors.size() > 0) {
194197
// If this transaction spends V3 parents, it cannot be too large.
195198
if (vsize > V3_CHILD_MAX_VSIZE) {
196-
return strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
197-
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE);
199+
return std::make_pair(strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
200+
ptx->GetHash().ToString(), ptx->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE),
201+
nullptr);
198202
}
199203

200204
// Check the descendant counts of in-mempool ancestors.
@@ -210,9 +214,10 @@ std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
210214
std::any_of(children.cbegin(), children.cend(),
211215
[&direct_conflicts](const CTxMemPoolEntry& child){return direct_conflicts.count(child.GetTx().GetHash()) > 0;});
212216
if (parent_entry->GetCountWithDescendants() + 1 > V3_DESCENDANT_LIMIT && !child_will_be_replaced) {
213-
return strprintf("tx %u (wtxid=%s) would exceed descendant count limit",
217+
return std::make_pair(strprintf("tx %u (wtxid=%s) would exceed descendant count limit",
214218
parent_entry->GetSharedTx()->GetHash().ToString(),
215-
parent_entry->GetSharedTx()->GetWitnessHash().ToString());
219+
parent_entry->GetSharedTx()->GetWitnessHash().ToString()),
220+
nullptr);
216221
}
217222
}
218223
return std::nullopt;

‎src/policy/v3_policy.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ static_assert(V3_CHILD_MAX_VSIZE + MAX_STANDARD_TX_WEIGHT / WITNESS_SCALE_FACTOR
5050
*
5151
* @returns debug string if an error occurs, std::nullopt otherwise.
5252
*/
53-
std::optional<std::string> SingleV3Checks(const CTransactionRef& ptx,
53+
std::optional<std::pair<std::string, CTransactionRef>> SingleV3Checks(const CTransactionRef& ptx,
5454
const CTxMemPool::setEntries& mempool_ancestors,
5555
const std::set<Txid>& direct_conflicts,
5656
int64_t vsize);

‎src/test/txvalidation_tests.cpp‎

Lines changed: 28 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
115115
const auto expected_error_str{strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
116116
tx_v2_from_v3->GetHash().ToString(), tx_v2_from_v3->GetWitnessHash().ToString(),
117117
mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())};
118-
BOOST_CHECK(*SingleV3Checks(tx_v2_from_v3, *ancestors_v2_from_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v3)) == expected_error_str);
118+
auto result_v2_from_v3{SingleV3Checks(tx_v2_from_v3, *ancestors_v2_from_v3, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v3))};
119+
BOOST_CHECK_EQUAL(result_v2_from_v3->first, expected_error_str);
120+
BOOST_CHECK_EQUAL(result_v2_from_v3->second, nullptr);
119121

120122
Package package_v3_v2{mempool_tx_v3, tx_v2_from_v3};
121123
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v2_from_v3, GetVirtualTransactionSize(*tx_v2_from_v3), package_v3_v2, empty_ancestors), expected_error_str);
@@ -130,8 +132,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
130132
const auto expected_error_str_2{strprintf("non-v3 tx %s (wtxid=%s) cannot spend from v3 tx %s (wtxid=%s)",
131133
tx_v2_from_v2_and_v3->GetHash().ToString(), tx_v2_from_v2_and_v3->GetWitnessHash().ToString(),
132134
mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())};
133-
BOOST_CHECK(*SingleV3Checks(tx_v2_from_v2_and_v3, *ancestors_v2_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3))
134-
== expected_error_str_2);
135+
auto result_v2_from_both{SingleV3Checks(tx_v2_from_v2_and_v3, *ancestors_v2_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3))};
136+
BOOST_CHECK_EQUAL(result_v2_from_both->first, expected_error_str_2);
137+
BOOST_CHECK_EQUAL(result_v2_from_both->second, nullptr);
135138

136139
Package package_v3_v2_v2{mempool_tx_v3, mempool_tx_v2, tx_v2_from_v2_and_v3};
137140
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v2_from_v2_and_v3, GetVirtualTransactionSize(*tx_v2_from_v2_and_v3), package_v3_v2_v2, empty_ancestors), expected_error_str_2);
@@ -147,7 +150,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
147150
const auto expected_error_str{strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
148151
tx_v3_from_v2->GetHash().ToString(), tx_v3_from_v2->GetWitnessHash().ToString(),
149152
mempool_tx_v2->GetHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())};
150-
BOOST_CHECK(*SingleV3Checks(tx_v3_from_v2, *ancestors_v3_from_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2)) == expected_error_str);
153+
auto result_v3_from_v2{SingleV3Checks(tx_v3_from_v2, *ancestors_v3_from_v2, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2))};
154+
BOOST_CHECK_EQUAL(result_v3_from_v2->first, expected_error_str);
155+
BOOST_CHECK_EQUAL(result_v3_from_v2->second, nullptr);
151156

152157
Package package_v2_v3{mempool_tx_v2, tx_v3_from_v2};
153158
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_from_v2, GetVirtualTransactionSize(*tx_v3_from_v2), package_v2_v3, empty_ancestors), expected_error_str);
@@ -162,8 +167,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
162167
const auto expected_error_str_2{strprintf("v3 tx %s (wtxid=%s) cannot spend from non-v3 tx %s (wtxid=%s)",
163168
tx_v3_from_v2_and_v3->GetHash().ToString(), tx_v3_from_v2_and_v3->GetWitnessHash().ToString(),
164169
mempool_tx_v2->GetHash().ToString(), mempool_tx_v2->GetWitnessHash().ToString())};
165-
BOOST_CHECK(*SingleV3Checks(tx_v3_from_v2_and_v3, *ancestors_v3_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3))
166-
== expected_error_str_2);
170+
auto result_v3_from_both{SingleV3Checks(tx_v3_from_v2_and_v3, *ancestors_v3_from_both, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_from_v2_and_v3))};
171+
BOOST_CHECK_EQUAL(result_v3_from_both->first, expected_error_str_2);
172+
BOOST_CHECK_EQUAL(result_v3_from_both->second, nullptr);
167173

168174
// tx_v3_from_v2_and_v3 also violates V3_ANCESTOR_LIMIT.
169175
const auto expected_error_str_3{strprintf("tx %s (wtxid=%s) would have too many ancestors",
@@ -215,8 +221,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
215221
BOOST_CHECK_EQUAL(ancestors->size(), 3);
216222
const auto expected_error_str{strprintf("tx %s (wtxid=%s) would have too many ancestors",
217223
tx_v3_multi_parent->GetHash().ToString(), tx_v3_multi_parent->GetWitnessHash().ToString())};
218-
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_multi_parent, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_parent)),
219-
expected_error_str);
224+
auto result{SingleV3Checks(tx_v3_multi_parent, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_parent))};
225+
BOOST_CHECK_EQUAL(result->first, expected_error_str);
226+
BOOST_CHECK_EQUAL(result->second, nullptr);
220227

221228
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_multi_parent, GetVirtualTransactionSize(*tx_v3_multi_parent), package_multi_parents, empty_ancestors),
222229
expected_error_str);
@@ -239,8 +246,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
239246
auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_multi_gen), m_limits)};
240247
const auto expected_error_str{strprintf("tx %s (wtxid=%s) would have too many ancestors",
241248
tx_v3_multi_gen->GetHash().ToString(), tx_v3_multi_gen->GetWitnessHash().ToString())};
242-
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_multi_gen, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_gen)),
243-
expected_error_str);
249+
auto result{SingleV3Checks(tx_v3_multi_gen, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_multi_gen))};
250+
BOOST_CHECK_EQUAL(result->first, expected_error_str);
251+
BOOST_CHECK_EQUAL(result->second, nullptr);
244252

245253
// Middle tx is what triggers a failure for the grandchild:
246254
BOOST_CHECK_EQUAL(*PackageV3Checks(middle_tx, GetVirtualTransactionSize(*middle_tx), package_multi_gen, empty_ancestors), expected_error_str);
@@ -256,8 +264,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
256264
auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child_big), m_limits)};
257265
const auto expected_error_str{strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
258266
tx_v3_child_big->GetHash().ToString(), tx_v3_child_big->GetWitnessHash().ToString(), vsize, V3_CHILD_MAX_VSIZE)};
259-
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_child_big, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child_big)),
260-
expected_error_str);
267+
auto result{SingleV3Checks(tx_v3_child_big, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child_big))};
268+
BOOST_CHECK_EQUAL(result->first, expected_error_str);
269+
BOOST_CHECK_EQUAL(result->second, nullptr);
261270

262271
Package package_child_big{mempool_tx_v3, tx_v3_child_big};
263272
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_v3_child_big, GetVirtualTransactionSize(*tx_v3_child_big), package_child_big, empty_ancestors),
@@ -298,9 +307,10 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
298307
const auto expected_error_str{strprintf("v3 child tx %s (wtxid=%s) is too big: %u > %u virtual bytes",
299308
tx_many_sigops->GetHash().ToString(), tx_many_sigops->GetWitnessHash().ToString(),
300309
total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, V3_CHILD_MAX_VSIZE)};
301-
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_many_sigops, *ancestors, empty_conflicts_set,
302-
GetVirtualTransactionSize(*tx_many_sigops, /*nSigOpCost=*/total_sigops, /*bytes_per_sigop=*/ DEFAULT_BYTES_PER_SIGOP)),
303-
expected_error_str);
310+
auto result{SingleV3Checks(tx_many_sigops, *ancestors, empty_conflicts_set,
311+
GetVirtualTransactionSize(*tx_many_sigops, /*nSigOpCost=*/total_sigops, /*bytes_per_sigop=*/ DEFAULT_BYTES_PER_SIGOP))};
312+
BOOST_CHECK_EQUAL(result->first, expected_error_str);
313+
BOOST_CHECK_EQUAL(result->second, nullptr);
304314

305315
Package package_child_sigops{mempool_tx_v3, tx_many_sigops};
306316
BOOST_CHECK_EQUAL(*PackageV3Checks(tx_many_sigops, total_sigops * DEFAULT_BYTES_PER_SIGOP / WITNESS_SCALE_FACTOR, package_child_sigops, empty_ancestors),
@@ -326,8 +336,9 @@ BOOST_FIXTURE_TEST_CASE(version3_tests, RegTestingSetup)
326336
auto ancestors{pool.CalculateMemPoolAncestors(entry.FromTx(tx_v3_child2), m_limits)};
327337
const auto expected_error_str{strprintf("tx %s (wtxid=%s) would exceed descendant count limit",
328338
mempool_tx_v3->GetHash().ToString(), mempool_tx_v3->GetWitnessHash().ToString())};
329-
BOOST_CHECK_EQUAL(*SingleV3Checks(tx_v3_child2, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child2)),
330-
expected_error_str);
339+
auto result{SingleV3Checks(tx_v3_child2, *ancestors, empty_conflicts_set, GetVirtualTransactionSize(*tx_v3_child2))};
340+
BOOST_CHECK_EQUAL(result->first, expected_error_str);
341+
BOOST_CHECK_EQUAL(result->second, nullptr);
331342
// If replacing the child, make sure there is no double-counting.
332343
BOOST_CHECK(SingleV3Checks(tx_v3_child2, *ancestors, {tx_mempool_v3_child->GetHash()}, GetVirtualTransactionSize(*tx_v3_child2))
333344
== std::nullopt);

‎src/validation.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -954,8 +954,8 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
954954
}
955955

956956
ws.m_ancestors = *ancestors;
957-
if (const auto err_string{SingleV3Checks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
958-
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "v3-rule-violation", *err_string);
957+
if (const auto err{SingleV3Checks(ws.m_ptx, ws.m_ancestors, ws.m_conflicts, ws.m_vsize)}) {
958+
return state.Invalid(TxValidationResult::TX_MEMPOOL_POLICY, "v3-rule-violation", err->first);
959959
}
960960

961961
// A transaction that spends outputs that would be replaced by it is invalid. Now

0 commit comments

Comments
 (0)