From 7d7f7a1189432b1b6245ba25df572229870567cb Mon Sep 17 00:00:00 2001 From: glozow Date: Wed, 13 Sep 2023 15:33:32 +0100 Subject: [PATCH] [policy] check for duplicate txids in package Duplicates of normal transactions would be found by looking for conflicting inputs, but this doesn't catch identical empty transactions. These wouldn't be valid but exiting early is good and AcceptPackage's result sanity checks assume non-duplicate transactions. --- src/policy/packages.cpp | 7 +++++++ src/test/txpackage_tests.cpp | 11 +++++++++++ test/functional/rpc_packages.py | 8 ++++---- 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/src/policy/packages.cpp b/src/policy/packages.cpp index 6e70a94088..a901ef8f38 100644 --- a/src/policy/packages.cpp +++ b/src/policy/packages.cpp @@ -37,6 +37,13 @@ bool CheckPackage(const Package& txns, PackageValidationState& state) std::unordered_set later_txids; std::transform(txns.cbegin(), txns.cend(), std::inserter(later_txids, later_txids.end()), [](const auto& tx) { return tx->GetHash(); }); + + // Package must not contain any duplicate transactions, which is checked by txid. This also + // includes transactions with duplicate wtxids and same-txid-different-witness transactions. + if (later_txids.size() != txns.size()) { + return state.Invalid(PackageValidationResult::PCKG_POLICY, "package-contains-duplicates"); + } + for (const auto& tx : txns) { for (const auto& input : tx->vin) { if (later_txids.find(input.prevout.hash) != later_txids.end()) { diff --git a/src/test/txpackage_tests.cpp b/src/test/txpackage_tests.cpp index c08d2748a6..01f0a836a3 100644 --- a/src/test/txpackage_tests.cpp +++ b/src/test/txpackage_tests.cpp @@ -66,6 +66,17 @@ BOOST_FIXTURE_TEST_CASE(package_sanitization_tests, TestChain100Setup) BOOST_CHECK(!CheckPackage(package_too_large, state_too_large)); BOOST_CHECK_EQUAL(state_too_large.GetResult(), PackageValidationResult::PCKG_POLICY); BOOST_CHECK_EQUAL(state_too_large.GetRejectReason(), "package-too-large"); + + // Packages can't contain transactions with the same txid. + Package package_duplicate_txids_empty; + for (auto i{0}; i < 3; ++i) { + CMutableTransaction empty_tx; + package_duplicate_txids_empty.emplace_back(MakeTransactionRef(empty_tx)); + } + PackageValidationState state_duplicates; + BOOST_CHECK(!CheckPackage(package_duplicate_txids_empty, state_duplicates)); + BOOST_CHECK_EQUAL(state_duplicates.GetResult(), PackageValidationResult::PCKG_POLICY); + BOOST_CHECK_EQUAL(state_duplicates.GetRejectReason(), "package-contains-duplicates"); } BOOST_FIXTURE_TEST_CASE(package_validation_tests, TestChain100Setup) diff --git a/test/functional/rpc_packages.py b/test/functional/rpc_packages.py index ae1a498e28..9c4960aa1e 100755 --- a/test/functional/rpc_packages.py +++ b/test/functional/rpc_packages.py @@ -212,8 +212,8 @@ class RPCPackagesTest(BitcoinTestFramework): coin = self.wallet.get_utxo() # tx1 and tx2 share the same inputs - tx1 = self.wallet.create_self_transfer(utxo_to_spend=coin) - tx2 = self.wallet.create_self_transfer(utxo_to_spend=coin) + tx1 = self.wallet.create_self_transfer(utxo_to_spend=coin, fee_rate=DEFAULT_FEE) + tx2 = self.wallet.create_self_transfer(utxo_to_spend=coin, fee_rate=2*DEFAULT_FEE) # Ensure tx1 and tx2 are valid by themselves assert node.testmempoolaccept([tx1["hex"]])[0]["allowed"] @@ -222,8 +222,8 @@ class RPCPackagesTest(BitcoinTestFramework): self.log.info("Test duplicate transactions in the same package") testres = node.testmempoolaccept([tx1["hex"], tx1["hex"]]) assert_equal(testres, [ - {"txid": tx1["txid"], "wtxid": tx1["wtxid"], "package-error": "conflict-in-package"}, - {"txid": tx1["txid"], "wtxid": tx1["wtxid"], "package-error": "conflict-in-package"} + {"txid": tx1["txid"], "wtxid": tx1["wtxid"], "package-error": "package-contains-duplicates"}, + {"txid": tx1["txid"], "wtxid": tx1["wtxid"], "package-error": "package-contains-duplicates"} ]) self.log.info("Test conflicting transactions in the same package")