9413ecdc66 fix: disable linter 'check-rpc-mapping.py' for composite commands (Konstantin Akimov)
f4bc19fb99 refactor: proper support for composite commands such as 'bls generate' (Konstantin Akimov)
Pull request description:
## Issue being fixed or feature implemented
We have composite commands such as 'bls generate' that do not exist in bitcoin's implementation of rpc.
It doesn't let to backport yet bitcoin#18531 which enforced extra checks for arguments name (name of rpc and list arguments in rpc help and actual implementation must match).
## What was done?
This PR improves support of composite commands in Dash Core. New style of composite commands are applied for `bls` composite commands: `bls generate` and `bls fromsecret` as proof of concept.
Once this PR is merged, I will provide similar fixes for other "compose" rpc commands: `protx`, `masternode` (and everything else if any).
Beside better validation of arguments and command names, it improves suggest menu in Qt app (see a screenshot) for composite commands.
## How Has This Been Tested?
Run unit and functional tests. Also extra tests to conduct in qt app:
- check suggest for 'bls ....' and 'help bls ...'
- check output of next commands:
```
help bls
help bls generate
help bls from secret
bls
bls generate
bls generate 1
```
- also let's see that old-fashion composite commands are not broken:
```
help protx
help protx diff
protx diff 00000021c7604b9992254f9f1ed91de5d65eaade33c773abea63a7b0e93293ee 000000e44f9894838ebf768b464177cfce8859dcf92b0509f5c2fba774315996
```
## Breaking Changes
N/A
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone
ACKs for top commit:
PastaPastaPasta:
utACK 9413ecdc66
Tree-SHA512: c8accd2f1ea1d2168d7a2bf25311c92b7839782fc300c587c7537880df5d3fbbf7e79cedb2c6fa88ab067b696994beaf7d92185ae3e64fe1a5f70ad9e8620bec
ddca614017 merge bitcoin-core/gui#325: Align numbers in the "Peer Id" column to the right (Kittywhiskers Van Gogh)
09a8fc1df9 merge bitcoin-core/gui#280: Remove user input from URI error message (Kittywhiskers Van Gogh)
a3ac6941a7 merge bitcoin-core/gui#194: Save/restore RPCConsole geometry only for window (Kittywhiskers Van Gogh)
504f8344ba merge bitcoin-core/gui#296: Do not use QObject::tr plural syntax for numbers with a unit symbol (Kittywhiskers Van Gogh)
c33afea7fd merge bitcoin-core/gui#166: Use enum type as switch argument in *TableModel (Kittywhiskers Van Gogh)
5a67b52725 merge bitcoin-core/gui#214: Disable requests context menu actions when appropriate (Kittywhiskers Van Gogh)
dc9b25e04c merge bitcoin-core/gui#219: Prevent the main window popup menu (Kittywhiskers Van Gogh)
7774bdb423 partial bitcoin-core/gui#79: Embed monospaced font (Kittywhiskers Van Gogh)
722bd8d7f3 qt: introduce GUIUtil::fixedPitchFont (Kittywhiskers Van Gogh)
afd0648959 merge bitcoin-core/gui#179: Add Type column to peers window, update peer details name/tooltip (Kittywhiskers Van Gogh)
57597df2f2 merge bitcoin-core/gui#203: Display plain "Inbound" in peer details (Kittywhiskers Van Gogh)
582c6ec870 merge bitcoin-core/gui#180: connection type follow-ups (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
| `develop` (d7413ffb) | This PR |
| ------------------------------------------------------------ | ------------------------------------------------------------ |
|  |  |
## Breaking Changes
None expected, changes are limited to refactoring, cleanup and refinement.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests **(note: N/A)**
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK ddca614017
Tree-SHA512: 8b07c6261c5e111f6db06be528aab7d3862bb31cf23e7df84a1e211ae1e1b5d52a51a520b24ed1b058ac19f845204fe458f16ba5920db9c1b1d1939a1fff6711
32f8fda7d6 merge bitcoin#24991: allow startup with -onlynet=onion -listenonion=1 (Kittywhiskers Van Gogh)
e67ed92d3d merge bitcoin#25173: add coverage for unknown network in -onlynet (Kittywhiskers Van Gogh)
77efd36112 merge bitcoin#24687: Check an invalid -i2psam will raise an init error (Kittywhiskers Van Gogh)
fb1416f7cb merge bitcoin#24205: improve network reachability test coverage and safety (Kittywhiskers Van Gogh)
7cb7479829 merge bitcoin#24663: add links to doc/cjdns.md (Kittywhiskers Van Gogh)
c736ebf566 merge bitcoin#24555: create initial doc/cjdns.md for CJDNS how-to documentation (Kittywhiskers Van Gogh)
554bd24186 partial bitcoin#24468: improve -onlynet help and related tor/i2p documentation (Kittywhiskers Van Gogh)
5436b6a82d merge bitcoin#24165: extend inbound eviction protection by network to CJDNS peers (Kittywhiskers Van Gogh)
d52724d039 merge bitcoin#22834: respect -onlynet= when making outbound connections (Kittywhiskers Van Gogh)
f9d1a9a00d merge bitcoin#23077: Full CJDNS support (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Depends on https://github.com/dashpay/dash/pull/6034
* Depends on https://github.com/dashpay/dash/pull/6035
* If `-proxy=` is given together with `-noonion` then the provided proxy will not be set as a proxy for reaching the Tor network. So it will not be possible to open manual connections to the Tor network for example with the `addnode` RPC. To mimic the old behavior use `-proxy=` together with `-onlynet=` listing all relevant networks except `onion`.
* [bitcoin#24165](https://github.com/bitcoin/bitcoin/pull/24165) has been backported _before_ [bitcoin#23758](https://github.com/bitcoin/bitcoin/pull/23758) and to account for this, minor changes were made in `src/test/net_peer_eviction_tests.cpp` (using `nTimeConnected` instead of `m_connected`). When backporting [bitcoin#23758](https://github.com/bitcoin/bitcoin/pull/23758), these changes will have to be reversed as they won't be covered by the cherry-pick diff.
* CJDNS support has been labelled as being introduced in Dash Core 21.0, in line with the milestone designation of the PR. Should `develop` be used for a new minor/patch release, `doc/cjdns.md` will have to be modified to reflect the correct version number.
## Breaking changes
No expected protocol or consensus changes.
## Checklist:
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK 32f8fda7d6
Tree-SHA512: e23b22ca5edbe4c4abeab0bc07780303e68e7c4cc46b7697300b0837c5acd3a98649b6b03bd07a23c827bd85f64210173027b0b0eea31872c031fa4ed04eeb0c
76279c1a37 Merge bitcoin/bitcoin#25149: refactor: Add thread safety annotation to `BanMan::SweepBanned()` (MacroFake)
6269c6f1db Merge bitcoin/bitcoin#25053: Guard `#include <config/bitcoin-config.h>` (fanquake)
d50f0b016f Merge bitcoin/bitcoin#24977: rpc: Explain active and internal in listdescriptors (fanquake)
3c44399d55 Merge bitcoin/bitcoin#24856: lint: Converting lint-assertions.sh to lint-assertions.py (laanwj)
e9f5b4b735 Merge bitcoin/bitcoin#24213: refactor: use Span in random.* (laanwj)
7e0474ac1c Merge bitcoin/bitcoin#24632: add `(none)` in -getinfo `Warnings:` if no warning returned (laanwj)
57e9b56bad Merge bitcoin/bitcoin#24145: mempool: Clear vTxHashes when mapTx is cleared (laanwj)
fe56d9b994 Merge bitcoin/bitcoin#24698: test: -peerblockfilters without -blockfilterindex raises an error (MarcoFalke)
3cabce645e Merge bitcoin/bitcoin#24472: fuzz: execute each file in dir without fuzz engine (MarcoFalke)
f5116a7d31 Merge bitcoin-core/gui#549: refactor: use std::chrono for formatDurationStr() helper (Hennadii Stepanov)
3fa8158510 Merge bitcoin/bitcoin#22317: doc: Highlight DNS requests part in tor.md (Andrew Chow)
72b62edd5a Merge bitcoin/bitcoin#23834: wallettool: Check that the dumpfile checksum is the correct size (laanwj)
ee9b3cdb0e Merge bitcoin/bitcoin#23979: test: wait for rather than assert presence of file in startupnotify test (MarcoFalke)
2ec5940399 Merge bitcoin/bitcoin#23532: test: add functional test for -startupnotify (MarcoFalke)
5a31be9608 Merge bitcoin/bitcoin#23812: test: fix intermittent failures in p2p_timeouts.py (MarcoFalke)
10828f5b3d Merge bitcoin/bitcoin#23733: fuzz: Move ISO8601 to one place (MarcoFalke)
7f39b5af41 Merge bitcoin/bitcoin#23635: test: Bump shellcheck version to 0.8.0 (fanquake)
Pull request description:
## Issue being fixed or feature implemented
Trivial batch of backports
## What was done?
trivial backports
## How Has This Been Tested?
Unit tests ran; waiting on CI
## Breaking Changes
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
Top commit has no ACKs.
Tree-SHA512: a3f97003e6441468951b827b2c3ea607740e5b9d36b96c2f93e45e7fb4088ecf4d0a2b7038de050ca0e7d61379c364969f4a8caff98ec1cc69016f4114e64c6a
f619f8ff80 merge bitcoin#20234: don't bind on 0.0.0.0 if binds are restricted to Tor (Kittywhiskers Van Gogh)
1698336fc8 merge bitcoin#22974: Improve performance of Good (Kittywhiskers Van Gogh)
29f4482e01 merge bitcoin#22911: Minor cleanups to asmap (Kittywhiskers Van Gogh)
5706edad5f merge bitcoin#22915: Remove confusing CAddrDB (Kittywhiskers Van Gogh)
3f69606827 merge bitcoin#22848: Expose BanMapToJson / BanMapFromJson (Kittywhiskers Van Gogh)
9065eed969 merge bitcoin#22791: Fix asmap/addrman initialization order bug (Kittywhiskers Van Gogh)
99b7812271 merge bitcoin#22849: Remove unused SERIALIZE_METHODS for CBanEntry (Kittywhiskers Van Gogh)
a30379c557 merge bitcoin#22740: Move serialization code to cpp (Kittywhiskers Van Gogh)
d4e79aa377 merge bitcoin#22697: Remove CAddrMan::Clear() function (Kittywhiskers Van Gogh)
77d8f6c918 merge bitcoin#22725: Move addrman ser/deser tests to addrman_tests.cpp (Kittywhiskers Van Gogh)
4ba3f49afc merge bitcoin#22627: De-duplicate Add() function (Kittywhiskers Van Gogh)
49af8185e0 merge bitcoin#20233: Make consistency checks a runtime option (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependency for https://github.com/dashpay/dash/pull/6043
* [bitcoin#22915](https://github.com/bitcoin/bitcoin/pull/22915) is backported before [bitcoin#21850](https://github.com/bitcoin/bitcoin/pull/21850), which is the reason {`Dump`, `Read`}`PeerAddresses` takes in an `const ArgsManager&` but doesn't do anything with it.
* This will need to be accounted for when backporting [bitcoin#21850](https://github.com/bitcoin/bitcoin/pull/21850)
## Breaking Changes
None expected. No changes to serialization format.
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [x] I have performed a self-review of my own code
- [x] I have commented my code, particularly in hard-to-understand areas **(note: N/A)**
- [x] I have added or updated relevant unit/integration/functional/e2e tests
- [x] I have made corresponding changes to the documentation **(note: N/A)**
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
PastaPastaPasta:
utACK f619f8ff80
Tree-SHA512: 19599f0322f049b123790a45530ecb2ec74b1463b47e0102921b489bad1d5cb671a94900b3b35dbd7c1d80bc0680f1ed01e6fac675ac0be81621878388be0bcd
88044a14d9b2c6c70a3330ee1545a9eb39d14d89 Guard `#include <config/bitcoin-config.h>` (Hennadii Stepanov)
Pull request description:
A fix for builds when the `HAVE_CONFIG_H` macro is not defined.
ACKs for top commit:
Empact:
Code Review ACK 88044a14d9
Tree-SHA512: f2bf1693c7671d7113dccaf66ae34a84719d86cb3271fa18b36611deab93a48d787b3ccfbd735d3b763017d709971cb1151d8d7f30390720009e6e2a6275b5b0
4637bbe448ae7370528f40092ce230c32602a6d6 rpc: Explain active and internal in listdescriptors (Andrew Chow)
Pull request description:
The current help text for active and internal in listdescriptors is not particularly helpful. They require the reader to already know what those terms mean. This help text is updated to actually explain the definitions of those words in context of a descriptor wallet.
ACKs for top commit:
S3RK:
ACK 4637bbe448ae7370528f40092ce230c32602a6d6
jarolrod:
ACK 4637bbe448ae7370528f40092ce230c32602a6d6
w0xlt:
ACK 4637bbe448
Tree-SHA512: 0af2c04f3b9920799cf616ad618bde9248eb9f74cc28f443b5b0f6646deba76e9b1415aca0865ad3bcc24aa6af0e9d07ad7b7cd80f0fe80838cf847f1b944426
172c2333f03aecb4c347c791537e13c296adbde2 Porting lint-assertions.sh to lint-assertions.py (hiago)
Pull request description:
This PR is converting `test/lint/lint-assertions.sh` to `test/lint/lint-assertions.py`. It's an item of #24783.
ACKs for top commit:
laanwj:
Tested ACK 172c2333f03aecb4c347c791537e13c296adbde2
Tree-SHA512: 94d5b03acfeaf2303fad95d489d6c3aa7bd655889ddaa807cc97e0613b8eb8f5ef094feee2a98d974606890deb554e76490a5c523d64eb5bc55afa6a43221aae
3ae7791bcaa88f5c68592673b8926ee807242ce7 refactor: use Span in random.* (pasta)
Pull request description:
~This PR does two things~
1. use a Span<unsigned char> for GetRandBytes and GetStrongRandBytes
~2. make GetRand a template for which any integral type can be used, where the default behavior is to return a random integral up to the max of the integral unless a max is provided.
This simplifies a lot of code from `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()`~
MarcoFalke this was inspired by your comment here: https://github.com/bitcoin/bitcoin/pull/24185#issuecomment-1025514263 about using Span, so hopefully I'll be able to get this PR done and merged 😂
~Also, if requested I could revert the `GetRand(std::numeric_limits<uint64_t>::max()` -> `GetRand<uint64_t>()` related changes if it ends up causing too many conflicts~
ACKs for top commit:
laanwj:
Thank you! Code review re-ACK 3ae7791bcaa88f5c68592673b8926ee807242ce7
Tree-SHA512: 12375a83b68b288916ba0de81cfcab4aac14389a66a36811ae850427435eb67dd55e47df9ac3ec47db4e214f4330139e548bec815fff8a3f571484ea558dca79
0cea7b10f1180e9993c14473e1a3b6525ef6ba01 print `(none)` if no warnings in -getinfo (/dev/fd0)
Pull request description:
Adds `(none)` in warnings when no warnings returned by -getinfo
Reviewers can test this by making the following change in `/src/warnings.cpp`:
```diff
bilingual_str GetWarnings(bool verbose)
{
bilingual_str warnings_concise;
std::vector<bilingual_str> warnings_verbose;
LOCK(g_warnings_mutex);
// Pre-release build warning
if (!CLIENT_VERSION_IS_RELEASE) {
- warnings_concise = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications");;
+ warnings_concise = _("");;
```
Before this pull request:
```
$ bitcoin-cli -getinfo
Chain: regtest
Blocks: 0
Headers: 0
Verification progress: 100.0000%
Difficulty: 4.656542373906925e-10
Network: in 0, out 0, total 0
Version: 239900
Time offset (s): 0
Proxies: n/a
Min tx relay fee rate (BTC/kvB): 0.00001000
Warnings:
```
After this pull request:
```diff
$ bitcoin-cli -getinfo
Chain: regtest
Blocks: 0
Headers: 0
Verification progress: 100.0000%
Difficulty: 4.656542373906925e-10
Network: in 0, out 0, total 0
Version: 239900
Time offset (s): 0
Proxies: n/a
Min tx relay fee rate (BTC/kvB): 0.00001000
Warnings: (none)
```
ACKs for top commit:
jonatack:
ACK 0cea7b10f1180e9993c14473e1a3b6525ef6ba01
laanwj:
Tested ACK 0cea7b10f1180e9993c14473e1a3b6525ef6ba01
Tree-SHA512: a12499d11ff84bc954db354f968eb1f5ee4999d8b80581fe0bdf604732b2e2f608cb5c35c4ca8cb5a430f3991954a6207f0758302618662e6b9505044cf2dc95
9d65ad365c539557e969a35d22723d92e0b422fe Clear vTxHashes when mapTx is cleared (Peter Bushnell)
Pull request description:
vTxHashes is a vector of all entries in mapTx, if you clear one you should clear the other, lest someone try to use the txiter in vTxHashes which would result in a segfault.
ACKs for top commit:
laanwj:
Code review ACK 9d65ad365c539557e969a35d22723d92e0b422fe
Tree-SHA512: 6832755e43ab7f528b46817aeadcb6ffdc965b97f59ab96bb053dedbb7e68155ba3db52286355dca33b509237f80eda249760b26db493762bc50d8e2cef16d8f
d6bc2322ed2e0674e027d39825fdadbb0db2c24a test: -peerblockfilters without -blockfilterindex raises an error (brunoerg)
Pull request description:
This PR adds test coverage for the following init error:
2a3e8fb359/src/init.cpp (L850)
Setting -peerblockfilters without -blockfilterindex should raise an error when initializing.
ACKs for top commit:
ccdle12:
Tested ACK d6bc2322ed
Tree-SHA512: e740c2ccde6bb1bb8381bb676a6d01bd5746cf9ce0c8dadd62067a6b9b380027bfe8b8cdeae9846a0ab18385f3dc5dff607fe5274cb55107d47470db00015fb2
f59bee3fb242c9e02781a35272cf9644f37e7fc1 fuzz: execute each file in dir without fuzz engine (Anthony Towns)
Pull request description:
Phony fuzzing (phuzzing)! Run the fuzz testing code against known inputs to detect errors. Advantage is you can easily test using the existing qa-assets datasets without having to compile with fuzzing enabled; disadvantage is that it doesn't do any actual fuzzing.
Example usage:
```
$ for a in ${QA_ASSETS}/fuzz_seed_corpus/*; do echo ${a##*/}; done | xargs -P8 -I {} /bin/sh -c "FUZZ={} test/fuzz/fuzz ${QA_ASSETS}/fuzz_seed_corpus/{}"
No fuzzer for address_deserialize.
No fuzzer for addrdb.
No fuzzer for banentry_deserialize.
addition_overflow: succeeded against 848 files in 0s.
asmap: succeeded against 981 files in 0s.
checkqueue: succeeded against 211 files in 0s.
...
```
(`-P8` says run 8 of the tasks in parallel)
If there are failures, the first one will be reported and the program will abort with output like:
```
fuzz: test/fuzz/versionbits.cpp:336: void (anonymous namespace)::versionbits_fuzz_target(FuzzBufferType): Assertion `exp_state != ThresholdState::FAILED' failed.
Error processing seed "corpus/versionbits/35345ae8e722234095810b1117a29b63af7621af"
```
Rebase of #22763, which was a rebase of #21496, but also reports the name of the fuzzer and the time taken.
Fixes#21461
Top commit has no ACKs.
Tree-SHA512: d8d046d4a309652eb13de42116276bf992480bc887ad3535a8ff18b354cb24826bc562b06af63802ec945c637f046563b6a5601d6321b46a5543127daafea09b
6f2593dc23565abaa3d176595cba6e07883f512e gui, refactor: use std::chrono for formatDurationStr() helper (Jon Atack)
Pull request description:
Updates `formatDurationStr()` to use the `chrono` standard lib. No change in behavior.
ACKs for top commit:
RandyMcMillan:
tACK 6f2593dc23565abaa3d176595cba6e07883f512e
shaavan:
ACK 6f2593dc23565abaa3d176595cba6e07883f512e
w0xlt:
tACK 6f2593d on Ubuntu 21.10 Qt 5.15.2
promag:
Code review ACK 6f2593dc23565abaa3d176595cba6e07883f512e.
Tree-SHA512: 61e9afdb1db779150df338e6af08727c34f69639add465c2f7003ff775d97dce3e78e78d325bc6dea5bc13f0fce9ef1c3506d13f1661a5e083e52bba8a32ba44
86a4a15bdcc96eb565ab80166642d71d542061a9 Highlight DNS request part (Prayank)
Pull request description:
_What?_
Highlight DNS requests part in Proxy section
_Why?_
1. DNS requests are very important while considering privacy
2. Lot of users might skip reading it because of the way it is mixed with everything else in the doc right now
3. I have seen lot of users ignoring DNS requests or unaware of such things while using privacy tools
_How?_
Initially I had tried keeping these lines separate from code block but [Jonatack didn't agree with the changes](https://github.com/bitcoin/bitcoin/pull/21157#discussion_r618177116). Harding suggested using [bold/italic in `<pre></pre>`](https://github.com/bitcoin/bitcoin/pull/21157#discussion_r644364143). I have used the suggestions from previous PR and added `---`
This is a part of alternative described in https://github.com/bitcoin/bitcoin/pull/22316
ACKs for top commit:
jonatack:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
Rspigler:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
achow101:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
RiccardoMasutti:
ACK 86a4a15
lsilva01:
ACK 86a4a15bdc
kristapsk:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
theStack:
ACK 86a4a15bdcc96eb565ab80166642d71d542061a9
Tree-SHA512: a4fe0e8c08df330e5ca78ce19ce74be7034c653f4374469d928908847a6debf385283e3a6da66de600566c7bab6290ccd35df26864aef94cbb3f294123391437
ac617cc141fe05bea0dc5e8f9df3da43c0945842 wallettool: Check that the dumpfile checksum is the correct size (Andrew Chow)
Pull request description:
After parsing the checksum, make sure that it is the size that we expect it to be.
This issue was reported by Pedro Baptista.
ACKs for top commit:
laanwj:
Code review ACK ac617cc141fe05bea0dc5e8f9df3da43c0945842
Tree-SHA512: 8135b3fb1f4f6b6c91cfbac7d1d3421f1f6c664a742c92940f68eae857f92ce49d042cc3aa5c2df6ef182825271483d65efc7543ec7a8ff047fd7c08666c8899
96eb0093d07c58ad3b02c49a5a4385da37a8e337 test: wait rather than assert presence of file in startupnotify test (fanquake)
Pull request description:
Should fix#23967.
ACKs for top commit:
brunoerg:
crACK 96eb0093d07c58ad3b02c49a5a4385da37a8e337
kristapsk:
utACK 96eb0093d07c58ad3b02c49a5a4385da37a8e337
Tree-SHA512: 9107970e45c027cfc6c6cbfcfd5a7d9f5956259bbbb11f5b180c3947126e42e62c0f8ffd69cf7b39b51c9c5b4fedbb753839d59aebe876be68c1484bb6065819
126853214a490ee840e83ca17c717c40cfbe6837 test: add functional test for -startupnotify (Bruno Garcia)
Pull request description:
This PR adds a functional test for -startupnotify. It basically starts the node passing a command on -startupnotify to create a file on tmp and then, we check if the file has been successfully created.
ACKs for top commit:
theStack:
Tested ACK 126853214a490ee840e83ca17c717c40cfbe6837
kristapsk:
re-ACK 126853214a490ee840e83ca17c717c40cfbe6837
Tree-SHA512: 5bf3e46124ee5c9d609c9993e6465d5a71a8d2275dcf07c8ce0549f013f7f8863d483b46b7164152f566468a689371ccb87f01cf118c3c9cac5b2be673b61a5c
0a1b6fa5a18f3efb2ac3e28a23a4fd5e1cf9eaf0 test: fix intermittent timeouts in p2p_timeouts.py (Martin Zumsande)
Pull request description:
Fixes #23800 by making sure that all peers are connected (i.e. `m_connected` is set) before the mocktime is bumped.
We can't wait for verack here, but we can wait for a debug log entry ("Added connection peer=2") instead.
In the failed CI runs (e.g. https://cirrus-ci.com/task/5600553806856192?logs=ci#L7469) different peers were added at different mocktimes.
ACKs for top commit:
naumenkogs:
ACK 0a1b6fa5a18f3efb2ac3e28a23a4fd5e1cf9eaf0
theStack:
Concept and approach ACK 0a1b6fa5a18f3efb2ac3e28a23a4fd5e1cf9eaf0
Tree-SHA512: 1a3c8a9a79339d4adc6ecb1731eb0d0eadb2e5024ad3c6779b4696691f85d6c3304ef8689746d0332150a4cf04489ca4b2ff3eeb0bb76feec28c1e4bb9dbca19
fa72dd314fe857d827d9b311bdf0453e9463746f fuzz: Move ISO8601 to one place (MarcoFalke)
Pull request description:
Seems confusing to split this to two places.
Also fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=42178
ACKs for top commit:
fanquake:
ACK fa72dd314fe857d827d9b311bdf0453e9463746f
Tree-SHA512: 637b0671078848ea417fdf66b92715602040fad34d4ca5f7b843a519a1cfeebe5d992a79a399deba39926905125681d66ab0dc05f66f79a26f3bf555e12fb0ba
5202bd1dc0942953c415ef39c14ffd48cbaab6b9 test: Bump shellcheck version to 0.8.0 (Hennadii Stepanov)
Pull request description:
Among [added](https://github.com/koalaman/shellcheck/blob/master/CHANGELOG.md#v080---2021-11-06) rules, SC2295 could be [useful](https://github.com/bitcoin/bitcoin/pull/23506#issuecomment-982201468) for us.
ACKs for top commit:
dongcarl:
Code Review ACK 5202bd1dc0942953c415ef39c14ffd48cbaab6b9
fanquake:
ACK 5202bd1dc0942953c415ef39c14ffd48cbaab6b9 - would have rather this just been a part of #23506 to avoid another PR and pointless rebasing.
Tree-SHA512: fd7ff801c71af03c5a5b2823b7daba25a430b3ead5e5e50a3663961ee2223e55d322aec91d79999814cd35bd7ed6e9415a0b797718ceb8c0b1dbdbb40c336b82
a62e17027f Merge bitcoin/bitcoin#23458: ci: Do not print `git log` for empty COMMIT_RANGE (MarcoFalke)
eec305b4b3 Merge bitcoin/bitcoin#22904: sync, log: inline lock contention logging macro to fix duration, improve BCLog::LogMsg() (merge-script)
54e6449d09 Merge bitcoin-core/gui#250: scripted-diff: Drop redundant QString calls (MarcoFalke)
d2032dd03f Merge #20845: net: Log to net debug in MaybeDiscourageAndDisconnect except for noban and manual peers (MarcoFalke)
a2f3ba0054 Merge bitcoin-core/gui#202: peers-tab: bug fix right panel toggle (MarcoFalke)
a5e7b029f2 Merge #21124: test: remove unnecessary assignment in bdb (MarcoFalke)
53f9d2ce2c Merge bitcoin-core/gui#161: Add PeerTableModel::StatsRole to prevent data layer violation (Jonas Schnelli)
Pull request description:
## Issue being fixed or feature implemented
Batch of trivial backports
## What was done?
Trivial backports
## How Has This Been Tested?
Built; haven't ran tests
## Breaking Changes
Didn't see any
## Checklist:
_Go over all the following points, and put an `x` in all the boxes that apply._
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK a62e17027f
Tree-SHA512: d14fb153e256ec7e2c5562e6c9361e0a890376092cbe78b59e88a1462bf2c32fc169f9bd0e00493ac68cdf47870c07023f52e3be7da807e2886b33dba7a8816a
095f07744cf500adc1f1587eb5b7a61df6e6b05f ci: Do not print `git log` for empty COMMIT_RANGE (Hennadii Stepanov)
Pull request description:
On master (77a2f5d30c5ecb764b8a7c098492e1f5cdec90f0) a CI lint task [log](https://api.cirrus-ci.com/v1/task/4817858858319872/logs/lint.log) exceeds 20K lines.
This PR fixes this issue.
ACKs for top commit:
MarcoFalke:
cr ACK 095f07744cf500adc1f1587eb5b7a61df6e6b05f
Tree-SHA512: 89180018aeccf1599cdf218924cbab12dcbae0f6674bb90e13b64e342cdd908a880b885039c23f0d1d03493e55a94fe04abf39481616ae6550c6a759f5ca9a35
f530202353a4f8bb444966559aa15681ab3cebc6 Make unexpected time type in BCLog::LogMsg() a compile-time error (Martin Ankerl)
bddae7e7ff7bb5931ed807acaef7336f2ee98476 Add util/types.h with ALWAYS_FALSE template (MarcoFalke)
498b323425d960274c40472a6a847afc1982201d log, timer: improve BCLog::LogMsg() (Jon Atack)
8d2f847ed913f15677ae978a412015ac844ffceb sync: inline lock contention logging macro to fix time duration (Jon Atack)
Pull request description:
Follow-up to #22736.
The first commit addresses the issue identified and reported by Martin Ankerl in https://github.com/bitcoin/bitcoin/pull/22736#discussion_r703019629 to fix the lock contention duration reporting.
The next three commits make improvements to the timer code in `BCLog::LogMsg()` and add `util/types.h` with an `ALWAYS_FALSE` template, that springboard from https://github.com/bitcoin/bitcoin/pull/22736#discussion_r702747920 by Marco Falke.
ACKs for top commit:
martinus:
re-ACK f530202353a4f8bb444966559aa15681ab3cebc6. I ran a fully synced node for about a day. My node was mostly idle though so not much was going on. I [wrote a little script](https://github.com/martinus/bitcoin-stuff/blob/main/scripts/parse-debuglog-contention-single.rb) to parse the `debug.log` and summarize the output to see if anything interesting was going on, here is the result:
theStack:
ACK f530202353a4f8bb444966559aa15681ab3cebc6
Tree-SHA512: 37d093eac5590e1b5846ab5994d0950d71e131177d1afe4a5f7fcd614270f977e0ea117e7af788e9a74ddcccab35b42ec8fa4db3a3378940d4988df7d21cdaaa
def1e64bb4fc902bfc19d6487b33cd940f0e0f34 scripted-diff: Drop redundant QString calls (Hennadii Stepanov)
Pull request description:
The return type of `QObject::tr` function _is_ `QString` 🐅
ACKs for top commit:
jarolrod:
ACK def1e64bb4fc902bfc19d6487b33cd940f0e0f34, tested on macOS 10.14.6 Qt 5.15.2
Tree-SHA512: ef405c87a30d6965f6887511d8666b6da57d258ca07833a3fa2dc9fd147d0539d33c57f7551ee13c1dd8024d6057139595c6ce5d088dd6efd7aa13db2a3eebdb
fa55159b9ede4a915f8ef9e5b90e3e99eadedf91 net: Log to net debug in MaybeDiscourageAndDisconnect except for noban and manual peers (MarcoFalke)
Pull request description:
The goal is to avoid local peers (e.g. untrusted peers on the local network or inbound onion peers via a local onion proxy) filling the debug log (and thus the disk).
ACKs for top commit:
practicalswift:
ACK fa55159b9ede4a915f8ef9e5b90e3e99eadedf91
vasild:
ACK fa55159b9ede4a915f8ef9e5b90e3e99eadedf91
Tree-SHA512: de233bf57334580f9b91f369fafd131d71c5ae25db25b09cc8fa8cbf34c0648f083c52260a6a912238751467e3c3c5f5d2309c145710753058d44a0003f88f4f
8353e8cecc3e34a8699b53849df0adab622c1c14 peers-tab: bug fix right panel toggle (randymcmillan)
Pull request description:
Initial Presentation:
When node row selected - panel is presented:
When network disabled - right panel is hidden:
ACKs for top commit:
jarolrod:
ACK 8353e8cecc3e34a8699b53849df0adab622c1c14
jonatack:
ACK 8353e8cecc3e34a8699b53849df0adab622c1c14 tested rebased on current master. Behavior is initially a bit surprising but this would allow more columns to be added to the peers tab window. Verified that selecting more than one peer, clicking on a column header, or running `disconnectnode "" <currently-selected-peer-id>` in the console (or on the CLI with the `-server` startup option) returns the window to its full size. If this is merged, it might be nice to have an obvious way to close the details area like a clickable "close this" icon in the upper left corner of the area.
Talkless:
tACK 8353e8cecc3e34a8699b53849df0adab622c1c14, tested on Debian Sid. Made `bitcoind` connect to `bitcoin-qt` with the PR changes, and after I quit the `bitcoind` instance, right panel do disappear, compared to the previous commit where it didn't.
Tree-SHA512: 8fc156f40bdd61e3ba8db333c729a2a07fd5f0fd1eed56f2fd2aa5ae5864756f8ab6fad74ae2fb0552ee7518b6d489f5800709e6c80c6f31f61fd8ce21cece5f
c9095b738fd4257ae5bdbb2ae38d3e7f41f51b64 test: remove unnecessary assignment in bdb (Bruno Garcia)
Pull request description:
This PR removes the unnecessary assignment to page_info['entries'] on line 54 since there is another assignment for it in line 59.
I think a lint (#21096) would detect cases like this one.
ACKs for top commit:
achow101:
ACK c9095b738fd4257ae5bdbb2ae38d3e7f41f51b64
theStack:
Code Review ACK c9095b738fd4257ae5bdbb2ae38d3e7f41f51b64
Tree-SHA512: 23377077c015b04361fd416b41bf6806ad0bdd4d264be6760f0fd3bc88d694d2cd52cae250519925c5d3b3c70715772714c3863f8fa181a2eb4883204ccdbf9d
b3e9bcaac85a64a1b41d534b28c8cfb1f08e14e5 qt, refactor: Drop no longer used PeerTableModel::getNodeStats function (Hennadii Stepanov)
49c604077c572fcdea8739eb3383467dbbbc5f52 qt: Use PeerTableModel::StatsRole (Hennadii Stepanov)
35007edf9c0f592303f0cbda3ade776c87fd80b1 qt: Add PeerTableModel::StatsRole (Hennadii Stepanov)
Pull request description:
This PR allows to access to the `CNodeCombinedStats` instance directly from any view object.
The `PeerTableModel::getNodeStats` member function removed as a kind of layer violation.
No behavior changes.
Also other pulls (bugfixes) are based on this one: #18 and #164.
ACKs for top commit:
jonatack:
Tested re-ACK b3e9bcaac85a64a1b41d534b28c8cfb1f08e14e5 per `git range-diff ae8f797 4c05fe0 b3e9bca`
promag:
Code review ACK b3e9bcaac85a64a1b41d534b28c8cfb1f08e14e5.
jonasschnelli:
utACK b3e9bcaac85a64a1b41d534b28c8cfb1f08e14e5
Tree-SHA512: 6ba50d5dd2c0373655d491ce8b130c47d598da2db5ff4b00633f447404c7e70f8562ead53ddf166e851384d9632ff9146a770c99845c2cdd3ff7250677e4c130
