Skip to content

Commit 8ab0c77

Browse files
committed
Merge bitcoin#22253: validation: distinguish between same tx and same-nonwitness-data tx in mempool
b7a8cd9 [test] submit same txid different wtxid as mempool tx (glozow) fdb4816 [validation] distinguish same txid different wtxid in mempool (glozow) Pull request description: On master, if you submit a transaction with the same txid but different witness to the mempool, it thinks the transactions are the same. Users submitting through `BroadcastTransaction()` (i.e. `sendrawtransaction` or the wallet) don't get notified that there's a different transaction in the mempool, although it doesn't crash. Users submitting through `testmempoolaccept()` will get a "txn-already-in-mempool" error. This PR simply distinguishes between `txn-already-in-mempool` and `txn-same-nonwitness-data-in-mempool`, without handling them differently: `sendrawtransaction` still will not throw, but `testmempoolaccept` will give you a different error. I believe the intention of bitcoin#19645 is to allow full swaps of transactions that have different witnesses but identical nonwitness data. Returning a different error message + adding a test was suggested: bitcoin#19645 (comment) so this is that PR. ACKs for top commit: naumenkogs: ACK b7a8cd9 jnewbery: Code review ACK b7a8cd9 theStack: Code-review ACK b7a8cd9 darosior: re-utACK b7a8cd9 Tree-SHA512: 9c6591edaf8727ba5b4675977adb8cbdef7288584003b6cd659828032dc92d2ae915800a8ef8b6fdffe112c1b660df72297a3dcf2e2e3e1f959c6cb3678c63ee
2 parents d1e4c56 + b7a8cd9 commit 8ab0c77

File tree

3 files changed

+123
-2
lines changed

3 files changed

+123
-2
lines changed

src/validation.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -587,9 +587,13 @@ bool MemPoolAccept::PreChecks(ATMPArgs& args, Workspace& ws)
587587
if (!CheckFinalTx(m_active_chainstate.m_chain.Tip(), tx, STANDARD_LOCKTIME_VERIFY_FLAGS))
588588
return state.Invalid(TxValidationResult::TX_PREMATURE_SPEND, "non-final");
589589

