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
d7c35d0538 refactor: remove `llmq::CQuorumManager` global, move to `LLMQContext` (Kittywhiskers Van Gogh)
5b86df6ca3 refactor: reduce `llmq::CQuorumManager` globals use, use args (Kittywhiskers Van Gogh)
1efd219173 refactor: remove `llmq::CQuorumBlockProcessor` global, move to `LLMQContext` (Kittywhiskers Van Gogh)
1d3afe742b evo: use `gsl::not_null` in `CTxMemPool::ConnectManagers` (Kittywhiskers Van Gogh)
805537e695 evo: add `CMNHFManager::`(`Dis`)`connectManagers` to remove global use (Kittywhiskers Van Gogh)
cbb6828ec7 refactor: access `llmq::CQuorumManager` through arg in `MNHFTx`, functions (Kittywhiskers Van Gogh)
Pull request description:
## Breaking Changes
None expected.
## 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:
knst:
utACK d7c35d0538
knst:
re-utACK d7c35d0538
PastaPastaPasta:
utACK d7c35d0538
Tree-SHA512: 07e7494bc061f85259a3a6cfb13d052403419cdc94fc51904ac03468bbdbd1ed74429546309d2bfc6a31ba184b93dc32842e49d2960819d19313bcb7e403008e
c3f8acf96d Merge bitcoin/bitcoin#21985: net: Return IPv6 scope id in `CNetAddr::ToStringIP()` (W. J. van der Laan)
28daf0d1c4 Merge bitcoin/bitcoin#22496: addrman: Remove addrman hotfixes (fanquake)
Pull request description:
bitcoin backports
Top commit has no ACKs.
Tree-SHA512: e208aee446ce5d4b29b43b9f86236b6f8ac1805572edcdf6355246c186947a5e783ecf77869525249436aa7fa2e10ae00e6b13aba7f6f04c8543e8fd382b9c8d
6c280adcd865ae3da4df53d630c9bf737283a56f net: Return IPv6 scope id in `CNetAddr::ToStringIP()` (W. J. van der Laan)
Pull request description:
If a scope id is provided, return it back in the string representation. Also bring back the test (now in platform independent fashion). Closes#21982. Includes #21961 (apart from the MacOS remark).
ACKs for top commit:
practicalswift:
cr ACK 6c280adcd865ae3da4df53d630c9bf737283a56f
Tree-SHA512: 77792c35679b6c3545fd3a8d3d74c4f515ac2ee9f02d983251aeaaac715d55c122bbb0141abbeac272011f15520b439bd2db4ec8541a58df9b366921d212ca5f
65332b1178c75e1f83415bad24918996a1524866 [addrman] Remove RemoveInvalid() (John Newbery)
Pull request description:
PRs #22179 and #22112 (EDIT: later reverted in #22497) added hotfix code to addrman to remove invalid addresses and mutate the ports of I2P entries after entering into addrman. Those hotfixes included at least two addrman data corruption bugs:
- #22467 (Assertion `nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size()' failed)
- #22470 (Changing I2P ports in addrman may wronly skip some entries from "new" buckets)
Hotfixing addrman is inherently dangerous. There are many members that have implicit assumptions on each others' state, and mutating those directly can lead to violating addrman's internal invariants.
Instead of trying to hotfix addrman, just don't insert any invalid addresses. For now, those are addresses which fail `CNetAddr::IsValid()`.
ACKs for top commit:
sipa:
utACK 65332b1178c75e1f83415bad24918996a1524866. I tried to reason through scenarios that could introduce inconsistencies with this code, but can't find any.
fanquake:
ACK 65332b1178c75e1f83415bad24918996a1524866 - Skipping the addition of invalid addresses (this code was initially added for Tor addrs) rather than adding all the invalids then removing them all when finishing unserializing seems like an improvement. Especially if it can be achieved with less code.
Tree-SHA512: 023113764cb475572f15da7bf9824b62b79e10a7e359af2eee59017df354348d2aeed88de0fd4ad7a9f89a0dad10827f99d70af6f1cb20abb0eca2714689c8d7
238978e25b fix: adjust `signrawtransactionwithkey` help text (UdjinM6)
Pull request description:
## Issue being fixed or feature implemented
`amount` was introduced in https://github.com/dashpay/dash/pull/3101. Double checked the code and yes, we do pass it around (for compatibility reasons) but it doesn’t affect the sig right now, you can set it to 0 or just skip it completely so it should be `optional`, not `required`. We even have a test that uses `signrawtransactionwithkey ` and ignores `amount` https://github.com/dashpay/dash/blob/master/test/functional/rpc_signrawtransaction.py#L19-L46.
NOTE: It might become required for `sighashtype` with `SIGHASH_DIP0143` flag after https://github.com/dashpay/dash/pull/5860 activation.
kudos to @pshenmic for noticing
## What was done?
Adjust help text
## How Has This Been Tested?
Run tests
## Breaking Changes
n/a
## Checklist:
- [x] 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)_
Top commit has no ACKs.
Tree-SHA512: e028c46b8b9c1663d9940642b08d56444ab4e5ab33015af1cb99265338b75f9e1c156cbbdd8e00f313bce87117019c769241cc4d83ccd6693ec0ffbaa8940e89
34c80473a8 Merge #19877: [test] clarify rpc_net & p2p_disconnect_ban functional tests (Wladimir J. van der Laan)
e42412924f Merge #19770: RPC: getpeerinfo: Deprecate "whitelisted" field (replaced by "permissions") (MarcoFalke)
f96966b7ea Merge #20043: doc: Add 19501 release notes (fanquake)
6a164eaea9 Merge #19501: send* RPCs in the wallet returns the "fee reason" (MarcoFalke)
b6c8d852e3 Merge #19725: [RPC] Add connection type to getpeerinfo, improve logs (MarcoFalke)
f86263b180 Merge #18202: refactor: consolidate sendmany and sendtoaddress code (Samuel Dobson)
fab41fd3c5 partial Merge #18878: test: Add test for conflicted wallet tx notifications (Wladimir J. van der Laan)
db5bd34ee8 Merge #19202: log: remove deprecated `db` log category (MarcoFalke)
Pull request description:
## Issue being fixed or feature implemented
Regular backports from bitcoin v21
## What was done?
- bitcoin/bitcoin#19202
- partial bitcoin/bitcoin#18878
- bitcoin/bitcoin#18202
- bitcoin/bitcoin#19725
- bitcoin/bitcoin#19501
- bitcoin/bitcoin#20043
- bitcoin/bitcoin#19770
- bitcoin/bitcoin#19877
## How Has This Been Tested?
Run unit/functional tests
## Breaking Changes
- (RPC) The `getpeerinfo` RPC no longer returns the `addnode` field by default. This
field will be fully removed in the next major release. It can be accessed
with the configuration option `-deprecatedrpc=getpeerinfo_addnode`. However,
it is recommended to instead use the `connection_type` field (it will return
`manual` when addnode is true)
- (Settings) The `sendtoaddress` and `sendmany` RPCs accept an optional `verbose=True`
argument to also return the fee reason about the sent tx.
- (Settings) The `-debug=db` logging category, which was deprecated in v0.18 and replaced by
`-debug=walletdb` to distinguish it from `coindb`, has been removed.
- (RPC) To make RPC `sendtoaddress` more consistent with `sendmany` the following error
`sendtoaddress` codes were changed from `-4` to `-6`:
- Insufficient funds
- Fee estimation failed
- Transaction has too long of a mempool chain
## Checklist:
- [x] 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
ACKs for top commit:
PastaPastaPasta:
utACK 34c80473a8
Tree-SHA512: 725a103e04c9c7d44a79da6f3f54e7745c7fb98ec906e7228ae16f7662d568e48c015c855902ff8485f2908f0f71815e769ca394cf6c3ca2e5fd920dd39cca74
0a23820923 merge bitcoin#27937: i2p documentation updates (Kittywhiskers Van Gogh)
1dc50d05cb merge bitcoin#26838: I2P documentation updates (Kittywhiskers Van Gogh)
5058519bbb merge bitcoin#25993: Add I2P guidance related to bandwidth and i2pd software version (Kittywhiskers Van Gogh)
859f59b423 merge bitcoin#26837: I2P network optimizations (Kittywhiskers Van Gogh)
9f8073419d merge bitcoin#26065: use the same destination type for transient and persistent addresses (Kittywhiskers Van Gogh)
9bf3829558 merge bitcoin#25355: add support for transient addresses for outbound connections (Kittywhiskers Van Gogh)
4977073b32 merge bitcoin#22497: remove ResetI2PPorts() (Kittywhiskers Van Gogh)
63d58534b6 merge bitcoin#22648: improve i2p/tor docs and i2p reachable unit tests (Kittywhiskers Van Gogh)
f04ce8bcda partial bitcoin#22229: consolidate to f-strings (part 1) (Kittywhiskers Van Gogh)
68ea6cc5c5 merge bitcoin#22589: update I2P hardcoded seeds and docs for 22.0 (Kittywhiskers Van Gogh)
Pull request description:
## Additional Information
* Dependent on https://github.com/dashpay/dash/pull/6034
## Breaking Changes
* With I2P connections, a new, transient address is used for each outbound connection if `-i2pacceptincoming=0`.
## 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:
re-utACK [0a23820](0a23820923); only changed commit title / text
Tree-SHA512: ef426263d7e81a5eec8555db21dcd24f5dbb65f5faa7013c7e0601a4c60ae675813aadf68ebc19936aa15bdbd8f6db88b4bd55dfaf4ac412abbda86fb1ac9c83
