## Issue being fixed or feature implemented
fix `p2p_quorum_data.py` test broken by #5276
## What was done?
adjust data request expiration timeout in tests
## How Has This Been Tested?
`./test/functional/test_runner.py p2p_quorum_data.py`
## Breaking Changes
n/a
## Checklist:
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
<!--
*** Please remove the following help text before submitting: ***
Provide a general summary of your changes in the Title above
Pull requests without a rationale and clear improvement may be closed
immediately.
Please provide clear motivation for your patch and explain how it
improves
Dash Core user experience or Dash Core developer experience
significantly:
* Any test improvements or new tests that improve coverage are always
welcome.
* All other changes should have accompanying unit tests (see
`src/test/`) or
functional tests (see `test/`). Contributors should note which tests
cover
modified code. If no tests exist for a region of modified code, new
tests
should accompany the change.
* Bug fixes are most welcome when they come with steps to reproduce or
an
explanation of the potential issue as well as reasoning for the way the
bug
was fixed.
* Features are welcome, but might be rejected due to design or scope
issues.
If a feature is based on a lot of dependencies, contributors should
first
consider building the system outside of Dash Core, if possible.
-->
## Issue being fixed or feature implemented
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
Before this fix, uniqueness of HPMN `platformNodeID` was checked only
while processing a block containing a `ProRegTx` or a `ProUpServTx`.
This is not enough as a `ProRegTx` or `ProUpServTx` containing duplicate
HPMN `platformNodeID` must be rejected at tx broadcast level.
## What was done?
<!--- Describe your changes in detail -->
Checking uniqueness when calling respective RPC and when receiving such
txs.
## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
## Breaking Changes
<!--- Please describe any breaking changes your code introduces -->
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [ ] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
## Issue being fixed or feature implemented
## What was done?
Implementation of 4k collateral HPMN.
## How Has This Been Tested?
## Breaking Changes
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e
tests
- [x] I have made corresponding changes to the documentation
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
---------
Co-authored-by: thephez <thephez@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
Co-authored-by: PastaPastaPasta <6443210+pastapastapasta@users.noreply.github.com>
Co-authored-by: UdjinM6 <1935069+Udjinm6@users.noreply.github.com>
Co-authored-by: Konstantin Akimov <545784+knst@users.noreply.github.com>
fa320975411af4f0e41771d89958a77fd7a2284b test: Create cached blocks not in the future (MarcoFalke)
Pull request description:
This avoids test failures when tests assume blocks are not from the future, like in wallet_dump: https://cirrus-ci.com/task/6607130193035264?command=ci#L3306
ACKs for top commit:
jonatack:
ACK fa320975411af4f0e41771d89958a77fd7a2284b
Tree-SHA512: 60b6882e0e1df8c5d67f034533407a45d3685983891b67ff4631072bfd0a93a325c7ca18758d7a2df252e4fcdb7c87321cb1e84458b22782e57e719eec634c22
## Issue being fixed or feature implemented
This should speed up test `feature_llmq_data_recovery.py` for 30% from
500+ seconds to ~350 seconds (running locally) and all other tests that
uses any quorum, such as `feature_llmq_simplepose.py`
Time of CI running is also decreased noticeable 168min -> 131min
21 jobs for
[pr-5091/knst/dash/functional-tests-delays](https://gitlab.com/dashpay/dash/-/commits/pr-5091/knst/dash/functional-tests-delays)
in 131 minutes and 20 seconds (queued for 4 seconds)
vs some other pull request:
23 jobs for
[pr-5100/UdjinM6/dash/fix_GetStateFor_perf](https://gitlab.com/dashpay/dash/-/commits/pr-5100/UdjinM6/dash/fix_GetStateFor_perf)
in 195 minutes and 33 seconds (queued for 28 minutes and 13 seconds)
## What was done?
decreased delays in functional tests
## How Has This Been Tested?
I run several times locally and run CI/CD to see that it doesn't fail
## Breaking Changes
no breaking changes
## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes
that apply. -->
- [x] I have performed a self-review of my own code
**For repository code-owners and collaborators only**
- [x] I have assigned this pull request to a milestone
fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 ci: Remove unparseable lines from supp file for old xenial clang tsan (MarcoFalke)
fa1bfc476c9208a4c412c8ca74d05f52bb47766f ci: ubsan report_error_type=1 and add suppressions (MarcoFalke)
fa69cef13e5aab8264339eb3d50a9e89d59efd87 test: Print stderr when subprocess fails (MarcoFalke)
2222c305866a77065ab5be24c1c252bae252bb59 test: Use char instead of unsigned char (MarcoFalke)
faa8023ce9a47b282e1fac3ca8b3a7bb0042935a ci: Bump to clang-8 for asan build to avoid segfaults on ppc64le (MarcoFalke)
Pull request description:
Use clang-8 instead of default clang (which is clang-6 on Bionic) to avoid spurious segfaults when running the ci system on ppc64le
ACKs for top commit:
practicalswift:
ACK fa40e48c50d8ccf42ce5e66c12390e2ed4b60e75 assuming Travis is happy -- diff looks correct :)
Tree-SHA512: f4f26232d3a0ef38da245869340f723d279a3db9823befbc735fb5a00024dae041c7306d7ae55d2488e6f86aa96cdea155b007aefb561fba505141e8dbc717dc
* tests: extend "age" period in `feature_llmq_connections.py`
see `NOTE`
* tests: sleep more in `wait_until` by default
Avoid overloading rpc with 20+ requests per second, 2 should be enough.
* tests: various fixes in `activate_dip0024`
- lower batch size
- no fast mode
- disable spork17 while mining
- bump mocktime on every generate call
* tests: bump mocktime on generate in `activate_dip8`
* tests: fix `reindex` option in `restart_mn`
Make sure nodes actually finished reindexing before moving any further.
* tests: trigger recovery threads and wait on mn restarts
* tests: sync blocks in `wait_for_quorum_data`
* tests: bump disconnect timeouts in `p2p_invalid_messages.py`
1 is too low for busy nodes
* tests: Wait for addrv2 processing before bumping mocktime in p2p_addrv2_relay.py
* tests: use `timeout_scale` option in `get_recovered_sig` and `isolate_node`
* tests: fix `wait_for...`s
* tests: fix `close_mn_port` banning test
* Bump MASTERNODE_SYNC_RESET_SECONDS to 900
This helps to avoid issues with 10m+ bump_mocktime on isolated nodes in feature_llmq_is_retroactive.py and feature_llmq_simplepose.py.
* style: fix extra whitespace
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
fac2fc4dd8a28b99e17c57e4ab6580a3231f1d0a test: Increase debugging to hunt down mempool_reorg intermittent failure (MarcoFalke)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 4094b44afaa623e58b69f8d0332e60f0150b9ae2fd8bb265210d85546d887672ab8a3435cd9b086be14f69ab5b17e0f9fae06bd8aec1e7947ca766dd72b577c4
cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a test: move sync_blocks and sync_mempool functions to test_framework.py (Roy Shao)
Pull request description:
This PR moves `sync_blocks` and `sync_mempool` out from `test_framework/util.py` to `test_framework/test_framework.py` so they can take contextual information of test framework into account.
* Change all reference callers to call functions from `test_framework.py`
* Remove `**kwargs` which is not used
* Take into account of `timeout_factor` when respecting timeout in function implementations.
* Pass all tests by running `./test/functional/test_runner.py`
fixes#18930
ACKs for top commit:
MarcoFalke:
ACK cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a , reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space 💫
Tree-SHA512: a79b2a3fa842fc26a7aacb834bb2aea88b3049916c0b754e60002a77ce94bb5954e0ea3b436bf268e9295efb62d721dfef263a09339a55c684ac3fda388c275e
38c3dd9c706e7e84b2a4dbaf1424a3f1c3b694fc docs: Add notes on how to diasble rpc timeout in functional tests while attatching gdb. (codeShark149)
784ae096259955ea7a294114f5c021c9489d2717 test: Add capability to disable RPC timeout in functional tests. (codeShark149)
Pull request description:
Many times, especially while debugging RPC callbacks to core using gdb, the test timeout kicks in before the response can get back. This can be annoying and requires restarting the functional test as well as gdb attachment.
This PR adds a `--notimeout` flag into `test_framework` and sets the `rpc_timeout` accordingly if the flag is set.
The same effect can be achieved with newly added `--factor` flag but keeping a separate flag that explicitly disables the timeout can be easier for new testers to find it out and separates its purpose from the `--factor` flag.
Requesting review ryanofsky jnewbery as per the IRC discussion.
Update: After initial round of review, the approach is modified to accommodate the functionality in already existing `--factor` flag. `--factor` is changed to `--timeout-factor` to express its intent better.
ACKs for top commit:
MarcoFalke:
ACK 38c3dd9c706e7e84b2a4dbaf1424a3f1c3b694fc and thanks for fixing up all my typos 😅
jnewbery:
ACK 38c3dd9c706e7e84b2a4dbaf1424a3f1c3b694fc.
Tree-SHA512: 9458dd1010288c62f8bb83f7a4893284fbbf938882dd65fc9e08810a910db07ef676e3100266028e5d4c8ce407b2267b3860595015da070c84a9d4a9816797db
2742c3428633b6ceaab6714635dc3adb74bf121b test: add factor option to adjust test timeouts (Harris)
Pull request description:
This PR adds a new option **factor** that can be used to adjust timeouts in various functional tests.
Several timeouts and functions from `authproxy`, `mininode`, `test_node` and `util` have been adapted to use this option. The factor-option definition is located in `test_framework.py`.
Fixes https://github.com/bitcoin/bitcoin/issues/18266
Also Fixes https://github.com/bitcoin/bitcoin/issues/18834
ACKs for top commit:
MarcoFalke:
Thanks! ACK 2742c3428633b6ceaab6714635dc3adb74bf121b
Tree-SHA512: 6d8421933ba2ac1b7db00b70cf2bc242d9842c48121c11aadc30b0985c4a174c86a127d6402d0cd73b993559d60d4f747872d21f9510cf4806e008349780d3ef
19139ee034d20ebab1b91d3ac13a8eee70b59374 Add documentation for test_shell submodule (JamesC)
f5112369cf91451d2d0bf574a9bfdaea04696939 Add TestShell class (James Chiang)
5155602a636c323424f75272ccec38588b3d71cd Move argparse() to init() (JamesC)
2ab01462f48b2d4e0d03ba842c3af8851c67c6f1 Move assert num_nodes is set into main() (JamesC)
614c645643e86c4255b98c663c10f2c227158d4b Clear TestNode objects after shutdown (JamesC)
6f40820757d25ff1ccfdfcbdf2b45b8b65308010 Add closing and flushing of logging handlers (JamesC)
6b71241291a184c9ee197bf5f0c7e1414417a0a0 Refactor TestFramework main() into setup/shutdown (JamesC)
ede8b7608e115364b5bb12e7f39d662145733de6 Remove network_event_loop instance in close() (JamesC)
Pull request description:
This PR refactors BitcoinTestFramework to encapsulate setup and shutdown logic into dedicated methods, and adds a ~~TestWrapper~~ TestShell child class. This wrapper allows the underlying BitcoinTestFramework to run _between user inputs_ in a REPL environment, such as a Jupyter notebook or any interactive Python3 interpreter.
The ~~TestWrapper~~ TestShell is motivated by the opportunity to expose the test-framework as a prototyping and educational toolkit. Examples of code prototypes enabled by ~~TestWrapper~~ TestShell can be found in the Optech [Taproot/Schnorr](https://github.com/bitcoinops/taproot-workshop) workshop repository.
Usage example:
```
>>> import sys
>>> sys.path.insert(0, "/path/to/bitcoin/test/functional")
```
```
>>> from test_framework.test_wrapper import TestShell
>>> test = TestShell()
>>> test.setup(num_nodes=2)
20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Initializing test directory /path/to/bitcoin_func_test_XXXXXXX
```
```
>>> test.nodes[0].generate(101)
>>> test.nodes[0].getblockchaininfo()["blocks"]
101
```
```
>>> test.shutdown()
20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Stopping nodes
20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Cleaning up /path/to/bitcoin_func_test_XXXXXXX on exit
20XX-XX-XXTXX:XX:XX.XXXXXXX TestFramework (INFO): Tests successful
```
**Overview of changes to BitcoinTestFramework:**
- Code moved to `setup()/shutdown()` methods.
- Argument parsing logic encapsulated by `parse_args` method.
- Success state moved to `BitcoinTestFramework.success`.
_During Shutdown_
- `BitcoinTestFramework` logging handlers are flushed and removed.
- `BitcoinTestFrameowork.nodes` list is cleared.
- `NetworkThread.network_event_loop` is reset. (NetworkThread class).
**Behavioural changes:**
- Test parameters can now also be set when overriding BitcoinTestFramework.setup() in addition to overriding `set_test_params` method.
- Potential exceptions raised in BitcoinTestFramework.setup() will be handled in main().
**Added files:**
- ~~test_wrapper.py~~ `test_shell.py`
- ~~test-wrapper.md~~ `test-shell.md`
ACKs for top commit:
jamesob:
ACK 19139ee034
jonatack:
ACK 19139ee034d20ebab1b91d3ac13a8eee70b59374
jnewbery:
Rather than invalidate the three ACKs for a minor nit, can you force push back to 19139ee034d20ebab1b91d3ac13a8eee70b59374 please? I think this PR was ready to merge before your last force push.
jachiang:
> Rather than invalidate the three ACKs for a minor nit, can you force push back to [19139ee](19139ee034) please? I think this PR was ready to merge before your last force push.
jnewbery:
ACK 19139ee034d20ebab1b91d3ac13a8eee70b59374
Tree-SHA512: 0c24f405f295a8580a9c8f1b9e0182b5d753eb08cc331424616dd50a062fb773d3719db4d08943365b1f42ccb965cc363b4bcc5beae27ac90b3460b349ed46b2
fa47330397 test: Speed up cache creation (MarcoFalke)
fa6ad7a5ec test: Bump MAX_NODES to 12 (MarcoFalke)
Pull request description:
When testing a combination of settings that affect the datadir (e.g. prune, blockfilter, ...) we may need a lot of datadirs.
Bump the maximum number of nodes proactively from 8 to 12, so that caches get populated with 12 node dirs, as opposed to 8.
Also, add an assert that the list of deterministic keys is exactly the number of max nodes (and not more than that.
Also, create the cache faster.
ACKs for commit fa4733:
laanwj:
utACK fa473303972b7dad600d949dc9b303d8136cb7e7
Tree-SHA512: 9803c765ed52d344102f5a3bce57b05d88a7429dcb05ed66ed6c881fda8d87c2834d02d21b95fe9f39c0efe3b8527e13cf94f006588cde22e8c2cd50b2d517a6
fa2cdc9ac2 test: Simplify create_cache (MarcoFalke)
fa25210d62 qa: Fix wallet_txn_doublespend issue (MarcoFalke)
1111aecbb5 qa: Always refresh stale cache to be out of ibd (MarcoFalke)
fab0d85802 qa: Remove mocktime unless required (MarcoFalke)
Pull request description:
When starting a test, we are always in IBD because the timestamps on cached blocks are in the past. Usually, we solve that by generating a block at the beginning of the test.
That is clumsy and might even lead to other problems such as #15360 and https://github.com/bitcoin/bitcoin/issues/14446#issuecomment-461926598
So fix that by getting rid of mocktime and always refreshing the last block of the cache when starting the test framework.
Should fix#14446
Tree-SHA512: 6af09800f9c86131349a103af617a54551f5f3f3260d38e14e3f30fdd3d91a0feb0100c56cbb12eae4aeac5571ae4b530b16345cbb831d2670237b53351a22c1
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
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>
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
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
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
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
3c3e31c3a4 [tests] Add wallet-tool test (João Barbosa)
49d2374acf [tools] Add wallet inspection and modification tool (Jonas Schnelli)
Pull request description:
Adds an offline tool `bitcoin-wallet-tool` for wallet creation and maintenance.
Currently this tool can create a new wallet file, display information on an existing wallet, and run the salvage and zapwallettxes maintenance tasks on an existing wallet. It can later be extended to support other common wallet maintenance tasks.
Doing wallet maintenance tasks in an offline tool makes much more sense (and is potentially safer) than having to spin up a full node.
Tree-SHA512: 75a28b8a58858d9d76c7532db40eacdefc5714ea5aab536fb1dc9756e2f7d750d69d68d59c50a68e633ce38fb5b8c3e3d4880db30fe01561e07ce58d42bceb2b
fa4b8c90d3 test: add_nodes can only be called once after set_test_params (MarcoFalke)
faa831102a Revert "tests: Support calling add_nodes more than once" (MarcoFalke)
Pull request description:
Writing tests should be straightforward and with little side-effects as possible.
I don't see how this is needed and can not be achieved with `self.num_nodes` (and `self.extra_args` et al.)
Tree-SHA512: 83a67f2cba9d97e21d80847ff405a4633fcb0d5674486efa57ee1813e46efe8709ae0fb462b8339a01ebeca5c4f2d29ecb1807d648b8fd9ee8ce336b08d580a8
4aabadbf44 tests: have combine_logs default to most recent test dir (James O'Beirne)
Pull request description:
Have `combine_logs.py` default to the most recent test directory if no argument is provided. This allows you to avoid an annoying copy-paste when iterating on a failing test, since you can do something like
```sh
alias testlogs='./test/functional/combine_logs.py -c | less'
./test/functional/some_test.py # fails
testlogs
```
Tree-SHA512: 919642ab09c314888a23c9491963b35b9da87e60deb740d1d5e816444aa9bdda5e519dc8ca131669f2d563167ef5f5abb14e22f20f47bf8362915ed578181846
a2a04a5abb Bugfix: Only run bitcoin-tx tests when bitcoin-tx is enabled (Luke Dashjr)
92af71cea9 configure: Make it possible to build only one of bitcoin-cli or bitcoin-tx (Luke Dashjr)
Pull request description:
Includes #5618 (which the reasons for rejecting no longer hold true)
Tree-SHA512: f30a8e4a2f70166b7cabef77c4674163b3a9da14c6a547d34f00d1056a19bf4d23e22851eea726fad2afc8735d5473ae91122c770b65ac3886663dc20e2c5b70
