Skip to content

Commit e70fb87

Browse files
committed
Merge bitcoin/bitcoin#23381: validation/refactor: refactoring for package submission
14cd7bf [test] call CheckPackage for package sanitization checks (glozow) 6876378 MOVEONLY: move package unit tests to their own file (glozow) c9b1439 MOVEONLY: mempool checks to their own functions (glozow) 9e910d8 scripted-diff: clean up MemPoolAccept aliases (glozow) fd92b0c document workspace members (glozow) 3d3e459 [validation] cache iterators to mempool conflicts (glozow) 36a8441 [validation/rpc] cache + use vsize calculated in PreChecks (glozow) 8fa2936 [validation] re-introduce bool for whether a transaction is RBF (glozow) cbb3598 [validation/refactor] store precomputed txdata in workspace (glozow) 0a79eab [validation] case-based constructors for ATMPArgs (glozow) Pull request description: This contains the refactors and moves within #22674. There are no behavior changes, so it should be simpler to review. ACKs for top commit: ariard: Code Review ACK 14cd7bf jnewbery: Code review ACK 14cd7bf laanwj: Code review ACK 14cd7bf, thanks for adding documentation and clarifying the code t-bast: Code Review ACK bitcoin/bitcoin@14cd7bf Tree-SHA512: 580ed48b43713a3f9d81cd9b573ef6ac44efe5df2fc7b7b7036c232b52952b04bf5ea92940cf73739f4fbd54ecf980cef58032e8a2efe05229ad0b3c639de8a0
2 parents cb4adbd + 14cd7bf commit e70fb87

File tree

6 files changed

+285
-195
lines changed

6 files changed

+285
-195
lines changed

src/Makefile.test.include

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ BITCOIN_TESTS =\
139139
test/transaction_tests.cpp \
140140
test/txindex_tests.cpp \
141141
test/txrequest_tests.cpp \
142+
test/txpackage_tests.cpp \
142143
test/txvalidation_tests.cpp \
143144
test/txvalidationcache_tests.cpp \
144145
test/uint256_tests.cpp \

