Skip to content

Commit

Permalink
[policy] check for duplicate txids in package
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
glozow committed Sep 13, 2023
1 parent b2ec032 commit 7d7f7a1
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 4 deletions.
7 changes: 7 additions & 0 deletions src/policy/packages.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ bool CheckPackage(const Package& txns, PackageValidationState& state)
std::unordered_set<uint256, SaltedTxidHasher> 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()) {
Expand Down
11 changes: 11 additions & 0 deletions src/test/txpackage_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions test/functional/rpc_packages.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ def test_conflicting(self):
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"]
Expand All @@ -222,8 +222,8 @@ def test_conflicting(self):
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")
Expand Down

0 comments on commit 7d7f7a1

Please sign in to comment.