32d665c2657793c8b2cc7248d26d80a940acfe20 test: fix "tx-size-small" errors after default address change (Sebastian Falbesoner)
Pull request description:
Addresses #17043, affects RBF and BIP68 functional tests.
The "tx-size-small" policy rule rejects transactions with a non-witness size of
smaller than 82 bytes (see `src/validation.cpp:MemPoolAccept::PreChecks(...)`),
which corresponds to a transaction with 1 segwit input and 1 P2WPKH output.
Through the default address change, the created test transactions have segwit
inputs now and sending to short scriptPubKeys might violate this rule. By
bumping the dummy scriptPubKey size to 22 bytes (= the size of a P2WPKH
scriptPubKey), on all occurences the problem is solved.
The dummy scriptPubKey has the format:
```21 <21-byte-long string of 'a' or 1s>```
ACKs for top commit:
instagibbs:
reACK 32d665c265 just s/Bytes/bytes/
MarcoFalke:
ACK 32d665c2657793c8b2cc7248d26d80a940acfe20
Tree-SHA512: 80e0386ff3c3f462901ba5c1e5ef2cbf095d9c0a40c8c3cfeacd4a3ab676afe744aa95b9eed77b4b3eec88bed930b33aa718117ed0977f6374e858a2f3bd5c57
3fd7e76f6d [tests] Move deterministic address import to setup_nodes (John Newbery)
Pull request description:
This requires a small changes to a few tests, but means that
deterministic addresses will always be imported (unless setup_nodes
behaviour is explicitly overridden).
Tidies up the way we import deterministic addresses, requested in review comment here: https://github.com/bitcoin/bitcoin/pull/14468#discussion_r225594586.
Tree-SHA512: 2b32edf500e286c463398487ab1153116a1dc90f64a53614716373311abdc83d8a251fdd8f42d1146b56e308664deaf62952113f66e98bc37f23968096d1a961
cf9ed307e6 qa: blocktools enforce named args for amount (MarcoFalke)
Pull request description:
Since #13669 changed some signatures, I think it might be worthwhile to enforce named args for primitive types such as amounts.
Tree-SHA512: 2733e7b6a20590b54bd54e81a09e3f5e2fadf4390bed594916b70729bcf485b048266012c1203369e0968032a2c6a2719107ac17ee925d8939af3df916eab1a6
Issues with current implementation: params list is not mentioning `baseBlockHashes`, `baseBlockHashesNb` looks excessive, no default values, handling of baseBlockHash-es is off by 1 (`3 + i` should be `4 + i`).
before:
```
> help quorum rotationinfo
quorum rotationinfo "blockRequestHash" baseBlockHashesNb extraShare
Get quorum rotation information
Arguments:
1. blockRequestHash (string, required) The blockHash of the request.
2. baseBlockHashesNb (numeric, required) Number of baseBlockHashes
3. extraShare (boolean, required) Extra share
```
after:
```
> help quorum rotationinfo
quorum rotationinfo "blockRequestHash" ( extraShare "baseBlockHash..." )
Get quorum rotation information
Arguments:
1. blockRequestHash (string, required) The blockHash of the request.
2. extraShare (boolean, optional, default=false) Extra share
3. baseBlockHash... (string, optional, default=) baseBlockHashes
```
* tests: move `move_to_next_cycle` to `DashTestFramework`
* tests: set correct defaults for `mine_cycle_quorum`
* tests: use correct quorum type in `create_islock`
* tests: fix `rpc_verifyislock.py`
* tests: fix `feature_llmq_is_cl_conflicts.py`
* tests: isolate zmq subscribers in `interface_zmq_dash.py`
this lets us call `test_*_publishers()` in any order and any number of times
* tests: check zmq for both deterministic and non-deterministic islocks
* tests: fix `wait_for_*` methods
should not override `all_ok` once it's `False`
* tests: force non-mns to switch to "fully synced" mnsync status in `setup_network`
like we do this for the controller node and all mns already
* Added GET_SNAPSHOT_INFO message handling
* Quorum members by rotation
* Quorum utils functions
* Handle GET_QUORUM_ROTATION_INFO with baseBlockHash from client
* Storing QuorumSnaphots in evoDB when requesting them
* Added DIP Enforcement param
* quorumIndex cache
* Quorum Rotation deployment control
* Usage of Bitsets for storing CQuorumSnapshots
* Correct handling of early quorum quarters
* More asserts
* Corrections
* Handling of quorumIndex
* Refactoring of truncate mechanism
* Various fixes
* Interface correction
* Added template type for indexed cache
* Added quorumIndex into commitmenHash
* Various changes
* Needs to update maqQuorumsCache along with indexedQuorumsCache
* Added CFinalCommitment version 2
* Renamed variables
* Fixes
* Refactoring & correct caching of quorumMembers by rotation
* Added assertions
* Refactoring
* Interface change
* Handling of previous DKG session failure
* Applied refactoring
* Build quarter members improvments
* Merge Quorum Rotation and Decreased fee into one deployment (DIP24)
* Added new LLMQ Type
* Added functional tests + refactoring
* Refactoring
* Spreaded Quorum creation and Quorum Index adaptation
* quorumIndex adaptations
* Added quorumIndex in CFinalCommitment
* Latest work
* Final refactoring
* Batch of refactoring
* Fixes for tests
* Fix for CFinalCommitment
* Fix for Quorums
* Fix
* Small changes
* Thread sync fic
* Safety changes
* Reuse mns when needed
* Refactoring
* More refactoring
* Fixes for rotationinfo handling
* Fix for rotation of members
* Correct order of MNs lists in Quorum Snapshots
* Adding extra logs
* Sync rotation quorums + qrinfo changes
* Fix + extra logs
* Removed redundant field
* Fix for null final commitment + refactoring
* Added timers in tests
* Fix for qrinfo message: quorumdiff and merkleRootQuorums
* Small changes for rotation test
* Remove reading from scanQuorumCache
* Added quorum list output
* Crash fix
* Experimental commit
* apply changes to specialtxman.cpp from specialtx.cpp
* all the changes
* substancially speed up feature_llmq_rotation.py
* reenable asserts, add check for reorgs
* Refactoring
* Added extra logs
* format
* trivial
* drop extra boost includes
* drop ContainsMN
* fix ScanQuorums
* check quorum hash and index in CFinalCommitment::Verify
* fix/tweak tests
* IsQuorumRotationEnabled should be aware of the context
* Calculating members based on earlier block.
* Fix for Quorum Members Cache
* Removed duplicate size of baseBlockHashes
* Adaptations of qrinfo to -8 mn lists
* Introduction of llmqTypeDIP24InstantSend
* Adaptation for llmqTypeDIP24InstantSend
* Adaptations for IS
* bump protocol version
* Added feature_llmq_is_migration test
* Various cleanups
* use unordered_lru_cache for quorumSnapshotCache
* trivial refactor ComputeQuorumMembersByQuarterRotation
* Reduced CFinalCommitment::quorumIndex from 32 to 16 bits
* Keep verified LLMQ relay connections
* Experimental Relay connection fix
* Fix for EnsureQuorumConnections rotation
* Using only valid Mns for checking
* Override of nPowTargetSpacing (devnet only)
* Show penalty score in masternode rpc
* fixups
* Rotation refactoring
* Update src/chainparams.cpp
* Replaced LogPrintf with LogPrint
* IS locking fix once DIP24 activation
* Various cleanup
* Updated MIN_MASTERNODE_PROTO_VERSION
* Introduce LLMQ_TEST_INSTANTSEND reg-test only quorum and actually test switching to dip0024 quorums
* Renamed field lastQuorumHashPerIndex
* Renamed to DIP0024
* chore: update nStartTime and nTimeout for mainnet / testnet for DEPLOYMENT_DIP0024
Co-authored-by: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
460fa8e0d9c1d08b5b5bca3fa02ba2763337aaf1 test: remove `import socket` in test_ipv6_local (brunoerg)
Pull request description:
Since this module (`socket`) is imported at the top of file, there is no need to import it again within the function.
ACKs for top commit:
MarcoFalke:
cr ACK 460fa8e0d9c1d08b5b5bca3fa02ba2763337aaf1
Tree-SHA512: 031c17a776dedaa21b3ec6458ca822304e76a5a3f4494406e6b7b04f08cc2abefcfe742c462b60c9b3e2fee3cd110a69ed5ad413357886dc7b823abc916ea40e
5db506ba5943868cc2c845f717508739b7f05714 tests: Add option --valgrind to run nodes under valgrind in the functional tests (practicalswift)
Pull request description:
What is better than fixing bugs? Fixing entire bug classes of course! :)
Add option `--valgrind` to run the functional tests under Valgrind.
Regular functional testing under Valgrind would have caught many of the uninitialized reads we've seen historically.
Let's kill this bug class once and for all: let's never use an uninitialized value ever again. Or at least not one that would be triggered by running the functional tests! :)
My hope is that this addition will make it super-easy to run the functional tests under Valgrind and thus increase the probability of people making use of it :)
Hopefully `test/functional/test_runner.py --valgrind` will become a natural part of the pre-release QA process.
**Usage:**
```
$ test/functional/test_runner.py --help
…
--valgrind run nodes under the valgrind memory error detector:
expect at least a ~10x slowdown, valgrind 3.14 or
later required
```
**Live demo:**
First, let's re-introduce a memory bug by reverting the recent P2P uninitialized read bug fix from PR #17624 ("net: Fix an uninitialized read in ProcessMessage(…, "tx", …) when receiving a transaction we already have").
```
$ git diff
diff --git a/src/consensus/validation.h b/src/consensus/validation.h
index 3401eb64c..940adea33 100644
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@@ -114,7 +114,7 @@ inline ValidationState::~ValidationState() {};
class TxValidationState : public ValidationState {
private:
- TxValidationResult m_result = TxValidationResult::TX_RESULT_UNSET;
+ TxValidationResult m_result;
public:
bool Invalid(TxValidationResult result,
const std::string &reject_reason="",
```
Second, let's test as normal without Valgrind:
```
$ test/functional/p2p_segwit.py -l INFO
2019-11-28T09:30:42.810000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test__fc8q3qo
…
2019-11-28T09:31:57.187000Z TestFramework (INFO): Subtest: test_non_standard_witness_blinding (Segwit active = True)
…
2019-11-28T09:32:08.265000Z TestFramework (INFO): Tests successful
```
Third, let's test with `--valgrind` and see if the test fail (as we expect) when the unitialized value is used:
```
$ test/functional/p2p_segwit.py -l INFO --valgrind
2019-11-28T09:32:33.018000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_gtjecx2l
…
2019-11-28T09:40:36.702000Z TestFramework (INFO): Subtest: test_non_standard_witness_blinding (Segwit active = True)
2019-11-28T09:40:37.813000Z TestFramework (ERROR): Assertion failed
ConnectionRefusedError: [Errno 111] Connection refused
```
ACKs for top commit:
MarcoFalke:
ACK 5db506ba5943868cc2c845f717508739b7f05714
jonatack:
ACK 5db506ba5943868cc2c845f717508739b7f05714
Tree-SHA512: 2eaecacf4da166febad88b2a8ee6d7ac2bcd38d4c1892ca39516b6343e8f8c8814edf5eaf14c90f11a069a0389d24f0713076112ac284de987e72fc5f6cc3795
* Compressed headers implementation.
First header is always compressed in a headers2 msg
Version is uncompressed if it’s not matched within the last 7 unique versions to be sent in the current msg
Service flag to signal that the peer supports compressed headers
If compressed headers services is active, the peer will receive headers compressed
If both sendheaders and sendheaders2 are sent, the peer will respond with compressed headers
Functional tests as for uncompressed headers
Updates regarding the existing functional tests to use the compressed headers if the NODE_HEADERS_COMPRESSED service flag is active
* style: add missing comma
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
fab17e8272f5f70213f186809479ee7a75898b1d test: Add basic test for BIP34 (MarcoFalke)
Pull request description:
BIP34 was disabled for testing, which explains why it had no test.
Fix that by enabling it and adding a test.
Tree-SHA512: 9cb5702d474117ce6420226eb93ee09d6fb5fc856fabc8b67abe56a088cd727674e0e5462000e1afa83b911374036f90abdbdde56a8c236a75572ed47e10a00f
fa178a6385 [rpc] mining: Omit uninitialized currentblockweight, currentblocktx (MarcoFalke)
Pull request description:
Previously we'd report "0", which could be mistaken for a valid number. E.g. the number of transactions is 0 or the block weight is 0, whatever that means.
Tree-SHA512: ee94ab203a329e272211b726f4c23edec4b09c650ec363b77fd59ad9264165d73064f78ebb9e11b5c2c543b73c157752410a307655560531c7d5444d203aa0ea
e3e1a5631e [test] functional: set cwd of nodes to tmpdir (Sjors Provoost)
Pull request description:
Any process launched by bitcoind will have `self.datadir` as its `cwd`.
Tree-SHA512: 0b311643bb96c7dc2f693774620173243b3add40bf373284695af2f0071823b23485289fd2ffe152b7f63e0bfe989b16720077cfc2ce33905f9b8e7f2630f3c0
* feat: introduce devnetVersion as a method for breaking changes to devnets
include devnetVersion in expected version for devnet connections
* feat: always use DGW on devnets
* fix p2p_connect_to_devnet.py
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
59e387705c7e55ec40400301346354fa2d0c613f test: add invalid tx templates for use in functional tests (James O'Beirne)
Pull request description:
This change adds a list of `CTransaction`-generating templates which each correspond to a specific type of invalid transaction. We then use this list to test for a wider variety of invalid tx types in `p2p_invalid_tx.py` and `feature_block.py`.
Consolidating all invalid tx types will allow us to more easily cover all tx reject cases from a variety of tests without repeating ourselves. Validation logic doesn't differ much between mempool and block acceptance, but there *is* a difference and we should be sure we're testing both comprehensively.
Right now, I've only added templates covering the tx reject types listed below but if this approach seems worthwhile I will expand the list to be fully comprehensive.
```
bad-txns-in-belowout
bad-txns-inputs-duplicate
bad-txns-too-many-sigops
bad-txns-vin-empty
bad-txns-vout-empty
bad-txns-vout-negative
```
Tree-SHA512: 05407f4a953fbd7c44c08bb49bb989cefd39a2b05ea00f5b3c92197a3f05e1b302f789e33832445734220e1c333d133aba385740b77b84139b170c583471ce20
568a1d72619371a45b14a8356d3f80bd0c0efabc fix ecdsa verify in test framework (Stepan Snigirev)
Pull request description:
This PR fixes a small bug in the test framework in `verify_ecdsa` function.
`r` in ecdsa signature is modulo curve order, so if the point `R` calculated during verification has x-coordinate that is larger than the curve order, the verification will fail in the test framework but pass in libsecp256k1.
Example (all in hex):
public key: `0289d889551598a0263746c01e5882ccf9b7dc4ca5a37108482c9d80de40e0a8cf`
der signature: `3006020104020104` (r = 4, s = 4)
message: `3232323232323232323232323232323232323232323232323232323232323232`
libsecp256k1 returns `true`, test framework returns `false`.
ACKs for top commit:
sipa:
utACK 568a1d72619371a45b14a8356d3f80bd0c0efabc
Tree-SHA512: 9e9c58498f10085d2ad85e95caff6c92793799d2a40696ef43febcd7d313c8c3d5ecec715ca903cbb8432a8a96bd0065d86d060966d4ee651c3871ce16c252bf
fad21a1a7aa8804f4699e5821f074f5d3845c78b test: Explain that a bug should be filed when the test fail (MarcoFalke)
Pull request description:
Without a bug report it is harder to fix the issue
ACKs for top commit:
hebasto:
ACK fad21a1a7aa8804f4699e5821f074f5d3845c78b, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.
fanquake:
ACK fad21a1a7aa8804f4699e5821f074f5d3845c78b
Tree-SHA512: db194e8f8c0f07b2f4c9ef27e456510959f89da69435cee71605d720e0ad06f18700973f5af25ea31a190b933eb35f2743f014878aa3f8293500e06b4907ebbd
a67352161c68fea9764cc31aff199f112d8572c6 test: skip tool_wallet test when bitcoin-wallet isn't compiled (fanquake)
e9277baed64e1d4054a102e40b39a9aed7839c2f test: skip wallet_listreceivedby test when the cli isn't compiled (fanquake)
621d398750d9f5ce3e7ec75ccb160b3534dcc436 test: skip bitcoin_cli test when the cli isn't compiled (fanquake)
Pull request description:
Don't try and run the `interface_bitcoin_cli.py` test when `bitcoin-cli` isn't available.
```bash
stdout:
2019-11-17T01:51:41.623000Z TestFramework (INFO): Initializing test directory /var/folders/z2/cn877pxd3czdfh47mfkmbwgm0000gn/T/test_runner_₿_🏃_20191116_205141/interface_bitcoin_cli_0
2019-11-17T01:51:41.890000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
File "/Users/michael/github/bitcoin/test/functional/test_framework/test_framework.py", line 111, in main
self.run_test()
File "/Users/michael/github/bitcoin/test/functional/interface_bitcoin_cli.py", line 18, in run_test
cli_response = self.nodes[0].cli("-version").send_cli()
File "/Users/michael/github/bitcoin/test/functional/test_framework/test_node.py", line 528, in send_cli
process = subprocess.Popen(p_args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
File "/Users/michael/.pyenv/versions/3.5.6/lib/python3.5/subprocess.py", line 676, in __init__
restore_signals, start_new_session)
File "/Users/michael/.pyenv/versions/3.5.6/lib/python3.5/subprocess.py", line 1289, in _execute_child
raise child_exception_type(errno_num, err_msg)
FileNotFoundError: [Errno 2] No such file or directory: '/Users/michael/github/bitcoin/src/bitcoin-cli'
```
Top commit has no ACKs.
Tree-SHA512: de27513a615d9d21271a0948e012c3209351e7374efd19bfa1bb9cda77e8fffe15d99e3424e4dbfa8cf826084f8af1670726f4703bd2b6093e7d37df4bea64f0
fa69588537bc91c0aedbc89ef1760d89cbffad75 test: Make PORT_MIN in test runner configurable (MarcoFalke)
Pull request description:
This is needed when some ports in the port range are used by other processes. Note that simply assigning the ports dynamically does not work:
* We spin up several nodes per test (each node gets its own port)
* We run several tests in parallel
So to avoid nodes from different tests colliding on ports, the port assignment must be deterministic (can not be dynamic).
Fixes: #10869
ACKs for top commit:
practicalswift:
ACK fa69588537bc91c0aedbc89ef1760d89cbffad75 -- diff looks correct
promag:
ACK fa69588537bc91c0aedbc89ef1760d89cbffad75.
Tree-SHA512: e79adb015e7de79064e2d14336c38bc9672bd779ad6c52917721897e73f617c39d32c068a369c26670002a6c4ab95a71ef3a6878ebdd9710e02f410e2f7bcd14
96299a9d6c0a6b9125a58a63ee3147e55d1b086b Test: Move common function assert_approx() into util.py (fridokus)
Pull request description:
To reduce code duplication, move `assert_approx` into common framework `util.py`.
`assert_approx()` is used in two functional tests.
ACKs for top commit:
theStack:
ACK 96299a9
practicalswift:
ACK 96299a9d6c0a6b9125a58a63ee3147e55d1b086b -- DRY is good and diff looks correct
fanquake:
ACK 96299a9d6c0a6b9125a58a63ee3147e55d1b086b - thanks for contributing 🍻
Tree-SHA512: 8e9d397222c49536c7b3d6d0756cc5af17113e5af8707ac48a500fff1811167fb2e03f3c0445b0b9e80f34935f4d57cfb935c4790f6f5463a32a67df5f736939
fadfd844de8c53034a97dfa6f771ffe9f523fba2 test: Remove unused connect_nodes_bi (MarcoFalke)
fa3b9ee8b2280af4bcbcfffff275aaf8dd125929 scripted-diff: test: Replace connect_nodes_bi with connect_nodes (MarcoFalke)
faaee1e39a91b3f603881655d3980c29af09852b test: Use connect_nodes when connecting nodes in the test_framework (MarcoFalke)
1111bb91f517838e5b9f778bf6b5a9c8d561e857 test: Reformat python imports to aid scripted diff (MarcoFalke)
Pull request description:
By default all test nodes are connected in a chain. However, instead of just a single connection between each pair of nodes, we end up with up to four connections for a "middle" node (two outbound, two inbound, from each side).
This is generally redundant (tx and block relay should succeed with just a single connection) and confusing. For example, test timeouts after a call to `sync_` may be racy and hard to reproduce. On top of that, the test `debug.log`s are hard to read because txs and block invs may be relayed on the same connection multiple times.
Fix this by inlining `connect_nodes_bi` in the two tests that need it, and then replace it with a single `connect_nodes` in all other tests.
Historic background:
`connect_nodes_bi` has been introduced as a (temporary?) workaround for bug #5113 and #5138, which has long been fixed in #5157 and #5662.
ACKs for top commit:
laanwj:
ACK fadfd844de8c53034a97dfa6f771ffe9f523fba2
jonasschnelli:
utACK fadfd844de8c53034a97dfa6f771ffe9f523fba2 - more of less a cleanup PR.
promag:
Tested ACK fadfd844de8c53034a97dfa6f771ffe9f523fba2, ran extended tests.
Tree-SHA512: 2d027a8fd150749c071b64438a0a78ec922178628a7dbb89fd1212b0fa34febd451798c940101155d3617c0426c2c4865174147709894f1f1bb6cfa336aa7e24
fa8489a15511f61a372473927e73c34692bbec23 test: Add test for BIP30 duplicate tx (MarcoFalke)
77770d95e2838d7665fa8f621e9e83d79f9b3196 test: Properly serialize BIP34 coinbase height (MarcoFalke)
Pull request description:
This adds a test for BIP30 to check that duplicate txs can exist in the blockchain given the first one was completely spent when the second one is added. (Requested by ajtowns in https://github.com/bitcoin/bitcoin/pull/16333#issuecomment-508604071)
We can not add a test that a later duplicate tx overwrites a previous one, because BIP30 is always enforced on regtest. If someone feels strongly about such a test, some Bitcoin Core code would have to be modified, which can be done in a follow up pull request.
Also, add a commit to fix the BIP34 test failures reported in https://github.com/bitcoin/bitcoin/pull/14633#issue-227712540
ACKs for top commit:
laanwj:
Code review ACK fa8489a15511f61a372473927e73c34692bbec23
Tree-SHA512: c707d0bdc93937263876b603425b53322a2a9f9ec3f50716ae2fa9de8ddc644beb22b26c1bfde7f4aab102633e096b354ef380db919176bd2cb44a2828f884aa
fae91a09c453a9a95c382df765bd71e54698d5b2 test: Remove incorrect and unused try-block in assert_debug_log (MarcoFalke)
Pull request description:
This try block has accidentally been added by me in fa3e9f7627784ee00980590e5bf044a0e1249999.
It was unused all the time, but commit 6011c9d72d1df5c2cd09de6f85c21eb4f7eb1ba8 added a `return` in the finally block, muting all exceptions.
This can be tested by adding an `assert False` after any `with ...assert_debug_log...:` line.
ACKs for top commit:
laanwj:
ACK fae91a09c453a9a95c382df765bd71e54698d5b2
ryanofsky:
utACK fae91a09c453a9a95c382df765bd71e54698d5b2. I didn't know returning inside a `finally` block would cancel pending exceptions or return values, but I guess this makes sense and is a good thing to be aware of.
Tree-SHA512: 47ed0165062060e9af055a3e92f1a529cd41d00476bfad64e3cd141ae084d22f926a343bb1257717e164e15459a59ab66aed198c95d18bf780d8cb0b76aa3298
6011c9d72d1df5c2cd09de6f85c21eb4f7eb1ba8 QA: fix rpc_setban.py race (Jonas Schnelli)
Pull request description:
The new `rpc_setban.py` test failes regularly on CIs due to a race between injecting the ban and testing the log "on the other side".
The problem is, that the test immediately after the `addnode` command on node0 checks for the `dropped (banned)` entry on node1 (without giving some time).
Adding a 2 seconds sleep seems to solve the race (I guess there is no better event-driven delay).
Example of a failed test: https://bitcoinbuilds.org/index.php?ansilog=bf743910-103f-4b54-9a97-960c471061bd.log#l2906
Top commit has no ACKs.
Tree-SHA512: 680f8ea3e5ddb07e93f824f1aeff4a459e25e6c14715a39fc7670e50506d7cf25925348672c5c2d8ba3e1243ccf5effbc2456bcd094fb96868349f8d26e008f1
9f9db39041 QA/mininode: Send all headers upfront in send_blocks_and_test to avoid sending an unconnected one (Luke Dashjr)
Pull request description:
While this doesn't currently trigger any problems, the network protocol does expect headers to be sent connectable in normal circumstances, and if too many are sent out of order will disconnect the peer.
ACKs for commit 9f9db3:
Tree-SHA512: 25b88718e4ba3d31aed2de7ece23fab9a0737fd6536c5e618ea8eb5a3a217dab0dffaebc4892df7993bcea7efb7c4fb5085fabebe99535b8f7fdde3c19df54ff
