mirror of
https://github.com/bitcoin/bitcoin.git
synced 2025-04-12 22:29:33 +02:00
Merge bitcoin/bitcoin#30784: test: add check that too large txs aren't put into orphanage
66d13c870284327abc89d36c0b5cc5f58e96f570 test: add check that large txs aren't put into orphanage (Sebastian Falbesoner)
ed7d2246661ec1789b7db0f21668270f0681ea4a test: add `BulkTransaction` helper to unit test transaction utils (Sebastian Falbesoner)
Pull request description:
This PR adds test coverage for the following check in `TxOrphanage::AddTx`, where large orphan txs are ignored in order to avoid memory exhaustion attacks:
5abb9b1af4/src/txorphanage.cpp (L22-L34)
Note that this code-path isn't reachable under normal circumstances, as txs larger than `MAX_STANDARD_TX_WEIGHT` are already rejected earlier in the course of doing the mempool standardness checks (see `MemPoolAccept::PreChecks` -> `IsStandardTx` -> `reason = "tx-size";`), so this is only relevant if tx standardness rules are disabled via `-acceptnonstdtxns=1`. The ignore path is checked ~~by asserting the debug log, which is ugly, but as far as I know there is currently no way to access the orphanage entries from the outside~~ via unit test that checks the return value of `AddTx`. As an alternative to adding test coverage, one might consider removing this check altogether (or replacing it with an `Assume`), as it's redundant as explained above.
ACKs for top commit:
maflcko:
review ACK 66d13c870284327abc89d36c0b5cc5f58e96f570
glozow:
ACK 66d13c870284327abc89d36c0b5cc5f58e96f570
tdb3:
re-ACK 66d13c870284327abc89d36c0b5cc5f58e96f570
Tree-SHA512: 88e8254ab5fca70c387a5992649ea6a704a65162999be972cc86bd74fc26c5f0f1e13e04856708d07ad5524cb77c0918e19663db92b3593e842469dfe04af6a1
This commit is contained in:
commit
f66011e88f
@ -3,12 +3,15 @@
|
||||
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
|
||||
|
||||
#include <arith_uint256.h>
|
||||
#include <consensus/validation.h>
|
||||
#include <policy/policy.h>
|
||||
#include <primitives/transaction.h>
|
||||
#include <pubkey.h>
|
||||
#include <script/sign.h>
|
||||
#include <script/signingprovider.h>
|
||||
#include <test/util/random.h>
|
||||
#include <test/util/setup_common.h>
|
||||
#include <test/util/transaction_utils.h>
|
||||
#include <txorphanage.h>
|
||||
|
||||
#include <array>
|
||||
@ -370,4 +373,21 @@ BOOST_AUTO_TEST_CASE(get_children)
|
||||
}
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(too_large_orphan_tx)
|
||||
{
|
||||
TxOrphanage orphanage;
|
||||
CMutableTransaction tx;
|
||||
tx.vin.resize(1);
|
||||
|
||||
// check that txs larger than MAX_STANDARD_TX_WEIGHT are not added to the orphanage
|
||||
BulkTransaction(tx, MAX_STANDARD_TX_WEIGHT + 4);
|
||||
BOOST_CHECK_EQUAL(GetTransactionWeight(CTransaction(tx)), MAX_STANDARD_TX_WEIGHT + 4);
|
||||
BOOST_CHECK(!orphanage.AddTx(MakeTransactionRef(tx), 0));
|
||||
|
||||
tx.vout.clear();
|
||||
BulkTransaction(tx, MAX_STANDARD_TX_WEIGHT);
|
||||
BOOST_CHECK_EQUAL(GetTransactionWeight(CTransaction(tx)), MAX_STANDARD_TX_WEIGHT);
|
||||
BOOST_CHECK(orphanage.AddTx(MakeTransactionRef(tx), 0));
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_SUITE_END()
|
||||
|
@ -3,6 +3,7 @@
|
||||
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
|
||||
|
||||
#include <coins.h>
|
||||
#include <consensus/validation.h>
|
||||
#include <script/signingprovider.h>
|
||||
#include <test/util/transaction_utils.h>
|
||||
|
||||
@ -69,3 +70,23 @@ std::vector<CMutableTransaction> SetupDummyInputs(FillableSigningProvider& keyst
|
||||
|
||||
return dummyTransactions;
|
||||
}
|
||||
|
||||
void BulkTransaction(CMutableTransaction& tx, int32_t target_weight)
|
||||
{
|
||||
tx.vout.emplace_back(0, CScript() << OP_RETURN);
|
||||
auto unpadded_weight{GetTransactionWeight(CTransaction(tx))};
|
||||
assert(target_weight >= unpadded_weight);
|
||||
|
||||
// determine number of needed padding bytes by converting weight difference to vbytes
|
||||
auto dummy_vbytes = (target_weight - unpadded_weight + (WITNESS_SCALE_FACTOR - 1)) / WITNESS_SCALE_FACTOR;
|
||||
// compensate for the increase of the compact-size encoded script length
|
||||
// (note that the length encoding of the unpadded output script needs one byte)
|
||||
dummy_vbytes -= GetSizeOfCompactSize(dummy_vbytes) - 1;
|
||||
|
||||
// pad transaction by repeatedly appending a dummy opcode to the output script
|
||||
tx.vout[0].scriptPubKey.insert(tx.vout[0].scriptPubKey.end(), dummy_vbytes, OP_1);
|
||||
|
||||
// actual weight should be at most 3 higher than target weight
|
||||
assert(GetTransactionWeight(CTransaction(tx)) >= target_weight);
|
||||
assert(GetTransactionWeight(CTransaction(tx)) <= target_weight + 3);
|
||||
}
|
||||
|
@ -26,4 +26,8 @@ CMutableTransaction BuildSpendingTransaction(const CScript& scriptSig, const CSc
|
||||
// the second nValues[2] and nValues[3] outputs paid to a TxoutType::PUBKEYHASH.
|
||||
std::vector<CMutableTransaction> SetupDummyInputs(FillableSigningProvider& keystoreRet, CCoinsViewCache& coinsRet, const std::array<CAmount,4>& nValues);
|
||||
|
||||
// bulk transaction to reach a certain target weight,
|
||||
// by appending a single output with padded output script
|
||||
void BulkTransaction(CMutableTransaction& tx, int32_t target_weight);
|
||||
|
||||
#endif // BITCOIN_TEST_UTIL_TRANSACTION_UTILS_H
|
||||
|
Loading…
x
Reference in New Issue
Block a user