fae7b14a04 qa: Make TestNodeCLI command optional in send_cli (MarcoFalke)
ffffb10a9f qa: Rename cli.args to cli.options (MarcoFalke)
Pull request description:
Makes the `command` optional, since there are valid bitcoin-cli calls that have no `command`:
* `bitcoin-cli -?`
* `bitcoin-cli -getinfo`
* ...
Also, rename self.args to self.options, since that is the name in the `bitcoin-cli -help` documentation.
Tree-SHA512: f49c06024e78423301d70782946d47c0fb97a26876afba0a1f71ed329f5d7124aee4c2df520c7af74079bf9937851902f7be9c54abecc28dc29274584804d46c
873beca6d [tests] Rename NodeConn and NodeConnCB (John Newbery)
Pull request description:
Final step in #11518
NodeConn -> P2PConnection
NodeConnCB -> P2PInterface
This is basically just a rename. Should be an easy review.
Tree-SHA512: fe1204b2b3d8182c5e324ffa7cb4099a47ef8536380e0bb9d37a5fccf76a24f548d1f1a7988ab8f830986a3058b670696de3fc891af5e5f75dbeb4e3273005d7
ee5efad6cf [tests] refactor node_network_limited (John Newbery)
b425131f5a [tests] remove redundant duplicate tests from node_network_limited (John Newbery)
2e02984591 [tests] node_network_limited - remove race condition (John Newbery)
dbfe294805 [tests] define NODE_NETWORK_LIMITED in test framework (John Newbery)
1285312048 [tests] fix flake8 warnings in node_network_limited.py (John Newbery)
Pull request description:
Fixes race condition in the node_network_limited test case introduced in #11740. Also tidies up the test and removes redundant duplicate tests.
Tree-SHA512: a5240fe35509d81a47c3d3b141a956378675926093e658d24be43027b20d3b5f0ba7c6017c8208487a1849d4fdfb911a361911d571423db7c50711250aba3011
* [tests] Remove mininode periodic (half-hour) ping messages
* [tests] Tidy up mininode
Add docstrings and renames some methods.
Also removes the redundant NodeConn.readable() method override.
* [tests] Move only: move NodeConnCB below NodeConn
This is required since NodeConnCB will inherit from NodeConn
after the next commit.
* [tests] Make NodeConnCB a subclass of NodeConn
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.
* [tests] Move version message sending from NodeConn to NodeConnCB
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.
* remove witness
Signed-off-by: Pasta <pasta@dashboost.org>
* Fix 11712
Co-authored-by: John Newbery <john@johnnewbery.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
09c6699900 [qa] Handle disconnect_node race (Suhas Daftuar)
Pull request description:
Several tests call disconnect_nodes() on each node-pair in rapid
succession, resulting in a race condition if a node disconnects a peer
in-between the calculation of the nodeid's to disconnect and the
invocation of the disconnectnode rpc call. Handle this.
Tree-SHA512: 3078cea0006fcb507c812004a777c505eb1e9dda7c6df12dbbe72395a73ff6f6760f597b6492054f5487b34534417ddef5fbad30553c135c288c4b7cfce79223
* [tests] Tidy up mininode.py module
Mostly move only. Adds a few extra comments.
* #11648 [tests] Move test_framework Bitcoin primitives into separate module
I manually recreated this commit, since we have A LOT of conflicts in mininode. However since it is primarily just a move, it was pretty easy to recreate
Signed-off-by: Pasta <pasta@dashboost.org>
* add import to messages.py
Signed-off-by: Pasta <pasta@dashboost.org>
* move import from mininode.py to messages.py
Signed-off-by: Pasta <pasta@dashboost.org>
* fix test failure
Signed-off-by: Pasta <pasta@dashboost.org>
* remove empty line at top of messages.py
Signed-off-by: pasta <pasta@dashboost.org>
* alphabetize MESSAGEMAP seperated by if it is dash specific or not
Signed-off-by: pasta <pasta@dashboost.org>
* remove accidentally added feefilter message
Signed-off-by: pasta <pasta@dashboost.org>
* Add missing getmnlistd/mnlistdiff messages to MESSAGEMAP
Co-authored-by: John Newbery <john@johnnewbery.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
This causes re-tries of LLMQ connections, which is required in cases
where 2 MNs tried to connect to each other and due to bad timing then
disconnected each other.
This is especially important when waiting for phase 1 (initialization),
as we might have skipped a whole DKG session before while the async DKG
session handler is still in the init phase (but for the old/skipped LLMQ).
75d0e4c544 [qa] Delete cookie file before starting node (Suhas Daftuar)
Pull request description:
When a node is restarted during a test after an unclean shutdown (such
as with -dbcrashratio), it's possible an old cookie file was left
behind. This can cause a race condition when restarting the node, where
the test framework might try to connect using credentials from the
old cookie file, just as the node will generate new credentials and
overwrite the old file.
Delete any such cookie file if present prior to startup.
Tree-SHA512: ae1e8bf8fd20e07c32b0715025693bb28b0e3dd34f328cae4346abf579b0c97b5db1c02782e1c46b7a3b6058d268b6d46b668e847658a6eed0be857ffb0d65dc
e36a0c0 [qa] Ensure bitcoind processes are cleaned up when tests end (Suhas Daftuar)
Pull request description:
When tests fail (such as due to a bug in the test, race condition, etc), it's possible that we could follow code paths that bypass our normal node shutdown that occurs in `TestNode.stop_node`. Add a destructor to `TestNode` that cleans this up.
Tree-SHA512: 72e04bc21462ebd0cb346fd1fe0540da454acfbad41923a0b06ea2317e9045b68e58f9adb02d8200891aca89a9d03a022eb72282aeb31a3b3afe7c6843a4b450
fa23105 qa: Cache only chain and wallet for regtest datadir (MarcoFalke)
Pull request description:
mempool.dat should be empty and I don't see a need to copy it around when restoring from the cache.
Tree-SHA512: f11ab69732db4dee0e9a0900570464e49085532b0cebc963877057112a7b985c477da3d32eb2093daabac9ada9e73b7c49881681ec5efa6101919b0af76001cf
9d7f839a2 test: Use os.path.join consistently in feature_pruning tests (Ben Woosley)
81b082277 test: Use wait_until in tests where time was used for polling (Ben Woosley)
Pull request description:
This is prompted by and builds on #12545, a nice cleanup / consolidation of patterns.
In cases where the exception message was meaningful, I tried to represent it as well in a comment.
I expect #12545 will go in first, but I'm happy to squash them if that's preferred.
Tree-SHA512: 7a861244001c87fd6b59b6bc248ee741ac8178f7255d6f1fda39bc693c5ff3b7de5f53d13afe9829aef6ea69153481edb0a9d5bc07c36c4f66b4315edd180bb4
fa41d68a2 qa: Fix python TypeError in script.py (MarcoFalke)
Pull request description:
`__repr__` returns string, so don't mix it with byte strings.
This fixes
```
TypeError: %b requires a bytes-like object, or an object that implements __bytes__, not 'str'
Tree-SHA512: fac06e083f245209bc8a36102217580b0f6186842f4e52a686225111b0b96ff93c301640ff5e7ddef6a5b4f1689071b16a9a8dc80f28e2b060ddee29edd24ec7
42e1b5d [test] Round target fee to 8 decimals in assert_fee_amount (Karl-Johan Alm)
Pull request description:
The output would produce arbitrary number of decimal points, sometimes resulting in 9 decimals:
```
AssertionError: Fee of 0.00000415 BTC too low! (Should be 0.000006175 BTC)
```
The above looks like the expected fee is 6175 sats when in reality it's 618.
Tree-SHA512: ddbff2926a88890d6e34a58db36f0b15a917a80064be6e40e9bcbec3f05ae6202d02adcd7873733945b043fa121d4a56dd930446ec368078fe1935cbfff898ce
ec527c6 Don't allow relative -walletdir paths (Russell Yanofsky)
Pull request description:
This makes it an error to explicitly specify a non-absolute -walletdir path, and also adds a debug.log warning if a relative rather than absolute -datadir path is configured.
Specifying paths relative to the current working directory in a daemon process can be dangerous, because files can fail to be located even if the configuration doesn't change, but the daemon is started up differently.
Specifying a relative -datadir now adds a warning to the debug log. It would not be backwards-compatible to forbid relative -datadir paths entirely, and it could also be inconvenient for command line testing.
Specifying a relative -walletdir now results in a startup error. But since the -walletdir option is new in 0.16.0, there should be no compatibility issues. Another reason not to use working directory paths for -walletdir specifically is that the default -walletdir is a "wallets" subdirectory inside the datadir, so it could be surprising that setting -walletdir manually would choose a directory rooted in a completely different location.
Tree-SHA512: 67cbdae677f82487a9031c5ec96b0205a488ab08718a0f4f49365e025119f3d7f6cfc88ba1eba04c1ecd8b9561a5b2c42e2e1a267af7c08c76b83e5e361f5a31
a14dbff39e Allow multiwallet.py to be used with --usecli (Russell Yanofsky)
f6ade9ce1a [tests] allow tests to be run with --usecli (John Newbery)
ff9a363ff7 TestNodeCLI batch emulation (Russell Yanofsky)
ca9085afc5 Prevent TestNodeCLI.args mixups (Russell Yanofsky)
fcfb952bca Improve TestNodeCLI output parsing (Russell Yanofsky)
Pull request description:
Lack of test coverage was pointed out by @jnewbery in https://github.com/bitcoin/bitcoin/pull/11687#discussion_r158133900
Tree-SHA512: 5f10e31abad11a5edab0da4e2515e39547adb6ab9e55e50427ab2eb7ec9a43d6b896b579b15863e5edc9beee7d8bf1c84d9dabd247be0760a1b9ae39e1e8ee02
fafdad0d4 qa: Remove unused NodeConn members (MarcoFalke)
Pull request description:
* `ver_send` and `ver_recv` were completely unused
* `rpc` was only used once, in p2p-segwit. Imo better only pass it to the constructor in that single test
Tree-SHA512: 7f85554d6d0fd2096516ca3c608811d5370da66cde35d8031bdc921607a7a4efdb26355896012f75f713f8df09e28d46ba46be69fd96a5898fabb1a25cbcb8ad
fb00c45c3 [tests] Explicitly disallow support for p2p versions below 60001 (John Newbery)
3858aabbd [tests] Remove support for p2p alert messages (John Newbery)
c0b127470 [tests] Remove support for bre-BIP31 ping messages (John Newbery)
2904e301c [tests] Remove dead code from mininode.py (John Newbery)
Pull request description:
This is the first part of #11518. It removes a ~150 lines of unused code from the mininode module:
- remove unused `deliver_sleep_time` and `EarlyDisconnectError` code
- remove support for pre-BIP31 ping messages
- remove support for alert message
- explicitly don't support p2p versions lower than 60001
Should be an easy ACK for reviewers. If all extended tests pass, then this code really was dead :)
Tree-SHA512: 508e612ceb0b094250d18e75522d51e6b14cd069443050ba4af34d6f890c58721cb5653e8bc000b60635b9474d035b0dcd9c509c0dcdb3a7501df17b787f83b0
readd is None check
Signed-off-by: Pasta <pasta@dashboost.org>
f89308532 [tests] Don't subclass from object for Python 3 (John Newbery)
8f9e3627e [tests] authproxy.py: tidy up __init__() (John Newbery)
323d8f61e [tests] fix flake8 warnings in authproxy.py (John Newbery)
fc0176d01 [tests] use python3 for authproxy.py (John Newbery)
Pull request description:
A few trivial tidyups in the test_framework:
- the test_framework can only be run in Python3, so remove the py2/3 compatibility workarounds in authproxy.py
- while there, do some general tidying up of the module - fix flake8 warnings, make initialization code more compact
- All classes in Python3 are new-style. No need to explicitly inherit from `object`.
Tree-SHA512: d15c93aa4b47c1ad7d05baa7a564053cf0294932e178c95ef335380113f42e1af314978d07d3b107292a8e3496fd840535b5571a9164182feaa062a1e9ff8b73
fix up mininode.py slightly
Signed-off-by: Pasta <pasta@dashboost.org>
c1e5d40 Make debugging test crash easier (MeshCollider)
8263f6a Create walletdir if datadir doesn't exist and fix tests (MeshCollider)
9587a9c Default walletdir is wallets/ if it exists (MeshCollider)
d987889 Add release notes for -walletdir and wallets/ dir (MeshCollider)
80c5cbc Add test for -walletdir (MeshCollider)
0530ba0 Add -walletdir parameter to specify custom wallet dir (MeshCollider)
Pull request description:
Closes#11348
Adds a `-walletdir` parameter which specifies a directory to use for wallets, allowing them to be stored separately from the 'main' data directory. Creates a new `wallets/` directory in datadir if this is the first time running, and defaults to using it if it exists.
Includes tests and release notes. Things which might need to be considered more:
- there is no 'lock' on the wallets directory, which might be needed?
- because this uses a new wallets/ directory by default, downgrading to an earlier version won't see the wallets in that directory (not a big deal though, users can just copy them up to the main dir)
- jnewbery suggested putting each wallet in its own directory, which is a good idea, but out of scope for this PR IMO. EDIT: this is being done in https://github.com/bitcoin/bitcoin/pull/11687
- doc/files.md needs updating (will do soon)
I also considered including a cleanup by removing caching of data directory paths and instead just initialise them once on startup (c.f. #3073), but decided it wasn't super relevant here will just complicate review.
Tree-SHA512: c8ac04bfe9a810c32055f2c8b8fa0d535e56125ceb8d96f12447dd3538bf3e5ee992b60b1cd2173bf5f3fa023a9feab12c9963593bf27ed419df929bb413398d
f6ffb14 [test] Add getblockchaininfo functional test (João Barbosa)
fd8f45f [test] Add restart_node to BitcoinTestFramework (João Barbosa)
Pull request description:
Adds functional test for `getblockchaininfo`. Also deals with the fact that `pruneheight` is only in the response when pruning is enabled (related to #11366).
Tree-SHA512: 56cdec0921f572874f2fdded0990d1722d1435c3ff9979e6bff1afdccdca6f8b214dbe8d7490cdac07b5758909db085132d14340de2cce943241f7ebde7e5b6c
1817398b3 mininode: add an optimistic write and disable nagle (Cory Fields)
Pull request description:
Disclaimer: I'm not familiar with asyncore, so I'm unclear how safe this is. It works for me (tm).
Because the poll/select loop may pause for 100msec before actually doing a send, and we have no way to force the loop awake, try sending from the calling thread if the queue is empty.
Also, disable nagle as all sends should be either full messages or unfinished sends.
This shaves an average of ~1 minute or so off of my accumulated runtime, and 10-15 seconds off of actual runtime.
Tree-SHA512: 6b61b8058e621dacf0b4dd353c10e3666fbda0691440eb6ebc432491ebada80a781dcd09291bf03e70112a41d3c2a0c91775ed08824b79bf8d0ebed11595c28b
* [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 #13048: [tests] Fix feature_block flakiness
c1d742025c [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 <jonnynewbs@gmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
0063d2c3d [tests] Make p2p-leaktests.py more robust (John Newbery)
Pull request description:
There has been an example of p2p-leaktests.py failing on travis in the new service bits test (introduced in #11001 . It appeared to me that the previous p2p connections had not been fully disconnected before attempting to add new p2p connections.
I've added a sleep and restarted the NetworkThread, but I don't know whether this will fix the problem, since I'm unable to reproduce the failure locally.
@MarcoFalke - not sure what you want to do here? I don't think this change could make things any worse.
Tree-SHA512: f5427c26267185a903c9b75bb3925bf153b8afce70c8e493bf8f585f57d809d20643b4ee69081300b211d22e960242aecc3d719f4ddd230aa08fdc5484b55055
* Rename LLMQ_5_60 to LLMQ_TEST
* Introduce -llmqtestparams which allows to modify LLMQ_TEST on regtest
Also add support in DashTestFramework
* Use parameters size=3, threshold=2 as default for LLMQ_TEST
And fall back to the old parameters where necessary
* Wait for all contributions, even when one member is lying
Otherwise we might end up continuing too fast, which would fail the DKG.
* Update src/chainparams.cpp
Co-Authored-By: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Otherwise we end up having the same list instance for every entry in
extra_args, which means modifying index 0 also affects all other indexes.
Noticed this while looking at "ps aux | grep dashd" output. It turned out
that parameters like "-dip3params" were passed multiple times to each node.
* Avoid unnecessary connections in dip3-deterministicmns.py
This saves ~15 seconds
* Generate more blocks per generate() call
* Use -vbparams to activate dip8 faster in tests
Avoids generating/syncing many unnecessary blocks
