Commit graph

23738 commits

Author SHA1 Message Date
Wladimir J. van der Laan 9e8e813df5
Merge #18410: Docs: Improve commenting for coins.cpp|h
21fa0a44ab [docs] use consistent naming for possible_overwrite (John Newbery)
2685c214cc [tests] small whitespace fixup (John Newbery)
e9936966c0 scripted-diff: Rename PRUNED to SPENT in coins tests (John Newbery)
c205979031 [docs] Improve commenting in coins.cpp|h (John Newbery)

Pull request description:

  - Add full commenting for spentness / DIRTYness / FRESHness and which combinations are valid
  - Remove the 'pruned' terminology, which doesn't make sense since per-txout chainstate db was merged (#10195).
  - Rename `potential_overwrite` to `possible_overwrite` to standardize terminology (there were previously examples of both, which made searching the codebase difficult).
  - Make other minor improvements to the comments

ACKs for top commit:
  jonatack:
    Re-ACK 21fa0a4 per `git diff 98bee55 21fa0a4` the only change since my previous review is the following code commenting diff in `src/coins.cpp::L177-179`;  rebuilt/ran unit tests anyway as a sanity check on the unit test changes.

Tree-SHA512: 391e01588ef5edb417250080cec17361f982c4454bc5f8c6d78bbd528c68a2bb94373297760691295c24660ce1022ad3ef7599762f736c8eed772ce096d38c3d
2020-04-22 14:23:56 +02:00
Wladimir J. van der Laan acb4fa0741
Merge #18665: Do not expose and consider -logthreadnames when it does not work
b91e4ae0d8 Do not expose and consider -logthreadnames when it does not work (Hennadii Stepanov)

Pull request description:

  There are conditions when the `HAVE_THREAD_LOCAL` macro is undefined what causes the `-logthreadnames` option does not work -- instead of thread names empty strings `[]` only are printed in the `debug.log` file.

  This PR does not exposes the `-logthreadnames` option in such cases.

  Refs:
  - #16059
  - #18652

ACKs for top commit:
  MarcoFalke:
    ACK b91e4ae0d8, looked at the diff, didn't test

Tree-SHA512: 3bd58e5ea603c69686589ddc94d6fa441cab4f712004378f2f1661e12638804ca03cfb6426e6393e55b6a095b325f3161d3c5371af05d7fc79d6d328227bf40c
2020-04-22 14:18:06 +02:00
Wladimir J. van der Laan 19032c750c
Merge #18612: script: Remove undocumented and unused operator+
ccccd51908 script: Remove undocumented and unused operator+ (MarcoFalke)

Pull request description:

  This operator has no documented use case and is also unused outside of test code. The test code and all other (imaginary) code that might use this operator is written more clear and concise by the existing CScript push operators for opcodes and data.

  Removing the operator is also going to protect against accidentally reintroducing bugs like this 6ff5f718b6 (diff-8458adcedc17d046942185cb709ff5c3L1135) (last time it was used).

ACKs for top commit:
  laanwj:
    ACK ccccd51908

Tree-SHA512: 43898ac77e4d9643d9f8ac6f8f65497a4f0bbb1fb5dcaecc839c3719aa36181ba77befb213e59a9f33a20a29e0173a0e9c4763b1930940b32c3d1598b3e39af9
2020-04-22 14:17:01 +02:00
fanquake c90a9e6fff
Merge #18713: scripts: Add MACHO stack canary check to security-check.py
8334ee31f8 scripts: add MACHO LAZY_BINDINGS test to test-security-check.py (fanquake)
7b99c7454c scripts: add MACHO Canary check to security-check.py (fanquake)

Pull request description:

  7b99c7454c uses `otool -Iv` to check for `___stack_chk_fail` in the macOS binaries. Similar to the [ELF check](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/security-check.py#L105). Note that looking for a triple underscore prefixed function (as opposed to two for ELF) is correct for the macOS binaries. i.e:
  ```bash
  otool -Iv bitcoind | grep chk
  0x00000001006715b8   509 ___memcpy_chk
  0x00000001006715be   510 ___snprintf_chk
  0x00000001006715c4   511 ___sprintf_chk
  0x00000001006715ca   512 ___stack_chk_fail
  0x00000001006715d6   517 ___vsnprintf_chk
  0x0000000100787898   513 ___stack_chk_guard
  ```

  8334ee31f8 is a follow up to #18295 and adds test cases to `test-security-check.py` that for some reason I didn't add at the time. I'll sort out #18434 so that we can run these tests in the CI.

ACKs for top commit:
  practicalswift:
    ACK 8334ee31f8: Mitigations are important. Important things are worth asserting :)
  jonasschnelli:
    utACK 8334ee31f8.

