c8330d4 qa: Use node.datadir instead of tmpdir in test framework (MarcoFalke)
Pull request description:
Commit c53c9831ee introduced the utility function `get_datadir_path`, however not all places in the code use this util function. Using the util function everywhere makes it easier to review pull requests related to the datadir.
This commit replaces datadir path creation with the `datadir` member of `TestNode`, which itself uses `get_datadir_path`.
Tree-SHA512: c75707ab7149d732a6d56152a5813138a33459d3d07577b60b89f2a207c83b7663fac5f203593677c9892d1c23a5eba4bd45c5c4ababf040d720b437240fcddf
fae1374 qa: Allow for partial_match when checking init error (MarcoFalke)
5812273 [Tests] Require exact match in assert_start_raises_init_eror() (John Newbery)
0ec08a6 [Tests] Move assert_start_raises_init_error method to TestNode (John Newbery)
Pull request description:
Extracted from #12379, because the changes are important on their own.
This allows for exact testing, since the match can be specified with a strict regex. Internal details (such as exact formatting of the error message) can still be fuzzed away by regex wildcards.
Tree-SHA512: 605d2c9c42362a32d42321b066637577a026d0bb8cfc1c9f5737a4ca6503ffe85457a5122cea6e1101053ccc6c8aa1bbae3602e1fa7d2988bf7d5c275f412f66
4d9b4256d8 Fix typos (Dimitris Apostolou)
Pull request description:
Unfortunately I messed up my repo while trying to squash #12593 so I created a PR with just the correct fixes.
Tree-SHA512: 295d77b51bd2a9381f1802c263de7ffb2edd670d9647391e32f9a414705b3c8b483bb0e469a9b85ab6a70919ea13397fa8dfda2aea7a398b64b187f178fe6a06
Several test scripts define a subclass of P2PInterface called TestNode.
This commit renames those to TestP2PConn since we already have a
TestNode class in the test framework.
-BEGIN VERIFY SCRIPT-
sed -i s/TestNode/TestP2PConn/ test/functional/*py test/functional/test_framework/comptool.py
_END VERIFY SCRIPT-
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
fa1436c42 [qa] util: Remove unused sync_chain (MarcoFalke)
Pull request description:
The util function `sync_blocks` already checks for equal chains, so we can remove the unused `sync_chain`.
Also cleaned up the errors that are printed in case of timeout:
```
AssertionError: Block sync timed out:
'72a3a3e9dcfd0a09204c3447af0f481d19641eeadbe6a91b8e680ed614bc7712'
'5032af4ae22ae7a21afdc9d9f516877309c4dd8ef2ecadb9354be7088439b4a6'
'5032af4ae22ae7a21afdc9d9f516877309c4dd8ef2ecadb9354be7088439b4a6'
```
and
```
AssertionError: Mempool sync timed out:
{'c2af943d9b321c36e0f5a153a9d3d8b11bdd46ceb28e38f5fd2c722e3edb3563'}
set()
set()
```
Tree-SHA512: cb4ad30e3e3773072f59afa2c81cfa27bd8f389a93f02acb919990298627fcfbaa53a3d3944d827cc8a5d009871e42a47ea09e9bb93e85c22e3af6a24a574e5d
* Z is the zone designator for the zero UTC offset.
* T is the delimiter used to separate date and time.
This makes it clear for the end-user that the date/time logged is
specified in UTC and not in the local time zone.
fa3528a85b qa: Fix some tests to work on native windows (MarcoFalke)
Pull request description:
This allows some more tests to be run natively on Windows
Tree-SHA512: 8097a82dc046be9f6bb0da634758c9afef7836960ca7a1f88f9acab9512dbf7bc26525b515faae407edab4620846cce2b427940298f822e250f23f924b4c7591
b156ff7c3 [tests] bind functional test nodes to 127.0.0.1 (Sjors Provoost)
Pull request description:
Replaces #12200 which broke `rpc_bind.py`.
Prevents OSX firewall allow-this-application-to-accept-inbound-connections permission popups and is generally safer.
To prevent binding to `127.0.0.1`, set `self.bind_to_localhost_only = False`.
cc @jnewbery
Tree-SHA512: 5e700124c91bd0cbdee83ca44910071d71d61d8842334755b685d14fbff6454d75de1ea7de67340370386f58b41361e80e90bb4dca5c4d5992f9d2b27985f999
Prevents OSX firewall allow-this-application-to-accept-inbound-connections
permission popups and is generally safer.
To prevent binding to 127.0.0.1, set self.bind_to_localhost_only = False.
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
eb91835 Add setter for g_initial_block_download_completed (Jonas Schnelli)
3f56df5 [QA] add NODE_NETWORK_LIMITED address relay and sync test (Jonas Schnelli)
158e1a6 [QA] fix mininode CAddress ser/deser (Jonas Schnelli)
fa999af [QA] Allow addrman loopback tests (add debug option -addrmantest) (Jonas Schnelli)
6fe57bd Connect to peers signaling NODE_NETWORK_LIMITED when out-of-IBD (Jonas Schnelli)
31c45a9 Accept addresses with NODE_NETWORK_LIMITED flag (Jonas Schnelli)
Pull request description:
Eventually connect to peers signalling NODE_NETWORK_LIMITED if we are out of IBD.
Accept and relay NODE_NETWORK_LIMITED peers in addrman.
Tree-SHA512: 8a238fc97f767f81cae1866d6cc061390f23a72af4a711d2f7158c77f876017986abb371d213d1c84019eef7be4ca951e8e6f83fda36769c4e1a1d763f787037
face7220b7 qa: Move common args to bitcoin.conf (MarcoFalke)
Pull request description:
Beside removing duplicates of the same args in the code, this actually helps with debugging after a test failure.
For example, `bitcoin-qt` has `server` turned off, so you'd have to turn it on every time, if you wanted to debug a temporary test datadir created by the test framework.
Also, `keypool` would fill up if you forget to specify `-keypool=1`.
Tree-SHA512: 996bec8738dc0fce7297ab1fc5b4fbe3aa31b9b6241f8c4e685db728925363ccaca6145ad1edc6bee2f360e02ac4b9a53fcdff74eb79de90793393742d52b559
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
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.
Splits signrwatransaction into a wallet version (signrawtransactionwithwallet) and
non-wallet version (signrawtransactionwithkey). signrawtransaction is marked as DEPRECATED
and will call the right signrawtransaction* command as per the parameters in order to
maintain compatibility.
Updated signrawtransactions test to use new RPCs
b22cce014 scripted-diff: validateaddress to getaddressinfo in tests (Andrew Chow)
b98bfc5ed Create getaddressinfo RPC and deprecate parts of validateaddress (Andrew Chow)
1598f3230 [rpc] Move DescribeAddressVisitor to rpc/util (John Newbery)
39633ecd5 [rpc] split wallet and non-wallet parts of DescribeAddressVisitor (John Newbery)
Pull request description:
This PR makes a new RPC command called `getaddressinfo` which relies on the wallet. It contains all of `validateaddress`'s address info stuff. Those parts in `validateaddress` have been marked as deprecated. The tests have been updated to use `getaddressinfo` except the `disablewallet` test which is the only test that actually uses `validateaddress` to validate an address.
Tree-SHA512: ce00ed0f2416200b8de1e0a75e8517c024be0b6153457d302c3879b3491cce28191e7c29aed08ec7d2eeeadc62918f5c43a7cb79cd2e4b6d9291bd83ec31c852
Change all instances of validateaddress to getaddressinfo since it seems that
no test actually uses validateaddress for actually validating addresses.
-BEGIN VERIFY SCRIPT-
find ./test/functional -path '*py' -not -path ./test/functional/wallet_disable.py -not -path ./test/functional/rpc_deprecated.py -not -path ./test/functional/wallet_address_types.py -exec sed -i'' -e 's/validateaddress/getaddressinfo/g' {} \;
-END VERIFY SCRIPT-
65682da [tests] bind functional test nodes to 127.0.0.1 (Sjors Provoost)
Pull request description:
Prevents OSX firewall allow-this-application-to-accept-inbound-connections permission popups and is generally safer.
To test, make an arbitrary whitespace change to `src/bitcoind.cpp` and recompile. This normally resets the firewall's memory.
Easiest way to reproduce a popup without running the test suite:
```sh
src/bitcoind -regtest -bind=127.0.0.1 # No popup
src/bitcoind -regtest # Popup
```
Tree-SHA512: ffa92f148a2ead2ceca978c285882979639be23eb31ad6a27aa81df9fdddba5d493719c92c09a351a81d638f6f739c351a721e42168d77ead60abe074df773d6
95e2e9a [tests] Change invalidtxrequest to use BitcoinTestFramework (John Newbery)
359d067 [tests] Fix flake8 warnings in invalidtxrequest (John Newbery)
c32cf9f [tests] Add P2PDataStore class (John Newbery)
cc046f6 [tests] Reduce NodeConn connection logging from info to debug (John Newbery)
Pull request description:
Next step in #10603
- first commit changes log level for an internal log from INFO to DEBUG. (Not really related, but I started finding the INFO level logging annoying when debuging test failures)
- second commit introduces a `P2PStub` class - a subclass of `NodeConnCB` which has its own block and tx store and responds appropriately to getdata requests. Not all the functionality is used in `invalidtxrequest.py`, but will be used in `invalidblockrequest.py` and `p2p-fullblocktest` when those are changed to use `BitcoinTestFramework`
- third commit tidies up `invalidtxrequest.py`
- fourth commit removes usage of `ComparisonTestFramework`
Tree-SHA512: f3085c73c15d6ce894e401490bce8a7fa7cf52b0c9d135ff7e351f1f6f517c99accab8588fcdc443f39ea8315329aaabd66b2baa32499df5a774737882030373
faefd29 qa: Prepare functional tests for Windows (MarcoFalke)
Pull request description:
* Pass `sys.executable` when calling a python script via the subprocess
module
* Don't remove the log file while it is still open and written to
* Properly use os.pathsep and os.path.sep when modifying the PATH
environment variable
* util-tests: Use os.path.join for Windows compatibility
Ref: #8227
Tree-SHA512: c507a536af104b3bde4366b6634099db826532bd3e7c35d694b5883c550920643b3eab79c76703ca67e1044ed09979e855088f7324321c8d52112514e334d614
* Pass `sys.executable` when calling a python script via the subprocess
module
* Don't remove the log file while it is still open and written to
* Properly use os.pathsep and os.path.sep when modifying the PATH
environment variable
* util-tests: Use os.path.join for Windows compatibility
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
Also warn if bitcoind is configured to use a relative -datadir path.
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 also 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.
5fecd84 [tests] Remove redundant import in blocktools.py test (Anthony Towns)
9b20bb4 [tests] Check tests conform to naming convention (Anthony Towns)
7250b4e [tests] README.md nit fixes (Anthony Towns)
82b2712 [tests] move witness util functions to blocktools.py (John Newbery)
1e10854 [tests] [docs] update README for new test naming scheme (John Newbery)
Pull request description:
Splitting #11774 into two parts -- this part updates the README with the proposed naming convention, and adds some checks to test_runner.py that the number of tests violating the naming convention doesn't increase too much. Idea is this part of the change should not introduce merge conflicts or require much rebasing, so reviews of the complicated bits won't become invalidated too often; while the second part will just be file renames, which will require regular rebasing and will introduce merge conflicts with pending PRs, but can be merged later, and should also be much easier to review, since it will only include relatively trivial changes.
Tree-SHA512: b96557d41714addbbfe2aed62fb5a48639eaeb1eb3aba30ac1b3a86bb3cb8d796c6247f9c414c4695c4bf54c0ec9968ac88e2f88fb62483bc1a2f89368f7fc80
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
b224a47a1 Add address_types test (Pieter Wuille)
7ee54fd7c Support downgrading after recovered keypool witness keys (Pieter Wuille)
940a21932 SegWit wallet support (Pieter Wuille)
f37c64e47 Implicitly know about P2WPKH redeemscripts (Pieter Wuille)
57273f2b3 [test] Serialize CTransaction with witness by default (Pieter Wuille)
cf2c0b6f5 Support P2WPKH and P2SH-P2WPKH in dumpprivkey (Pieter Wuille)
37c03d3e0 Support P2WPKH addresses in create/addmultisig (Pieter Wuille)
3eaa003c8 Extend validateaddress information for P2SH-embedded witness (Pieter Wuille)
30a27dc5b Expose method to find key for a single-key destination (Pieter Wuille)
985c79552 Improve witness destination types and use them more (Pieter Wuille)
cbe197470 [refactor] GetAccount{PubKey,Address} -> GetAccountDestination (Pieter Wuille)
0c8ea6380 Abstract out IsSolvable from Witnessifier (Pieter Wuille)
Pull request description:
This implements a minimum viable implementation of SegWit wallet support, based on top of #11389, and includes part of the functionality from #11089.
Two new configuration options are added:
* `-addresstype`, with options `legacy`, `p2sh`, and `bech32`. It controls what kind of addresses are produced by `getnewaddress`, `getaccountaddress`, and `createmultisigaddress`.
* `-changetype`, with the same options, and by default equal to `-addresstype`, that controls what kind of change is used.
All wallet private and public keys can be used for any type of address. Support for address types dependent on different derivation paths will need a major overhaul of how our internal detection of outputs work. I expect that that will happen for a next major version.
The above also applies to imported keys, as having a distinction there but not for normal operations is a disaster for testing, and probably for comprehension of users. This has some ugly effects, like needing to associate the provided label to `importprivkey` with each style address for the corresponding key.
To deal with witness outputs requiring a corresponding redeemscript in wallet, three approaches are used:
* All SegWit addresses created through `getnewaddress` or multisig RPCs explicitly get their redeemscripts added to the wallet file. This means that downgrading after creating a witness address will work, as long as the wallet file is up to date.
* All SegWit keys in the wallet get an _implicit_ redeemscript added, without it being written to the file. This means recovery of an old backup will work, as long as you use new software.
* All keypool keys that are seen used in transactions explicitly get their redeemscripts added to the wallet files. This means that downgrading after recovering from a backup that includes a witness address will work.
These approaches correspond to solutions 3a, 1a, and 5a respectively from https://gist.github.com/sipa/125cfa1615946d0c3f3eec2ad7f250a2. As argued there, there is no full solution for dealing with the case where you both downgrade and restore a backup, so that's also not implemented.
`dumpwallet`, `importwallet`, `importmulti`, `signmessage` and `verifymessage` don't work with SegWit addresses yet. They're remaining TODOs, for this PR or a follow-up. Because of that, several tests unexpectedly run with `-addresstype=legacy` for now.
Tree-SHA512: d425dbe517c0422061ab8dacdc3a6ae47da071450932ed992c79559d922dff7b2574a31a8c94feccd3761c1dffb6422c50055e6dca8e3cf94a169bc95e39e959
test_framework accepts a new --usecli parameter. Running the test with
this parameter will cause all RPCs to be sent through bitcoin-cli rather
than directly over http. By default, individual test cases do not
support --usecli, and self.supports_cli must be set to True in the
set_test_params method.
We can make supports_cli default to True in future once we know which
tests will fail with use_cli.
Change TestNodeCLI.__call__() to return a new instance instead of modifying the
existing instance. This way, it's possible to create different cli objects that
have their own options (for example -rpcwallet options to connect to different
wallets), and options set for a single call (`node.cli(options).method(args)`)
will no longer leak into future calls.
Add network thread_start(), network_thread_running() and
network_thread_join() utility functions in mininode.py and use
network_thread_running() in network thread assertions.
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
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.
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
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
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
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
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.
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
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
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
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.
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
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
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
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.
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.
TestNode is a class responsible for all state related to a bitcoind node
under test. It stores local state, is responsible for tracking the
bitcoind process and delegates unrecognised messages to the RPC
connection.
This commit changes start_nodes and stop_nodes to start and stop the
bitcoind nodes in parallel, making test setup and teardown much faster.
cc5d38f4b Add option to attach a python debugger if test fails (John Newbery)
Pull request description:
Adds a simple option to the test_framework to attach pdb if the test fails.
Helpful for catching and debugging intermittent failures: Run the test in a loop with this option. The first failure will cause execution to pause and nodes will be left running for interactive debugging.
@sdaftuar
Tree-SHA512: 01cfae15fa3f04ed6ec6a99fef60a6c6a59723429309e81eacd6767caf12f5758f59b337804291ecab33a38a2958f36e2b513d201bee72a2eeb207a67046f952
In v0.15, we disconnect nodes that send us version messages with
unsupported service bits (1 << 5 and 1 << 7). This commit adds a test
that bitcoind will disconnect those nodes before August 1st 2018, and won't
disconnect those nodes after August 1st 2018.
