[QA] Test suite update + P2P invalid blocks and invalid tx functional tests coverage. #2346
Conversation
This file contains 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
furszy
force-pushed
the
2021_updating_functional_test
branch
from
cd123af
to
ac6fff3
Compare
random-zebra
previously approved these changes
Apr 30, 2021
There was a problem hiding this comment.
Tested ACK ac6fff3
Really nice backport. The p2p_invalid_* tests are a thing of beauty 💜
We should probably be able to go ahead and kill the ComparisonTestFramework now.
Left a minor non-blocking comment, which could also be tackled in another PR (as well as picking the duration from last GA run, and updating the comment and sorting inside test_runner).
Adaptation from John Newbery's btc@c32cf9f62285b5cd18a5064aee91f0802f0f87a8 to our full block sync process. P2PDataStore subclasses NodeConnCB. This class keeps a store of txs and blocks and responds to getdata requests from the node-under-test.
Adaptation of btc@fac3e22b18cd29053bc17065fd75db7b84ba6f40
…0-5137. CVE-2018-17144 and CVE-2012-2459 are only partially tested for regression. - CVE-2018-17144 is not tested for the inflation bug. - CVE-2012-2459 is only tested for the mutated block being rejected, not for the original block being accepted afterwards. This commit fixes that limitation. Also added functional test for CVE-2010-5137.
…ality + init 'acceptnonstdtxn' option to skip (most) "non-standard transaction" checks, for testnet/regtest only.
furszy
force-pushed
the
2021_updating_functional_test
branch
from
ac6fff3
to
2d994c4
Compare
|
Updated, nits tackled. |
random-zebra
approved these changes
May 4, 2021
There was a problem hiding this comment.
ACK 2d994c4
Going to merge this now, as it is only touching the python framework (aside from the policy change of setting fRequireStandard by default on regtest), and it's needed for a follow-up I'm working on.
random-zebra
added a commit
that referenced
this pull request
May 17, 2021
…son test framework 48e5d54 [Tests][Refactoring] Remove 'magic bytes' in p2p_invalid_* tests (random-zebra) fcda6cc [QA][Cleanup] Retire the Comparison Test Framework (random-zebra) 3633e42 [Tests] Fix and resurrect feature_block.py (random-zebra) 77645bf [BUG][Tests] Add on_getblocks to P2PDataStore interface (random-zebra) 030517b [Refactoring] pass next block height to IncrementExtraNonce (random-zebra) 0f0c97c [BUG] Fix block with invalid PoW in zerocoin_rejection_tests (random-zebra) b19bee9 [Refactoring][BUG] Never change chain-params after setup in unit tests (random-zebra) 810a880 [Tests] Update PIVX specific constants bits/block-size/block-version (random-zebra) d0777a7 [Refactoring] Add fPowNoRetargeting consensus param + fix regtest diff (random-zebra) 93fcc2e [Params] Fix regtest coinbase - mine it with lower diff (random-zebra) 4cf7222 [Tests] Refactor wallet_sapling_transactions_validations_tests (random-zebra) 44a1c44 [Refactor] Option to mine mempool txes in TestChainSetup::CreateBlock (random-zebra) 0b670c8 [BUG] Properly recompute coinbase and sapling root hash in CreateBlock (random-zebra) f5f72ca [Trivial] Update some state logs (tx oversize and inputs missing/spent) (random-zebra) 9d552f9 [Refactor][Tests] Do not change chain when not needed (random-zebra) bd6e591 [Refactoring] remove SaplingActive check in GUI/RPC (random-zebra) b2410ae [BUG] Fix assertion in error in assert_debug_log (random-zebra) Pull request description: Follow-up to #2346. The original goal here was to remove the `ComparisonTestFramework`, following bitcoin#11818. But, before doing that, had to update the old `feature_block` test, which was still using the framework, even though the test was disabled. This is one of the most important functional tests, as it checks all block-related consensus rules, so, just keeping it disabled was not an option. Aside from being very long (1200+ lines), the test was taking ages to mine any single block. This is because it relies on `next_block` doing the actual proof of work (`CBlock::solve`), given the regtest nBits (difficulty is fixed on regtest). Now, since all networks (mainnet/testnet/regtest) share the same genesis block, the regtest difficulty was way higher than it should have been. We never had issues with this, because we are bailing out early in `CheckProofOfWork`, essentially not performing any proof of work check on regtest. So, in order to fix `feature_block.py`, there were two options: either modify `next_block` to keep a fixed nonce, or update the regtest chain parameters, creating a new genesis block with low-diff nBits, and actually checking the proof of work. I chose the latter. This triggered the need to refactor some unit-tests that were changing chain params (from `MAIN` to `REGTEST`) without reloading the genesis block in the block index (which wasn't needed before, because, as explained, all networks had the same genesis). After this PR, the chain is set only inside the test fixtures, and is never changed inside the single test cases (so there is no need to reload the chain index during execution). ACKs for top commit: furszy: ACK 48e5d54 . Fuzzbawls: ACK 48e5d54 Tree-SHA512: 213cad1be7eade941ec5be59fc0b8753eeb11aef55ae74052f59b6c668c4b8ab87efe77221545bdbb2d7b5beeb34b6123005ed2964def6673080cc809c508145
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Introduced the
P2PDataStoreclass into the functional test suite and updated it up to a point where was able to enable thep2p_invalid_block.pyandp2p_invalid_tx.pyfunctional tests. Plus updated both tests to a much recent version, expanding its testing coverage as well.Now the test suite is covering the primary CVE that btc had in the past.
As both tests were simply unusable before and we have a different synchronization process than upstream, plus we didn't have some params and init/global flags, this work isn't following a PR by PR back port path. Had to made my own way up to the complete tests support.
Main pilar PRs from upstream from where this work was based:
p2p_invalid_tx.py,p2p_invalid_block.pyandmininode.pychanges).