qa: Add test for orphan handling #13003
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Concept ACK - nice! |
ryanofsky
reviewed
Apr 17, 2018
| - add all txs to our tx_store | ||
| - send tx messages for all txs | ||
| - if success is True: assert that the tx is accepted to the mempool | ||
| - if success is False: assert that the tx is not accepted to the mempool |
Contributor
There was a problem hiding this comment.
"success is False" line unintentionally deleted?
Contributor
There was a problem hiding this comment.
It looks like including both "if success is ..." lines would be repetitive.
Contributor
There was a problem hiding this comment.
It is a little repetitive, but I think the description is vague without that line because "not asserting" and "asserting not" mean different things.
| network_thread_start() | ||
|
|
||
| self.reconnect_p2p() | ||
| self.bootstrap_p2p() # Add one p2p connection to the node |
Contributor
There was a problem hiding this comment.
Note to reviewers: The updates in feature_block.py just tweak the network_thread_start/network_thread_join calls to happen in a more logical order, and be consistent with other tests. This is internal test cleanup which is not directly related to the other changes in this PR.
| Helper to connect and wait for version handshake.""" | ||
| for _ in range(num_connections): | ||
| self.nodes[0].add_p2p_connection(P2PDataStore()) | ||
| network_thread_start() |
Contributor
There was a problem hiding this comment.
It would be logical to start the network thread adding connections, right? Just asking because it's weird to me to see shutdown order not be the reverse of initialization order like:
network_thread_start()
add_p2p_connection()
...
disconnect_p2ps()
network_thread_join()
Member
Author
There was a problem hiding this comment.
In test/functional/README.md it says "Call network_thread_start() after all P2PInterface objects are created ..."
Member
Author
|
Added a commit to address the documentation nit |
Member
|
utACK fa02c5b |
codablock
added a commit
to codablock/dash
that referenced
this pull request
Jan 8, 2020
bitcoin#13003 was backported out of order which causes missed changes.
UdjinM6
pushed a commit
to dashpay/dash
that referenced
this pull request
Jan 11, 2020
…nTestFramework (#3277) * [tests] Change feature_block.py to use BitcoinTestFramework * [tests] Fix flake8 warnings in feature_block.py * [tests] Tidy up feature_block.py - move all helper methods to the end - remove block, create_tx and create_and_sign_tx shortcuts - remove --runbarelyexpensive option, since it defaults to True and it's unlikely that anyone ever runs the test with this option set to false. * [tests] Add logging to feature_block.py * [tests] Improve assert message when wait_until() fails * Merge bitcoin#13048: [tests] Fix feature_block flakiness c1d7420 [tests] Fix feature_block flakiness (John Newbery) Pull request description: feature_block.py occasionally fails on Travis. I believe this is due to a a race condition when reconnecting to bitcoind after a subtest that expects disconnection. If the test runs ahead and sends the INV for the subsequent test before we've received the initial sync getheaders, then we may end up sending two headers messages - one as a response to the initial sync getheaders and one in response to the INV getheaders. If both of those headers fail validation with a DoS score of 50 or higher, then we'll unexpectedly be disconnected. There is only one validation failure that has a DoS score bewteen 50 and 100, which is high-hash. That's why the test is failing immediately after the "Reject a block with invalid work" subtest. Fix is to wait for the initial getheaders from the peer before we start populating our blockstore. That way we won't have any invalid headers to respond to it with. Tree-SHA512: dc17d795fcfaf0f8c0bf1e9732b5e11fbc8febbfafba4c231b7c13a5404a2c297dcd703a7a75bc7f353c893e12efc87f424f2201abd47ba5268af32d4d2e841f * Temporarely rename MAX_BLOCK_SIZE -> MAX_BLOCK_BASE_SIZE We'll undo this after the next commit. This avoids merge many conflicts and makes reviewing easier. * Rename MAX_BLOCK_BASE_SIZE back to MAX_BLOCK_SIZE * Use DoS score of 100 for bad-blk-sigops This was accidently changed to 10 while backporting bitcoin#7287 and causes test failures in p2p-fullblocktest.py * Use allowOptimisticSend=true when sending reject messages This fixes test failures in p2p-fullblocktest.py which expects reject messages to be sent/received before connections get closed. * Fix p2p-fullblocktest.py - CBlock and friends are still in test_framework.mininode - "-whitelist" causes connections to not be dropped, which in turn causes sync_blocks with reconnect=True to fail - "bad-cb-amount" does not cause a ban in Dash, so reconnect must be False - Dash already bans when a header is received which is a child of an invalid header, causing block requests to never happen * Backport missing changes from bitcoin#13003 bitcoin#13003 was backported out of order which causes missed changes. * Bump p2p-fullblocktest timeouts * Increase RPC timeout in p2p-fullblocktest.py Co-authored-by: John Newbery <[email protected]> Co-authored-by: MarcoFalke <[email protected]>
gades
pushed a commit
to cosanta/cosanta-core
that referenced
this pull request
Jun 30, 2021
…nTestFramework (dashpay#3277) * [tests] Change feature_block.py to use BitcoinTestFramework * [tests] Fix flake8 warnings in feature_block.py * [tests] Tidy up feature_block.py - move all helper methods to the end - remove block, create_tx and create_and_sign_tx shortcuts - remove --runbarelyexpensive option, since it defaults to True and it's unlikely that anyone ever runs the test with this option set to false. * [tests] Add logging to feature_block.py * [tests] Improve assert message when wait_until() fails * Merge bitcoin#13048: [tests] Fix feature_block flakiness c1d7420 [tests] Fix feature_block flakiness (John Newbery) Pull request description: feature_block.py occasionally fails on Travis. I believe this is due to a a race condition when reconnecting to bitcoind after a subtest that expects disconnection. If the test runs ahead and sends the INV for the subsequent test before we've received the initial sync getheaders, then we may end up sending two headers messages - one as a response to the initial sync getheaders and one in response to the INV getheaders. If both of those headers fail validation with a DoS score of 50 or higher, then we'll unexpectedly be disconnected. There is only one validation failure that has a DoS score bewteen 50 and 100, which is high-hash. That's why the test is failing immediately after the "Reject a block with invalid work" subtest. Fix is to wait for the initial getheaders from the peer before we start populating our blockstore. That way we won't have any invalid headers to respond to it with. Tree-SHA512: dc17d795fcfaf0f8c0bf1e9732b5e11fbc8febbfafba4c231b7c13a5404a2c297dcd703a7a75bc7f353c893e12efc87f424f2201abd47ba5268af32d4d2e841f * Temporarely rename MAX_BLOCK_SIZE -> MAX_BLOCK_BASE_SIZE We'll undo this after the next commit. This avoids merge many conflicts and makes reviewing easier. * Rename MAX_BLOCK_BASE_SIZE back to MAX_BLOCK_SIZE * Use DoS score of 100 for bad-blk-sigops This was accidently changed to 10 while backporting bitcoin#7287 and causes test failures in p2p-fullblocktest.py * Use allowOptimisticSend=true when sending reject messages This fixes test failures in p2p-fullblocktest.py which expects reject messages to be sent/received before connections get closed. * Fix p2p-fullblocktest.py - CBlock and friends are still in test_framework.mininode - "-whitelist" causes connections to not be dropped, which in turn causes sync_blocks with reconnect=True to fail - "bad-cb-amount" does not cause a ban in Dash, so reconnect must be False - Dash already bans when a header is received which is a child of an invalid header, causing block requests to never happen * Backport missing changes from bitcoin#13003 bitcoin#13003 was backported out of order which causes missed changes. * Bump p2p-fullblocktest timeouts * Increase RPC timeout in p2p-fullblocktest.py Co-authored-by: John Newbery <[email protected]> Co-authored-by: MarcoFalke <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.