aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-04-21bench: add CAddrMan benchmarksVasil Dimov
The added benchmarks exercise the public methods Add(), GetAddr(), Select() and Good().
2020-04-21Merge #18721: test: Fix linter issueMarcoFalke
60cdcf30a4ddd29907513f32b2e607e092c96179 test: Fix linter issue (Hennadii Stepanov) Pull request description: Top commit has no ACKs. Tree-SHA512: 4fa0103526fed4b1399d3a6b83cea1e509cefff36d7e16ed1499d22afdc49e72053ac3d0634d858838ec6c296af7134131ea4d509c46de99da557567bc75d711
2020-04-21test: Fix linter issueHennadii Stepanov
2020-04-21Merge #18672: test: add further BIP37 size limit checks to p2p_filter.pyMarcoFalke
c7437185589926ec8def2af6bede6a407b3d2e4a 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 https://github.com/bitcoin/bitcoin/pull/18628/commits/fa4c29bc1d2425f861845bae4f3816d9817e622a, 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 c7437185589926ec8def2af6bede6a407b3d2e4a robot-visions: ACK c7437185589926ec8def2af6bede6a407b3d2e4a jonasschnelli: utACK c7437185589926ec8def2af6bede6a407b3d2e4a. Seems to fix it: https://bitcoinbuilds.org/index.php?build=2524 Tree-SHA512: a03e7639263eb36a381922afb4e1d0ed2ae286f2ad2e7bbd922509a043ddf6cfd08747e01d54d29bfb8f54b66908f653974b9c347e4ca4f43332b586778893be
2020-04-20Merge #18190: tests: Add fuzzing harness for Golomb-Rice coding ↵MarcoFalke
(GolombRiceEncode/GolombRiceDecode) 69749fbe6a95f45eb7a695a5f89be87e55c91fb8 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-20Merge #18676: build: Check libevent minimum version in configure scriptWladimir J. van der Laan
b68e71796792a9da9daa0a4e759d284d15595230 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 https://github.com/bitcoin/bitcoin/pull/18676/commits/b68e71796792a9da9daa0a4e759d284d15595230 laanwj: ACK b68e71796792a9da9daa0a4e759d284d15595230 Tree-SHA512: 9825c42aeb166165e99fe5eaf74dbb47c2b51aecdbe53c5ae949fe126e1b8e8b6fe8d228fdde4e8daa4243e5907954202f42eb23c71629e4b2b92a7d4eb892e4
2020-04-20test: add further BIP37 size limit checks to p2p_filter.pySebastian Falbesoner
also unified method of detecting misbehaviour (using assert_debug_log instead of checking peer's banscore)
2020-04-20tests: Add fuzzing harness for Golomb-Rice coding ↵practicalswift
(GolombRiceEncode/GolombRiceDecode)
2020-04-20Merge #18705: ci: Remove xenial tsan workaroundMarcoFalke
faebcd4e8d3bb72f0b8ce849716241be8b46c1da ci: Remove xenial tsan workaround (MarcoFalke) Pull request description: ACKs for top commit: hebasto: ACK faebcd4e8d3bb72f0b8ce849716241be8b46c1da Tree-SHA512: 5d7e15be211e526948f863f573dbb5a97005262241ba4a07858346cba3a17cb24b1473df347224e0e4f2b22201750e27fba80ffe0d1dddf85f4e8f9341a8c129
2020-04-20Merge #17579: [refactor] Merge getreceivedby tally into GetReceived functionMarcoFalke
a1d5b12ec07d2f7aa9fa955a6dd99e8a2be5cb25 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 https://github.com/bitcoin/bitcoin/commit/a1d5b12ec07d2f7aa9fa955a6dd99e8a2be5cb25 meshcollider: utACK a1d5b12ec07d2f7aa9fa955a6dd99e8a2be5cb25 Tree-SHA512: 43d9cd92f7c2c6a8b9c7509aa85a9b9233a6cfec1c43a9062e3bdfb83515413d1feafa8938c828351278ba22bd31c47e62ab5341e4bddc2493103b094d73b047
2020-04-20Merge #17831: rpc: doc: Fix and extend getblockstats examplesMarcoFalke
709998467e1c1bc7980662c9f88fbc7964602d33 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 https://github.com/bitcoin/bitcoin/pull/17831/commits/709998467e1c1bc7980662c9f88fbc7964602d33 Tree-SHA512: 84a5b7f449f06fff785bc0afbc1a7dfd55454bc76c52a8945e91556f87f3edfdc5a1780faab8fcfd6c415b734295b7c67d2e04ba7b6cfa91a77758af5dda53ae
2020-04-20Merge #18544: net: limit BIP37 filter lifespan (active between ↵MarcoFalke
'filterload'..'filterclear') a9ecbdfcaa15499644d16e9c8ad2c63dfc45b37b test: add more inactive filter tests to p2p_filter.py (Sebastian Falbesoner) 5eae034996b340c19cebab9efb6c89d20fe051ef 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: https://github.com/bitcoin/bitcoin/blob/c0b389b33516fb3eaaad7c30bd11dba768882a7e/src/net.h#L812 and after a loaded filter is removed again through a `filterclear` message: https://github.com/bitcoin/bitcoin/blob/c0b389b33516fb3eaaad7c30bd11dba768882a7e/src/net_processing.cpp#L3201 The behaviour was introduced by commit https://github.com/bitcoin/bitcoin/commit/37c6389c5a0ca63ae3573440ecdfe95d28ad8f07 (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) https://github.com/bitcoin/bitcoin/blob/c0b389b33516fb3eaaad7c30bd11dba768882a7e/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) https://github.com/bitcoin/bitcoin/blob/c0b389b33516fb3eaaad7c30bd11dba768882a7e/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-20Merge #18691: test: add wait_for_cookie_credentials() to framework for ↵MarcoFalke
rpcwait tests 92fe537cf704dfb4ae830c8c8b382f08c4893e65 test: fix intermittent race condition in interface_bitcoin_cli.py (Jon Atack) c648e636b2f230db5f1d1137088685f74ae42454 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-19test: fix intermittent race condition in interface_bitcoin_cli.pyJon Atack
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-19test: add wait_for_cookie_credentials() to test frameworkJon Atack
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-19Merge #18601: wallet: Refactor WalletRescanReserver to use wallet referenceMarcoFalke
fc289b7898fb90d4800675b69c0bb9b42df5599f 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 fc289b7898fb90d4800675b69c0bb9b42df5599f Tree-SHA512: b03e33f2d9df2870436aa3284137fd022dd89ea96a1b170fa27f8685ad4f986e6c4ba5975a84966c30d18430a4014d7d8740a1dff2f985c9ef8226ed18e69db9
2020-04-19Merge #18610: scripted-diff: test: replace command with msgtype (naming)MarcoFalke
9df32e820d83aa74e2f175d8d63b5666b8b4ef0e 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 9df32e820d83aa74e2f175d8d63b5666b8b4ef0e . 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-19wallet: Refactor WalletRescanReserver to use wallet referenceJoão Barbosa
2020-04-19ci: Remove xenial tsan workaroundMarcoFalke
2020-04-19Merge #15761: Replace -upgradewallet startup option with upgradewallet RPCMarcoFalke
0d32d661481f099af572e7a08a50e17bcc165c44 Remove -upgradewallet startup option (Andrew Chow) 92263cce5b6c6b66296dadda5f29724611db0160 Add upgradewallet RPC (Andrew Chow) 1e48796c99b63aa8fa8451ce7b0c20759ea43500 Make UpgradeWallet a member function of CWallet (Andrew Chow) c988f27937bc79c90f4eed48552c72f1b66dc044 Have UpgradeWallet take the version to upgrade to and an error message out parameter (Andrew Chow) 183323712398e26ddcf3a9dc048aaa9900a91f5a Only run UpgradeWallet if the wallet needs to be upgraded (Andrew Chow) 9c16b1735f8e530ce68d678e9ca0eceb2ceb3520 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 0d32d661481f099af572e7a08a50e17bcc165c44 darosior: ACK 0d32d661481f099af572e7a08a50e17bcc165c44 MarcoFalke: ACK 0d32d661481f099af572e7a08a50e17bcc165c44 🚵 Tree-SHA512: b425bf6f5d605e26506889d63c780895482f07cbc086193218e031e8504d3072d41e90d65cd41bcc98ee4c1eb048954bc5d4ac85435f7394892373aac89a3b0a
2020-04-19Merge #18675: tests: Don't initialize PrecomputedTransactionData in ↵MarcoFalke
txvalidationcache tests 3718ae2ef8dd2559e435bf8d7f5ed5217611ce81 [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 3718ae2ef8dd2559e435bf8d7f5ed5217611ce81 sipa: utACK 3718ae2ef8dd2559e435bf8d7f5ed5217611ce81 Tree-SHA512: bc9c095035a7072a2a91941df38cdbb969e817264efbaa6dcb88cc3ab132d9264aa0751fa588d1a5e45f37b4d2bb1903cda078765f0bbcc87d9cc47cbec5356a
2020-04-19Merge #18633: test: Properly raise FailedToStartError when rpc shutdown ↵MarcoFalke
before warmup finished (take 2) fa03713e133e3017112fdd5c278e0c8643054578 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 fa03713e133e3017112fdd5c278e0c8643054578 Tree-SHA512: 6e5b214ed9101bf8ebe7472dcc1f9e9d128e2575c93ec00c8d0774ae1a9b52a8c2a653a45a0eab8d881570b08dd5ffeddf5aca88a10438c366e1f633253cb0b5
2020-04-19Merge #18695: test: Replace boost::mutex with std::mutexfanquake
27abd1a4f4c7a3d092d59edbbaa1e0f324c8b0ef 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 https://github.com/bitcoin/bitcoin/pull/18695/commits/27abd1a4f4c7a3d092d59edbbaa1e0f324c8b0ef sipa: utACK 27abd1a4f4c7a3d092d59edbbaa1e0f324c8b0ef Tree-SHA512: 062eed360a68910fb71552fd892bfd097442718a237446cfb8350bfd5d807da7251ead2b9755e1d7022598774ed23fa5432a589ac6f8cadddab404b439883466
2020-04-18test: Properly raise FailedToStartError when rpc shutdown before warmup ↵MarcoFalke
finished (take 2)
2020-04-18Merge #18692: test: Bump timeout in wallet_import_rescanMarcoFalke
fabfcad8764bb8f807b0ac5f3482b414278a4525 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 fabfcad8764bb8f807b0ac5f3482b414278a4525 -- patch looks correct Tree-SHA512: 8fd60a05380349f521d0e814d2f268702dfbe57c7567a4f6e94435498dfdd32909179d75fded44757ecb1a93a4045842bc6d00bfd6cd18ba751513461359c7b0
2020-04-18Merge #17219: wallet: allow transaction without change if keypool is emptySamuel Dobson
92bcd70808b9cac56b184903aa6d37baf9641b37 [wallet] allow transaction without change if keypool is empty (Sjors Provoost) 709f8685ac37510aa145ac259753583c82280038 [wallet] CreateTransaction: simplify change address check (Sjors Provoost) 5efc25f9638866941028454cfa9bae27f1519cb4 [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 92bcd70808b9cac56b184903aa6d37baf9641b37 jonasschnelli: utACK 92bcd70808b9cac56b184903aa6d37baf9641b37 achow101: ACK 92bcd70808b9cac56b184903aa6d37baf9641b37 meshcollider: Code review ACK 92bcd70808b9cac56b184903aa6d37baf9641b37 Tree-SHA512: 07b8c8251f57061c58a85ebf0359be63583c23bac7a2c4cefdc14820c0cdebcc90a2bb218e5ede0db11d1e204cda149e056dfd18614642070b3d56efe2735006
2020-04-17rpc: doc: Fix and extend getblockstats examplesAdam Soltys
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-18test: Replace boost::mutex with std::mutexHennadii Stepanov
2020-04-17Merge #18682: fuzz: http_request workaround for libevent < 2.1.1MarcoFalke
6f8b498d186df5aa08dbb9ca8fdeab6652f1db5e 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 https://github.com/libevent/libevent/commit/8ac3c4c25bea4b9948ab91cd00605bf34fc0bd72 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 6f8b498d186df5aa08dbb9ca8fdeab6652f1db5e, tested on xenial: Tree-SHA512: 3b9e0147b8aea22e417d418e3b6d4905f5be131c2b0ae4b0f8b9411c5606d2e22f1b23e1ecc6980ecab907c61404de09e588aae1ac43cf70cf9e8d006bbdee73
2020-04-17test: Bump timeout in wallet_import_rescanMarcoFalke
2020-04-17Merge #18641: test: Create cached blocks not in the futureMarcoFalke
fa320975411af4f0e41771d89958a77fd7a2284b 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 fa320975411af4f0e41771d89958a77fd7a2284b Tree-SHA512: 60b6882e0e1df8c5d67f034533407a45d3685983891b67ff4631072bfd0a93a325c7ca18758d7a2df252e4fcdb7c87321cb1e84458b22782e57e719eec634c22
2020-04-17Merge #18683: ci: Disable valgrind functionl tests on forked repos to avoid ↵MarcoFalke
timeouts faceeae49ae85fb644996fedb970ad9d0e297ca5 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 faceeae49ae85fb644996fedb970ad9d0e297ca5, tested on my own repo: https://travis-ci.org/github/hebasto/bitcoin/jobs/676257500 Tree-SHA512: 50e44edf94fcb997438eeaf7308b2b58a0854141ecb1fbb0ba7bf5ed662f19b60899f966f579cca90ad5e789234d0e90122d8c2c854da70339058adc3a475fa6
2020-04-17fuzz: http_request workaround for libevent < 2.1.1Sebastian Falbesoner
Before libevent 2.1.1, internal functions names didn't end with an underscore.
2020-04-17Merge #18607: rpc: Fix named arguments in documentationMarcoFalke
fa168d754221a83cab0d2984a02c41cf6819e873 rpc: Document all aliases for first arg of listtransactions (MarcoFalke) fa5b1f067fcf8bebb23455dd8a16cde5068e79cd rpc: Document all aliases for second arg of getblock (MarcoFalke) fa86a4bbfc000593ae4ad9dcdaec3fd0c0406086 rpc: Rename first arg of generateblock RPC to "output" (MarcoFalke) Pull request description: This fixes a bug found with #18531: * Currently the named argument for `generateblock` is documented as `address/descriptor`, but the server only accepts a named argument of `address`. Fix it by changing the name to `output` for both the documentation and the server code. Also, add tests to prove the server understands the new name `output`. * Unrelated to that, there have been a bunch of aliases in the server code that are not present in the source code of the documentation. Fix that by adding the alias to the source code of the documentation. Only the first alias is displayed in the rendered documentation. Also, add tests to prove the server actually understands all aliases. ACKs for top commit: pierreN: Tested ACK fa168d7 tests, help messages Tree-SHA512: 05e15628e3a667b296f3783d20f764b450b959451b5360c7eaf5993156582d47a0f5882330ca2493b851eb46324d504953b90c875bc88a15c9e8c89eb3ef8d92
2020-04-17Merge #18673: scripted-diff: Sort test includesMarcoFalke
fa4632c41714dfaa699bacc6a947d72668a4deef test: Move boost/stdlib includes last (MarcoFalke) fa488f131fd4f5bab0d01376c5a5013306f1abcd scripted-diff: Bump copyright headers (MarcoFalke) fac5c373006a9e4bcbb56843bb85f1aca4d87599 scripted-diff: Sort test includes (MarcoFalke) Pull request description: When writing tests, often includes need to be added or removed. Currently the list of includes is not sorted, so developers that write tests and have `clang-format` installed will either have an unrelated change (sorting) included in their commit or they will have to manually undo the sort. This pull preempts both issues by just sorting all includes in one commit. Please be aware that this is **NOT** a change to policy to enforce clang-format or any other developer guideline or process. Developers are free to use whatever tool they want, see also #18651. Edit: Also includes a commit to bump the copyright headers, so that the touched files don't need to be touched again for that. ACKs for top commit: practicalswift: ACK fa4632c41714dfaa699bacc6a947d72668a4deef jonatack: ACK fa4632c41714dfaa, light review and sanity checks with gcc build and clang fuzz build Tree-SHA512: 130a8d073a379ba556b1e64104d37c46b671425c0aef0ed725fd60156a95e8dc83fb6f0b5330b2f8152cf5daaf3983b4aca5e75812598f2626c39fd12b88b180
2020-04-17Merge #18664: fuzz: fix unused variable compiler warningMarcoFalke
eab7367e25e35688a4d4a6c96701dd7149134df5 fuzz: fix unused variable compiler warning (Jon Atack) Pull request description: Fixes the compiler warning while hopefully not invalidating the existing seeds. Added an explanatory comment. ``` test/fuzz/locale.cpp:59:19: warning: unused variable 'random_int32' [-Wunused-variable] const int32_t random_int32 = fuzzed_data_provider.ConsumeIntegral<int32_t>(); ``` ACKs for top commit: practicalswift: ACK eab7367e25e35688a4d4a6c96701dd7149134df5 Tree-SHA512: 4c90784518027cd3f85acd18030201efe4018f9da46365fef934e9a53a0b923031fec4c884a2da2f14232b6060aeb9016ac09950a18e31395de048548ecbc836
2020-04-17ci: Disable valgrind functionl tests on forked repos to avoid timeoutsMarcoFalke
2020-04-17Merge #18467: rpc: Improve documentation and return value of settxfeeMarcoFalke
38677274f931088218eeb1f258077d3387f39c89 rpc: settxfee respects -maxtxfee wallet setting (Fabian Jahr) bda84a08a0ac92dff6cadc99cf9bb8c3fadd7e13 rpc: Add documentation for deactivating settxfee (Fabian Jahr) Pull request description: ~~Closes 18315~~ `settxfee` can be deactivated by passing 0 as the fee. That does not seem to be documented for the user so this PR adds it in the description. The return value of a simple boolean seems also too simplified given the multiple dimensions that this deactivation feature enables. I.e. it does not seem intuitive if the returned boolean shows that the call succeeded or if means that `settxfee` is active. My suggested solution is to change the return value to a JSON object that included the "active" state and the currently set fee rate. Examples: ``` $ src/bitcoin-cli settxfee 0.0000000 { "active": false, "fee_rate": "0.00000000 BTC/kB" } $ src/bitcoin-cli settxfee 0.0001 { "active": true, "fee_rate": "0.00010000 BTC/kB" } ``` ACKs for top commit: MarcoFalke: ACK 38677274f931088218eeb1f258077d3387f39c89, seems useful to error out early instead of later #16257 🕍 jonatack: ACK 38677274f931088218eeb meshcollider: LGTM, utACK 38677274f931088218eeb1f258077d3387f39c89 Tree-SHA512: 642813b5cf6612abb4b6cb63728081a6bd1659d809e0149c8f56060b6da7253fee989b3b202854f3051df3773c966799af30b612648c466b099f00590f356548
2020-04-17fuzz: fix unused variable compiler warningJon Atack
2020-04-17Merge #17824: wallet: Prefer full destination groups in coin selectionSamuel Dobson
a2324e4d3f47f084b07a364c9a360a0bf31e86a0 test: Improve naming and logging of avoid_reuse tests (Fabian Jahr) 1abbdac6777bc5396d17a6772c8176a354730997 wallet: Prefer full destination groups in coin selection (Fabian Jahr) Pull request description: Fixes #17603 (together with #17843) In the case of destination groups of >10 outputs existing in a wallet with `avoid_reuse` enabled, the grouping algorithm is adding left-over outputs as an "incomplete" group to the list of groups even when a full group has already been added. This leads to the strange behavior that if there are >10 outputs for a destination the transaction spending from that will effectively use `len(outputs) % 10` as inputs for that transaction. From the original PR and the code comment I understand the correct behavior should be the usage of 10 outputs. I opted for minimal changes in the current code although there maybe optimizations possible for cases with >20 outputs on a destination this sounds like too much of an edge case right now. ACKs for top commit: jonatack: Re-ACK a2324e4 achow101: ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0 kallewoof: ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0 meshcollider: Tested ACK a2324e4d3f47f084b07a364c9a360a0bf31e86a0 (verified the new test fails on master without this change) Tree-SHA512: 4743779c5d469fcd16df5baf166024b1d3c8eaca151df1e8281b71df62b29541cf7bfee3f8ab48d83e3b34c9256e53fd38a7b146a54c79f9caa44cce3636971a
2020-04-17build: Set libevent minimum version to 2.0.21Hennadii Stepanov
2020-04-17Merge #18670: refactor: Remove unused methods CBloomFilter::reset()/clear()MarcoFalke
69ffddc83e0f3e265bf6cf7ae31489ae629fe6be refactor: Remove unused methods CBloomFilter::reset()/clear() (Sebastian Falbesoner) Pull request description: The method `CBloomFilter::reset()` was introduced by commit d2d7ee0e863b286e1c9f9c54659d494fb0a7712d in 2015, but was never ever used, as far as I could find. As discovered by MarcoFalke, the method `clear()` is also unused outside of unit tests and is hence also removed. ACKs for top commit: MarcoFalke: re-ACK 69ffddc83e0f3e265bf6cf7ae31489ae629fe6be jonatack: ACK 69ffddc83e0f3e2, code review, compiled a fuzz build and started the bloom_filter fuzz test as a sanity check. promag: ACK 69ffddc83e0f3e265bf6cf7ae31489ae629fe6be. Tree-SHA512: 6c53678545ad8e2fa1ffc0a8838e450462f26748a60632f738dc020f0eb494ae2c32841e6256e266ed9140177257a78b707123421942f3819a14ffcb9a99322f
2020-04-17test: Move boost/stdlib includes lastMarcoFalke
2020-04-17Merge #18262: bnb: exit selection when best_waste is 0Samuel Dobson
9b5950db8683f9b4be03f79ee0aae8a780b01a4b bnb: exit selection when best_waste is 0 (Andrew Chow) Pull request description: If we find a solution which has no waste, just use that. This solution is what we would consider to be optimal, and other solutions we find would have to also have 0 waste, so they are equivalent to the first one with 0 waste. Thus we can optimize by just choosing the first one with 0 waste. Closes #18257 ACKs for top commit: instagibbs: utACK https://github.com/bitcoin/bitcoin/pull/18262/commits/9b5950db8683f9b4be03f79ee0aae8a780b01a4b meshcollider: utACK 9b5950db8683f9b4be03f79ee0aae8a780b01a4b Tree-SHA512: 59565ff4a3d8281e7bc0ce87065a34c8d8bf8a95f628ba96b4fe89f1274979165aea6312e5f1f21b418c8c484aafc5166d22d9eff9d127a8192498625d58c557
2020-04-17refactor: Remove unused methods CBloomFilter::reset()/clear()Sebastian Falbesoner
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
2020-04-16Merge #18662: test: Replace gArgs with local argsman in benchMarcoFalke
faf989f93695d29099f6e152d5a2e117cd304183 util: Document why ArgsManager (con/de)structor is not inline (MarcoFalke) fae00a77e2589cc784650e3e60e1b99c22ca8a7b bench: Remove unused argsman.ClearArgs (MarcoFalke) fa46aebeb1fc419b227524eab8352d9a7fc1f981 scripted-diff: Replace gArgs with local argsman in bench (MarcoFalke) fa2bc4141df59f2e38fef863723b433250295d20 tools: Add unused argsman to bench_bitcoin (MarcoFalke) Pull request description: All utilities use the same gArgs global that the node uses. This is generally fine and does not lead to issues unless a bench test is going to spin up a NodeContext via the TestingSetup. In that case the two uses of gArgs conflict and currently it needs to be cleared: https://github.com/bitcoin/bitcoin/blob/544709763e1f45148d1926831e07ff03487673ee/src/bench/bench_bitcoin.cpp#L76 One solution would be to do nothing, because the current code works with that workaround. Another solution would be to not use the same global in all binaries. ACKs for top commit: promag: ACK faf989f93695d29099f6e152d5a2e117cd304183. ryanofsky: Code review ACK faf989f93695d29099f6e152d5a2e117cd304183. Just new commit added restoring forward declaration Tree-SHA512: 8ee4b28eee294d41c002f801fa844b0c23c919a3061f5109638701db0947b3b0ea28caa7311ae5f126fc660648bbaa0890853e6b06bdc5868692f52ba8c05f66
2020-04-16Merge #18598: gitian: Add missing automake package to gitian-win-signer.ymlWladimir J. van der Laan
e44aeefaaed8d698d1b9004b66f85384397b1a75 gitian: Add missing automake package to gitian-win-signer.yml (Andrew Chow) Pull request description: automake is needed to build osslsigncode otherwise autogen.sh fails with the docker virtualization method. ACKs for top commit: hebasto: ACK e44aeefaaed8d698d1b9004b66f85384397b1a75, for `osslsigncode-1.7.1` we did not run `autogen.sh` in the past. fanquake: ACK e44aeefaaed8d698d1b9004b66f85384397b1a75 jonatack: ACK e44aeef Tree-SHA512: a0e615c1b099ee1c469ce41f886f2ece6746234a5a800743a4e8be671e4114fd30e1c35bc0ddcb75778409564129d0fde7ac4e3d70b0f7691f97f729f34c8e0c
2020-04-16Merge #18667: ci: Limit cache size regardless of NO_DEPENDSMarcoFalke
0c6318788beaf1a31aeba5a21f3f8bb5c07cea6c ci: Limit cache size regardless of NO_DEPENDS (Hennadii Stepanov) Pull request description: Close #18666. ACKs for top commit: MarcoFalke: ACK 0c6318788beaf1a31aeba5a21f3f8bb5c07cea6c . Depends has ccache disabled anyway and is cached regardless of whether ccache is there or not, see #17248 Tree-SHA512: b1bf98be0f844b4704abd177841b014f3900be8160496f0d12596310db607b4f544547e8c3cbfcf17c086a78afd251653363f3dd467b769ac0062bc19adc8144
2020-04-16[tests] Don't initialize PrecomputedTransactionData in txvalidationcache testsJohn Newbery
PrecomputedTransactionData is initialized inside CheckInputScripts(). No need to pre-initialize it before calling into CheckInputScripts().
2020-04-16scripted-diff: Bump copyright headersMarcoFalke
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT-