Age | Commit message (Collapse) | Author |
|
The added benchmarks exercise the public methods Add(), GetAddr(),
Select() and Good().
|
|
60cdcf30a4ddd29907513f32b2e607e092c96179 test: Fix linter issue (Hennadii Stepanov)
Pull request description:
Top commit has no ACKs.
Tree-SHA512: 4fa0103526fed4b1399d3a6b83cea1e509cefff36d7e16ed1499d22afdc49e72053ac3d0634d858838ec6c296af7134131ea4d509c46de99da557567bc75d711
|
|
|
|
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
|
|
(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
|
|
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
|
|
also unified method of detecting misbehaviour
(using assert_debug_log instead of checking peer's banscore)
|
|
(GolombRiceEncode/GolombRiceDecode)
|
|
faebcd4e8d3bb72f0b8ce849716241be8b46c1da ci: Remove xenial tsan workaround (MarcoFalke)
Pull request description:
ACKs for top commit:
hebasto:
ACK faebcd4e8d3bb72f0b8ce849716241be8b46c1da
Tree-SHA512: 5d7e15be211e526948f863f573dbb5a97005262241ba4a07858346cba3a17cb24b1473df347224e0e4f2b22201750e27fba80ffe0d1dddf85f4e8f9341a8c129
|
|
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
|
|
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
|
|
'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
|
|
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
|
|
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.
|
|
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.
|
|
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
|
|
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
|
|
|
|
|
|
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
|
|
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
|
|
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
|
|
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
|
|
finished (take 2)
|
|
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
|
|
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
|
|
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>
|
|
|
|
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
|
|
|
|
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
|
|
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
|
|
Before libevent 2.1.1, internal functions names didn't end with an underscore.
|
|
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
|
|
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
|
|
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
|
|
|
|
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
|
|
|
|
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
|
|
|
|
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
|
|
|
|
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
|
|
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
|
|
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
|
|
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
|
|
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
|
|
PrecomputedTransactionData is initialized inside CheckInputScripts(). No need
to pre-initialize it before calling into CheckInputScripts().
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
|