Skip to content

Commit

Permalink
Merge #20126: test: p2p_leak_tx.py improvements (use MiniWallet, add …
Browse files Browse the repository at this point in the history
…p2p_lock acquires)

5b77f80 test: add p2p_lock acquires in p2p_leak_tx.py (Sebastian Falbesoner)
cc8c682 test: use MiniWallet for p2p_leak_tx.py (Sebastian Falbesoner)

Pull request description:

  This PR enables one more of the non-wallet functional tests (p2p_leak_tx.py) to be run even with the Bitcoin Core wallet disabled by using the new MiniWallet instead, as proposed in #20078. It also adds missing p2p_lock acquires that need to be held while modifying internal p2p Interface state (in this case the `last_message` dictionary) to avoid data races.

ACKs for top commit:
  laanwj:
    Code review ACK 5b77f80

Tree-SHA512: 6661bc6e3491a9af4bf040f379e5955c525136397e99d3eadde92e247580d0d87efff750e6d3b1f6d9a4e578144a433a982f574ef056b44dd6bca33873a1bae6
  • Loading branch information
MarcoFalke committed Oct 13, 2020
2 parents ad90dd9 + 5b77f80 commit cd6e193
Showing 1 changed file with 11 additions and 8 deletions.
19 changes: 11 additions & 8 deletions test/functional/p2p_leak_tx.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,12 @@
"""Test that we don't leak txs to inbound peers that we haven't yet announced to"""

from test_framework.messages import msg_getdata, CInv, MSG_TX
from test_framework.p2p import P2PDataStore
from test_framework.p2p import p2p_lock, P2PDataStore
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
)
from test_framework.wallet import MiniWallet


class P2PNode(P2PDataStore):
Expand All @@ -21,31 +22,33 @@ class P2PLeakTxTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1

def skip_test_if_missing_module(self):
self.skip_if_no_wallet()

def run_test(self):
gen_node = self.nodes[0] # The block and tx generating node
gen_node.generate(1)
miniwallet = MiniWallet(gen_node)
# Add enough mature utxos to the wallet, so that all txs spend confirmed coins
miniwallet.generate(1)
gen_node.generate(100)

inbound_peer = self.nodes[0].add_p2p_connection(P2PNode()) # An "attacking" inbound peer

MAX_REPEATS = 100
self.log.info("Running test up to {} times.".format(MAX_REPEATS))
for i in range(MAX_REPEATS):
self.log.info('Run repeat {}'.format(i + 1))
txid = gen_node.sendtoaddress(gen_node.getnewaddress(), 0.01)
txid = miniwallet.send_self_transfer(from_node=gen_node)['wtxid']

want_tx = msg_getdata()
want_tx.inv.append(CInv(t=MSG_TX, h=int(txid, 16)))
inbound_peer.last_message.pop('notfound', None)
with p2p_lock:
inbound_peer.last_message.pop('notfound', None)
inbound_peer.send_and_ping(want_tx)

if inbound_peer.last_message.get('notfound'):
self.log.debug('tx {} was not yet announced to us.'.format(txid))
self.log.debug("node has responded with a notfound message. End test.")
assert_equal(inbound_peer.last_message['notfound'].vec[0].hash, int(txid, 16))
inbound_peer.last_message.pop('notfound')
with p2p_lock:
inbound_peer.last_message.pop('notfound')
break
else:
self.log.debug('tx {} was already announced to us. Try test again.'.format(txid))
Expand Down

0 comments on commit cd6e193

Please sign in to comment.