maxmustermann/dogecoin
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge #18247: test: Wait for both veracks in add_p2p_connection

Browse source 
faf1d04731 test: Remove redundant sync_with_ping after add_p2p_connection (MarcoFalke)
fa90647045 test: Wait for both veracks in add_p2p_connection (MarcoFalke)

Pull request description:

  This fixes the race in p2p_blocksonly

  E.g. https://travis-ci.org/MarcoFalke/bitcoin-core/jobs/657038844#L4500

  ```
   ...
   test  2020-03-01T20:58:28.825000Z TestFramework.mininode (DEBUG): Closed connection to: 127.0.0.1:11828
   node0 2020-03-01T20:58:28.825642Z [net] disconnecting peer=0
   node0 2020-03-01T20:58:28.825826Z [net] Cleared nodestate for peer=0
   node0 2020-03-01T20:58:28.875835Z [http] Received a POST request for / from 127.0.0.1:53448
   node0 2020-03-01T20:58:28.876067Z [httpworker.0] ThreadRPCServer method=getmempoolinfo user=__cookie__
   test  2020-03-01T20:58:28.877000Z TestFramework.mininode (DEBUG): Connecting to Bitcoin Node: 127.0.0.1:11828
   test  2020-03-01T20:58:28.878000Z TestFramework.mininode (DEBUG): Connected & Listening: 127.0.0.1:11828
   test  2020-03-01T20:58:28.878000Z TestFramework.mininode (DEBUG): Send message to 127.0.0.1:11828: msg_version(nVersion=70014 nServices=9 nTime=Sun Mar  1 20:58:28 2020 addrTo=CAddress(nServices=1 ip=127.0.0.1 port=11828) addrFrom=CAddress(nServices=1 ip=0.0.0.0 port=0) nNonce=0x164D5DEB952A4A0B strSubVer=b'/python-mininode-tester:0.0.3/' nStartingHeight=-1 nRelay=1)
   node0 2020-03-01T20:58:28.883808Z [net] Added connection peer=1
   node0 2020-03-01T20:58:28.883950Z [net] connection from 127.0.0.1:33798 accepted
   node0 2020-03-01T20:58:28.884300Z [msghand] received: version (116 bytes) peer=1
   node0 2020-03-01T20:58:28.884483Z [msghand] sending version (114 bytes) peer=1
   node0 2020-03-01T20:58:28.884700Z [msghand] send version message: version 70015, blocks=200, us=[::]:0, peer=1
   node0 2020-03-01T20:58:28.884765Z [msghand] sending verack (0 bytes) peer=1
   test  2020-03-01T20:58:28.885000Z TestFramework.mininode (DEBUG): Received message from 127.0.0.1:11828: msg_version(nVersion=70015 nServices=1033 nTime=Sun Mar  1 20:58:28 2020 addrTo=CAddress(nServices=0 ip=0.0.0.0 port=0) addrFrom=CAddress(nServices=1033 ip=0.0.0.0 port=0) nNonce=0x4A0F2F4C549B3399 strSubVer=b'/Satoshi:0.19.99(testnode0)/' nStartingHeight=200 nRelay=0)
   test  2020-03-01T20:58:28.885000Z TestFramework.mininode (DEBUG): Send message to 127.0.0.1:11828: msg_verack()
   test  2020-03-01T20:58:28.885000Z TestFramework.mininode (DEBUG): Received message from 127.0.0.1:11828: msg_verack()
   node0 2020-03-01T20:58:28.885004Z [msghand] receive version message: /python-mininode-tester:0.0.3/: version 70014, blocks=-1, us=127.0.0.1:11828, peer=1
   test  2020-03-01T20:58:28.886000Z TestFramework (INFO): Check that txs from rpc are not rejected and relayed to other peers
   node0 2020-03-01T20:58:28.886556Z [http] Received a POST request for / from 127.0.0.1:53448
   node0 2020-03-01T20:58:28.886783Z [httpworker.1] ThreadRPCServer method=getpeerinfo user=__cookie__
   node0 2020-03-01T20:58:28.889032Z [http] Received a POST request for / from 127.0.0.1:53448
   node0 2020-03-01T20:58:28.889294Z [httpworker.2] ThreadRPCServer method=testmempoolaccept user=__cookie__
   node0 2020-03-01T20:58:28.891655Z [http] Received a POST request for / from 127.0.0.1:53448
   node0 2020-03-01T20:58:28.891963Z [httpworker.3] ThreadRPCServer method=sendrawtransaction user=__cookie__
   node0 2020-03-01T20:58:28.893115Z [httpworker.3] Enqueuing TransactionAddedToMempool: txid=af34fc5ff9ea8babbd4083fbb79ffd2ad5aff1d6def803c07ca5aeed880bd60f wtxid=af34fc5ff9ea8babbd4083fbb79ffd2ad5aff1d6def803c07ca5aeed880bd60f
   node0 2020-03-01T20:58:28.893443Z [scheduler] TransactionAddedToMempool: txid=af34fc5ff9ea8babbd4083fbb79ffd2ad5aff1d6def803c07ca5aeed880bd60f wtxid=af34fc5ff9ea8babbd4083fbb79ffd2ad5aff1d6def803c07ca5aeed880bd60f
   node0 2020-03-01T20:58:28.894814Z [msghand] received: verack (0 bytes) peer=1
   node0 2020-03-01T20:58:28.894937Z [msghand] sending sendheaders (0 bytes) peer=1
   node0 2020-03-01T20:58:28.895087Z [msghand] sending sendcmpct (9 bytes) peer=1
   node0 2020-03-01T20:58:28.895235Z [msghand] sending sendcmpct (9 bytes) peer=1
   node0 2020-03-01T20:58:28.895430Z [msghand] sending ping (8 bytes) peer=1
   node0 2020-03-01T20:58:28.895896Z [msghand] initial getheaders (199) to peer=1 (startheight:-1)
   test  2020-03-01T20:58:28.896000Z TestFramework.mininode (DEBUG): Received message from 127.0.0.1:11828: msg_sendheaders()
   node0 2020-03-01T20:58:28.896016Z [msghand] sending getheaders (645 bytes) peer=1
   node0 2020-03-01T20:58:28.896607Z [msghand] sending feefilter (8 bytes) peer=1
   test  2020-03-01T20:58:28.897000Z TestFramework.mininode (DEBUG): Received message from 127.0.0.1:11828: msg_sendcmpct(announce=False, version=2)
   test  2020-03-01T20:58:28.897000Z TestFramework.mininode (DEBUG): Received message from 127.0.0.1:11828: msg_sendcmpct(announce=False, version=1)
   test  2020-03-01T20:58:28.897000Z TestFramework.mininode (DEBUG): Received message from 127.0.0.1:11828: msg_ping(nonce=f735096062d217b5)
   test  2020-03-01T20:58:28.897000Z TestFramework.mininode (DEBUG): Send message to 127.0.0.1:11828: msg_pong(nonce=f735096062d217b5)
   test  2020-03-01T20:58:28.897000Z TestFramework.mininode (DEBUG): Received message from 127.0.0.1:11828: msg_getheaders(locator=CBlockLocator(nVersion=70014 vHave=[48924041037103782797700918670732352379567180837453042168545380831411841797392, 28010422273815860773972769588722664110955084223364219183119416607410792753789, 5954376895683677137597080246740451260829355661937599865380797589540815086241, 14500403275336359851183244421245184901482464358719551678581030092830439955257, 17853919108052771837249729512111680264864054213441538187113939176285784834878, 28843166929059356839755035875664073555480989477... (msg truncated)
   test  2020-03-01T20:58:28.897000Z TestFramework.mininode (DEBUG): Received message from 127.0.0.1:11828: msg_feefilter(feerate=000003e8)
   node0 2020-03-01T20:58:28.898144Z [msghand] received: pong (8 bytes) peer=1
   node0 2020-03-01T20:59:28.338539Z [scheduler] Feeding 13446 bytes of dynamic environment data into RNG
   test  2020-03-01T20:59:28.908000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: ''''
                                             def test_function():
                                                 assert self.is_connected
                                                 if not self.last_message.get('tx'):
                                                     return False
                                                 return self.last_message['tx'].tx.rehash() == txid
                                     '''
   test  2020-03-01T20:59:28.908000Z TestFramework (ERROR): Assertion failed
                                     Traceback (most recent call last):
                                       File "/home/travis/build/MarcoFalke/bitcoin-core/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/test_framework.py", line 112, in main
                                         self.run_test()
                                       File "/home/travis/build/MarcoFalke/bitcoin-core/build/bitcoin-x86_64-pc-linux-gnu/test/functional/p2p_blocksonly.py", line 57, in run_test
                                         self.nodes[0].p2p.wait_for_tx(txid)
                                       File "/home/travis/build/MarcoFalke/bitcoin-core/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/mininode.py", line 369, in wait_for_tx
                                         wait_until(test_function, timeout=timeout, lock=mininode_lock)
                                       File "/home/travis/build/MarcoFalke/bitcoin-core/build/bitcoin-x86_64-pc-linux-gnu/test/functional/test_framework/util.py", line 234, in wait_until
                                         raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
                                     AssertionError: Predicate ''''
                                             def test_function():
                                                 assert self.is_connected
                                                 if not self.last_message.get('tx'):
                                                     return False
                                                 return self.last_message['tx'].tx.rehash() == txid
                                     ''' not true after 60 seconds

ACKs for top commit:
  jonatack:
    ACK faf1d04

Tree-SHA512: 3b1a38a5c87d11c610eee0988f0c4af9bfcd978df9ac718ef611f663df2fd4a0eb04e077df5e940d15971bb2f22328fb6021cacccb6902f1e527f288ad2c4a2c
This commit is contained in:
MarcoFalke 2020-04-01 05:08:30 +02:00
parent eb773f1063 faf1d04731
commit 67a72466e1
No known key found for this signature in database
GPG key ID: CE2B75697E69A548
5 changed files with 33 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
test/functional/p2p_dos_header_tree.py
View file

@ -47,8 +47,7 @@ class RejectLowDifficultyHeadersTest(BitcoinTestFramework):
self.log.info("Feed all non-fork headers, including and up to the first checkpoint")
self.nodes[0].add_p2p_connection(P2PInterface())
self.nodes[0].p2p.send_message(msg_headers(self.headers))
self.nodes[0].p2p.sync_with_ping()
self.nodes[0].p2p.send_and_ping(msg_headers(self.headers))
assert {
'height': 546,
'hash': '000000002a936ca763904c3c35fce2f3556c559c0214345d31b1bcebf76acb70',
@ -65,8 +64,7 @@ class RejectLowDifficultyHeadersTest(BitcoinTestFramework):
# On node 0 it succeeds because checkpoints are disabled
self.restart_node(0, extra_args=['-nocheckpoints'])
self.nodes[0].add_p2p_connection(P2PInterface())
self.nodes[0].p2p.send_message(msg_headers(self.headers_fork))
self.nodes[0].p2p.sync_with_ping()
self.nodes[0].p2p.send_and_ping(msg_headers(self.headers_fork))
assert {
"height": 2,
"hash": "00000000b0494bd6c3d5ff79c497cfce40831871cbf39b1bc28bd1dac817dc39",
@ -76,8 +74,7 @@ class RejectLowDifficultyHeadersTest(BitcoinTestFramework):
# On node 1 it succeeds because no checkpoint has been reached yet by a chain tip
self.nodes[1].add_p2p_connection(P2PInterface())
self.nodes[1].p2p.send_message(msg_headers(self.headers_fork))
self.nodes[1].p2p.sync_with_ping()
self.nodes[1].p2p.send_and_ping(msg_headers(self.headers_fork))
assert {
"height": 2,
"hash": "00000000b0494bd6c3d5ff79c497cfce40831871cbf39b1bc28bd1dac817dc39",

1
test/functional/p2p_invalid_messages.py
View file

@ -140,7 +140,6 @@ class InvalidMessagesTest(BitcoinTestFramework):
# Node is still up.
conn = node.add_p2p_connection(P2PDataStore())
conn.sync_with_ping()
def test_magic_bytes(self):
conn = self.nodes[0].add_p2p_connection(P2PDataStore())

8
test/functional/p2p_leak.py
View file

@ -19,6 +19,7 @@ from test_framework.util import wait_until
banscore = 10
class CLazyNode(P2PInterface):
def __init__(self):
super().__init__()
@ -88,6 +89,7 @@ class CNodeNoVerackIdle(CLazyNode):
self.send_message(msg_ping())
self.send_message(msg_getaddr())
class P2PLeakTest(BitcoinTestFramework):
def set_test_params(self):
self.num_nodes = 1
@ -96,7 +98,11 @@ class P2PLeakTest(BitcoinTestFramework):
def run_test(self):
no_version_bannode = self.nodes[0].add_p2p_connection(CNodeNoVersionBan(), send_version=False, wait_for_verack=False)
no_version_idlenode = self.nodes[0].add_p2p_connection(CNodeNoVersionIdle(), send_version=False, wait_for_verack=False)
no_verack_idlenode = self.nodes[0].add_p2p_connection(CNodeNoVerackIdle())
no_verack_idlenode = self.nodes[0].add_p2p_connection(CNodeNoVerackIdle(), wait_for_verack=False)
# Wait until we got the verack in response to the version. Though, don't wait for the other node to receive the
# verack, since we never sent one
no_verack_idlenode.wait_for_verack()
wait_until(lambda: no_version_bannode.ever_connected, timeout=10, lock=mininode_lock)
wait_until(lambda: no_version_idlenode.ever_connected, timeout=10, lock=mininode_lock)

9
test/functional/p2p_timeouts.py
View file

@ -27,11 +27,13 @@ from test_framework.messages import msg_ping
from test_framework.mininode import P2PInterface
from test_framework.test_framework import BitcoinTestFramework
class TestP2PConn(P2PInterface):
def on_version(self, message):
# Don't send a verack in response
pass
class TimeoutsTest(BitcoinTestFramework):
def set_test_params(self):
self.setup_clean_chain = True
@ -41,10 +43,14 @@ class TimeoutsTest(BitcoinTestFramework):
def run_test(self):
# Setup the p2p connections
no_verack_node = self.nodes[0].add_p2p_connection(TestP2PConn())
no_verack_node = self.nodes[0].add_p2p_connection(TestP2PConn(), wait_for_verack=False)
no_version_node = self.nodes[0].add_p2p_connection(TestP2PConn(), send_version=False, wait_for_verack=False)
no_send_node = self.nodes[0].add_p2p_connection(TestP2PConn(), send_version=False, wait_for_verack=False)
# Wait until we got the verack in response to the version. Though, don't wait for the other node to receive the
# verack, since we never sent one
no_verack_node.wait_for_verack()
sleep(1)
assert no_verack_node.is_connected
@ -81,5 +87,6 @@ class TimeoutsTest(BitcoinTestFramework):
assert not no_version_node.is_connected
assert not no_send_node.is_connected
if __name__ == '__main__':
TimeoutsTest().main()

15
test/functional/test_framework/test_node.py
View file

@ -468,7 +468,19 @@ class TestNode():
p2p_conn.peer_connect(**kwargs, net=self.chain)()
self.p2ps.append(p2p_conn)
if wait_for_verack:
# Wait for the node to send us the version and verack
p2p_conn.wait_for_verack()
# At this point we have sent our version message and received the version and verack, however the full node
# has not yet received the verack from us (in reply to their version). So, the connection is not yet fully
# established (fSuccessfullyConnected).
#
# This shouldn't lead to any issues when sending messages, since the verack will be in-flight before the
# message we send. However, it might lead to races where we are expecting to receive a message. E.g. a
# transaction that will be added to the mempool as soon as we return here.
#
# So syncing here is redundant when we only want to send a message, but the cost is low (a few milliseconds)
# in comparision to the upside of making tests less fragile and unexpected intermittent errors less likely.
p2p_conn.sync_with_ping()
return p2p_conn
@ -487,6 +499,7 @@ class TestNode():
p.peer_disconnect()
del self.p2ps[:]
class TestNodeCLIAttr:
def __init__(self, cli, command):
self.cli = cli
@ -498,6 +511,7 @@ class TestNodeCLIAttr:
def get_request(self, *args, **kwargs):
return lambda: self(*args, **kwargs)
def arg_to_cli(arg):
if isinstance(arg, bool):
return str(arg).lower()
@ -506,6 +520,7 @@ def arg_to_cli(arg):
else:
return str(arg)
class TestNodeCLI():
"""Interface to bitcoin-cli for an individual node"""