124e1ee134 doc: Add release notes for getindexinfo RPC (Fabian Jahr)
c447b09458 test: Add tests for getindexinfo RPC (Fabian Jahr)
667bc7a7f7 rpc: Add getindexinfo RPC (Fabian Jahr)
Pull request description:
As I was playing with indices a I was missing an RPC that gives information about the active indices in the node. I think this can be helpful for many users, especially since there are some new index candidates coming up (#14053, #18000) that can give a quick overview without the user having to parse the logs.
Feature summary:
- Adds new RPC `listindices` (placed in Util section)
- That RPC only lists the actively running indices
- For each index it gives the name, whether it is synced and up to which block height it is synced
ACKs for top commit:
laanwj:
Re-ACK 124e1ee134
jonatack:
Code review re-ACK 124e1ee per `git range-diff a57af89 47a5372 124e1ee` no change since my last re-ACK, rebase only
Tree-SHA512: 3b7174c87951e6457fef099f530337803906baf32fb64261410b8def2c0917853d6a1bf3059cd590b1cc1523608f8916dafb327a431d27ecbf8d7454406b5b35
ed5cd12869 test: Distinguish between nodes(bitcoind) and peers(mininodes) in p2p_leak.py (Dhruv Mehta)
f6f082b934 test: remove `CNodeNoVersionIdle` from p2p_leak.py (Dhruv Mehta)
45cf55ccac test: remove `CNodeNoVersionMisbehavior` from p2p_leak.py (Dhruv Mehta)
Pull request description:
- Removes `CNodeNoVersionMisbehavior` per recommendation at https://github.com/bitcoin/bitcoin/pull/19657#issuecomment-669926458
- Removes `CNodeNoVersionIdle` because it is similarly unnecessary
- As someone new to the codebase, I found it easier to understand it if `no_version_disconnect_node` tries to overwhelm the peer with any message that is not version/verack.
- Per recommendation at https://github.com/bitcoin/bitcoin/pull/19727#pullrequestreview-468093555, made a clear distinction between nodes(bitcoind) and peers(mininode interface implementations)
ACKs for top commit:
jnewbery:
tested ACK ed5cd12869
amitiuttarwar:
utACK ed5cd12869
Tree-SHA512: 310a24c91fd837e7f65177edb55fe6142fb3559fae7867c5cdd9c9a23b1a02202b935ca9a82633fa7649f3de2fa221f6da906a7b5e499fc20f7254085033757d
71e0f07e9c util: remove unused c-string variant of atoi64() (Sebastian Falbesoner)
Pull request description:
This is another micro-PR "removing old cruft with potentially sharp edges" (quote by practicalswift, see #19739). Gets rid of the c-string variant of the function `atoi64()`, which is only used in fuzzers and on one place with `wallet/wallet.h` (where it is originally a `std::string` anyways and uses `.c_str()` -- this method call can simply be removed.)
ACKs for top commit:
practicalswift:
ACK 71e0f07e9c -- diff looks correct
laanwj:
ACK 71e0f07e9c
Tree-SHA512: 4d1d28e2f5274fdbe0652e7a0f83dd416f4d19c1e1a49979927960a3ad40b0990eeaa4374656bf2c6998a965a14d62c1bc78303b7d583d3307c17828030a8e3b
356988e200 util: make EncodeBase58Check consume Spans (Sebastian Falbesoner)
f0fce0675d util: make EncodeBase58 consume Spans (Sebastian Falbesoner)
Pull request description:
This PR improves the interfaces for the functions `EncodeBase58{Check}` by using Spans, in a similar fashion to e.g. PRs #19660, #19687. Note that on the master branch there are currently two versions of `EncodeBase58`: one that takes two pointers (marking begin and end) and another one that takes a `std::vector<unsigned char>` const-ref. The PR branch only leaves one generic Span-interface, both simplifying the interface and allowing more generic containers to be passed. The same is done for `EncodeBase58Check`, where only one interface existed but it's more generic now (e.g. a std::array can be directly passed, as done in the benchmarks).
ACKs for top commit:
laanwj:
Code review ACK 356988e200
Tree-SHA512: 47cfccdd7f3a2d4694bb8785e6e5fd756daee04ce1652ee59a7822e7e833b4a441ae9362b9bd67ea020d2b5b7d927629c9addb6abaa9881d8564fd3b1257f512
fa0538e94d ci: Set cirrus RAM to 8GB (MarcoFalke)
fa41810d0e ci: Run valgrind fuzzer on cirrus (MarcoFalke)
Pull request description:
The first commit should fix the 50min timeout in forked repos. Similar to #19424. E.g. https://travis-ci.org/github/bitcoin-core/gui/builds/718322267
The second commit should fix#19744
Top commit has no ACKs.
Tree-SHA512: c765098dfa913ca49b1d1eee99aaa83e4b9eb191b7ad5e652e3f04744fe8670dd3ef4215832b8e2b5bac0273d24f607fc275e72f566326108ba42ab57228ffd4
It's also clearer to have `no_version_disconnect_node` send a message
other than version or verack in order to reach the peer discouragement
threshold.
72ae20fc14 tests: add sync_all to fix race condition in wallet groups test (Karl-Johan Alm)
Pull request description:
This most likely fixes#19749, the intermittent CI issues with wallet_groups.
This fix is also included in #19743.
Top commit has no ACKs.
Tree-SHA512: dd6ef7f89829483e2278191c21fe0912b51fd2187c10a0fa158339c5ab9f22d93b733ae10f17ef25d8b64f44e596e66dba8d7db5c009343472f422ce4cd67d8f
2f8a4c9a06 build: Enable some commonly enabled compiler diagnostics (practicalswift)
Pull request description:
Enable some commonly enabled compiler diagnostics as discussed in #17344.
| Compiler diagnostic | no# of emitted unique GCC warnings in `master` | no# of emitted unique Clang warnings in `master` |
| ------------- | ------------- | ------------- |
| `-Wduplicated-branches`: Warn if `if`/`else` branches have duplicated code | 0 | Not supported |
| `-Wduplicated-cond`: Warn if `if`/`else` chain has duplicated conditions | 0 | Not supported |
| `-Wlogical-op`: Warn about logical operations being used where bitwise were probably wanted | 0 | Not supported |
| `-Woverloaded-virtual`: Warn if you overload (not `override`) a virtual function | 0 | 0 |
| ~~`-Wunused-member-function`: Warn on unused member function~~ | Not supported | 2 |
| ~~`-Wunused-template`: Warn on unused template~~ | Not supported | 1 |
There is a large overlap between this list and [Jason Turner's list of recommended compiler diagnostics in the Collaborative Collection of C++ Best Practices (`cppbestpractices`) project](https://github.com/lefticus/cppbestpractices/blob/master/02-Use_the_Tools_Available.md#gcc--clang). There is also an overlap with the recommendations given in the [C++ Core Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) (with editors Bjarne Stroustrup and Herb Sutter).
Closes#17344.
ACKs for top commit:
jonatack:
ACK 2f8a4c9a06 no warnings for me with these locally on debian 5.7.10-1 (2020-07-26) x86_64 with gcc 10 and clang 12
fanquake:
ACK 2f8a4c9a06 - no-longer seeing any obvious issues with doing this.
hebasto:
ACK 2f8a4c9a06, no new warnings in Travis jobs.
Tree-SHA512: f669ea22b31263a555f999eff6a9d65750662e95831b188c3192a2cf0127fb7b5136deb762a6b0b7bbdfb0dc6a40caf48251a62b164fffb81dd562bdd15ec3c8
4792cad88c doc: comment out and add annotation to unused MSG_FILTERED_WITNESS_BLOCK (Adam Jonas)
Pull request description:
Commenting out and adding a note to unused `MSG_FILTERED_WITNESS_BLOCK` [defined in BIP144](https://github.com/bitcoin/bips/blob/master/bip-0144.mediawiki#relay).
There was an attempt to make use of this in https://github.com/bitcoin/bitcoin/pull/10350, but it was closed due to lack of support. (h/t sdaftuar for pointing to the PR and jnewbery for the idea)
ACKs for top commit:
jnewbery:
Obvious ACK 4792cad88c
theStack:
ACK 4792cad88c📜
MarcoFalke:
cr ACK 4792cad88c good to keep it around in a comment to avoid accidental future re-assignment
practicalswift:
ACK 4792cad88c
Tree-SHA512: 22327ddded643ae50fdb529e4529a9b464f74e90620d0d2079a11070eaa8afe8363f6e14cca52f3bec2c9f87ee13e318edc6c5193761c94b8ae77be353a8da1f
fa55c1d5fd build: Add Werror=range-loop-analysis (MarcoFalke)
Pull request description:
The warning is implicitly enabled for Bitcoin Core. Also explicitly since commit d92204c900.
To avoid "fix range loop" follow-up refactors, we have two options:
* Disable the warning, so that issues never appear
* Enable it as an error, so that issues are either caught locally or by ci
ACKs for top commit:
fanquake:
ACK fa55c1d5fd
practicalswift:
ACK fa55c1d5fd -- pre-review fix-up is better than post-review fix-up
hebasto:
re-ACK fa55c1d5fd
Tree-SHA512: 019aa133f254af8882c1d5d10c420d9882305db0fc2aa9dad7d285168e2556306c3eedcc03bd30e63f11eae4cc82b648d83fb6e9179d6a6364651fb602d70134
7668db3b08 Move only: Move CDiskTxPos to its own file (Marcin Jachymiak)
Pull request description:
Moves `CDiskTxPos` it its own file so it can be used without the `txindex.h` include elsewhere. Originally part of #14053.
ACKs for top commit:
jnewbery:
utACK 7668db3b08
promag:
ACK 7668db3b08.
Tree-SHA512: b108e980ad04e43d1323410c3683a82bed70aee7795f5d8a2afbaf32a07ba598571f00b047bdde15048124b17178bcbd10654c48461beac988e9643cb2df664c
7f13dfb587 test: test the implicit avoid partial spends functionality (Karl-Johan Alm)
b82067bf69 wallet: try -avoidpartialspends mode and use its result if fees are below threshold (Karl-Johan Alm)
Pull request description:
The `-avoidpartialspends` feature is normally disabled, as it may affect the optimal fee for payments. This PR introduces a new parameter `-maxapsfee` (max avoid partial spends fee) which acts on the following values:
* -1: disable partial spend avoidance completely (do not even try it)
* 0: only do partial spend avoidance if fees are the same or better as the regular coin selection
* 1..∞: use APS variant if the absolute fee difference is less than or equal to the max APS fee
For values other than -1, the code will now try partial spend avoidance once, and if that gives a value within the accepted range, it will use that.
Example: -maxapsfee=0.00001000 means the wallet will do regular coin select, APS coin select, and then pick AKS iff the absolute fee difference is <= 1000 satoshi.
Edit: updated this to reflect the fact we are now using a max fee.
ACKs for top commit:
fjahr:
tested ACK 7f13dfb587
achow101:
ACK 7f13dfb587
jonatack:
ACK 7f13dfb58, code review, debug build, verified the test fails with `AssertionError: not(2 == 1)` for the number of vouts when `-maxapsfee=0.0001` is changed to 0, and verified the new logging with an added assertion.
meshcollider:
Code review ACK 7f13dfb587
Tree-SHA512: 475929df57f6191bb4e36bfbcad5a280a64bb0ecd8767b76cb2e44e2301235d0eb294a3f2fac5bbf15d35d7ecfba47acb2285feadb883c9ce31c08377e3afb3c
767073fb96 Shrink CAddress from 48 to 40 bytes on x64 (Vasil Dimov)
Pull request description:
`CAddress` inherits `CService` which is 28 bytes (on 64 bit machines).
`CAddress` then adds two member variables - one that requires 4 byte
alignment (`nTime`) and one that requires 8 byte alignment
(`nServices`).
Declare the smaller one first so that it fits in bytes 29..32.
On 32 bit machines this change has no effect and `CAddress` remains 40
bytes.
ACKs for top commit:
laanwj:
ACK 767073fb96
theStack:
ACK 767073fb96
Tree-SHA512: 73d6a4fcfa2687b4076950801871252e369510ecf09f820576dbeca9ee3ee94d14672e7d5596cb45fedd9e4b973dd0716a2ea3f13fc3058b4b697d036a7c9db0
9e7894357e test: speedup p2p_feefilter.py by whitelisting peers (immediate tx relay) (Sebastian Falbesoner)
fe3f0cc44e test: use wait_until for invs matching in p2p_feefilter.py (Sebastian Falbesoner)
6d941923c3 test: add logging for p2p_feefilter.py (Sebastian Falbesoner)
Pull request description:
This PR gives some love to the functional test `p2p_feefilter.py` by introducing the following changes:
* add missing log messages for the `test_feefilter` subtest (the main one)
* deduplicate code by using the utility function `wait_until` (already using the [recently introduced version](12410b1feb)) instead of a manual condition checking loop with `time.sleep`
* improve naming of the function `matchAllInvs` (more expressive name, snake_case) and moving it from global namespace to the connection class `FeefilterConn`
* speeding up the test significantly by the good ol' method of activating immediate tx relay (as seen on e.g. https://github.com/bitcoin/bitcoin/pull/17121, https://github.com/bitcoin/bitcoin/pull/17124, https://github.com/bitcoin/bitcoin/pull/17340, https://github.com/bitcoin/bitcoin/pull/17362, ...):
```
master branch:
$ time ./p2p_feefilter.py
...
real 0m39.367s
user 0m1.227s
sys 0m0.571s
PR branch:
$ time ./p2p_feefilter.py
...
real 0m9.386s
user 0m1.120s
sys 0m0.577s
```
ACKs for top commit:
instagibbs:
code review ACK 9e7894357e
jonatack:
re-ACK 9e78943 per `git range-diff 3ab2582 ea74a3c 9e78943`
Tree-SHA512: fe21c1c5413df9165fea916b5d5f609d3ba33e7b5c3364b38eb824fcc55d9e6abddf27116cbc0b325913d451a73c44542040fb916aec9c46f805c6e12f6f10cf
* Removed coinbasetxn from the getblocktemplate help outputs
* Added the missing name for transactions in the help outputs
* Added getblocktemplate help outputs for longpollid and default_witness_commitment
* Added more clarity to capabilities, rules, and coinbaseaux for getblocktemplate help (credit to luke-jr)
Co-authored-by: Luke Dashjr <luke+github_public@dashjr.org>
fa330ec2fe test: Remove confusing and broken use of wait_until global (MarcoFalke)
fa6583c30b ci: Set increased --timeout-factor by default (MarcoFalke)
Pull request description:
Assuming that tests don't have a logic error or race, setting a high timeout should not cause any issues. The tests will still pass just as fast in the fastest case, but it allows for some buffer in case of slow disks or otherwise starved ci machines.
Fixes#19729
ACKs for top commit:
hebasto:
ACK fa330ec2fe, I have reviewed the code, and it looks OK, I agree it can be merged.
Tree-SHA512: 3da80ee008c7b08bab5fdaf7804d57c79d6fed49a7d37b9c54fc89756659fcb9981fd10afc4d07bd90d93c1699fd410a0110a3cd34d016873759d114ce3cd82d