590-
// is it already in the memory pool?
591-
if (m_pool.exists(hash)) {
590+
if (m_pool.exists(GenTxid(true, tx.GetWitnessHash()))) {
591+
// Exact transaction already exists in the mempool.
592592
return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-already-in-mempool");
593+
} else if (m_pool.exists(GenTxid(false, tx.GetHash()))) {
594+
// Transaction with the same non-witness data but different witness (same txid, different
595+
// wtxid) already exists in the mempool.
596+
return state.Invalid(TxValidationResult::TX_CONFLICT, "txn-same-nonwitness-data-in-mempool");
593597
}
594598

595599
// Check for conflicts with in-memory transactions
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
#!/usr/bin/env python3
2+
# Copyright (c) 2021 The Bitcoin Core developers
3+
# Distributed under the MIT software license, see the accompanying
4+
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
5+
"""
6+
Test mempool acceptance in case of an already known transaction
7+
with identical non-witness data different witness.
8+
"""
9+
10+
from test_framework.messages import (
11+
COIN,
12+
COutPoint,
13+
CTransaction,
14+
CTxIn,
15+
CTxInWitness,
16+
CTxOut,
17+
sha256,
18+
)
19+
from test_framework.script import (
20+
CScript,
21+
OP_0,
22+
OP_ELSE,
23+
OP_ENDIF,
24+
OP_EQUAL,
25+
OP_HASH160,
26+
OP_IF,
27+
OP_TRUE,
28+
hash160,
29+
)
30+
from test_framework.test_framework import BitcoinTestFramework
31+
from test_framework.util import (
32+
assert_equal,
33+
)
34+
35+
class MempoolWtxidTest(BitcoinTestFramework):
36+
def set_test_params(self):
37+
self.num_nodes = 1
38+
self.setup_clean_chain = True
39+
40+
def run_test(self):
41+
node = self.nodes[0]
42+
43+
self.log.info('Start with empty mempool and 101 blocks')
44+
# The last 100 coinbase transactions are premature
45+
blockhash = node.generate(101)[0]
46+
txid = node.getblock(blockhash=blockhash, verbosity=2)["tx"][0]["txid"]
47+
assert_equal(node.getmempoolinfo()['size'], 0)
48+
49+
self.log.info("Submit parent with multiple script branches to mempool")
50+
hashlock = hash160(b'Preimage')
51+
witness_script = CScript([OP_IF, OP_HASH160, hashlock, OP_EQUAL, OP_ELSE, OP_TRUE, OP_ENDIF])
52+
witness_program = sha256(witness_script)
53+
script_pubkey = CScript([OP_0, witness_program])
54+
55+
parent = CTransaction()
56+
parent.vin.append(CTxIn(COutPoint(int(txid, 16), 0), b""))
57+
parent.vout.append(CTxOut(int(9.99998 * COIN), script_pubkey))
58+
parent.rehash()
59+
60+
privkeys = [node.get_deterministic_priv_key().key]
61+
raw_parent = node.signrawtransactionwithkey(hexstring=parent.serialize().hex(), privkeys=privkeys)['hex']
62+
parent_txid = node.sendrawtransaction(hexstring=raw_parent, maxfeerate=0)
63+
node.generate(1)
64+
65+
# Create a new transaction with witness solving first branch
66+
child_witness_script = CScript([OP_TRUE])
67+
child_witness_program = sha256(child_witness_script)
68+
child_script_pubkey = CScript([OP_0, child_witness_program])
69+
70+
child_one = CTransaction()
71+
child_one.vin.append(CTxIn(COutPoint(int(parent_txid, 16), 0), b""))
72+
child_one.vout.append(CTxOut(int(9.99996 * COIN), child_script_pubkey))
73+
child_one.wit.vtxinwit.append(CTxInWitness())
74+
child_one.wit.vtxinwit[0].scriptWitness.stack = [b'Preimage', b'\x01', witness_script]
75+
child_one_wtxid = child_one.getwtxid()
76+
child_one_txid = child_one.rehash()
77+
78+
# Create another identical transaction with witness solving second branch
79+
child_two = CTransaction()
80+
child_two.vin.append(CTxIn(COutPoint(int(parent_txid, 16), 0), b""))
81+
child_two.vout.append(CTxOut(int(9.99996 * COIN), child_script_pubkey))
82+
child_two.wit.vtxinwit.append(CTxInWitness())
83+
child_two.wit.vtxinwit[0].scriptWitness.stack = [b'', witness_script]
84+
child_two_wtxid = child_two.getwtxid()
85+
child_two_txid = child_two.rehash()
86+
87+
assert_equal(child_one_txid, child_two_txid)
88+
assert child_one_wtxid != child_two_wtxid
89+
90+
self.log.info("Submit one child to the mempool")
91+
txid_submitted = node.sendrawtransaction(child_one.serialize().hex())
92+
assert_equal(node.getrawmempool(True)[txid_submitted]['wtxid'], child_one_wtxid)
93+
94+
# testmempoolaccept reports the "already in mempool" error
95+
assert_equal(node.testmempoolaccept([child_one.serialize().hex()]), [{
96+
"txid": child_one_txid,
97+
"wtxid": child_one_wtxid,
98+
"allowed": False,
99+
"reject-reason": "txn-already-in-mempool"
100+
}])
101+
testres_child_two = node.testmempoolaccept([child_two.serialize().hex()])[0]
102+
assert_equal(testres_child_two, {
103+
"txid": child_two_txid,
104+
"wtxid": child_two_wtxid,
105+
"allowed": False,
106+
"reject-reason": "txn-same-nonwitness-data-in-mempool"
107+
})
108+
109+
# sendrawtransaction will not throw but quits early when the exact same transaction is already in mempool
110+
node.sendrawtransaction(child_one.serialize().hex())
111+
# sendrawtransaction will not throw but quits early when a transaction with the same non-witness data is already in mempool
112+
node.sendrawtransaction(child_two.serialize().hex())
113+
114+
115+
if __name__ == '__main__':
116+
MempoolWtxidTest().main()

test/functional/test_runner.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@
279279
'feature_asmap.py',
280280
'mempool_unbroadcast.py',
281281
'mempool_compatibility.py',
282+
'mempool_accept_wtxid.py',
282283
'rpc_deriveaddresses.py',
283284
'rpc_deriveaddresses.py --usecli',
284285
'p2p_ping.py',

0 commit comments

Comments
 (0)