Commit Graph

45 Commits

Author SHA1 Message Date
Wladimir J. van der Laan
90bc40d297
Merge #13004: Print to console by default when not run with -daemon
6a3b0d3 Print to console by default when not run with -daemon (Evan Klitzke)

Pull request description:

  Cherry-picked ef6fa1c38e1bd115d1cce155907023d79da379d8 from the "up for grabs" PR: "Smarter default behavior for -printtoconsole" (#12689).

  See previous review in #12689.

Tree-SHA512: 8923a89b9c8973286d53e960d3c464b1cd026cd5a5911ba62f9f972c83684417dc4004101815dfe987fc1e1baaec1fdd90748a0866bb5548e974d77b3135d43b
2021-04-18 00:37:04 -04:00
Wladimir J. van der Laan
5179599526 Merge #12493: [wallet] Reopen CDBEnv after encryption instead of shutting down
c1dde3a949b36ce9c2155777b3fa1372e7ed97d8 No longer shutdown after encrypting the wallet (Andrew Chow)
d7637c5a3f1d62922594cdfb6272e30dacf60ce9 After encrypting the wallet, reload the database environment (Andrew Chow)
5d296ac810755dc47f105eb95b52b7e2bcb8aea8 Add function to close all Db's and reload the databae environment (Andrew Chow)
a769461d5e37ddcb771ae836254fdc69177a28c4 Move BerkeleyEnvironment deletion from internal method to callsite (Andrew Chow)

Pull request description:

  This is the replacement for #11678 which implements @ryanofsky's [suggestion](https://github.com/bitcoin/bitcoin/pull/11678#pullrequestreview-76464511).

  Shutting down the software was to prevent the BDB environment from writing unencrypted private keys to disk in the database log files, as was noted [here](https://bitcointalk.org/index.php?topic=51474.msg616068#msg616068). This PR replaces the shutdown behavior with a CDBEnv flush, close, and reopen which achieves the same effect: everything is cleanly flushed and closed, the log files are removed, and then the environment reopened to continue normal operation.

  To ensure that no unencrypted private keys are in the log files after encrypting the wallet, I wrote [this script](https://gist.github.com/achow101/7f7143e6c3d3fdc034d3470e72823e9d) to pull private keys from the original wallet file and searches for these keys in the log files (note that you will have to change your file paths to make it work on your own machine).

  As for concerns about private keys being written to slack space or being kept in memory, these behaviors no longer exist after the original wallet encryption PR and the shutting down solution from 2011.

  cc @ryanofsky

Tree-SHA512: 34b894283b0677a873d06dee46dff8424dec85a2973009ac9b84bcf3d22d05f227c494168c395219d9aee3178e420cf70d4b3eeacc9785aa86b6015d25758e75
2021-04-05 03:31:50 +02:00
UdjinM6
f22d5e2aaa
tests: Apply --timeoutscale option to rpc timeouts (#3982) 2021-02-08 14:39:05 +01:00
UdjinM6
783f07e9f7
tests: Add uacomment in P2PConnection and use it to create (correct network specific) strSubVer (#3969) 2021-01-25 19:35:17 +01:00
Kolby Moroz Liebl
e197f976e1
backport bitcoin#16509 and add devnet test (#3946)
* Merge #16509: test: Adapt test framework for chains other than "regtest"

faf36838bdba7393960fce6ad0c56dc1f93f5870 test: Avoid hardcoding the chain name in combine_logs (MarcoFalke)
fa8a1d7ba30040f8c74f93fc41a61276c255a6a6 test: Adapt test framework for chains other than "regtest" (MarcoFalke)
68f546635d5de2ccfedadeabc7bc79e12e5eca6a test: Fix “local variable 'e' is assigned to but never used” (Ben Woosley)

Pull request description:

  This is required for various work in progress:

  * testchains #8994
  * signet #16411
  * some of my locally written tests

  While it will be unused in the master branch as of now, it will make all of those pull requests shorter. Thus review for non-regtest tests can focus on the actual changes and not some test framework changes.

ACKs for top commit:
  jonatack:
    ACK faf36838bdba7393960fce6ad0c56dc1f93f5870, ran tests and reviewed the code.

Tree-SHA512: 35add66c12cab68f2fac8f7c7d47c604d3f24eae9336ff78f83e2c92b3dc08a25e7f4217199bac5393dd3fb72f945bba9c001d6fbb8efd298c88858075fcb3d6

* Add devnet support for tests

* test: make sure devnet can connect to each other and start

* Partial merge bitcoin/bitcoin#16681: Tests: Use self.chain instead of 'regtest' in almost all current tests, revert one TODO while at it

Co-authored-by: MarcoFalke <falke.marco@gmail.com>
Co-authored-by: Jorge Timón <jtimon@jtimon.cc>
2021-01-22 09:58:07 -05:00
PastaPastaPasta
1c8f475a7f
Merge pull request #3882 from PastaPastaPasta/backports-0.17-pr25
Backports 0.17 pr25
2020-12-19 00:35:31 -06:00
Wladimir J. van der Laan
e6089da994
Merge #13823: qa: quote path in authproxy for external multiwallets
fa67505e1ea007bdc081bc7425fb83d5455d8308 qa: Quote wallet name for rpc path (MarcoFalke)

Pull request description:

  When using external multiwallets they are specified by their full path which might contain non-ascii characters (e.g. umlauts or emojis).

  Fix this by url-quoting the path.

Tree-SHA512: 7cc66514579d9f602f88a6817c5ab43a44c6d3711df452dc904173f0bc34e2c0b2c34877497f06b61f6720c532fa183053f54622dc454e316c89cee7eaa72463
2020-12-18 12:55:45 -06:00
Wladimir J. van der Laan
6d46698979
Merge #12076: qa: Use node.datadir instead of tmpdir in test framework
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
2020-12-15 22:06:42 -06:00
pasta
185e4b128c
Merge #12443: qa: Move common args to bitcoin.conf
face722 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
2020-12-15 11:06:26 -06:00
Wladimir J. van der Laan
90038da3d9
Merge #13022: [qa] Attach node index to test_node AssertionError and print messages
80a5e59 [qa] Attach node index to test_node AssertionError and print messages (James O'Beirne)

Pull request description:

  In the midst of fighting with https://github.com/bitcoin/bitcoin/pull/12873 it became apparent that there're a number of assertions and print statements which are emitted by test nodes but don't identify the node in question. This change makes debugging a bit easier by adding identifying information to non-logger test_node-related error messages.

Tree-SHA512: 7cc86f2c81f4b3fdba15ec9a2d21a84c4b083629e845e82288087c3affbbdc5c68e74067621856cc97fe84fbc8cb4f5ca4977a51ef381e5d74515df8eb001239

fix 13022

Signed-off-by: pasta <pasta@dashboost.org>
2020-11-17 14:34:34 -05:00
MarcoFalke
04b375a95d
Merge #12810: [Tests] Fix a typo at assert_start_raises_init_error() and use specific exception for initialization error
8394300859 [Tests] fix a typo in TestNode.assert_start_raises_init_error() (Roman Zeyde)

Pull request description:

  `self.wait_util_stopped()` should be `self.wait_until_stopped()`.

  Also, use a specific Exception subclass for indicating node failure to start (instead of using `AssetionError` and an `except Exception` clause).

  Following https://travis-ci.org/bitcoin/bitcoin/jobs/359066226#L2726 and depending on #12806 (which fixes the root cause of the Travis test failure).

Tree-SHA512: 7bd5a95586a412472ef9dffdb086789d7275ddaf862724e21cebb3418d0c97e6d89b4d1a58375e42114060d028403d6eab89e3a1e9a833ffe8dadf3439ab1fe2
2020-10-26 20:35:28 -04:00
Wladimir J. van der Laan
7a880f29df
Merge #12718: [Tests] Require exact match in assert_start_raises_init_eror (jnewbery)
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
Signed-off-by: pasta <pasta@dashboost.org>
2020-10-22 11:36:39 -04:00
pasta
e09719c2f9
add missing "import os.path"
Signed-off-by: pasta <pasta@dashboost.org>
2020-07-22 12:18:11 -05:00
Wladimir J. van der Laan
8adbc386a5
Merge #13051: qa: Normalize executable location
fa811b0 qa: Normalize executable location (MarcoFalke)

Pull request description:

  This removes the need to override the executable locations by just reading them from the config file. Beside making the code easier to read, running individual test on Windows is now possible by default (without providing further command line arguments).

  Note: Of course, it is still possible to manually specify the location through the `BITCOIND` environment variable, e.g. `bitcoin-qt`.

Tree-SHA512: bee6d22246796242d747120ca18aaab089f73067de213c9111182561985c5912228a0b0f7f9eec025ecfdb44db031f15652f30d67c489d481c995bb3232a7ac7
2020-07-22 12:12:46 -05:00
Wladimir J. van der Laan
773a16daa3
Merge #12482: [tests] bind functional test nodes to 127.0.0.1
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
2020-07-08 20:04:38 -05:00
UdjinM6
99ecf1d76c
Merge pull request #3526 from PastaPastaPasta/backport-14733-14024
Backport 14733 14024
2020-06-13 21:16:17 +03:00
MarcoFalke
6ab97cbfca Merge #14024: qa: Add TestNode::assert_debug_log
fa3e9f7627 qa: Add TestNode::assert_debug_log (MarcoFalke)

Pull request description:

  Closes #13006

Tree-SHA512: 8e2d2331039d70525a425aad65a4eaf9b83fb1f85a4260d69453176f04dbede6dd9b7bb4d5f089b46cf8f8c98571aa6ba7fac8fa6847bb3bdf6a6ad21a27b1a7
2020-06-12 23:50:03 -05:00
MarcoFalke
bb035972ee
Merge #12716: Fix typos and cleanup in various files
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
Signed-off-by: pasta <pasta@dashboost.org>
2020-06-11 23:20:48 -05:00
UdjinM6
469d6aa737
Some Dashification (#3513)
* Trivial Dashification

* Tweak getnetworkinfo and dumpwallet help text

We don't have RBF and Segwit

* CopyrightHolders should also check for missing "Dash Core" copyright
2020-06-11 11:39:04 +03:00
Alexander Block
a6d6c94a74 Allow to pass extra dashd args when running tests 2020-04-17 22:09:52 +02:00
Alexander Block
305485418b In disconnect_p2ps(), wait for p2p connections to disappear from getpeerinfo() 2020-04-17 07:53:07 +02:00
Alexander Block
755a23ca00
Always pass current mocktime to started nodes (#3423)
Instead of using the initial value. This removes the need for manually
passing of -mocktime when restarting nodes.

It also fixes a few flaky test cases where nodes are getting restarted.
2020-04-17 07:52:06 +02:00
MarcoFalke
01d3f009c5
Merge #12089: qa: Make TestNodeCLI command optional in send_cli
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
2020-04-08 11:46:27 -05:00
PastaPastaPasta
52bf616cc4
Merge #11712: [tests] Split NodeConn from NodeConnCB (#3373)
* [tests] Remove mininode periodic (half-hour) ping messages

* [tests] Tidy up mininode

Add docstrings and renames some methods.
Also removes the redundant NodeConn.readable() method override.

* [tests] Move only: move NodeConnCB below NodeConn

This is required since NodeConnCB will inherit from NodeConn
after the next commit.

* [tests] Make NodeConnCB a subclass of NodeConn

This makes NodeConnCB a subclass of NodeConn, and
removes the need for the client code to know
anything about the implementation details of NodeConnCB.

NodeConn can now be swapped out for any other implementation
of a low-level connection without changing client code.

* [tests] Move version message sending from NodeConn to NodeConnCB

This commit moves the logic that sends a version message
on connection from NodeConn to NodeConnCB. NodeConn should
not be aware of the semantics or meaning of the P2P payloads.

* remove witness

Signed-off-by: Pasta <pasta@dashboost.org>

* Fix 11712

Co-authored-by: John Newbery <john@johnnewbery.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
2020-04-05 14:12:45 +03:00
MarcoFalke
9b1ebdb161
Merge #12902: [qa] Handle potential cookie race when starting node
75d0e4c544 [qa] Delete cookie file before starting node (Suhas Daftuar)

Pull request description:

  When a node is restarted during a test after an unclean shutdown (such
  as with -dbcrashratio), it's possible an old cookie file was left
  behind. This can cause a race condition when restarting the node, where
  the test framework might try to connect using credentials from the
  old cookie file, just as the node will generate new credentials and
  overwrite the old file.

  Delete any such cookie file if present prior to startup.

Tree-SHA512: ae1e8bf8fd20e07c32b0715025693bb28b0e3dd34f328cae4346abf579b0c97b5db1c02782e1c46b7a3b6058d268b6d46b668e847658a6eed0be857ffb0d65dc
2020-03-24 11:43:10 -05:00
Wladimir J. van der Laan
7afc672d80
Merge #12904: [qa] Ensure bitcoind processes are cleaned up when tests end
e36a0c0 [qa] Ensure bitcoind processes are cleaned up when tests end (Suhas Daftuar)

Pull request description:

  When tests fail (such as due to a bug in the test, race condition, etc), it's possible that we could follow code paths that bypass our normal node shutdown that occurs in `TestNode.stop_node`.  Add a destructor to `TestNode` that cleans this up.

Tree-SHA512: 72e04bc21462ebd0cb346fd1fe0540da454acfbad41923a0b06ea2317e9045b68e58f9adb02d8200891aca89a9d03a022eb72282aeb31a3b3afe7c6843a4b450
2020-03-24 11:43:10 -05:00
UdjinM6
73258b363b
Merge pull request #3354 from PastaPastaPasta/backports-0.16-pr10
Backports 0.16 pr10
2020-03-10 14:57:35 +03:00
Wladimir J. van der Laan
64e33f715f
Merge #12220: Error if relative -walletdir is specified
ec527c6 Don't allow relative -walletdir paths (Russell Yanofsky)

Pull request description:

  This makes it an error to explicitly specify a non-absolute -walletdir path, and also adds a debug.log warning if a relative rather than absolute -datadir path is configured.

  Specifying paths relative to the current working directory in a daemon process can be dangerous, because files can fail to be located even if the configuration doesn't change, but the daemon is started up differently.

  Specifying a relative -datadir now adds a warning to the debug log. It would not be backwards-compatible to forbid relative -datadir paths entirely, and it could also be inconvenient for command line testing.

  Specifying a relative -walletdir now results in a startup error. But since the -walletdir option is new in 0.16.0, there should be no compatibility issues. Another reason not to use working directory paths for -walletdir specifically is that the default -walletdir is a "wallets" subdirectory inside the datadir, so it could be surprising that setting -walletdir manually would choose a directory rooted in a completely different location.

Tree-SHA512: 67cbdae677f82487a9031c5ec96b0205a488ab08718a0f4f49365e025119f3d7f6cfc88ba1eba04c1ecd8b9561a5b2c42e2e1a267af7c08c76b83e5e361f5a31
2020-03-04 10:13:31 -06:00
MarcoFalke
cc2cd7291b
Merge #11970: Add test coverage for bitcoin-cli multiwallet calls
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
2020-03-04 10:13:24 -06:00
MarcoFalke
4ec4e349d2
Merge #11677: qa: Remove unused NodeConn members
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
2020-02-29 11:51:03 -06:00
Alexander Block
21d33dd56d
Merge #14413: tests: Allow closed rpc handler in assert_start_raises_init_error (#3157)
62c304ea48 tests: Allow closed http server in assert_start_raises_init_error (Chun Kuan Lee)

Pull request description:

  The rpc handler may be unregistered when http server haven't been closed yet. So it may be allowable to get -342 `non-JSON HTTP response with \'%i %s\' from server` (503 Service Unavailable)

  See https://ci.appveyor.com/project/DrahtBot/bitcoin/build/master.2001. It shows "Rejecting request while shutting down" between "RPC stopped" and "Stopped HTTP server"

Tree-SHA512: e1f50ab9096cf23494ccc2850c01516c4a75f112c99108759157b22fce2011682a4b88216f206702f6a56e960182c00098053ad75f13aa0eafe27046adae63da
2019-10-17 11:59:21 +02:00
Wladimir J. van der Laan
fefe07003b Merge #14670: http: Fix HTTP server shutdown
28479f926f21f2a91bec5a06671c60e5b0c55532 qa: Test bitcond shutdown (João Barbosa)
8d3f46ec3938e2ba17654fecacd1d2629f9915fd http: Remove timeout to exit event loop (João Barbosa)
e98a9eede2fb48ff33a020acc888cbcd83e24bbf http: Remove unnecessary event_base_loopexit call (João Barbosa)
6b13580f4e3842c11abd9b8bee7255fb2472b6fe http: Unlisten sockets after all workers quit (João Barbosa)
18e968581697078c36a3c3818f8906cf134ccadd http: Send "Connection: close" header if shutdown is requested (João Barbosa)
02e1e4eff6cda0bfc24b455a7c1583394cbff6eb rpc: Add wait argument to stop (João Barbosa)

Pull request description:

  Fixes #11777. Reverts #11006. Replaces #13501.

  With this change the HTTP server will exit gracefully, meaning that all requests will finish processing and sending the response, even if this means to wait more than 2 seconds (current time allowed to exit the event loop).

  Another small change is that connections are accepted even when the server is stopping, but HTTP requests are rejected. This can be improved later, especially if chunked replies are implemented.

  Briefly, before this PR, this is the order or events when a request arrives (RPC `stop`):
   1. `bufferevent_disable(..., EV_READ)`
   2. `StartShutdown()`
   3. `evhttp_del_accept_socket(...)`
   4. `ThreadHTTP` terminates (event loop exits) because there are no active or pending events thanks to 1. and 3.
   5. client doesn't get the response thanks to 4.

  This can be verified by applying
  ```diff
       // Event loop will exit after current HTTP requests have been handled, so
       // this reply will get back to the client.
       StartShutdown();
  +    MilliSleep(2000);
       return "Bitcoin server stopping";
   }
  ```
  and checking the log output:
  ```
      Received a POST request for / from 127.0.0.1:62443
      ThreadRPCServer method=stop user=__cookie__
      Interrupting HTTP server
  **  Exited http event loop
      Interrupting HTTP RPC server
      Interrupting RPC
      tor: Thread interrupt
      Shutdown: In progress...
      torcontrol thread exit
      Stopping HTTP RPC server
      addcon thread exit
      opencon thread exit
      Unregistering HTTP handler for / (exactmatch 1)
      Unregistering HTTP handler for /wallet/ (exactmatch 0)
      Stopping RPC
      RPC stopped.
      Stopping HTTP server
      Waiting for HTTP worker threads to exit
      msghand thread exit
      net thread exit

      ... sleep 2 seconds ...

      Waiting for HTTP event thread to exit
      Stopped HTTP server
  ```

  For this reason point 3. is moved right after all HTTP workers quit. In that moment HTTP replies are queued in the event loop which keeps spinning util all connections are closed. In order to trigger the server side close with keep alive connections (implicit in HTTP/1.1) the header `Connection: close` is sent if shutdown was requested. This can be tested by
  ```
  bitcoind -regtest
  nc localhost 18443
  POST / HTTP/1.1
  Authorization: Basic ...
  Content-Type: application/json
  Content-Length: 44

  {"jsonrpc": "2.0","method":"stop","id":123}
  ```

  Summing up, this PR:
   - removes explicit event loop exit — event loop exits once there are no active or pending events
   - changes the moment the listening sockets are removed — explained above
   - sends header `Connection: close` on active requests when shutdown was requested which is relevant when it's a persistent connection (default in HTTP 1.1) — libevent is aware of this header and closes the connection gracefully
   - removes event loop explicit break after 2 seconds timeout

Tree-SHA512: 4dac1e86abe388697c1e2dedbf31fb36a394cfafe5e64eadbf6ed01d829542785a8c3b91d1ab680d3f03f912d14fc87176428041141441d25dcb6c98a1e069d8
2019-10-14 13:45:09 +02:00
Wladimir J. van der Laan
6e6e950fca Merge #11641: qa: Only allow disconnecting all NodeConns
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
2019-10-03 14:04:54 +02:00
MarcoFalke
7c163b1ffd Merge #11182: [tests] Add P2P interface to TestNode
32ae82f5c [tests] use TestNode p2p connection in tests (John Newbery)
5e5725cc2 [tests] Add p2p connection to TestNode (John Newbery)
b86c1cd20 [tests] fix TestNode.__getattr__() method (John Newbery)

Pull request description:

  Final two steps of #10082 : Adding the "mininode" P2P interface to `TestNode`

  This PR adds the mininode P2P interface to `TestNode`. It simplifies the process for opening a P2P connection to the node-under-test from this:

  ```python
  node0 = NodeConnCB()
  connections = []
  connections.append(NodeConn('127.0.0.1', p2p_port(0), self.nodes[0], node0))
  node0.add_connection(connections[0])
  ```

  to this:

  ```python
  self.nodes[0].add_p2p_connection(p2p_conn_type=NodeConnCB)
  ```

  The first commit adds the infrastructure to `test_node.py`. The second updates the individual test cases to use it. Can be separated if this is too much review for one PR.

Tree-SHA512: 44f1a6320f44eefc70489ae8350c6a16ad1a6035e4b9b7bafbdf19f5905ed0e2db85beaaf4758eec3059dd89a375a47a45352a029f39f57a86ab38a9ae66650e
2019-10-03 14:04:54 +02:00
João Barbosa
102c3f2cec [test] Replace check_output with low level version 2019-09-24 17:47:35 +02:00
João Barbosa
6245ce95e1 [test] Add support for custom arguments to TestNodeCLI 2019-09-24 17:47:35 +02:00
MarcoFalke
ded01879d0 Merge #11067: [qa] TestNode: Add wait_until_stopped helper method
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
2019-09-24 17:47:35 +02:00
MarcoFalke
17bb230d74 Merge #11121: TestNode tidyups
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
2019-09-24 16:58:03 +02:00
Alexander Block
ebfec2218a Dashify BITCOINCLI 2019-09-24 16:58:03 +02:00
MarcoFalke
1adc2001a8 Merge #10798: [tests] [utils] test bitcoin-cli
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
2019-09-24 16:58:03 +02:00
Wladimir J. van der Laan
d09f88d985 Merge #11077: [tests] fix timeout issues from TestNode
2b4ea52 [tests] fix timeout issues from TestNode (John Newbery)

Pull request description:

  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.

Tree-SHA512: 42a62a5459eea2e5d83b44dae2a5ccc7b15eb7fef8f8745ff04884dbba8f79d66ffdd65c67d37f6865b36da3f522bcdd0d6ea99861d7ce86dd8a56dc29cd643f
2019-09-24 16:58:03 +02:00
Wladimir J. van der Laan
0a8ad8b56d Merge #11091: test: Increase initial RPC timeout to 60 seconds
c1470a0 test: Increase initial RPC timeout to 60 seconds (Wladimir J. van der Laan)

Pull request description:

  When running the tests locally with a parallelism of 4 on an otherwise busy system, RPC can take quite a wait to come up.

  With the current timeout tests often fail with "Unable to connect to bitcoind".

  Change the timeout to 60 seconds just to be safe.

Tree-SHA512: 0c08cc8ce3f25ba2882beac2a50d1fcdd7c8c3bd6e3a8707813f94f2d39c14e2139ba1ddf7f9b66013d4c7f55db92d3f4aa88b433d855fd21e82842e350e459a
2019-09-24 16:58:03 +02:00
Alexander Block
1596b1dfaa Move -createwalletbackups=0 into test_node.py
Not all tests use test_framework.py, but all use test_node.py
2019-09-24 16:58:03 +02:00
Alexander Block
4049754f4a Dashify test_node.py 2019-09-24 16:58:03 +02:00
MarcoFalke
f55da3aa54 Merge #10711: [tests] Introduce TestNode
789733891 [tests] Introduce TestNode (John Newbery)

Pull request description:

  Continues #10082

  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.

  On my vm, this changeset reduces total test_runner runtime for the base set of tests
  (including building the cache) from 263s to 195s (a 25% speedup). Note that the time
  reported by test_runner does not include time spent building the cache:

  *with TestNode*:
  ```
  → date +"%T" ; ./test_runner.py -q ; date +"%T"
  12:48:04
  ..................................................................................................................................................................................................................................................................................................................................
  TEST                           | STATUS    | DURATION

  abandonconflict.py             | ✓ Passed  | 12 s
  bip68-112-113-p2p.py           | ✓ Passed  | 19 s
  blockchain.py                  | ✓ Passed  | 8 s
  bumpfee.py                     | ✓ Passed  | 13 s
  decodescript.py                | ✓ Passed  | 3 s
  disablewallet.py               | ✓ Passed  | 3 s
  disconnect_ban.py              | ✓ Passed  | 6 s
  fundrawtransaction.py          | ✓ Passed  | 37 s
  getchaintips.py                | ✓ Passed  | 4 s
  httpbasics.py                  | ✓ Passed  | 3 s
  import-rescan.py               | ✓ Passed  | 4 s
  importmulti.py                 | ✓ Passed  | 6 s
  importprunedfunds.py           | ✓ Passed  | 3 s
  invalidblockrequest.py         | ✓ Passed  | 4 s
  invalidtxrequest.py            | ✓ Passed  | 4 s
  keypool.py                     | ✓ Passed  | 7 s
  listsinceblock.py              | ✓ Passed  | 4 s
  listtransactions.py            | ✓ Passed  | 33 s
  mempool_limit.py               | ✓ Passed  | 4 s
  mempool_persist.py             | ✓ Passed  | 15 s
  mempool_reorg.py               | ✓ Passed  | 4 s
  mempool_resurrect_test.py      | ✓ Passed  | 3 s
  mempool_spendcoinbase.py       | ✓ Passed  | 3 s
  merkle_blocks.py               | ✓ Passed  | 3 s
  multi_rpc.py                   | ✓ Passed  | 4 s
  net.py                         | ✓ Passed  | 3 s
  nulldummy.py                   | ✓ Passed  | 3 s
  p2p-compactblocks.py           | ✓ Passed  | 28 s
  p2p-fullblocktest.py           | ✓ Passed  | 126 s
  p2p-leaktests.py               | ✓ Passed  | 8 s
  p2p-mempool.py                 | ✓ Passed  | 3 s
  p2p-segwit.py                  | ✓ Passed  | 59 s
  p2p-versionbits-warning.py     | ✓ Passed  | 8 s
  preciousblock.py               | ✓ Passed  | 3 s
  prioritise_transaction.py      | ✓ Passed  | 5 s
  proxy_test.py                  | ✓ Passed  | 3 s
  rawtransactions.py             | ✓ Passed  | 9 s
  receivedby.py                  | ✓ Passed  | 19 s
  reindex.py                     | ✓ Passed  | 12 s
  rest.py                        | ✓ Passed  | 9 s
  rpcnamedargs.py                | ✓ Passed  | 3 s
  segwit.py                      | ✓ Passed  | 7 s
  sendheaders.py                 | ✓ Passed  | 24 s
  signmessages.py                | ✓ Passed  | 3 s
  signrawtransactions.py         | ✓ Passed  | 3 s
  txn_clone.py                   | ✓ Passed  | 4 s
  txn_doublespend.py --mineblock | ✓ Passed  | 4 s
  uptime.py                      | ✓ Passed  | 3 s
  wallet-accounts.py             | ✓ Passed  | 3 s
  wallet-dump.py                 | ✓ Passed  | 7 s
  wallet-encryption.py           | ✓ Passed  | 8 s
  wallet-hd.py                   | ✓ Passed  | 15 s
  wallet.py                      | ✓ Passed  | 31 s
  walletbackup.py                | ✓ Passed  | 104 s
  zapwallettxes.py               | ✓ Passed  | 9 s
  zmq_test.py                    | ○ Skipped | 0 s

  ALL                            | ✓ Passed  | 735 s (accumulated)
  Runtime: 189 s

  12:51:19
  ```

  *master*:
  ```
  → date +"%T" ; ./test_runner.py -q ; date +"%T"
  12:40:13
  ..........................................................................................................................................................................................................................................................................................................................................................................................................................................
  TEST                           | STATUS    | DURATION

  abandonconflict.py             | ✓ Passed  | 15 s
  bip68-112-113-p2p.py           | ✓ Passed  | 19 s
  blockchain.py                  | ✓ Passed  | 8 s
  bumpfee.py                     | ✓ Passed  | 20 s
  decodescript.py                | ✓ Passed  | 3 s
  disablewallet.py               | ✓ Passed  | 3 s
  disconnect_ban.py              | ✓ Passed  | 8 s
  fundrawtransaction.py          | ✓ Passed  | 36 s
  getchaintips.py                | ✓ Passed  | 11 s
  httpbasics.py                  | ✓ Passed  | 7 s
  import-rescan.py               | ✓ Passed  | 16 s
  importmulti.py                 | ✓ Passed  | 10 s
  importprunedfunds.py           | ✓ Passed  | 5 s
  invalidblockrequest.py         | ✓ Passed  | 4 s
  invalidtxrequest.py            | ✓ Passed  | 3 s
  keypool.py                     | ✓ Passed  | 7 s
  listsinceblock.py              | ✓ Passed  | 11 s
  listtransactions.py            | ✓ Passed  | 37 s
  mempool_limit.py               | ✓ Passed  | 4 s
  mempool_persist.py             | ✓ Passed  | 23 s
  mempool_reorg.py               | ✓ Passed  | 7 s
  mempool_resurrect_test.py      | ✓ Passed  | 3 s
  mempool_spendcoinbase.py       | ✓ Passed  | 3 s
  merkle_blocks.py               | ✓ Passed  | 10 s
  multi_rpc.py                   | ✓ Passed  | 6 s
  net.py                         | ✓ Passed  | 6 s
  nulldummy.py                   | ✓ Passed  | 3 s
  p2p-compactblocks.py           | ✓ Passed  | 30 s
  p2p-fullblocktest.py           | ✓ Passed  | 126 s
  p2p-leaktests.py               | ✓ Passed  | 8 s
  p2p-mempool.py                 | ✓ Passed  | 3 s
  p2p-segwit.py                  | ✓ Passed  | 62 s
  p2p-versionbits-warning.py     | ✓ Passed  | 8 s
  preciousblock.py               | ✓ Passed  | 8 s
  prioritise_transaction.py      | ✓ Passed  | 7 s
  proxy_test.py                  | ✓ Passed  | 10 s
  rawtransactions.py             | ✓ Passed  | 15 s
  receivedby.py                  | ✓ Passed  | 28 s
  reindex.py                     | ✓ Passed  | 12 s
  rest.py                        | ✓ Passed  | 12 s
  rpcnamedargs.py                | ✓ Passed  | 3 s
  segwit.py                      | ✓ Passed  | 12 s
  sendheaders.py                 | ✓ Passed  | 26 s
  signmessages.py                | ✓ Passed  | 3 s
  signrawtransactions.py         | ✓ Passed  | 3 s
  txn_clone.py                   | ✓ Passed  | 10 s
  txn_doublespend.py --mineblock | ✓ Passed  | 10 s
  uptime.py                      | ✓ Passed  | 3 s
  wallet-accounts.py             | ✓ Passed  | 3 s
  wallet-dump.py                 | ✓ Passed  | 6 s
  wallet-encryption.py           | ✓ Passed  | 8 s
  wallet-hd.py                   | ✓ Passed  | 18 s
  wallet.py                      | ✓ Passed  | 69 s
  walletbackup.py                | ✓ Passed  | 130 s
  zapwallettxes.py               | ✓ Passed  | 15 s
  zmq_test.py                    | ○ Skipped | 0 s

  ALL                            | ✓ Passed  | 936 s (accumulated)
  Runtime: 242 s

  12:44:36
  ```

Tree-SHA512: 6dfc4c11fd0caf7de6954c93679cf22c3df0acc6f432e616d1151062a61f456faa8ae2fe670b427868af55bb564802df84c8fd76e90b4b338750dbc23f46ad88
2019-09-24 16:58:03 +02:00