src/rpc/rawtransaction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ static RPCHelpMan testmempoolaccept()
977977
if (tx_result.m_result_type == MempoolAcceptResult::ResultType::VALID) {
978978
const CAmount fee = tx_result.m_base_fees.value();
979979
// Check that fee does not exceed maximum fee
980-
const int64_t virtual_size = GetVirtualTransactionSize(*tx);
980+
const int64_t virtual_size = tx_result.m_vsize.value();
981981
const CAmount max_raw_tx_fee = max_raw_tx_fee_rate.GetFee(virtual_size);
982982
if (max_raw_tx_fee && fee > max_raw_tx_fee) {
983983
result_inner.pushKV("allowed", false);

src/test/txpackage_tests.cpp

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// Copyright (c) 2021 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <consensus/validation.h>
6+
#include <key_io.h>
7+
#include <policy/packages.h>
8+
#include <policy/policy.h>
9+
#include <primitives/transaction.h>
10+
#include <script/script.h>
11+
#include <script/standard.h>
12+
#include <test/util/setup_common.h>
13+
#include <validation.h>
14+
15+
#include <boost/test/unit_test.hpp>
16+
17+
BOOST_AUTO_TEST_SUITE(txpackage_tests)
18+
19+
// Create placeholder transactions that have no meaning.
20+
inline CTransactionRef create_placeholder_tx(size_t num_inputs, size_t num_outputs)
21+
{
22+
CMutableTransaction mtx = CMutableTransaction();
23+
mtx.vin.resize(num_inputs);
24+
mtx.vout.resize(num_outputs);
25+
auto random_script = CScript() << ToByteVector(InsecureRand256()) << ToByteVector(InsecureRand256());
26+
for (size_t i{0}; i < num_inputs; ++i) {
27+
mtx.vin[i].prevout.hash = InsecureRand256();
28+
mtx.vin[i].prevout.n = 0;
29+
mtx.vin[i].scriptSig = random_script;
30+
}
31+
for (size_t o{0}; o < num_outputs; ++o) {
32+
mtx.vout[o].nValue = 1 * CENT;
33+
mtx.vout[o].scriptPubKey = random_script;
34+
}
35+
return MakeTransactionRef(mtx);
36+
}
37+
38+
BOOST_FIXTURE_TEST_CASE(package_sanitization_tests, TestChain100Setup)
39+
{
40+
// Packages can't have more than 25 transactions.
41+
Package package_too_many;
42+
package_too_many.reserve(MAX_PACKAGE_COUNT + 1);
43+
for (size_t i{0}; i < MAX_PACKAGE_COUNT + 1; ++i) {
44+
package_too_many.emplace_back(create_placeholder_tx(1, 1));
45+
}
46+
PackageValidationState state_too_many;
47+
BOOST_CHECK(!CheckPackage(package_too_many, state_too_many));
48+
BOOST_CHECK_EQUAL(state_too_many.GetResult(), PackageValidationResult::PCKG_POLICY);
49+
BOOST_CHECK_EQUAL(state_too_many.GetRejectReason(), "package-too-many-transactions");
50+
51+
// Packages can't have a total size of more than 101KvB.
52+
CTransactionRef large_ptx = create_placeholder_tx(150, 150);
53+
Package package_too_large;
54+
auto size_large = GetVirtualTransactionSize(*large_ptx);
55+
size_t total_size{0};
56+
while (total_size <= MAX_PACKAGE_SIZE * 1000) {
57+
package_too_large.push_back(large_ptx);
58+
total_size += size_large;
59+
}
60+
BOOST_CHECK(package_too_large.size() <= MAX_PACKAGE_COUNT);
61+
PackageValidationState state_too_large;
62+
BOOST_CHECK(!CheckPackage(package_too_large, state_too_large));
63+
BOOST_CHECK_EQUAL(state_too_large.GetResult(), PackageValidationResult::PCKG_POLICY);
64+
BOOST_CHECK_EQUAL(state_too_large.GetRejectReason(), "package-too-large");
65+
}
66+
67+
BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup)
68+
{
69+
LOCK(cs_main);
70+
unsigned int initialPoolSize = m_node.mempool->size();
71+
72+
// Parent and Child Package
73+
CKey parent_key;
74+
parent_key.MakeNewKey(true);
75+
CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey()));
76+
auto mtx_parent = CreateValidMempoolTransaction(/* input_transaction */ m_coinbase_txns[0], /* vout */ 0,
77+
/* input_height */ 0, /* input_signing_key */ coinbaseKey,
78+
/* output_destination */ parent_locking_script,
79+
/* output_amount */ CAmount(49 * COIN), /* submit */ false);
80+
CTransactionRef tx_parent = MakeTransactionRef(mtx_parent);
81+
82+
CKey child_key;
83+
child_key.MakeNewKey(true);
84+
CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey()));
85+
auto mtx_child = CreateValidMempoolTransaction(/* input_transaction */ tx_parent, /* vout */ 0,
86+
/* input_height */ 101, /* input_signing_key */ parent_key,
87+
/* output_destination */ child_locking_script,
88+
/* output_amount */ CAmount(48 * COIN), /* submit */ false);
89+
CTransactionRef tx_child = MakeTransactionRef(mtx_child);
90+
const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {tx_parent, tx_child}, /* test_accept */ true);
91+
BOOST_CHECK_MESSAGE(result_parent_child.m_state.IsValid(),
92+
"Package validation unexpectedly failed: " << result_parent_child.m_state.GetRejectReason());
93+
auto it_parent = result_parent_child.m_tx_results.find(tx_parent->GetWitnessHash());
94+
auto it_child = result_parent_child.m_tx_results.find(tx_child->GetWitnessHash());
95+
BOOST_CHECK(it_parent != result_parent_child.m_tx_results.end());
96+
BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(),
97+
"Package validation unexpectedly failed: " << it_parent->second.m_state.GetRejectReason());
98+
BOOST_CHECK(it_child != result_parent_child.m_tx_results.end());
99+
BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(),
100+
"Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason());
101+
102+
103+
// A single, giant transaction submitted through ProcessNewPackage fails on single tx policy.
104+
CTransactionRef giant_ptx = create_placeholder_tx(999, 999);
105+
BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > MAX_PACKAGE_SIZE * 1000);
106+
auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {giant_ptx}, /* test_accept */ true);
107+
BOOST_CHECK(result_single_large.m_state.IsInvalid());
108+
BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX);
109+
BOOST_CHECK_EQUAL(result_single_large.m_state.GetRejectReason(), "transaction failed");
110+
auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash());
111+
BOOST_CHECK(it_giant_tx != result_single_large.m_tx_results.end());
112+
BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size");
113+
114+
// Check that mempool size hasn't changed.
115+
BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize);
116+
}
117+
BOOST_AUTO_TEST_SUITE_END()

