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
ff8a9b0 [tests] Add combinedlogslen argument to test_runner.py (John Newbery)
dba94ea [tests] [travis-ci] Move Travis functional test log post processing to test_runner (John Newbery)
bba1c54 [tests] Improve logging shutdown and add hint for combine_logs (John Newbery)
Pull request description:
Replaces #11779 . Notes from that PR:
> Currently, when a functional test fails, the debug logs are printed sequentially to the travis log. This makes debugging race conditions based on the travis log hard. Instead, all logs events should be combined and sorted by their timestamp, then appended to the travis log.
@MarcoFalke
Tree-SHA512: 56c80067d6a2c92f7e6a35e3ae5160637a0de052d9da593c7be6e02233544a93c66d62456f903f85e2edc09e31ab4bdafd1aed1d9897ae48c634f82631f856f7
e9dfa9bcc [tests] Move version message sending from NodeConn to NodeConnCB (John Newbery)
dad596fc3 [tests] Make NodeConnCB a subclass of NodeConn (John Newbery)
e30d40438 [tests] Move only: move NodeConnCB below NodeConn (John Newbery)
4d5059856 [tests] Tidy up mininode (John Newbery)
f2ae6f32a [tests] Remove mininode periodic (half-hour) ping messages (John Newbery)
ec59523c5 [tests] Remove rpc property from TestNode in p2p-segwit.py. (John Newbery)
Pull request description:
This is the final step in #11518, except for possibly renaming - for motivation, please see that PR.
If this is merged, then migrating the test framework from asyncore to asyncio should be easier (I say should because I haven't dug too deeply into what would be required).
Requesting review from @ryanofsky , since he always has good feedback on these refactor PRs, and I'd appreciate his take on this refactor. Note particularly that I've reverted the change suggested here: https://github.com/bitcoin/bitcoin/pull/11182#discussion_r148859555 . The idea, as always, is to present a simple interface to the test writer.
Tree-SHA512: 94dd467a13ec799b101108cf47d4dccb6f6240b601e375e3d785313333bbb389c26072a50759aca663bbf3d6c8b867b99e36ae8800ab8ea115e0496c151926ce
Instead of calling combine_logs from test_runner when the environment variable
PYTHON_DEBUG is set, add a combinedlogslen argument. This makes it easier
to call when running locally, and allows a different length of combined logs
to be printed.
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.
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.
9d42cc333 [tests] address review comments (John Newbery)
f0c4ab9a7 [tests] fix flakiness in sendheaders.py (John Newbery)
25fd6e2c2 [tests] refactor check_last_announcement() in sendheaders.py (John Newbery)
f39d4bbd1 [tests] tidy up BaseNode in sendheaders.py (John Newbery)
2613c545f [tests] fix flake8 warnings in sendheaders.py (John Newbery)
Pull request description:
This PR should fix the intermittent failure of sendheaders.py described in #11673. The first three commits are tidying up and refactoring the file. The final commit _fix flakiness in sendheaders.py_ fixes the intermittent failures. The commit message for that commit describes the problems that are being fixed.
I think @laanwj @MeshCollider @MarcoFalke have seen these failures.
fixes#11673
Tree-SHA512: 278e1af85f2eae00f970f2d8ef33686dd52b4f62180dea4cfdaff7bcf3287c6f1c2930355d99461a12f0c51c4d42cc3b1cb3275174134028ca4d06ffc24c18dd
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
Currently it's possible to accidentally type e.g.
bitcoin-cli -getinfo getbalance
and get an answer which can be confusing; the trialing arguments are
just ignored.
To avoid this, throw an error if the user provides arguments to
`-getinfo`.
1135c79 [tests] Tidy up mininode.py module (John Newbery)
f9cd9b1 [tests] Move test_framework Bitcoin primitives into separate module (John Newbery)
Pull request description:
Second part of #11518.
Moves the primitive Bitcoin datastructures and message classes into their own module, and tidies up the mininode.py module.
- First commit is almost entirely move-only
- Second commit is mostly move-only, but also does a little tidying.
Tree-SHA512: 5d74802677f1ab788e43188653106a96fffd9ab1fe3aa6a4eb94e5807de5dd5c8ee212296f45e8d16c7e3d95cfc4891677e812b7944bd3ab604e04b3b88aa06e
Fixes to sources of intermittent failure in sendheaders.py
- at the start of test_null_locators(), a new block is generated and
then a getheaders is sent. check_last_accouncement() is called to assert
that the headers message is received. However, the new block triggers an
inv to be sent over both P2P connections, so there's a race. If the inv
is received at the wrong time, the test fails.
- test_null_locators() ends by sending a block to the node under test.
At the start of test_nonnull_locators(), a block is mined and
check_last_announcement() is called to assert that the inv received is
for the same block. That means there's a race: if the inv from the block
sent in test_null_locators() is received at the wrong time, the test
fails.
All calls of check_last_announcement() asserted that the return
value was True. Just assert inside the function instead. This
gives better debug information if the assert fails.
Also only check the contents of the most recent inv and header if
check_last_announcement() is called with the relevant argument.
28f8b66 Diagnose unsuitable outputs in lockunspent(). (Eelis)
Pull request description:
Fixes#2667.
This is a simplified version of pull request #3574, which was abandoned by its author.
I added some tests as well.
Tree-SHA512: e63e00dec8b1b232079380183805cb0b0b18c78ea6bea769837949aab984689d7f68b2ccfe66b1873517b040b9e616ce0eb058575c3d4382aa8c26eebcf1f14e
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
faaa7db qa: Only allow disconnecting all NodeConns (MarcoFalke)
Pull request description:
Disconnecting the connection with `index=0` makes no sense when there are more than one connections, as the list "rotates around" and populates index 0 after `del`.
Just disconnect all NodeConns in any case.
Tree-SHA512: e5cf540823fccb31634b5a11501f54222be89862e80ccafc28bc06726480f8d2153b8c1b6f859fa6a6d087876251d48a6c6035bccdaaf16831e300bc17ff613d
5e0ba8f8c [wallet] getreceivedbyaddress should return error if address is not mine (John Newbery)
ea0cd24f7 [tests] Tidy up receivedby.py (John Newbery)
Pull request description:
Two commits:
- First commit tidies up the `receivedby.py` test (and speeds it up by factor of two)
- Second commit changes getreceivedbyaddress to return error if the address is not found in wallet, and adds test to `receivedby.py`
Tree-SHA512: e41342dcbd037a6b440cbe4ecd3b8ed589e18e477333f0d866f3564e948e0f5231e497d5ffb66da4e6680eb772d9f0cf839125098bb68b92d04a5ee35c6c0a81
11413646b [trivial] (whitespace only) fix getblockchaininfo alignment (John Newbery)
bd9c18171 [rpc] Add initialblockdownload to getblockchaininfo (John Newbery)
Pull request description:
Exposing whether the node is in IBD would help for testing, and may be useful in general, particularly for developers.
First discussed in #10357 here: https://github.com/bitcoin/bitcoin/pull/10357#pullrequestreview-59963870
> ... we could simplify this (and possibly other) tests by just adding a way to know if a node is in IBD. I'd like to do that, but I'm not sure it makes sense to complicate this PR with discussion over how that information should be made available. Eg it's not clear to me that the notion of being in IBD is worth exposing to the casual user, versus a hidden rpc call or something, since the definition has changed over time, and may continue to change in the future. But I still do agree that at least for testing purposes it would be far simpler to expose the field somehow...
This PR currently implements the simplest way of doing this: adding an `initialblockdownload` field to `getblockchaininfo`. Other approaches we could take:
1. add a new debug RPC method that exposes `IBD` and potentially other information.
2. add a parameter to `getblockchaininfo`, eg `debug_info`, which would cause it to return debug information including IBD
3. add a query string to the url `?debug=true` which would cause RPCs to return additional debug information.
I quite like the idea of (3). Feedback on these and other approaches very much welcomed!
@sdaftuar @laanwj
Tree-SHA512: a6dedd47f8c9bd38769cc597524466250041136feb33500644b9c48d0ffe4e3eeeb2587b5bbc6420364ebdd2667df807fbb50416f9a7913bbf11a14ea86dc0d4
- Fix flake8 warnings
- Remove the useless get_sub_array_from_array() function
- Reduce runtime for receivedby.py by about half by only using two nodes
748157913 [tests] Make comp test framework more debuggable (John Newbery)
Pull request description:
We should remove the comparison test framework entirely (see #10603).
Until we do that, let's make it a bit more debuggable. Currently, if there's an assert in the framework, it's very difficult to track down where we are in the test generator. Make the logging a bit better to help with debugging.
Before this PR:
```
→ ./p2p-fullblocktest.py
2017-10-09 14:05:11.302000 TestFramework (INFO): Initializing test directory /tmp/user/1000/testzdnax_yr
2017-10-09 14:05:11.557000 TestFramework.mininode (INFO): Connecting to Bitcoin Node: 127.0.0.1:11975
2017-10-09 14:05:11.712000 TestFramework.comptool (INFO): Test 1: PASS
2017-10-09 14:05:11.947000 TestFramework.comptool (INFO): Test 2: PASS
2017-10-09 14:05:12.057000 TestFramework.comptool (INFO): Test 3: PASS
2017-10-09 14:05:12.058000 TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 117, in main
self.run_test()
File "./p2p-fullblocktest.py", line 72, in run_test
self.test.run()
File "/home/ubuntu/bitcoin/test/functional/test_framework/comptool.py", line 306, in run
assert test_number != 4
AssertionError
2017-10-09 14:05:12.059000 TestFramework (INFO): Stopping nodes
2017-10-09 14:05:14.203000 TestFramework (WARNING): Not cleaning up dir /tmp/user/1000/testzdnax_yr
2017-10-09 14:05:14.204000 TestFramework (ERROR): Test failed. Test logging available at /tmp/user/1000/testzdnax_yr/test_framework.log
```
With this PR:
```
→ ./p2p-fullblocktest.py
2017-10-09 14:03:54.069000 TestFramework (INFO): Initializing test directory /tmp/user/1000/testuey7t3tf
2017-10-09 14:03:54.329000 TestFramework.mininode (INFO): Connecting to Bitcoin Node: 127.0.0.1:11783
2017-10-09 14:03:54.383000 TestFramework.comptool (INFO): Running test 1: ./p2p-fullblocktest.py line 184
2017-10-09 14:03:54.496000 TestFramework.comptool (INFO): Running test 2: ./p2p-fullblocktest.py line 193
2017-10-09 14:03:54.758000 TestFramework.comptool (INFO): Running test 3: ./p2p-fullblocktest.py line 205
2017-10-09 14:03:54.867000 TestFramework.comptool (INFO): Running test 4: ./p2p-fullblocktest.py line 208
2017-10-09 14:03:54.867000 TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/home/ubuntu/bitcoin/test/functional/test_framework/test_framework.py", line 117, in main
self.run_test()
File "./p2p-fullblocktest.py", line 72, in run_test
self.test.run()
File "/home/ubuntu/bitcoin/test/functional/test_framework/comptool.py", line 309, in run
assert test_number != 4
AssertionError
2017-10-09 14:03:54.868000 TestFramework (INFO): Stopping nodes
2017-10-09 14:03:56.950000 TestFramework (WARNING): Not cleaning up dir /tmp/user/1000/testuey7t3tf
2017-10-09 14:03:56.950000 TestFramework (ERROR): Test failed. Test logging available at /tmp/user/1000/testuey7t3tf/test_framework.log
```
Tree-SHA512: 5525958b0098d661c281bd955c92c72bf81359464376e96d44e6c88f18aea57ba08270ecd564edda4b47d674e3b27d20e5b1060544bf8dd5c6a68103d2bb35b8
725b79a [test] Verify node doesn't send headers that haven't been fully validated (Russell Yanofsky)
3788a84 Do not send (potentially) invalid headers in response to getheaders (Matt Corallo)
Pull request description:
Nowhere else in the protocol do we send headers which are for
blocks we have not fully validated except in response to getheaders
messages with a null locator. On my public node I have not seen any
such request (whether for an invalid block or not) in at least two
years of debug.log output, indicating that this should have minimal
impact.
Tree-SHA512: c1f6e0cdcdfb78ea577d555f9b3ceb1b4b60eff4f6cf313bfd8b576c9562d797bea73abc23f7011f249ae36dd539c715f3d20487ac03ace60e84e1b77c0c1e1a
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
bc9c0a7 Improve wallet-accounts test (Russell Yanofsky)
Pull request description:
Add comments and
- Verify sending to a account causes getaccountaddress to generate new addresses.
- Verify sending to a account causes getreceivedbyaccount to return amount received.
- Verify ways setaccount updates the accounts of existing addresses.
Tree-SHA512: 4facc8d10fb23847081e8081839d4050faa855cca40f34012b82297bf1a8cfdb975b26314bc518dc6b3f5997452a7acf309f9627e0dff359347c628b37014bfa
The mininode module includes code to support p2p versions below
60001. However, the test_framework does not support versions
of Bitcoin Core before V0.13.0. Remove code supporting
p2p versions before 60001 (which has never been run).
BIP31 support was added to Bitcoin Core in version 0.6.1. Our test
framework is incompatible with Bitcoin Core versions that old, so remove
all special logic for handling pre-BIP31 pings.
p2p connections can now be added to TestNode instances.
This commit also updates the example test to use the new
p2p interface in TestNode to demonstrate usage.
A future commit will update the existing tests to use p2p through the
TestNode.
d61845818 Have SegWit active by default (Pieter Wuille)
4bd89210a Unit tests for always-active versionbits. (Anthony Towns)
d07ee77ab Always-active versionbits support (Pieter Wuille)
18e071841 [consensus] Pin P2SH activation to block 173805 on mainnet (John Newbery)
526023aa7 Improve handling of BIP9Deployment limits (Anthony Towns)
Pull request description:
Most tests shouldn't have to deal with the now-historical SegWit activation transition (and other deployments, but SegWit is certainly the hardest one to accomodate).
This PR makes a versionbits starttime of -1 equal to "always active", and enables it by default for SegWit on regtest. Individual tests can override this by using the existing `-vbparams` option.
A few unit tests and functional tests are adapted to indeed override vbparams, as they specifically test the transition.
This is in preparation for wallet SegWit support, but I thought having earlier eyes on it would be useful.
Tree-SHA512: 3f07a7b41cf46476e6c7a5c43244e68c9f41d223482cedaa4c02a3a7b7cd0e90cbd06b84a1f3704620559636a2268f5767d4c52d09c1b354945737046f618fe5
5d465e396 Ensure backupwallet fails when attempting to backup to source file (Tomas van der Wansem)
Pull request description:
Previous behaviour was to destroy the wallet (to zero-length)
This fixes#11375
Tree-SHA512: bfd1738659b15e3f23b6bbdf55ec12269c62c820bf701daec19500b52bd5845bb5516733c6f76f36197eb155182a8a35dc239ad4de2ef1e59bbb0f124a455759
f3d4adf Make p2p-acceptablock not an extended test (Matt Corallo)
00dcda6 [qa] test that invalid blocks on an invalid chain get a disconnect (Matt Corallo)
015a525 Reject headers building on invalid chains by tracking invalidity (Matt Corallo)
932f118 Accept unrequested blocks with work equal to our tip (Matt Corallo)
3d9c70c Stop always storing blocks from whitelisted peers (Matt Corallo)
3b4ac43 Rewrite p2p-acceptblock in preparation for slight behavior changes (Matt Corallo)
Pull request description:
@sdaftuar pointed out that the version in #11487 was somewhat DoS-able as someone could feed you a valid chain that forked off the the last checkpoint block and force you to do lots of work just walking backwards across blocks for each new block they gave you. We came up with a few proposals but settled on the one implemented here as likely the simplest without obvious DoS issues. It uses our existing on-load mapBlockIndex walk to make sure everything that descends from an invalid block is marked as such, and then simply caches blocks which we attempted to connect but which were found to be invalid. To avoid DoS issues during IBD, this will need to depend on #11458.
Includes tests from #11487.
Tree-SHA512: 46aff8332908e122dae72ceb5fe8cd241902c2281a87f58a5fb486bf69d46458d84a096fdcb5f3e8e07fbcf7466232b10c429f4d67855425f11b38ac0bf612e1
659b206 Make listsinceblock refuse unknown block hash (Russell Yanofsky)
Pull request description:
Change suggested by @theuni who noticed listsinceblock would ignore invalid block hashes causing it to return a completely unfiltered list of transactions.
Tree-SHA512: 3c8fb160265780d1334e856e853ab48e2e18372b8f1fc71ae480c3f45317048cc1fee0055d5c58031981a91b9c2bdbeb8e49a889d04ecba61729ce8109f2ce3f
This is a simple change that makes our accept requirements the
same as our request requirements, (ever so slightly) further
decoupling our consensus logic from our FindNextBlocksToDownload
logic in net_processing.
Removes checking whitelisted behavior (which will be removed, the
difference in behavior here makes little sense) and no longer
requires that blocks at the same work as our tip be dropped if not
requested (in part because we *do* request those blocks).
e065249 Add unit test for outbound peer eviction (Suhas Daftuar)
5a6d00c Permit disconnection of outbound peers on bad/slow chains (Suhas Daftuar)
c60fd71 Disconnecting from bad outbound peers in IBD (Suhas Daftuar)
Pull request description:
The first commit will disconnect an outbound peer that serves us a headers chain with insufficient work while we're in IBD.
The second commit introduces a way to disconnect outbound peers whose chains fall out of sync with ours:
For a given outbound peer, we check whether their best known block (which is known from the blocks they announce to us) has at least as much work as our tip. If it doesn't, we set a 20 minute timeout, and if we still haven't heard about a block with as much work as our tip had when we set the timeout, then we send a single getheaders message, and wait 2 more minutes. If after two minutes their best known block has insufficient work, we disconnect that peer.
We protect 4 of our outbound peers (who provide some "good" headers chains, ie a chain with at least as much work as our tip at some point) from being subject to this logic, to prevent excessive network topology changes as a result of this algorithm, while still ensuring that we have a reasonable number of nodes not known to be on bogus chains.
We also don't require our peers to be on the same chain as us, to prevent accidental partitioning of the network in the event of a chain split. Note that if our peers are ever on a more work chain than our tip, then we will download and validate it, and then either reorg to it, or learn of a consensus incompatibility with that peer and disconnect. This PR is designed to protect against peers that are on a less work chain which we may never try to download and validate.
Tree-SHA512: 2e0169a1dd8a7fb95980573ac4a201924bffdd724c19afcab5efcef076fdbe1f2cec7dc5f5d7e0a6327216f56d3828884f73642e00c8534b56ec2bb4c854a656
Change suggested by Cory Fields <cory-nospam-@coryfields.com> who noticed
listsinceblock would ignore invalid block hashes causing it to return a
completely unfiltered list of transactions.
Add comments and
- Verify sending to a account causes getaccountaddress to generate new addresses.
- Verify sending to a account causes getreceivedbyaccount to return amount received.
- Verify ways setaccount updates the accounts of existing addresses.
6d51eaefe qa: Fix race condition in sendheaders.py (Suhas Daftuar)
c96b2e4f0 qa: Fix replace-by-fee race condition failures (Suhas Daftuar)
Pull request description:
I think #11407 broke replace-by-fee by introducing a race condition. I was observing frequent failures of replace-by-fee locally, always with a mempool sync failure (the sync call was added in #11407).
It appeared to me like there were two causes: sometimes the node would be in IBD and not request the transaction that was relayed; other times the blocks generated in make_utxo wouldn't have relayed quickly enough for the spend of the transaction to be accepted. I believe I've fixed both potential errors.
ping @instagibbs
Edit: I found a race condition in the sendheaders.py test, where if the verack from the python node wasn't processed before the first block in the test was generated, then no block announcement would go out to that peer, breaking the test. Fixed by adding a sync_with_ping after waiting for verack.
Tree-SHA512: 6ad160966e432c151c1ce6e88ae67e60e47123523bda3755cf7697a00e1a5ba38de8561751826e3d7cf0e492f8c2aec298e1b4de8424ebbaf497f099a1ef1d07
Make sure wallet databases have unique fileids. If they don't, throw an error.
BDB caches do not work properly when more than one open database has the same
fileid, because values written to one database may show up in reads to other
databases.
Bitcoin will never create different databases with the same fileid, but users
can create them by manually copying database files.
BDB caching bug was reported by Chris Moore <dooglus@gmail.com>
https://github.com/bitcoin/bitcoin/issues/11429Fixes#11429
cc9ee80 Improve ZMQ functional test (João Barbosa)
Pull request description:
After #11439, this PR only improves:
- test comments;
- simplicity by removing *duplicate* tests;
- also removes duplicate code.
Tree-SHA512: 3636fa9694c827128128742ad31e635d19670c3645aef8e7b1cb46069c21631e8b0db059486a6f6e7eee237a23d93bce6df95190394b5a8dcfce31a49a72d17f
fafa003 qa: Remove never used return value of sync_with_ping (MarcoFalke)
fa9de37 qa: Make tmpdir option an absolute path (MarcoFalke)
Pull request description:
This should fix issues with the multiwallet test and its symlinks
when the tmpdir is a relative path.
Rather than fixing os.symlink to work with paths relative to a
directory descriptor, which does not work on Windows, normalize
the path instead.
Tree-SHA512: 189690f3d065ea2f0f48e06775c86d513d0916c7c86312432e8e16df160e65539e288c2bd53d49a4180735fa940f6fcd52b506ccd7d9815651a9b1a69850dda6
c5dfa90 [tests] Add uacomment tests (Cristian Mircea Messel)
Pull request description:
Checks for setting the value, max length and reserved characters
Tree-SHA512: a62e2cf8e455a3cd3987c0855f7bfc49de47504c01263e3573366e3cbff400c5678224773d4f1e4ac684fff34d987994e490a0978c4da05ff2a4bfa972c84723
7a91ceb5e [QA] Add RPC based rescan test (Jonas Schnelli)
c77170fbd [Wallet] add rescanblockchain <start_height> <stop_height> RPC command (Jonas Schnelli)
Pull request description:
A RPC rescan command is much more flexible for the following reasons:
* You can define the start and end-height
* It can be called during runtime
* It can work in multiwallet environment
Tree-SHA512: df67177bad6ad1d08e5a621f095564524fa3eb87204c2048ef7265e77013e4b1b29f991708f807002329a507a254f35e79a4ed28a2d18d4b3da7a75d57ce0ea5
This should fix issues with the multiwallet test and symlinks
when the tmpdir is a relative path.
Rather than fixing os.symlink to work with paths relative to a
directory descriptor, which does not work on Windows, normalize
the path instead.
4526d21 Add test for multiwallet batch RPC calls (Russell Yanofsky)
74182f2 Add missing batch rpc calls to python coverage logs (Russell Yanofsky)
505530c Add missing multiwallet rpc calls to python coverage logs (Russell Yanofsky)
9f67646 Make AuthServiceProxy._batch method usable (Russell Yanofsky)
e02007a Limit AuthServiceProxyWrapper.__getattr__ wrapping (Russell Yanofsky)
edafc71 Fix uninitialized URI in batch RPC requests (Russell Yanofsky)
Pull request description:
This fixes "Wallet file not specified" errors when making batch wallet RPC calls with more than one wallet loaded. This issue was reported by @NicolasDorier in https://github.com/bitcoin/bitcoin/issues/11257
Request URI is not used for anything except multiwallet request dispatching, so this change has no other effect.
Tree-SHA512: b3907af48a6323f864bb045ee2fa56b604188b835025ef82ba3d81673244c04228d796323cec208a676e7cd578a95ec7c7ba1e84d0158b93844d5dda8f6589b9
857b32b [tests] Add -walletnotify functional test (João Barbosa)
df18d29 [tests] Add -blocknotify functional test (João Barbosa)
9c72a46 [tests] Tidy up forknotify.py (John Newbery)
Pull request description:
This patch adds the missing functional tests for `-blocknotify` and `-walletnotify` notifications. The `-alertnotify` test file `forknotify.py` is renamed to `notifications.py` to accommodate the new tests. Credits to @jnewbery for this cleanup and unification.
Tree-SHA512: ee7b28b9f1bd225587efaefa6427c6d080ccb50ead390b23d94717c137a317183b37be00da0b2bffedd4192a363c971dea510d78d29278eb9fa76374f7855c09
eff4bd8 [test] P2P functional test for certain fingerprinting protections (Jim Posen)
a2be3b6 [net] Ignore getheaders requests for very old side blocks (Jim Posen)
Pull request description:
Sending a getheaders message with an empty locator and a stop hash is a request for a single header by hash. The node will respond with headers for blocks not in the main chain as well as those in the main chain. To avoid fingerprinting, the node should, however, ignore requests for headers on side branches that are too old. This replicates the logic that currently exists for `getdata` requests for blocks.
Tree-SHA512: e04ef61e2b73945be6ec5977b3c5680b6dc3667246f8bfb67afae1ecaba900c0b49b18bbbb74869f7a37ef70b6ed99e78ebe0ea0a1569369fad9e447d720ffc4
47ba8cf71 scripted-diff: rename assert_raises_jsonrpc to assert_raises_rpc error (John Newbery)
677d893ff [tests] do not allow assert_raises_message to be called with JSONRPCException (John Newbery)
5864e9c16 [tests] remove direct testing on JSONRPCException from individual test cases (John Newbery)
Pull request description:
I did this a few months ago (here: #9707), but a few new examples have crept back in.
When testing RPC failures, the test case should always assert the error value and message, to ensure that the failure was for the correct reason. Not doing that can hide bugs in the test code and mean that the test is not testing the correct behaviour.
RPC failure testing should use the utility function `assert_raises_jsonrpc()` (renamed in the final commit of this PR to `assert_raises_rpc_error()`.
This PR does the following:
- changes all remaining instances of tests directly testing on `JSONRPCException` to calls to `assert_raises_jsonrpc()`
- prevents `assert_raises_message()` from being called with `JSONRPCException`
- scripted-diff changes `assert_raises_jsonrpc()` to `assert_raises_rpc_error()`
Tree-SHA512: 2cc5e320704ec623a6e5a27d3c2c81cea86b502e29896f03bb5bf92cc36725132c1144410aecdf49e90d4577d512ee467d50d8184e9d5c5d0870bfc931316a5a
aa57590d7 Update importprivkey named args documentation (Dusty Williams)
Pull request description:
Addresses issue #11462 by updating the documentation for the importprivkey arguments to the correct names, and updates the functional test importprunedfunds.py to use named arguments when calling importprivkey.
Tree-SHA512: 64e14bf89c8c6eec9c37f6ec0c9fc0012fdb035d9ec32cd652110c75abaa922ec5c7523d6ec5098c8a7b42124159b5e330e070974eb79b8b92816f8d61074523
Fixes#11462. Updated documentation for importprivkey function to use the correct name for the first argument.
Also updates a call to importprivkey to use named args in functional test.
0cd9273 rpc: Prevent `dumpwallet` from overwriting files (Wladimir J. van der Laan)
Pull request description:
Prevent arbitrary files from being overwritten by `dumpwallet`. There have been reports that users have overwritten wallet files this way. It may also avoid other security issues.
Fixes#9934. Adds mention to release notes and adds a test.
Tree-SHA512: 268c98636d40924d793b55a685a0b419bafd834ad369edaec08227ebe26ed4470ddea73008d1c4beb10ea445db1b0bb8e3546ba8fc2d1a411ebd4a0de8ce9120
Split off AuthServiceProxy.get_request method to make it easier to batch RPC
requests without duplicating code and remove leading underscore from _batch
method.
This does not change any existing behavior.
Change AuthServiceProxyWrapper.__getattr__ to only wrap proxied attributes, not
real attributes. This way AuthServiceProxyWrapper can continue logging RPC
calls without complicating other object usages, and special case handling for
the .url property can be dropped.
1088b5322 add functional test for mempoolreplacement command line arg (Gregory Sanders)
Pull request description:
Currently untested.
Tree-SHA512: 2dd9d55a3499844e48b3774df9155fd650220b0761da45d16869570356bb0ed17a88d4efa4302a517dd96e1e9cb34113661b3c9df688736f6849201a3d544deb
07704c1 Add some tests for getchaintxstats (Akio Nakamura)
3336676 Fix getchaintxstats() (Akio Nakamura)
Pull request description:
1. calculate nblocks more adaptive.
-> set default nblocks to min (blocks for 1 month, target block's height - 1)
-> before PR: if not specify nblocks-parameter, illegal parameter error will happen when target block height is below nblocks.
2. correct error message.
-> nblocks accepts [1 .. block's height -1] . so add a word "-1".
3. add check 0-divide.
-> if nTimeDiff = 0 then use UniValue(UniValue::VNULL) and returns {... "txrate": null} .
-> before PR: if nTimeDiff = 0 then returns {... "txrate":} and bitcoin-cli cannot handle the response.
Tree-SHA512: e1962ce7bb05a5bc7dec03eb04a8e7578f50fdb68927fcfc0a2232905ef4d679293eee148ebe0866682d209a8c458d21fbe71715e7311adb81f37089aae1ed93
49f869f Fix bip68-sequence rpc test (Johnson Lau)
Pull request description:
The test mined 1 extra block for the ACTIVE state. Test added to catch the right moment of LOCKED_IN->ACTIVE transaction
Tree-SHA512: a42477cf0b137e7e3b7c6c7b2530101cfad4e4f59866170b8fc0d655c43b3144aad6bca4287a4a8df4c28d7cf08d3f8df166975ad2e8dcb7d2cc15de60cf11cd
f97ab35fa qa: Fix bug introduced in p2p-segwit.py (Suhas Daftuar)
a7820422e qa: Treat mininode p2p exceptions as fatal (Suhas Daftuar)
Pull request description:
#11121 inadvertently broke the constructor for the `TestNode()` object in `p2p-segwit.py`, silently breaking at least one of the tests.
Although the python code was raising exceptions due to a `TestNode()` object not existing (or having the right type), mininode was masking these from anyone running the test through the test_runner (like travis), because it catches all exceptions during message delivery and just prints a log message and continues. Such "graceful" handling of errors is almost certainly something we don't want in our test suite, so the first commit here attempts to prevent that type of failure from ever being masked.
The second commit fixes the particular bug in `p2p-segwit.py`.
Tree-SHA512: b6646e3cb1e05c35c28e8899c44104bf2e2d0384643ca87042ab7f6ec0960d89f5bf25a7b95bab6e32d401c20a6018226160500f6ddceb923e81ffb04adb4f2f
8213838 [Qt] tolerate BIP173/bech32 addresses during input validation (Jonas Schnelli)
06eaca6 [RPC] Wallet: test importing of native witness scripts (NicolasDorier)
fd0041a Use BIP173 addresses in segwit.py test (Pieter Wuille)
e278f12 Support BIP173 in addwitnessaddress (Pieter Wuille)
c091b99 Implement BIP173 addresses and tests (Pieter Wuille)
bd355b8 Add regtest testing to base58_tests (Pieter Wuille)
6565c55 Convert base58_tests from type/payload to scriptPubKey comparison (Pieter Wuille)
8fd2267 Import Bech32 C++ reference code & tests (Pieter Wuille)
1e46ebd Implement {Encode,Decode}Destination without CBitcoinAddress (Pieter Wuille)
Pull request description:
Builds on top of #11117.
This adds support for:
* Creating BIP173 addresses for testing (through `addwitnessaddress`, though by default it still produces P2SH versions)
* Sending to BIP173 addresses (including non-v0 ones)
* Analysing BIP173 addresses (through `validateaddress`)
It includes a reformatted version of the [C++ Bech32 reference code](https://github.com/sipa/bech32/tree/master/ref/c%2B%2B) and an independent implementation of the address encoding/decoding logic (integrated with CTxDestination). All BIP173 test vectors are included.
Not included (and intended for other PRs):
* Full wallet support for SegWit (which would include automatically adding witness scripts to the wallet during automatic keypool topup, SegWit change outputs, ...) [see #11403]
* Splitting base58.cpp and tests/base58_tests.cpp up into base58-specific code, and "address encoding"-code [see #11372]
* Error locating in UI for BIP173 addresses.
Tree-SHA512: 238031185fd07f3ac873c586043970cc2db91bf7735c3c168cb33a3db39a7bda81d4891b649685bb17ef90dc63af0328e7705d8cd3e8dafd6c4d3c08fb230341
395cef7 Change getmininginfo errors field to warnings (Andrew Chow)
8502b20 Unify help text for GetWarnings output in get*info RPCs (Andrew Chow)
f77f0e4 Add warnings field to getblockchaininfo (Andrew Chow)
Pull request description:
The `getblockchaininfo` output does not contain the `errors` field which the `getinfo`, `getmininginfo`, and `getnetworkinfo` RPCs have. It should have it as the errors pertain to the blockchain. This PR adds that field.
This PR also unifies the help text for the `errors` field and its output position so that all of the `get*info` commands are consistent.
`getnetworkinfo`'s `errors` field is named `warnings`. I did not change this even though it is inconsistent since this naming has been in use for a long time.
Tree-SHA512: 385ab6acfee67fc8816f4d51ab2bd7a623264c7973906dfbab0a171f199e9db16fde19093a5bc3dfbdd4ff5f19d2186b646eb6b3bae0a4d7c9add43650a4a9d9
5e69a43 Add test for bitcoin-cli -getinfo (John Newbery)
3826253 rpc: Handle `getinfo` locally in bitcoin-cli w/ `-getinfo` (Wladimir J. van der Laan)
Pull request description:
Since @laanwj doesn't want to maintain these changes anymore, I will.
This PR is a revival of #8843. I have addressed @jnewbery's comments.
Regarding atomicity, I don't think that is a concern here. This is explicitly a new API and those who use it will know that this is different and that it is not atomic.
Tree-SHA512: 9664ed13a5557bda8c43f34d6527669a641f260b7830e592409b28c845258fc7e0fdd85dd42bfa88c103fea3ecdfede5f81e3d91870e2accba81c6d6de6b21ff
Changes the errors field to warnings. To maintain compatibility,
the errors field is deprecated and enabled by starting bitcoind with
-deprecatedrpc=getmininginfo
048e0c3e2 [rpc] [tests] Add deprecated RPC test (Cristian Mircea Messel)
d4cdbd6fb [rpc] Deprecate estimatefee RPC (John Newbery)
Pull request description:
Deprecates estimatefee in v0.16, for final removal in v0.17.
This commit introduces a phased removal of RPC methods. RPC method is
disabled by default in version x, but can be enabled by using the
`-deprecatedrpc=<methodname>` argument. RPC method is removed entirely in version
(x+1).
This gives users fair warning that an RPC is to be removed, and time to change client software if necessary. Deprecation warnings in RPC return values or release notes are easily ignored.
This is a more generic version of the approach I tried to use in #10841, which too late to make it into v0.15.
Tree-SHA512: 9695a600e84b812974387333e4a6805d18972da30befb754e9e4da77cd9815d00c5cc2ee0b0350bdbbdb5fdc6ba47789f8b2c6f5b15c8cd5a1deefcc4832da30
Deprecate estimatefee in v0.16, for final removal in v0.17.
This commit introduces a phased removal of RPC methods. RPC method is
disabled by default in version x, but can be enabled by using the
`-deprecatedrpc=<method>` argument. RPC method is removed entirely in
version (x+1).
Prevent arbitrary files from being overwritten. There have been reports
that users have overwritten wallet files this way. It may also avoid
other security issues.
Fixes#9934. Adds mention to release notes and adds a test.
fadf31e wallet: Display non-HD error on first run (MarcoFalke)
Pull request description:
On current master a fresh wallet created with `-usehd=0` is silently created as HD wallet.
An error should be displayed on the first run.
Also, this restores a test that was removed in c22a53cFixes: #11313
Tree-SHA512: 226a4129984324f88a431c7e2726383f6841711f0227d8e9f5b4f89d4bb9f2b8e922e6cf0a6f91d6efa747d139543a236b9f29326fc5d1e5d6f1dea2465d9b85
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
assumevalid.py would try to send over a closed P2P connection in a loop,
hitting the following failure many times:
TestFramework.mininode (ERROR): Cannot send message. No connection to node!
The test still passes, but this is a lot of noise in the test log.
Just check that the connection is open before trying to send.
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.
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
fadd0c16b [qa] zapwallettxes: Wait up to 3s for mempool reload (MarcoFalke)
Pull request description:
There had been intermittent test failures on zapwallettxes, as no time was allotted to reload the mempool.
Tree-SHA512: 993254d2aaca6ea42fceefffed0cf90bdda91c64150179ef2a11337c3fe2cc6bf42b83ea9d9a1a575204fbde2676d7203443b00d053e8c2ed43e017c09d3ab11
8fdb6f912 [tests] fixup dbcrash interaction with add_nodes() (John Newbery)
Pull request description:
Another conflict with #11121. Apologies - this is entirely my fault. I didn't run the extended test suite after rebasing on master.
@MarcoFalke @sdaftuar
Tree-SHA512: eefce1d1c63dc4a63c5e030a541e046ad4832e8a709c0a8aad40ffdc4712b2065486778b406dfa57cfd34e66db86064278ee3fea8f2c2afd2390772875e6fa3e
* This removes block-size-limiting code in favor of GBT clients
doing the limiting themselves (if at all).
* -blockmaxsize is deprecated and only used to calculate an implied
blockmaxweight, addressing confusion from multiple users.
* getmininginfo's currentblocksize return value was returning
garbage values, and has been removed, also removing a
GetSerializeSize call in some block generation inner loops and
potentially addressing some performance edge cases.
713a92073 Remove usehd option and warn when it is used (Andrew Chow)
d4c18f733 Bump wallet version number to 159900 (Andrew Chow)
Pull request description:
Bump the wallet version number to 159900 so that new wallets made without a default key will no longer work on previous versions at all. Also remove the `usehd` option to avoid weird interaction with wallet version numbers and HD-ness of wallets.
Tree-SHA512: dd7965505bfad6a926c79afd423236f509229a398a8398076f8d57d90a5974243f9459a61225c4daee560c796f427445c9e55a3ad528a3a97a9123ca6a1269ab
aece8a463 (finally) remove getinfo in favor of more module-specific infos (Matt Corallo)
Pull request description:
I see no reason not to have done this in 0.13, let alone for 0.15.
Tree-SHA512: ed3e36f99e9cb90304089e5957ddfbf74141e3e77d850e498e9e45dd8bc1deb9fe36b3fec4c43243023268670a45808de3c23d660df76fa27db6688814c464a5
1aa97ee08 Add savemempool RPC (Lawrence Nahum)
467cbbcbf Add return value to DumpMempool (Lawrence Nahum)
Pull request description:
Adds a simple parameterless rpc command to dump the mempool.
Rationale:
Sometimes there can be a crash for whatever reason (bug, power loss, etc) causing the mempool.dat file to not be saved.
This change allows to script/cron the rpc call to have more regular saves to the file as well as cli/ad-hoc.
This should solve issue https://github.com/bitcoin/bitcoin/issues/11086
Tree-SHA512: e856ae9777425a4521279c9b58e69285d8e374790bebefd3284cf91931eac0e456f86224f427a087a01bf70440bf6e439fa02c8a34940eb1046ae473e98b6aaa
617c459c6 qa: rpc test for wtxid in mempool entry (Suhas Daftuar)
7e5d5965d RPC: add wtxid to mempool entry output (Suhas Daftuar)
Pull request description:
We already cache this information in the mempool, so including it in the output of rpc calls is basically free.
Tree-SHA512: 2757e1bfca028103937e4b76ce1a5d805846bad5d3d9dd631dcc5f87721bcc0e9d19e437e02053ef1dd3b38b503f0fca8c0b8492cac37dfbd70256a3665f704c
faa8d9581 [qa] TestNode: Add wait_until_stopped helper method (MarcoFalke)
Pull request description:
This adds a helper method `wait_until_stopped` to the `TestNode` class. This should prevent numerous `time.sleep()` over all places.
Additionally, the timeout behavior is restored. (Was removed by the introduction of `TestNode`.)
This should prevent tests from running indefinitely by accident.
Tree-SHA512: 7133fc64d55711869c4e372e9d30625c98f1237fb3578c24a26900d9319831f10eb95592d7b08e536fa706158dffb0abf9197f11c5d9ef605c880628e1a6533f
eac64bb7a [qa] Test nMinimumChainWork (Suhas Daftuar)
0311836f6 Allow setting nMinimumChainWork on command line (Suhas Daftuar)
Pull request description:
As discussed briefly here: https://botbot.me/freenode/bitcoin-core-dev/2017-02-28/?msg=81712308&page=4
This adds a hidden command line option for setting `nMinimumChainWork`, which allows us to test this parameter in our functional tests, as well as allowing for niche use cases like syncing nodes that are otherwise disconnected from the network.
See also #10345, which proposes a new use of `nMinimumChainWork`.
Tree-SHA512: fe4d8f4f289697615c98d8760f1cc74c076110310ea0b5b875fcab78c127da9195b4eb84148aebacc7606c246e5773d3f13bd5d9559d0a8bffac20a3a28c62df
Nodes don't consider themselves out of "initial block download" until
their active chain has more work than nMinimumChainWork.
While in initial block download, nodes won't relay blocks to their
peers, so test that this parameter functions as intended by verifying
that block relay only succeeds past a given node once its
nMinimumChainWork has been exceeded.
7148b74dc [tests] Functional tests must explicitly set num_nodes (John Newbery)
5448a1471 [tests] don't override __init__() in individual tests (John Newbery)
6cf094a02 [tests] Avoid passing around member variables in test_framework (John Newbery)
36b626867 [tests] TestNode: separate add_node from start_node (John Newbery)
be2a2ab6a [tests] fix - use rpc_timeout as rpc timeout (John Newbery)
Pull request description:
Some additional tidyups after the introduction of TestNode:
- commit 1 makes TestNode use the correct rpc timeout. This should have been included in #11077
- commit 2 separates `add_node()` from `start_node()` as originally discussed here: https://github.com/bitcoin/bitcoin/pull/10556#discussion_r121161453 with @kallewoof . The test writer no longer needs to assign to `self.nodes` when starting/stopping nodes.
- commit 3 adds a `set_test_params()` method, so individual tests don't need to override `__init__()` and call `super().__init__()`
Tree-SHA512: 0adb030623b96675b5c29e2890ce99ccd837ed05f721d0c91b35378c5ac01b6658174aac12f1f77402e1d38b61f39b3c43b4df85c96952565dde1cda05b0db84
Almost all test scripts currently need to override the __init__()
method. When they do that they need to call into super().__init__() as
the base class does some generic initialization.
This commit makes the base class __init__() call into set_test_params()
method. Individual test cases can override set_test_params() to setup
their test parameters.
Separates the act of creating a TestNode object from starting the node.
The test_framework now keeps track of its list of TestNodes, and test
writers can call start_node() and stop_node() without having to update
the self.nodes list.
4f2905b76 Add getmininginfo functional test (Cristian Mircea Messel)
Pull request description:
Add `getmininginfo` functional test in `mining.py`
Tree-SHA512: 12be9cfb37e9ac4c6625fc06051704c8a8dfd7271c2654f994c7659c8810e4b7a4335105ae159315308bcd45b65589bab1829bd134d2f4cabf74d63f2e5d22fe
6d2d2eb49 RPC: gettxout: Slightly improve doc and tests (Jorge Timón)
Pull request description:
Slightly related to https://github.com/bitcoin/bitcoin/pull/10822 in the sense that I felt the documentation and testing wasn't as good as it could be while writing it.
Ping @sipa since we discussed this on IRC.
Tree-SHA512: a0b3ffdac65245a0429e772fc2d8bcc1e829b02c70fb2af6ee0b7578cae46683f6c51a824b4d703d4dc3f99b6f03a658d6bbc818bf32981516f24124249a211d
c6ec4358a [tests] Add bitcoin_cli.py test script (John Newbery)
b23549f6e [tests] add TestNodeCLI class for calling bitcoin-cli for a node (John Newbery)
Pull request description:
We don't test bitcoin-cli at all. That means that we can miss inconsistencies between the bitcoin-cli client and the RPC interface, such as #10698 and #10747. It also means that the various bitcoin-cli options and features are untested and regressions could be silently introduced.
Let's fix that.
This PR adds bitcoin-cli testing in the python functional test_framework:
1. Add a bitcoin_cli.py test script that tests bitcoin-cli. At the moment it only tests that the result of `getinfo` is the same if you run it as an RPC or through bitcoin-cli, but can easily be extended to test additional bitcoin-cli features
**EDIT: `--usecli` option is moved to a separate PR. This PR now only covers the bitcoin_cli.py test.**
2. ~Add a `--usecli` option to the test framework. This changes the test to use bitcoin-cli for all RPC calls instead of using direct HTTP requests. This is somewhat experimental. It works for most tests, but there are some cases where it can't work transparently because:~
- ~the testcase is asserting on a specific error code, and bitcoin-cli returns a different error code from the direct RPC~
- ~we're sending a very large RPC request (eg `submitblock`) and it can't be serialized into a shell bitcoin-cli call.~
~I think that even though `--usecli` doesn't work on all tests, it's still a useful experimental feature. Future potential enhancements:~
- ~enhance the framework to automatically skip tests that are known to fail with bitcoin-cli if the `--usecli` option is used.~
- ~run a subset of tests in Travis with `-usecli`~
This builds on and requires the `TestNode` PR #10711 . As an aside, this is a good demonstration of how tidy it is to add additional features/interfaces now that test node logic/state is encapsulated in a TestNode class.
Addresses #10791
Tree-SHA512: a1e6be12e8e007f6f67b3d3bbcd142d835787300831eb38e6027a1ad25ca9d79c4bc99a41b19e31ee95205cba1b3b2d21a688b5909316aad70bfc2b4eb6d8a52
08ce33f8e qa: Move wait_until to util (MarcoFalke)
Pull request description:
This moves `wait_until` to `util.py` to make it generally available to python tests.
Also, `wait_until` now takes an optional lock that is acquired while testing the predicate.
Previously the lock was always acquired, even when it was not necessary, cf. `disconnect_ban.py`.
Tree-SHA512: 18e452a017a6566fa8ad09bde058e1b841e167039dc63299e70cfa7a6dcbc779581e60ca3e8eb2f1b610767d5208b9376c203eb11015b250fd0542b5eb4215a8
Fixes a couple of bugs from the introduction of TestNode:
- test scripts were no longer able to specify a custom timeout for
starting a node. Therefore tests with nodes that take a long time to
start up (eg pruning.py) would fail.
- the test for whether a node has failed on start up was broken
by changing 'assert x is None' to 'assert not x'. Since
subprocess.poll() can return None (indicating the node is still running)
or 0 (indicating the node exited with return code 0), this was a
regression.
When running the tests locally with a parallelism of 4 on an otherwise
busy system, RPC can take quite a wait to come up.
Change the timeout to 60 seconds just to be safe.
e53615b Remove vchDefaultKey and have better first run detection (Andrew Chow)
Pull request description:
Removes vchDefaultKey which was only used for first run detection. Improves wallet first run detection by checking to see if any keys were read from the database.
This also fixes a (rather contrived) case where an encrypted non-HD wallet has corruption such that the default key is no longer valid and is loaded into a Core version that supports HD wallets. This causes a runtime exception since a new hd master key is generated as the software believes the wallet file is newly created but cannot add the generated key to the wallet since it is encrypted. I was only able to replicate this error by creating a non-hd wallet, encrypting it, then editing the wallet using `db_dump` and `db_load` before loading the wallet with hd enabled. This problem has been reported by [two](https://bitcointalk.org/index.php?topic=1993244.0) [users](https://bitcointalk.org/index.php?topic=1746976.msg17511261#msg17511261) so it is something that can happen, although that raises the question of "what corrupted the default key".
~P.S. I don't know what's up with the whitespace changes. I think my text editor is doing something stupid but I don't think those are important enough to attempt undoing them.~ Undid those
Tree-SHA512: 63b485f356566e8ffa033ad9b7101f7f6b56372b29ec2a43b947b0eeb1ada4c2cfe24740515d013aedd5f51aa1890dfbe499d2c5c062fc1b5d272324728a7d55
1d8df0141 Fix MD formatting in REST-interface.md and spelling mistake in test_runner.py (MeshCollider)
41f3e84aa Fix inconsistencies and grammar in various files (MeshCollider)
Pull request description:
Just a simple fix of some inconsistent capitalization, formatting and grammar in a few files (no code changes)
Tree-SHA512: 60b12a5a5c69a1af4a25b7db0b32ed806ed62ad2966cee08b3792a7cfa7f51848fd485349b4c09e60a7eedfdf55ee730c51daa066d6e226ae404c93342bf3e13
f42fc1d50 doc: spelling fixes (klemens)
Pull request description:
patch contains some spelling fixes ( just in comments ) as found by a bot ( http://www.misfix.org, https://github.com/ka7/misspell_fixer ).
Tree-SHA512: ba6046cfcd81b0783420daae7d776be92dd7b85a593e212f8f1b4403aca9b1b6af12cef7080d4ea5ed4a14952fd25e4300109a59c414e08f5395cdb9947bb750
Removes vchDefaultKey which was only used for first run detection.
Improves wallet first run detection by checking to see if any keys
were read from the database.
This will now also check for a valid defaultkey for backwards
compatibility reasons and to check for any corruption.
Keys will stil be generated on the first one, but there won't be
any shown in the address book as was previously done.
