node_network_limited had a race condition, since wait_for_block()
doesn't do what you might expect. It only checks the most recent block
received over the P2P interface (perhaps we should rename the method
wait_for_most_recent_block() to avoid future confusion). The test can
fail if the node sends us invs for other blocks, we respond with a
getdata, and the node sends us one of those blocks in the 0.05 second
wait_until loop window.
Fix this by not responding to inv messages with getdata messages.
assumevalid.py, example_test.py and p2p-acceptblocks.py add
p2p_connections after the NetworkThread has been started. This isn't
permitted. Fix test to restart the network thread when adding new
connections.
p2p-leaktest.py had a potential race condition if the NetworkThread
hadn't terminated by the time we tried to restart it.
Add network thread_start(), network_thread_running() and
network_thread_join() utility functions in mininode.py and use
network_thread_running() in network thread assertions.
Instead of calling combine_logs from test_runner when the environment variable
PYTHON_DEBUG is set, add a combinedlogslen argument. This makes it easier
to call when running locally, and allows a different length of combined logs
to be printed.
This commit moves the logic that sends a version message
on connection from NodeConn to NodeConnCB. NodeConn should
not be aware of the semantics or meaning of the P2P payloads.
This makes NodeConnCB a subclass of NodeConn, and
removes the need for the client code to know
anything about the implementation details of NodeConnCB.
NodeConn can now be swapped out for any other implementation
of a low-level connection without changing client code.
Currently it's possible to accidentally type e.g.
bitcoin-cli -getinfo getbalance
and get an answer which can be confusing; the trialing arguments are
just ignored.
To avoid this, throw an error if the user provides arguments to
`-getinfo`.
Fixes to sources of intermittent failure in sendheaders.py
- at the start of test_null_locators(), a new block is generated and
then a getheaders is sent. check_last_accouncement() is called to assert
that the headers message is received. However, the new block triggers an
inv to be sent over both P2P connections, so there's a race. If the inv
is received at the wrong time, the test fails.
- test_null_locators() ends by sending a block to the node under test.
At the start of test_nonnull_locators(), a block is mined and
check_last_announcement() is called to assert that the inv received is
for the same block. That means there's a race: if the inv from the block
sent in test_null_locators() is received at the wrong time, the test
fails.
All calls of check_last_announcement() asserted that the return
value was True. Just assert inside the function instead. This
gives better debug information if the assert fails.
Also only check the contents of the most recent inv and header if
check_last_announcement() is called with the relevant argument.
MarcoFalke