src/test/txvalidation_tests.cpp

Lines changed: 0 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -50,99 +50,4 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_reject_coinbase, TestChain100Setup)
5050
BOOST_CHECK_EQUAL(result.m_state.GetRejectReason(), "coinbase");
5151
BOOST_CHECK(result.m_state.GetResult() == TxValidationResult::TX_CONSENSUS);
5252
}
53-
54-
// Create placeholder transactions that have no meaning.
55-
inline CTransactionRef create_placeholder_tx(size_t num_inputs, size_t num_outputs)
56-
{
57-
CMutableTransaction mtx = CMutableTransaction();
58-
mtx.vin.resize(num_inputs);
59-
mtx.vout.resize(num_outputs);
60-
auto random_script = CScript() << ToByteVector(InsecureRand256()) << ToByteVector(InsecureRand256());
61-
for (size_t i{0}; i < num_inputs; ++i) {
62-
mtx.vin[i].prevout.hash = InsecureRand256();
63-
mtx.vin[i].prevout.n = 0;
64-
mtx.vin[i].scriptSig = random_script;
65-
}
66-
for (size_t o{0}; o < num_outputs; ++o) {
67-
mtx.vout[o].nValue = 1 * CENT;
68-
mtx.vout[o].scriptPubKey = random_script;
69-
}
70-
return MakeTransactionRef(mtx);
71-
}
72-
73-
BOOST_FIXTURE_TEST_CASE(package_tests, TestChain100Setup)
74-
{
75-
LOCK(cs_main);
76-
unsigned int initialPoolSize = m_node.mempool->size();
77-
78-
// Parent and Child Package
79-
CKey parent_key;
80-
parent_key.MakeNewKey(true);
81-
CScript parent_locking_script = GetScriptForDestination(PKHash(parent_key.GetPubKey()));
82-
auto mtx_parent = CreateValidMempoolTransaction(/* input_transaction */ m_coinbase_txns[0], /* vout */ 0,
83-
/* input_height */ 0, /* input_signing_key */ coinbaseKey,
84-
/* output_destination */ parent_locking_script,
85-
/* output_amount */ CAmount(49 * COIN), /* submit */ false);
86-
CTransactionRef tx_parent = MakeTransactionRef(mtx_parent);
87-
88-
CKey child_key;
89-
child_key.MakeNewKey(true);
90-
CScript child_locking_script = GetScriptForDestination(PKHash(child_key.GetPubKey()));
91-
auto mtx_child = CreateValidMempoolTransaction(/* input_transaction */ tx_parent, /* vout */ 0,
92-
/* input_height */ 101, /* input_signing_key */ parent_key,
93-
/* output_destination */ child_locking_script,
94-
/* output_amount */ CAmount(48 * COIN), /* submit */ false);
95-
CTransactionRef tx_child = MakeTransactionRef(mtx_child);
96-
const auto result_parent_child = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {tx_parent, tx_child}, /* test_accept */ true);
97-
BOOST_CHECK_MESSAGE(result_parent_child.m_state.IsValid(),
98-
"Package validation unexpectedly failed: " << result_parent_child.m_state.GetRejectReason());
99-
auto it_parent = result_parent_child.m_tx_results.find(tx_parent->GetWitnessHash());
100-
auto it_child = result_parent_child.m_tx_results.find(tx_child->GetWitnessHash());
101-
BOOST_CHECK(it_parent != result_parent_child.m_tx_results.end());
102-
BOOST_CHECK_MESSAGE(it_parent->second.m_state.IsValid(),
103-
"Package validation unexpectedly failed: " << it_parent->second.m_state.GetRejectReason());
104-
BOOST_CHECK(it_child != result_parent_child.m_tx_results.end());
105-
BOOST_CHECK_MESSAGE(it_child->second.m_state.IsValid(),
106-
"Package validation unexpectedly failed: " << it_child->second.m_state.GetRejectReason());
107-
108-
// Packages can't have more than 25 transactions.
109-
Package package_too_many;
110-
package_too_many.reserve(MAX_PACKAGE_COUNT + 1);
111-
for (size_t i{0}; i < MAX_PACKAGE_COUNT + 1; ++i) {
112-
package_too_many.emplace_back(create_placeholder_tx(1, 1));
113-
}
114-
auto result_too_many = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_too_many, /* test_accept */ true);
115-
BOOST_CHECK(result_too_many.m_state.IsInvalid());
116-
BOOST_CHECK_EQUAL(result_too_many.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
117-
BOOST_CHECK_EQUAL(result_too_many.m_state.GetRejectReason(), "package-too-many-transactions");
118-
119-
// Packages can't have a total size of more than 101KvB.
120-
CTransactionRef large_ptx = create_placeholder_tx(150, 150);
121-
Package package_too_large;
122-
auto size_large = GetVirtualTransactionSize(*large_ptx);
123-
size_t total_size{0};
124-
while (total_size <= MAX_PACKAGE_SIZE * 1000) {
125-
package_too_large.push_back(large_ptx);
126-
total_size += size_large;
127-
}
128-
BOOST_CHECK(package_too_large.size() <= MAX_PACKAGE_COUNT);
129-
auto result_too_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, package_too_large, /* test_accept */ true);
130-
BOOST_CHECK(result_too_large.m_state.IsInvalid());
131-
BOOST_CHECK_EQUAL(result_too_large.m_state.GetResult(), PackageValidationResult::PCKG_POLICY);
132-
BOOST_CHECK_EQUAL(result_too_large.m_state.GetRejectReason(), "package-too-large");
133-
134-
// A single, giant transaction submitted through ProcessNewPackage fails on single tx policy.
135-
CTransactionRef giant_ptx = create_placeholder_tx(999, 999);
136-
BOOST_CHECK(GetVirtualTransactionSize(*giant_ptx) > MAX_PACKAGE_SIZE * 1000);
137-
auto result_single_large = ProcessNewPackage(m_node.chainman->ActiveChainstate(), *m_node.mempool, {giant_ptx}, /* test_accept */ true);
138-
BOOST_CHECK(result_single_large.m_state.IsInvalid());
139-
BOOST_CHECK_EQUAL(result_single_large.m_state.GetResult(), PackageValidationResult::PCKG_TX);
140-
BOOST_CHECK_EQUAL(result_single_large.m_state.GetRejectReason(), "transaction failed");
141-
auto it_giant_tx = result_single_large.m_tx_results.find(giant_ptx->GetWitnessHash());
142-
BOOST_CHECK(it_giant_tx != result_single_large.m_tx_results.end());
143-
BOOST_CHECK_EQUAL(it_giant_tx->second.m_state.GetRejectReason(), "tx-size");
144-
145-
// Check that mempool size hasn't changed.
146-
BOOST_CHECK_EQUAL(m_node.mempool->size(), initialPoolSize);
147-
}
14853
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)