5da96210fc doc: release note for getpeerinfo last_block/last_transaction (Jon Atack)
cfef5a2c98 test: rpc_net.py logging and test naming improvements (Jon Atack)
21c57bacda test: getpeerinfo last_block and last_transaction tests (Jon Atack)
8a560a7d57 rpc: expose nLastBlockTime/TXTime as getpeerinfo last_block/transaction (Jon Atack)
02fbe3ae0b net: add nLastBlockTime/TXTime to CNodeStats, CNode::copyStats (Jon Atack)
Pull request description:
This PR adds inbound peer eviction criteria `nLastBlockTime` and `nLastTXTime` to `CNodeStats` and `CNode::copyStats`, which then allows exposing them in the next commit as `last_transaction` and `last_block` Unix Epoch Time fields in RPC `getpeerinfo`.
This may be useful for writing missing eviction tests. I'd also like to add `lasttx` and `lastblk` columns to the `-netinfo` dashboard as described in https://github.com/bitcoin/bitcoin/pull/19643#issuecomment-671093420.
Relevant discussion at the p2p irc meeting http://www.erisian.com.au/bitcoin-core-dev/log-2020-08-11.html#l-549:
```text
<jonatack> i was specifically trying to observe and figure out how to test https://github.com/bitcoin/bitcoin/issues/19500
<jonatack> which made me realise that i didn't know what was going on with my peer conns in enough detail
<jonatack> i'm running bitcoin locally with nLastBlockTime and nLastTXTime added to getpeerinfo for my peer connections dashboard
<jonatack> sipa: is there a good reason why that (eviction criteria) data is not exposed through getpeerinfo currently?
<sipa> jonatack: nope; i suspect just nobody ever added it
<jonatack> sipa: thanks. will propose.
```
The last commit is optional, but I think it would be good to have logging in `rpc_net.py`.
ACKs for top commit:
jnewbery:
Code review ACK 5da96210fc
theStack:
Code Review ACK 5da96210fc
darosior:
ACK 5da96210fc
Tree-SHA512: 2db164bc979c014837a676e890869a128beb7cf40114853239e7280f57e768bcb43bff6c1ea76a61556212135281863b5290b50ff9d24fce16c5b89b55d4cd70
daed542a12 [net_processing] Move ProcessMessage to PeerLogicValidation (John Newbery)
c556770b5e [net_processing] Change PeerLogicValidation to hold a connman reference (John Newbery)
Pull request description:
Rather than ProcessMessage() being a static function in net_processing.cpp, make it a private member function of PeerLogicValidation. This is the start of moving static functions and global variables into PeerLogicValidation to make it better encapsulated.
ACKs for top commit:
jonatack:
ACK daed542a12 code review and debug tested
promag:
Code review ACK daed542a12.
MarcoFalke:
re-ACK daed542a12, only change is removing second commit 🎴
theStack:
Code Review ACK daed542a12
Tree-SHA512: ddebf410d114d9ad5a9e536950018ff333a347c035d74fcc101fb4a3f20a281782c7eac2b7d1bd1c8f6bc7e59f5b5630fb52c2e1b4c32df454fa584673bd021e
366913e307 build: AX_BOOST_THREAD serial 33 (Igor Cota)
cf0681133a build: disable D-Bus on Android by default (Igor Cota)
Pull request description:
I've been trying to build for Android on different OSes/Gitian with varying success. Build system is quite the beast and sometimes it doesn't get it right. To make sure it does these three little tweaks make the Android build more robust:
- disable D-Bus (Android doesn't support it and has its own way to trigger notifications)
- don't flag `-lpthread` when linking Boost, [Bionic has built-in support](https://stackoverflow.com/questions/30801752/android-ndk-and-pthread)
- ~~add `-static-libstdc++` to linker flags. This avoids having to bundle `libc++_shared` with CLI apps, still necessary with `bitcoin-qt` though (thanks Sjors)~~
I think these are small and fairly straightforward so I put them all into this one PR.
ACKs for top commit:
fanquake:
ACK 366913e307
Tree-SHA512: 31465fd228a5877c20aa2a05f98242d4eeb328b9b35bd1a7a3dcfb1ef51379d84053a81ade5a65436ffc1bc8ccd21f11ed0539eb10e827d182c0c04394629af0
fa30d5282c doc: Remove label from good first issue template (MarcoFalke)
Pull request description:
Good first issues aren't that frequent that manually assigning the label is a problem, but this fixes the spam problem (e.g. https://twitter.com/GoodFirstIssues/status/1295455089491161088 )
ACKs for top commit:
jnewbery:
ACK fa30d5282c
Tree-SHA512: 59e7c707637cc328e2443c2b7e5d2c82ef151739ad5afb6cea1a60501318dc8c4c81c95591eed8172581ac99d43cf826dcdd547e096eff1038137853af67a975
15ae4a17c4 test/fuzz: add a seed corpus generation option to the test_runner (Antoine Poinsot)
Pull request description:
This adds a startup option to test/fuzz/test_runner.py which allows to generate seed corpus to the passed `seed_dir` instead of using them.
ACKs for top commit:
MarcoFalke:
ACK 15ae4a17c4
Tree-SHA512: f80ad58e48cc45272eace33dbf377848f31cbd6a25433786d50e9700f70185dff6513f71d885d0727ed57a2aa49163bfbdbc51a8091e99b4b1bae71e1504e6a5
4148f55dd0 docs: Correct description for getblockstats's txs field (Nadav Ivgi)
Pull request description:
It does count the coinbase transaction.
Refs #19766
ACKs for top commit:
MarcoFalke:
ACK 4148f55dd0
theStack:
ACK 4148f55dd0
Tree-SHA512: ccd420f19242efbbbecfe822c825363bc89e26618834de0d805f5cdb07461c8bdc6e077c61ea8cd0d40564a96c67d8a71c68175c8543bb849909d7ae375b2a92
5067c5acc3 [test] Add test for getblockheader verboseness (Torhte Butler)
Pull request description:
Improve test coverage by adding a test for getblockheader with verbose argument set to false.
ACKs for top commit:
theStack:
ACK 5067c5acc3
Tree-SHA512: e55593f1026a89dc7b796fa985b4cbcdb596e91d80d42dfb0660bda1692aaa35749ec29f9cd7032803f6225afb323f085df1ef6a9982de87be8e098f7253cdd5
8ed2f1ed78 Remove unused includes (Marcin Jachymiak)
cf095a53fc Move comment about BaseIndex::DB from TxIndex::DB (Marcin Jachymiak)
Pull request description:
Moves a comment about the `BaseIndex::DB` from the `TxIndex::DB` into the correct place. Originally part of https://github.com/bitcoin/bitcoin/pull/14053.
ACKs for top commit:
fanquake:
ACK 8ed2f1ed78
Tree-SHA512: cb4e2b916c7ab996961cc2e1d910bc4b8a1700eb32b70fc1657ca720117a7a84f7337fe5e4fb30e047aa92c31eaa976eaaa5cb8f861877f2ff6f4a59bb94f4e9
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.