Tree-SHA512: 1aa5ded34bbd187eddb112b27278deb328bfc21ac82316b20fab6ad894f223b239a76b53dab0ac1770d194c1760fcc40d4da91ec09959ba4fc8eadedb173936a
2020-04-22 16:40:32 +08:00
fanquake 7d1a3bda21
Merge #18709: doc: note why we can't use thread_local with glibc back compat
b155fcda51 doc: fix typo in configure.ac (fanquake)
20a30922fb doc: note why we can't use thread_local with glibc back compat (fanquake)

Pull request description:

  Given that we went through a [gitian build](https://github.com/bitcoin/bitcoin/pull/18681) to remember why this is the case, we might as well make a note of it in configure.ac.

  [From #18681](https://github.com/bitcoin/bitcoin/pull/18681#issuecomment-615526634):

  Looking at the Linux build log, this has failed with:
  ```bash
  Checking glibc back compat...
  bitcoind: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bitcoind: failed IMPORTED_SYMBOLS
  bitcoin-cli: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bitcoin-cli: failed IMPORTED_SYMBOLS
  bitcoin-tx: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bitcoin-tx: failed IMPORTED_SYMBOLS
  bitcoin-wallet: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bitcoin-wallet: failed IMPORTED_SYMBOLS
  test/test_bitcoin: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  test/test_bitcoin: failed IMPORTED_SYMBOLS
  bench/bench_bitcoin: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  bench/bench_bitcoin: failed IMPORTED_SYMBOLS
  qt/bitcoin-qt: symbol __cxa_thread_atexit_impl from unsupported version GLIBC_2.18
  ```

  `__cxa_thread_atexit_impl` is used for [thread_local variable destruction](https://sourceware.org/glibc/wiki/Destructor%20support%20for%20thread_local%20variables):

  > To implement this support, glibc defines __cxa_thread_atexit_impl exclusively for use by libstdc++ (which has the __cxa_thread_atexit to wrap around it), that registers destructors for thread_local variables in a list. Upon thread or process exit, the destructors are called in reverse order in which they were added.

  As suggested, this only became available in glibc 2.18. From the [2.18 release notes](https://sourceware.org/legacy-ml/libc-alpha/2013-08/msg00160.html):

  > * Add support for calling C++11 thread_local object destructors on thread
    and program exit.  This needs compiler support for offloading C++11
    destructor calls to glibc.

ACKs for top commit:
  hebasto:
    ACK b155fcda51

Tree-SHA512: 5b9567e4a70598a4b0b91956f44ae0d93091db17c84cbf9817dac6cfa992c97d3438a8b1bb66644c74891f2149e44984daed445d22de93ca8858c5b0eabefb40
2020-04-22 14:46:19 +08:00
John Newbery 21fa0a44ab [docs] use consistent naming for possible_overwrite
And other general comment improvements for adding coins.
2020-04-21 14:19:15 -04:00
John Newbery 2685c214cc [tests] small whitespace fixup
Required after scripted-diff in previous commit.
2020-04-21 14:19:15 -04:00
John Newbery e9936966c0 scripted-diff: Rename PRUNED to SPENT in coins tests
-BEGIN VERIFY SCRIPT-
sed -i -e 's/PRUNED,/SPENT ,/g' ./src/test/coins_tests.cpp
sed -i -e 's/PRUNED/SPENT/g' ./src/test/coins_tests.cpp
-END VERIFY SCRIPT-
2020-04-21 14:19:15 -04:00
John Newbery c205979031 [docs] Improve commenting in coins.cpp|h
Remove references to 'pruned' coins, which don't exist since the move
to per-txout coins db.
2020-04-21 14:18:03 -04:00
MarcoFalke b6a5dc90bf
Merge #18384: [test] more specific feature_segwit test error messages and fixing incorrect comments
3c21db7b78 [test] add 8 error messages to feature_segwit and change version to type (Gloria Zhao)

Pull request description:

  Followup to [this](https://github.com/bitcoin/bitcoin/pull/15169/files#r303673472) comment on functional test feature_segwit.py verifying that unsigned witness transactions are invalid.

  (1) Changes 8 error messages from "mandatory-script-verify-flag" to "non-mandatory-script-verify-flag" and with more specific error messages.
  (2) Edits comments that incorrectly describe the test, namely that the `v` variable corresponds to using P2WSH versus P2WPKH, not witness versions.

ACKs for top commit:
  MarcoFalke:
    ACK 3c21db7b78 🍾

Tree-SHA512: 3734ea3762667636c4fb20f5285634ab94d6b3527b7390fcc5e41b4582829dfe0099beabeaed42098613d168ede3385a6ffcd73989d1fa9dbd18004f5e9cf083
2020-04-21 13:42:21 -04:00
Gloria Zhao 3c21db7b78 [test] add 8 error messages to feature_segwit and change version to type
P2WPKH witness program without signature -> throws "hash mismatch" error
P2WSH witness program without signature -> throws "empty witness" error
same errors for P2SH_P2WPKH and P2SH_P2WSH respectively when passed redeemScript but no signature
P2SH_P2WPKH and P2SH_P2WSH with no signature fail with "Operation not valid with current stack size" when not signed due to missing input
change VER to TYPE and constants WIT_V0 to P2WPKH=0 and WIT_V1 to P2WSH=1
2020-04-21 08:17:42 -07:00
MarcoFalke 9ea4d8326a
Merge #18704: test: Increase debugging to hunt down mempool_reorg intermittent failure
fac2fc4dd8 test: Increase debugging to hunt down mempool_reorg intermittent failure (MarcoFalke)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: 4094b44afaa623e58b69f8d0332e60f0150b9ae2fd8bb265210d85546d887672ab8a3435cd9b086be14f69ab5b17e0f9fae06bd8aec1e7947ca766dd72b577c4
2020-04-21 10:58:10 -04:00
MarcoFalke 977328338e
Merge #18721: test: Fix linter issue
60cdcf30a4 test: Fix linter issue (Hennadii Stepanov)

Pull request description:

Top commit has no ACKs.

Tree-SHA512: 4fa0103526fed4b1399d3a6b83cea1e509cefff36d7e16ed1499d22afdc49e72053ac3d0634d858838ec6c296af7134131ea4d509c46de99da557567bc75d711
2020-04-21 08:05:42 -04:00
Hennadii Stepanov 60cdcf30a4
test: Fix linter issue 2020-04-21 14:58:51 +03:00
MarcoFalke 4ad6144ed0
Merge #18672: test: add further BIP37 size limit checks to p2p_filter.py
c743718558 test: add further BIP37 size limit checks to p2p_filter.py (Sebastian Falbesoner)

Pull request description:

  This is a follow-up PR to #18628. In addition to the hash-functions limit test introduced with commit fa4c29bc1d, it adds checks for the following size limits as defined in [BIP37](https://github.com/bitcoin/bips/blob/master/bip-0037.mediawiki):

  ad message type `filterload`:
  > The filter itself is simply a bit field of arbitrary byte-aligned size. The maximum size is **36,000 bytes**.

  ad message type `filteradd`:
  > The data field must be smaller than or equal to **520 bytes** in size (the maximum size of any potentially matched object).

  Also introduces new constants for the limits (or reuses the max script size constant in case for the `filteradd` limit).

  Also fixes #18711 by changing the misbehaviour check on "filteradd without filterset" (introduced with #18544) below to also use the more commonly used `assert_debug_log` method.

ACKs for top commit:
  MarcoFalke:
    ACK c743718558
  robot-visions:
    ACK c743718558
  jonasschnelli:
    utACK c743718558. Seems to fix it: https://bitcoinbuilds.org/index.php?build=2524

Tree-SHA512: a03e7639263eb36a381922afb4e1d0ed2ae286f2ad2e7bbd922509a043ddf6cfd08747e01d54d29bfb8f54b66908f653974b9c347e4ca4f43332b586778893be
2020-04-21 07:26:18 -04:00
fanquake 8334ee31f8
scripts: add MACHO LAZY_BINDINGS test to test-security-check.py
I didn't add the relevant test in #18295.
2020-04-21 11:32:06 +08:00
fanquake 7b99c7454c
scripts: add MACHO Canary check to security-check.py 2020-04-21 11:32:01 +08:00
fanquake b155fcda51
doc: fix typo in configure.ac
s/liner/linker/
2020-04-21 10:05:43 +08:00
MarcoFalke c4c3f110eb
Merge #18190: tests: Add fuzzing harness for Golomb-Rice coding (GolombRiceEncode/GolombRiceDecode)
69749fbe6a tests: Add fuzzing harness for Golomb-Rice coding (GolombRiceEncode/GolombRiceDecode) (practicalswift)

Pull request description:

  Add fuzzing harness for Golomb-Rice coding (`GolombRiceEncode`/`GolombRiceDecode`).

  Test this PR using:

  ```
  $ make distclean
  $ ./autogen.sh
  $ CC=clang CXX=clang++ ./configure --enable-fuzz \
        --with-sanitizers=address,fuzzer,undefined
  $ make
  $ src/test/fuzz/golomb_rice
  …
  ```

Top commit has no ACKs.

Tree-SHA512: 1b26512301b8c22ab3b804d9b9e4baf933f26f8c05e462d583863badcec7e694548a34849a0d7c4ff7d58b19f6338b51819976ecf642bc4659b04ef71182d748
2020-04-20 15:32:41 -04:00
Wladimir J. van der Laan 56d2ff8a8f
Merge #18676: build: Check libevent minimum version in configure script
b68e717967 build: Set libevent minimum version to 2.0.21 (Hennadii Stepanov)

Pull request description:

  The non-`pkg-config` path is ignored as there is a hope to get rid of all of them in #18307.

  As xenial has [libevent 2.0.21](https://packages.ubuntu.com/xenial-updates/libevent-2.0-5) only, the default bionic Docker image is used in the _"[no depends, only system libs, sanitizers: thread (TSan), no wallet]"_ CI test.

ACKs for top commit:
  theStack:
    utACK b68e717967
  laanwj:
    ACK b68e717967

Tree-SHA512: 9825c42aeb166165e99fe5eaf74dbb47c2b51aecdbe53c5ae949fe126e1b8e8b6fe8d228fdde4e8daa4243e5907954202f42eb23c71629e4b2b92a7d4eb892e4
2020-04-20 18:43:07 +02:00
Sebastian Falbesoner c743718558 test: add further BIP37 size limit checks to p2p_filter.py
also unified method of detecting misbehaviour
(using assert_debug_log instead of checking peer's banscore)
2020-04-20 18:22:21 +02:00
practicalswift 69749fbe6a tests: Add fuzzing harness for Golomb-Rice coding (GolombRiceEncode/GolombRiceDecode) 2020-04-20 14:57:48 +00:00
MarcoFalke fc00e651e4
Merge #18705: ci: Remove xenial tsan workaround
faebcd4e8d ci: Remove xenial tsan workaround (MarcoFalke)

Pull request description:

ACKs for top commit:
  hebasto:
    ACK faebcd4e8d

Tree-SHA512: 5d7e15be211e526948f863f573dbb5a97005262241ba4a07858346cba3a17cb24b1473df347224e0e4f2b22201750e27fba80ffe0d1dddf85f4e8f9341a8c129
2020-04-20 10:16:12 -04:00
MarcoFalke 3be119c0f6
Merge #17579: [refactor] Merge getreceivedby tally into GetReceived function
a1d5b12ec0 Merge getreceivedby tally into GetReceived function (Andrew Toth)

Pull request description:

  This PR merges the tally code of `getreceivedbyaddress` and `getreceivedbylabel` into a single function `GetReceived`. This reduces repeated code and makes it similar to `listreceivedbyaddress` and `listreceivedbylabel`, which use the function `ListReceived`. It will also make the change in #14707 simpler and easier to review.

ACKs for top commit:
  theStack:
    re-ACK a1d5b12ec0
  meshcollider:
    utACK a1d5b12ec0

Tree-SHA512: 43d9cd92f7c2c6a8b9c7509aa85a9b9233a6cfec1c43a9062e3bdfb83515413d1feafa8938c828351278ba22bd31c47e62ab5341e4bddc2493103b094d73b047
2020-04-20 10:05:32 -04:00
Hennadii Stepanov b91e4ae0d8
Do not expose and consider -logthreadnames when it does not work 2020-04-20 14:17:49 +03:00
MarcoFalke 5e5dd9918e
Merge #17831: rpc: doc: Fix and extend getblockstats examples
709998467e rpc: doc: Fix and extend getblockstats examples (Adam Soltys)

Pull request description:

  This pull fixes the example curl command for `getblockstats` which doesn't work as is because it's missing a comma between the params and has single quotes around the second parameter.

  It also adds an additional example of getting block stats by hash by using a known workaround (#15412) to get bitcoin-cli to treat the hash parameter as JSON instead of a string since there is ongoing deliberation about how or whether to fix the root issue (#15448).

ACKs for top commit:
  theStack:
    ACK 709998467e

Tree-SHA512: 84a5b7f449f06fff785bc0afbc1a7dfd55454bc76c52a8945e91556f87f3edfdc5a1780faab8fcfd6c415b734295b7c67d2e04ba7b6cfa91a77758af5dda53ae
2020-04-20 07:15:45 -04:00
MarcoFalke da4cbb7927
Merge #18544: net: limit BIP37 filter lifespan (active between 'filterload'..'filterclear')
a9ecbdfcaa test: add more inactive filter tests to p2p_filter.py (Sebastian Falbesoner)
5eae034996 net: limit BIP37 filter lifespan (active between 'filterload' and 'filterclear') (Sebastian Falbesoner)

Pull request description:

  This PR fixes https://github.com/bitcoin/bitcoin/issues/18483. On the master branch, there is currently _always_ a BIP37 filter set for every peer: if not a specific filter is set through a `filterload` message, a default match-everything filter is instanciated and pointed to via the `CBloomFilter` default constructor; that happens both initially, when the containing structure `TxRelay` is constructed:

  c0b389b335/src/net.h (L812)

  and after a loaded filter is removed again through a `filterclear` message:

  c0b389b335/src/net_processing.cpp (L3201)

  The behaviour was introduced by commit 37c6389c5a (an intentional covert fix for [CVE-2013-5700](https://github.com/bitcoin/bitcoin/pull/18515), according to gmaxwell).

  This default match-everything filter leads to some unintended side-effects:
  1. `getdata` request for filtered blocks (i.e. type `MSG_FILTERED_BLOCK`) are always responded to with `merkleblock`s, even if no filter was set by the peer, see issue #18483 (strictly speaking, this is a violation of BIP37) c0b389b335/src/net_processing.cpp (L1504-L1507)
  2. if a peer sends a `filteradd` message without having loaded a filter via `filterload` before, the intended increasing of the banscore never happens (triggered if `bad` is set to true, a few lines below) c0b389b335/src/net_processing.cpp (L3182-L3186)

  This PR basically activates the `else`-branch code paths for all checks of `pfilter` again (on the master branch, they are dead code) by limiting the pointer's lifespan: instead of always having a filter set, the `pfilter` is only pointing to a `CBloomFilter`-instance after receiving a `filterload` message and the instance is destroyed again (and the pointer nullified) after receiving a `filterclear` message.

  Here is a before/after comparison in behaviour:
  | code part / scenario                          |    master branch                   |   PR branch                                          |
  | --------------------------------------------- | ---------------------------------- | ---------------------------------------------------- |
  | `getdata` processing for `MSG_FILTERED_BLOCK` | always responds with `merkleblock` | only responds if filter was set via `filterload`     |
  | `filteradd` processing, no filter was loaded  | nothing                            | peer's banscore increases by 100 (i.e. disconnect)   |

  On the other code parts where `pfilter` is checked there is no change in the logic behaviour (except that `CBloomFilter::IsRelevantAndUpdate()` is unnecessarily called and immediately returned in the master branch).
  Note that the default constructor of `CBloomFilter` is only used for deserializing the received `filterload` message and nowhere else. The PR also contains a functional test checking that sending `getdata` for filtered blocks is ignored by the node if no bloom filter is set.

ACKs for top commit:
  MarcoFalke:
    re-ACK a9ecbdfcaa, only change is in test code 🕙

Tree-SHA512: 1a656a6d74ccaf628e7fdca063ba63fbab2089e0b6d0a11be9bbd387c2ee6d3230706ff8ffc1a55711481df3d4547137dd7c9d9184d89eaa43ade4927792d0b6
2020-04-20 06:59:53 -04:00
MarcoFalke dc5da7fe7b
Merge #18691: test: add wait_for_cookie_credentials() to framework for rpcwait tests
92fe537cf7 test: fix intermittent race condition in interface_bitcoin_cli.py (Jon Atack)
c648e636b2 test: add wait_for_cookie_credentials() to test framework (Jon Atack)

Pull request description:

  This PR adds a `wait_for_cookie_credentials()` method to the test framework and calls it before the `-rpcwait` tests, to avoid an intermittent race condition on the CI run with Valgrind where the cookie file isn't written yet when the CLI call with `-rpcwait` arrives to `get_auth_cookie()`.

  To reproduce/test, build with
  ```diff
  diff --git a/src/httprpc.cpp b/src/httprpc.cpp
  index 60c4d06f12..3dd06c4758 100644
  --- a/src/httprpc.cpp
  +++ b/src/httprpc.cpp
  @@ -291,6 +291,7 @@ static bool InitRPCAuthentication()
   bool StartHTTPRPC()
   {
       LogPrint(BCLog::RPC, "Starting HTTP RPC server\n");
  +    UninterruptibleSleep(std::chrono::seconds{11});
       if (!InitRPCAuthentication())
  ```
  then run the test normally and with valgrind
  ```
  test/functional/interface_bitcoin_cli.py -l debug
  valgrind test/functional/interface_bitcoin_cli.py -l debug
  ```
  Thanks to Marco Falke for all the help. Closes #18684.

Top commit has no ACKs.

Tree-SHA512: 1b76635b5b1d6b05138affef7ab788aa3bc3fc75b0c69ba778ecdf81063cfe02a8dd7667cfd63a6c6e19b2dac47d7a8b755e334d8af5c0ab9d4026808ee96c83
2020-04-20 06:13:59 -04:00
fanquake 20a30922fb
doc: note why we can't use thread_local with glibc back compat 2020-04-20 16:58:54 +08:00
Jon Atack 92fe537cf7
test: fix intermittent race condition in interface_bitcoin_cli.py
by calling wait_for_cookie_credentials() to ensure the cookie file is written
and auth credentials available for testing the CLI -rpcwait option before the
RPC connection is up.
2020-04-19 18:12:05 +02:00
Jon Atack c648e636b2
test: add wait_for_cookie_credentials() to test framework
to be able to ensure the cookie file is written and auth credentials available
when testing CLI/RPC commands before the RPC connection is up.
2020-04-19 18:12:03 +02:00
MarcoFalke f8102d9088
Merge #18601: wallet: Refactor WalletRescanReserver to use wallet reference
fc289b7898 wallet: Refactor WalletRescanReserver to use wallet reference (João Barbosa)

Pull request description:

  Simple refactor to `WalletRescanReserver` to use wallet reference instead of pointer.

  Complements #18259.

ACKs for top commit:
  MarcoFalke:
    ACK fc289b7898

Tree-SHA512: b03e33f2d9df2870436aa3284137fd022dd89ea96a1b170fa27f8685ad4f986e6c4ba5975a84966c30d18430a4014d7d8740a1dff2f985c9ef8226ed18e69db9
2020-04-19 10:32:43 -04:00
MarcoFalke d2882a012b
Merge #18610: scripted-diff: test: replace command with msgtype (naming)
9df32e820d scripted-diff: test: replace command with msgtype (Sebastian Falbesoner)

Pull request description:

  This is a follow-up PR to https://github.com/bitcoin/bitcoin/pull/18533, which changed the naming of `strCommand` to `msg_type` in the network processing code. The same approach is done here for the function test framework, to get rid of the wrong "command" terminology for network mesage types. (Commands are usually used in the CLI or RPC context, so using the same name in the network message context would only be confusing.)

  The commit was created through the following steps:
  1. search for all occurences of the string "command" within the folder `test/functional`
  ```git grep -i command test/functional > command_finds```
  2. manually sort out all false-positives, i.e. occurences of "command" which describe commands in the correct sense (mostly CLI or RPC related, also some with Socks5)
  3. put the remaining occurences into a scripted-diff (a quite simple one, actually) that renames "command" to "msgtype" in the concerned files.

  The name `msgtype` was intentionally chosen without the underscore `_` as classes beginning with `msg_` define concrete types of messages.

ACKs for top commit:
  MarcoFalke:
    ACK 9df32e820d . Makes sense that tests use the same naming as Bitcoin Core. See `NetMsgType` here: https://doxygen.bitcoincore.org/namespace_net_msg_type.html

Tree-SHA512: cd0ee08a382910b7f10ce583acdaf4f8a39f9ba4a22434a914415727eedd98bac538de9bf6633574d5eb86f62558bc8dcb638a3289d99b04f8481f34e7a9a0c7
2020-04-19 09:18:21 -04:00
João Barbosa fc289b7898 wallet: Refactor WalletRescanReserver to use wallet reference 2020-04-19 14:04:37 +01:00
MarcoFalke faebcd4e8d
ci: Remove xenial tsan workaround 2020-04-19 08:52:49 -04:00
MarcoFalke fac2fc4dd8
test: Increase debugging to hunt down mempool_reorg intermittent failure 2020-04-19 08:40:20 -04:00
MarcoFalke b470c75847
Merge #15761: Replace -upgradewallet startup option with upgradewallet RPC
0d32d66148 Remove -upgradewallet startup option (Andrew Chow)
92263cce5b Add upgradewallet RPC (Andrew Chow)
1e48796c99 Make UpgradeWallet a member function of CWallet (Andrew Chow)
c988f27937 Have UpgradeWallet take the version to upgrade to and an error message out parameter (Andrew Chow)
1833237123 Only run UpgradeWallet if the wallet needs to be upgraded (Andrew Chow)
9c16b1735f Move wallet upgrading to its own function (Andrew Chow)

Pull request description:

  `-upgradewallet` is largely incompatible with many recent wallet features and versions. For example, it was disabled if multiple wallets were used and would not work with encrypted wallets that were being upgraded to HD.

  This PR does away with the old method of upgrading upon startup and instead allows users to upgrade their wallets via an `upgradewallet` RPC. This does largely the same thing as the old `-upgradewallet` option but because the wallet is loaded, it can be unlocked to upgrade to HD. Furthermore it is compatible with multiwallet as it works on the individual wallet that is specified by the RPC.

ACKs for top commit:
  meshcollider:
    Code review ACK 0d32d66148
  darosior:
    ACK 0d32d66148
  MarcoFalke:
    ACK 0d32d66148 🚵

Tree-SHA512: b425bf6f5d605e26506889d63c780895482f07cbc086193218e031e8504d3072d41e90d65cd41bcc98ee4c1eb048954bc5d4ac85435f7394892373aac89a3b0a
2020-04-19 07:06:42 -04:00
MarcoFalke a998c5185b
Merge #18675: tests: Don't initialize PrecomputedTransactionData in txvalidationcache tests
3718ae2ef8 [tests] Don't initialize PrecomputedTransactionData in txvalidationcache tests (John Newbery)

Pull request description:

  PrecomputedTransactionData is initialized inside CheckInputScripts(). No need to pre-initialize it before calling into CheckInputScripts().

  Normally, I wouldn't bother, but we're making changes to `PrecomputedTransactionData` in #17977 which would break these tests without removing these constructions. Might as well get these changes out of the way here.

ACKs for top commit:
  robot-visions:
    ACK 3718ae2ef8
  sipa:
    utACK 3718ae2ef8

Tree-SHA512: bc9c095035a7072a2a91941df38cdbb969e817264efbaa6dcb88cc3ab132d9264aa0751fa588d1a5e45f37b4d2bb1903cda078765f0bbcc87d9cc47cbec5356a
2020-04-19 06:18:21 -04:00
MarcoFalke b690b24eb2
Merge #18633: test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2)
fa03713e13 test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2) (MarcoFalke)

Pull request description:

  actually (?) fix #18561

  See most recent traceback https://travis-ci.org/github/bitcoin/bitcoin/jobs/674668692#L7062

  I believe the reason the error is still there is that ConnectionResetError is derived from OSError:

  ConnectionResetError(ConnectionError(OSError))

  And IOError is an alias for OSError since python 3.3, see https://docs.python.org/3/library/exceptions.html#IOError

  So fix that by renaming IOError to the alias OSError and move the less specific catch clause down a few lines.

ACKs for top commit:
  jonatack:
    ACK fa03713e13

Tree-SHA512: 6e5b214ed9101bf8ebe7472dcc1f9e9d128e2575c93ec00c8d0774ae1a9b52a8c2a653a45a0eab8d881570b08dd5ffeddf5aca88a10438c366e1f633253cb0b5
2020-04-19 06:10:06 -04:00
fanquake d65631171c
Merge #18695: test: Replace boost::mutex with std::mutex
27abd1a4f4 test: Replace boost::mutex with std::mutex (Hennadii Stepanov)

Pull request description:

  This PR replaces `boost::mutex` with `std::mutex` in the `scheduler_tests` test suite.

ACKs for top commit:
  theStack:
    ACK 27abd1a4f4
  sipa:
    utACK 27abd1a4f4

Tree-SHA512: 062eed360a68910fb71552fd892bfd097442718a237446cfb8350bfd5d807da7251ead2b9755e1d7022598774ed23fa5432a589ac6f8cadddab404b439883466
2020-04-19 11:53:25 +08:00
MarcoFalke fa03713e13
test: Properly raise FailedToStartError when rpc shutdown before warmup finished (take 2) 2020-04-18 14:58:58 -04:00
MarcoFalke 6ae99aab5d
Merge #18692: test: Bump timeout in wallet_import_rescan
fabfcad876 test: Bump timeout in wallet_import_rescan (MarcoFalke)

Pull request description:

  Avoid timeouts when starting the node, also make error message more verbose

ACKs for top commit:
  practicalswift:
    ACK fabfcad876 -- patch looks correct

Tree-SHA512: 8fd60a05380349f521d0e814d2f268702dfbe57c7567a4f6e94435498dfdd32909179d75fded44757ecb1a93a4045842bc6d00bfd6cd18ba751513461359c7b0
2020-04-18 12:57:16 -04:00
Samuel Dobson bbb1ba1814
Merge #17219: wallet: allow transaction without change if keypool is empty
92bcd70808 [wallet] allow transaction without change if keypool is empty (Sjors Provoost)
709f8685ac [wallet] CreateTransaction: simplify change address check (Sjors Provoost)
5efc25f963 [wallet] translate "Keypool ran out" message (Sjors Provoost)

Pull request description:

  Extracted from #16944

  First this PR simplifies the check when generating a change address, by dropping `CanGetAddresses` and just letting `reservedest.GetReservedDestination` do this check.

  Second, when the keypool is empty, instead of immediately giving up, we create a dummy change address and pass that to coin selection. If we didn't need the change address (e.g. when spending the entire balance), then it's all good. If we did need a change address, we throw the original error.

ACKs for top commit:
  fjahr:
    Code review ACK 92bcd70808
  jonasschnelli:
    utACK 92bcd70808
  achow101:
    ACK 92bcd70808
  meshcollider:
    Code review ACK 92bcd70808

Tree-SHA512: 07b8c8251f57061c58a85ebf0359be63583c23bac7a2c4cefdc14820c0cdebcc90a2bb218e5ede0db11d1e204cda149e056dfd18614642070b3d56efe2735006
2020-04-18 22:00:26 +12:00
Adam Soltys 709998467e
rpc: doc: Fix and extend getblockstats examples
This fixes the example curl command for `getblockstats` which is missing
a comma between the params and has single quotes around the second
parameter.

Besides fixing the existing example, this commit adds an additional
example of getting block stats by hash by using a known workaround with
bitcoin-cli to get it to treat the hash parameter as a JSON string by
wrapping it in both single and double quotes.

Co-Authored-By: Andrew Toth <andrewstoth@gmail.com>
Co-Authored-By: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
2020-04-17 20:40:08 -07:00
Hennadii Stepanov 27abd1a4f4
test: Replace boost::mutex with std::mutex 2020-04-18 01:51:05 +03:00
MarcoFalke 895c71e535
Merge #18682: fuzz: http_request workaround for libevent < 2.1.1
6f8b498d18 fuzz: http_request workaround for libevent < 2.1.1 (Sebastian Falbesoner)

Pull request description:

  The fuzz test `http_request` calls the following two internal libevent functions:
  * `evhttp_parse_firstline_`
  * `evhttp_parse_headers_`

  Before libevent 2.1.1 however, internal functions names didn't end with an underscore (see libevent commit 8ac3c4c25b and [Changelog for 2.1.1.-alpha](https://github.com/libevent/libevent/blob/master/ChangeLog#L1830) when the change was first mentioned) hence the build fails with a linking error.
  This PR adds a preprocessor workaround to the test that checks for the libevent version (via ~`_EVENT_NUMERIC_VERSION`~ `LIBEVENT_VERSION_NUMBER`) and creates wrapper functions mapping to naming scheme without underscore in case the version is older than 2.1.1.

  Tested with Ubuntu Xenial 16.04.6 LTS and clang-8.

ACKs for top commit:
  hebasto:
    ACK 6f8b498d18, tested on xenial:

Tree-SHA512: 3b9e0147b8aea22e417d418e3b6d4905f5be131c2b0ae4b0f8b9411c5606d2e22f1b23e1ecc6980ecab907c61404de09e588aae1ac43cf70cf9e8d006bbdee73
2020-04-17 17:17:11 -04:00
MarcoFalke fabfcad876
test: Bump timeout in wallet_import_rescan 2020-04-17 17:09:27 -04:00
MarcoFalke c54295c1a2
Merge #18641: test: Create cached blocks not in the future
fa32097541 test: Create cached blocks not in the future (MarcoFalke)

Pull request description:

  This avoids test failures when tests assume blocks are not from the future, like in wallet_dump: https://cirrus-ci.com/task/6607130193035264?command=ci#L3306

ACKs for top commit:
  jonatack:
    ACK fa32097541

Tree-SHA512: 60b6882e0e1df8c5d67f034533407a45d3685983891b67ff4631072bfd0a93a325c7ca18758d7a2df252e4fcdb7c87321cb1e84458b22782e57e719eec634c22
2020-04-17 14:04:14 -04:00
MarcoFalke d7bdba460c
Merge #18683: ci: Disable valgrind functionl tests on forked repos to avoid timeouts
faceeae49a ci: Disable valgrind functionl tests on forked repos to avoid timeouts (MarcoFalke)

Pull request description:

  Allows people to fork our repo and run the tests again

  Also print more cache stats

ACKs for top commit:
  hebasto:
    ACK faceeae49a, tested on my own repo: https://travis-ci.org/github/hebasto/bitcoin/jobs/676257500

Tree-SHA512: 50e44edf94fcb997438eeaf7308b2b58a0854141ecb1fbb0ba7bf5ed662f19b60899f966f579cca90ad5e789234d0e90122d8c2c854da70339058adc3a475fa6
2020-04-17 14:00:04 -04:00
Sebastian Falbesoner 6f8b498d18 fuzz: http_request workaround for libevent < 2.1.1
Before libevent 2.1.1, internal functions names didn't end with an underscore.
2020-04-17 19:00:19 +02:00