685a44c60122a8e9bf69076bdab117dd55469373 fuzz: set fSuccessfullyConnected in connman harness (frankomosh)
Pull request description:
The connman fuzz harness never sets `fSuccessfullyConnected=true` on nodes added through `AddTestNode()`. `NodeFullyConnected()` gates `ForEachNode()` on that flag, making its callback unreachable in the current harness.
Set `fSuccessfullyConnected=true` before `AddTestNode()` to simulate a node that has completed the version handshake.
ACKs for top commit:
maflcko:
lgtm ACK 685a44c60122a8e9bf69076bdab117dd55469373
sedited:
ACK 685a44c60122a8e9bf69076bdab117dd55469373
marcofleon:
ACK 685a44c60122a8e9bf69076bdab117dd55469373
Tree-SHA512: 2c696b8674cb465f468642b5fd37a467bc34dcbf61dc901d784fd2fe0dd13ced5cd6bd9873bcce0f8e60e25d450052e9a562ece08abeb2ab6472e584dba65c40
551875360cdb3b9997dded22b929039eac635fd0 ci: Use arch-appropriate binaries in lint install (will)
Pull request description:
In testing #34547 it has been observed that the lint container does not run on aarch64-linux without `qemu binfmt` (or similar).
This is because some tools are hardcoded to download x64 linux binaries. This has meant the linter works fine on:
- x64 linux
- aarch64 MacOS (via Rosetta)
- platforms using qemu
But does not work on e.g. aarch64-linux _without qemu_.
`shellcheck`` offer many platforms: https://github.com/koalaman/shellcheck/releases/tag/v0.11.0 and `mlc` offers are least x64 and aarch64 linux https://github.com/becheran/mlc/releases/tag/v1.2.0.
Try to download the correct binary for the platform using `uname` detection. This should see the linter work on native aarch64 + amd64, whilst maintaining current (emulated) compatibility.
ACKs for top commit:
maflcko:
lgtm ACK 551875360cdb3b9997dded22b929039eac635fd0
Tree-SHA512: 636cccbed3ffff995549c666b0cad1aa9790291a73a0f2212f0374c8878bd916c04e4ecb17fac1611fc2d72d363cececeeaa997af918ad4225355231376ff7b0
Replace the hardcoded x86_64 binary name with $(uname --machine) so the
correct binary is downloaded when building the lint container, where at
all possible.
bde35d61f930a7cd2011aeb9f5443f3188484e80 depends: capnp 1.4.0 (fanquake)
Pull request description:
Update capnp in depends to [`1.4.0`](https://github.com/capnproto/capnproto/releases/tag/v1.4.0).
It contains a number of bugfixes, and fixes for 2 CVEs, of which I think only `Fix benign(?) buffer overrun in async readMessage()` is relevant to us, and it seems to be considered benign:
> This is technically undefined behavior (a buffer overrun), but we suspect that it is benign with all known memory allocators. In C++, a zero-sized allocation (made with `operator new(0)`, as is the case here) is required to return a unique pointer, different from any other such allocation. Because of this, all common memory allocators round up a zero-byte allocation to a word-sized allocation (32-bit or 64-bit, depending on the architecture). The overrun written to this allocation was exactly one pointer in size, so always fits into the actual allocation space.
> Nevertheless, the code is in fact relying on undefined behavior, and it is theoretically possible that some memory allocator implements zero-sized allocations in a way that would make this overrun dangerous.
See https://github.com/capnproto/capnproto/compare/release-1.3.0...release-1.4.0 for all changes since 1.3.0.
ACKs for top commit:
sedited:
ACK bde35d61f930a7cd2011aeb9f5443f3188484e80
janb84:
ACK bde35d61f930a7cd2011aeb9f5443f3188484e80
hebasto:
ACK bde35d61f930a7cd2011aeb9f5443f3188484e80.
Tree-SHA512: 33a6c12684b9a6046a38c3b9dd1a5730db352eae07b5dbfe7244228fde3d1627d039c0e0ba7d35fe0968f91a0f476c239fa8f2e356a37b8ac975ac268d271bc2
6202acd284bc0284fc9b144fdc39774f112fcdf2 test: addrman: successive failures in the last week for IsTerrible (brunoerg)
f611d3bdaf035280f44d5183a8322c0a0c6e3d27 refactor: addrman: move consts to .h (brunoerg)
Pull request description:
This PR adds test coverage for the case that an address is considered terrible if we had N successive failures in the last week.
It kills the following mutant (https://corecheck.dev/mutation/src/addrman.cpp#L88):
```diff
diff --git a/src/addrman.cpp b/src/addrman.cpp
index e3981e6a40..f8045491c1 100644
--- a/src/addrman.cpp
+++ b/src/addrman.cpp
@@ -65,7 +65,7 @@ bool AddrInfo::IsTerrible(NodeSeconds now) const
}
if (now - m_last_success > ADDRMAN_MIN_FAIL && nAttempts >= ADDRMAN_MAX_FAILURES) { // N successive failures in the last week
- return true;
+ return false;
}
return false;
```
ACKs for top commit:
frankomosh:
re-ACK 6202acd284bc0284fc9b144fdc39774f112fcdf2
naiyoma:
tACK 6202acd284bc0284fc9b144fdc39774f112fcdf2
danielabrozzoni:
tACK 6202acd284bc0284fc9b144fdc39774f112fcdf2
sedited:
ACK 6202acd284bc0284fc9b144fdc39774f112fcdf2
Tree-SHA512: b4736ef91b75ba4c060dc18a2b796aee94d0d8be5ca58b9b873156248cd0dc6910595595e0f56d75bffff4f94c035319ac8428f7d79f07fe685bdba27a188829
Without this, NodeFullyConnected() filters out every fuzz-constructed node, making ForEachNode's callback unreachable (0/1.13M branch hits from my end).
fa90b21430b4f3bae6f4092a27e611bb06f2777f test: Remove unused feature_segwit.py functions (MarcoFalke)
fa6b05c96ffb6cef42bf8907759d0d99f97a2e22 test: Remove unused CUSTOM_._COUNT (MarcoFalke)
fa7bac94d87a1b25f8b33ebd29ce47bf442f137e test: Remove unused wait_for_addr, firstAddrnServices, on_addr (MarcoFalke)
fa388a35855cc53d1a48d464c1bdcf793a0e2968 test: Remove unused self.p2p_conn_index = 1 (MarcoFalke)
fa803710e2783c92e803d0c4e57ae32b34947738 test: Remove unused AddressType (MarcoFalke)
fab5072ce1396484b9caa7320da4dc3c98070adc ci: Remove vulture (MarcoFalke)
Pull request description:
Currently, `vulture` is run with `--min-confidence=100`, which reduces its checks to dead code after control statements, which is nice, but not really a common nor severe issue. See the discussion in https://github.com/bitcoin/bitcoin/issues/34810#issuecomment-4045927137 and commit 5c005363a880c136cc44ff2456a402e398fcbf44, which had to remove dead code manually.
Reducing the confidence has shown to be too brittle/tedious in the past, so remove the tool for now from CI.
Of course, removing this from CI does not prevent anyone from running it locally and removing dead code.
Fixes https://github.com/bitcoin/bitcoin/issues/34810
ACKs for top commit:
fanquake:
ACK fa90b21430b4f3bae6f4092a27e611bb06f2777f
willcl-ark:
ACK fa90b21430b4f3bae6f4092a27e611bb06f2777f
Tree-SHA512: 6a5998470dae3a17baec29b70b02333f4cd9b81bc4c6a05b56085ff1ba527ed7bdeccd17b09d9ad785ae03c97982ee1f3147e4df3bd537c66b02e9a44d0e5f15
111864ac30126dc64a9e21d4e1b5e3d9ef4e5358 qa: Avoid duplicating output in case the diff is the same (Hodlinator)
c2e28d455af8fbb8d6074dc26590e61b9764d761 ci: Enable `wallet_multiwallet.py` in "Windows, test cross-built" job (Hodlinator)
850a80c1999e671b6cce33d8545af06adf5f77f0 qa: Disable parts of the test when running under Windows or root (Hodlinator)
fb803e3c79e52305df74ae30e77fd36900a49c24 qa: Test scanning errors individually (Hodlinator)
ed43ce57cce53612f13ac7c6db59fa7ac60e31c4 qa: Check for platform-independent part of error message (Hodlinator)
64a098a9b6263dbdeea25f89f4c9fe3c53943dd1 refactor(qa): Break apart ginormous run_test() (Hodlinator)
bb1aff7ed7e4bd6618dfe75b5faa9956c3adead4 move-only(qa): Move wallet creation check down to others (Hodlinator)
d1a4ddb58ef676d4e7436cc3bcdf5fb3008b4b6f refactor(qa): Lift out functions to outer scopes (Hodlinator)
c811e47367d531b69c10e3fc976df764e79f13e2 scripted-diff: self.nodes[0] => node (Hodlinator)
73cf858911056717a4ebe97cd250f3a506136eff refactor(qa): Remove unused option (Hodlinator)
Pull request description:
Makes the functional test compatible with *Linux->Windows cross-built executables*.
Main parts:
* Commit "qa: Check for platform-independent part of error message" switches to match on platform-independent part of error message.
* Commit "qa: Test scanning errors individually" disentangles code causing the same error message substring, based on #31410.
* Commit "qa: Disable parts of the test when running under Windows or root" enables the test to be run on Windows, based in part on https://github.com/bitcoin/bitcoin/pull/31410#issuecomment-3554721014.
Also:
* Removes unused option in wallet_multiwallet.py.
* Breaks apart wallet_multiwallet.py's `run_test()` into smaller test functions.
* Improves `assert_equal()` output for dicts.
Fixes#31409.
ACKs for top commit:
achow101:
ACK 111864ac30126dc64a9e21d4e1b5e3d9ef4e5358
janb84:
re ACK 111864ac30126dc64a9e21d4e1b5e3d9ef4e5358
w0xlt:
reACK 111864ac30126dc64a9e21d4e1b5e3d9ef4e5358
Tree-SHA512: 4e3ff92588ac9f2611fc963ce62097b6c0dd4d4eb8da7952c72619c7b554ff3cae5163fe1886d4d9bbd7af1acca5b846411e7f5b46f9bddb08719b61108efbba
af0da2fce2146a005f54b900a1fd545a6278173a crypto: Use `secure_allocator` for `AES256CBC*::iv` (David Gumberg)
d53852be316dd2322a7e621a81ee33e1b234c530 crypto: Use `secure_allocator` for `AES256_ctx` (David Gumberg)
8c6fedaa81882be1e2db4a9a2291205d774fa4c6 build: `lockedpool.cpp` kernel -> crypto (David Gumberg)
51ac1abf6fdb6340110397a959681d52e29f73f7 bench: Add wallet encryption benchmark (David Gumberg)
9a158725161f726eb6747811e9e9335eef83f616 wallet: Make encryption derivation clock mockable (David Gumberg)
ae5485fa0d258f060a22c399a4e257eaf96e67b5 refactor: Generalize derivation target calculation (David Gumberg)
Pull request description:
Fixes#31744
Reuse `secure_allocator` for `AES256_ctx` in the aes 256 encrypters and decrypters and the `iv` of `AES256CBC` encrypters and decrypters. These classes are relevant to `CCrypter`, used for encrypting wallets, and my understanding is that if an attacker knows some or all of the contents of these data structures (`AES256_ctx` & `iv`) they might be able to decrypt a user's wallet.
Presently the `secure_allocator` tries to protect sensitive data with `mlock()` on POSIX systems and `VirtualLock()` on Windows to prevent memory being paged to disk, and by zero'ing out memory contents on deallocation with `memory_cleanse()` which is similar to `OPENSSL_cleanse()` by scaring compilers away from optimizing `memset` calls on non-Windows systems, and using `SecureZeroMemory()` on Windows.
ACKs for top commit:
achow101:
ACK af0da2fce2146a005f54b900a1fd545a6278173a
furszy:
utACK af0da2fce2146a005f54b900a1fd545a6278173a
theStack:
re-ACK af0da2fce2146a005f54b900a1fd545a6278173a
Tree-SHA512: 49067934fd2f2b285fc7b1a7c853fd2d4475431b3a811ae511f61074dc71a99a0826c3ab40ab4a5dfc84b2b9914a90c920d2484b38ac19502e3bd6170ad27622
d62f46eed4be8998f4aa75e20bd0dca2e453313f doc: Add `INSTALL.md` to Linux release tarballs (Hennadii Stepanov)
Pull request description:
Closes https://github.com/bitcoin/bitcoin/issues/32097:
> Better I think would be to add instructions for the most popular desktop distros.
ACKs for top commit:
willcl-ark:
utACK d62f46eed4be8998f4aa75e20bd0dca2e453313f
achow101:
ACK d62f46eed4be8998f4aa75e20bd0dca2e453313f
janb84:
re ACK d62f46eed4be8998f4aa75e20bd0dca2e453313f
w0xlt:
ACK d62f46eed4be8998f4aa75e20bd0dca2e453313f
sedited:
ACK d62f46eed4be8998f4aa75e20bd0dca2e453313f
Tree-SHA512: eab6904b6acc63eaf0e13112f52d856ee630ce4110fb368f4f5f170655def91706bb7c7f70c123186a51461ea3d48ef026a4e026d665675f1fd76f2d0bbca68c
d03e3be246f64065002268e74ee9a834089de37a ci: check macos bundle structure and codesigning (fanquake)
66d80d57b48982d8301e21321a9d82780586908d macdeploy: use plugins dir to find plugins (fanquake)
ab137cbfe2763f2306e52c7c5f1860d87defc636 macdeploy: subprocess out to zip rather than shutil.make_archive (fanquake)
Pull request description:
Fix bundle format.
Add a CI check that codesigning works.
Fixes#34744.
ACKs for top commit:
Sjors:
tACK d03e3be246f64065002268e74ee9a834089de37a
hebasto:
ACK d03e3be246f64065002268e74ee9a834089de37a, tested on macOS Tahoe 26.3.1.
sedited:
ACK d03e3be246f64065002268e74ee9a834089de37a
Tree-SHA512: 5a7db896952edf338ff4fe8c934f1e1c992642850a99d5fafbb1212c6979601b3b72b6f3af880fb6f6ac8759cd4102e9f01792abb05410ceaf36cbffaec48e47
20fb7618b001937befbf8757e5881674b4a55147 args: make most ArgsManager members private (w0xlt)
22b40f34f33f5e5555d0b378f005516ecfb45e6c args: replace cs_args RecursiveMutex with Mutex (w0xlt)
3a16ec8582bee1dfc82857205ba1037c17a735cb test: scope cs_args locks to avoid recursive locking (w0xlt)
70b51fef7afd705d109718447097935e49cc0c25 args: eliminate all recursive locking of cs_args (w0xlt)
7d61e03c701215e2956353195dff4734210a2765 args: extract lock-requiring internal helpers (w0xlt)
Pull request description:
Part of #19303.
Replace `ArgsManager::cs_args` from `RecursiveMutex` to `Mutex`.
The conversion follows the pattern established in prior `RecursiveMutex` removals (e.g. `CAddrMan` in #19238, `CBlockPolicyEstimator` in #22014): extract private lock-held helpers with trailing underscore naming (`GetSetting_()`, `GetArgFlags_()`, `GetPathArg_()`), then replace recursive calls in methods that already hold `cs_args` with those helpers.
ACKs for top commit:
l0rinc:
ACK 20fb7618b001937befbf8757e5881674b4a55147
rkrux:
Concept ACK 20fb7618b0
sedited:
Re-ACK 20fb7618b001937befbf8757e5881674b4a55147
hebasto:
ACK 20fb7618b001937befbf8757e5881674b4a55147, only rebased and suggested changes since my recent [review](https://github.com/bitcoin/bitcoin/pull/34745#pullrequestreview-3916715262).
Tree-SHA512: 7ab4278737f00deaa3f3da75e08469f91e95aa31e916820d02af737c754751ae4f73c1c8650f120eeff142a134f9209cf581499696a7b88ffc83d296515e40f2
9085dee476d21daab786e88848597c755beecee4 qa: Add lock order annotation for TxMempool::cs (David Gumberg)
Pull request description:
I added this to see if static analysis would catch any lock order inversions while investigating #34731, it did not catch any but it is a worthwhile defense against statically-analyzable inversions of `cs_main->TxMempool::cs`.
ACKs for top commit:
sedited:
tested ACK 9085dee476d21daab786e88848597c755beecee4
hebasto:
ACK 9085dee476d21daab786e88848597c755beecee4.
w0xlt:
ACK 9085dee476d21daab786e88848597c755beecee4
Tree-SHA512: 498605362e7b197b7643a3f9ffd04589bc9b245aca1d245d289efa145906248c1a72f9fbf4835a9378aabe170b747dc0cc033105915a2bf7a9989de368710c02
c2732146d14e5e0b932b14ea177ac8bcd25c9f84 doc: fix process name typo in multiprocess.md (ViniciusCestarii)
Pull request description:
In this [flow](https://github.com/bitcoin/bitcoin/blob/master/doc/design/multiprocess.md#example-use-cases-and-flows), the bitcoin-wallet process acts as the client initiating the RPC request, while the bitcoin-node process acts as the server receiving it. So at "The `getBlockHash` method of the generated `Chain` server subclass in `bitcoin-wallet` receives a Cap’n Proto request object with the `height` parameter, and calls the `getBlockHash` method on its local `Chain` object with the provided `height`." it should say bitcoin-node not bitcoin-wallet.
This PR fix this typo.
ACKs for top commit:
ryanofsky:
Code review ACK c2732146d14e5e0b932b14ea177ac8bcd25c9f84. Nice catch!
Tree-SHA512: 7227dc96e556ad36f1f39cc1033d80c92e8c6a47cf61ab68401122b1ef8075068224494e4ffb7094eca8dc325294cb96e561d3e997af7d434158031485f92677
57bfa864fe69ea5610399f9db60cf2299930703a test: use static methods and clarify comment in addr_relay (stratospher)
7ee8c0abc629f0658b6c36f36b5da11c51cbb39d test: protect outbound connection from eviction in getaddr_test (stratospher)
ecb5ce6e76ef9391d636e7e4f6be7e3f7ed05550 test: fix addr relay test silent pass and wrong peerinfo index (stratospher)
Pull request description:
couple of improvements in the addr relay test:
- fixes the silent test pass discovered in https://github.com/bitcoin/bitcoin/pull/34717#issuecomment-3990585763 (will remove this if that PR gets merged - the test doesn't fail even though #34717 changes the behaviour)
- correct wrong peerinfo index
- prevent intermittent disconnection warnings like the one shown below by protecting outbound peer from `ConsiderEviction`
```
TestFramework (INFO): Check that we answer getaddr messages only once per connection
TestFramework.p2p (WARNING): Connection lost to 127.0.0.1:58829 due to [Errno 54] Connection reset by peer
```
- remove a no longer applicable test comment since we don't need to send initial GETADDR for intial self announcement anymore
ACKs for top commit:
Bortlesboat:
ACK 57bfa864fe69. Ran both `p2p_addr_relay.py` and `p2p_addr_selfannouncement.py` locally, both pass. Good catch on the stale `peerinfo` reference in `inbound_blackhole_tests` — that would silently check the wrong peer.
naiyoma:
ACK 57bfa864fe69ea5610399f9db60cf2299930703a
mzumsande:
Code Review ACK 57bfa864fe69ea5610399f9db60cf2299930703a
Tree-SHA512: 22e4f87f66569bfa629a68a8b440cd21b5285c6dad6eb7926514f2d74e16fe3711525b264f82765c83020be976a0438b8d2ab1e48e7c0b7d85437ee672d52324
d236415649e21a4848b5685b6ba41da82c8dde73 rpc: Refactor gettxspendingprevout to be easier to parse (Fabian Jahr)
Pull request description:
This refactoring commit was originally part of #34635 but the the change was some-what controversial: https://github.com/bitcoin/bitcoin/pull/34635#discussion_r2833541912 and the discussion was blocking the rest of the uncontroversial changes from moving forward.
It looks like #34748 is trying to achieve something similar now, we'll have to see how the different approached converge but at least there seems to be some more conceptual interest than appeared on #34635 originally.
ACKs for top commit:
hodlinator:
ACK d236415649e21a4848b5685b6ba41da82c8dde73
sedited:
Re-ACK d236415649e21a4848b5685b6ba41da82c8dde73
furszy:
utACK d236415649
Tree-SHA512: 3df70bb8f143247f66d52430b3d8a4ecc5f0b80aa68ef3fc9082739b6f1be02fa9ef88ff134bb01034c9b6006400342e71fa8dc0a203d8e13b4bf6e21d1b05a8
9a968ad35efe04bfd86299c5dfbc48ada41bc46e ci: bump cirruslabs actions versions (will)
Pull request description:
Fix CI deprecation warnings of form: `Node.js 20 actions are deprecated. The following actions are running on Node.js 20 and may not work as expected: cirruslabs/cache/restore@v4.`
As these are hiding in `.github/actions/**/*.yml` they were forgotten by the (two) previous PR(s) 😢https://github.com/bitcoin/bitcoin/pull/34802https://github.com/bitcoin/bitcoin/pull/34344
ACKs for top commit:
maflcko:
lgtm ACK 9a968ad35efe04bfd86299c5dfbc48ada41bc46e
Tree-SHA512: 2de22cac1dbe7b3df3a69114b1cd84cb8ca5338c6b03c7181d535de247778202301457cfb8d537f6ce118c8d08e0bdcc6ce316ab3d946f606c17e2974a1f14c0
be6d24ec22cc2ef8086098d6322b024e07c5758c guix: Make guix-clean less destructive (Hodlinator)
Pull request description:
* Show preview and ask for confirmation before git clean unless used with "--force"
* Error out when user tries to pass args such as "guix-clean --help"
ACKs for top commit:
kevkevinpal:
reACK [be6d24e](be6d24ec22)
janb84:
ACK be6d24ec22cc2ef8086098d6322b024e07c5758c
sedited:
ACK be6d24ec22cc2ef8086098d6322b024e07c5758c
willcl-ark:
ACK be6d24ec22cc2ef8086098d6322b024e07c5758c
Tree-SHA512: 19fc6e5dc68f2886f5fb970ff39d040a8088eedca56ad5b86622297614b0d164df9ddd5a8b24b972060e9dcde732de5e3346866b53963524a4277a0a56220570
fa6497ba71e9573d341c1c051af09b3ec2fc8d74 build: Set AUTHOR_WARNING on warnings (MarcoFalke)
Pull request description:
Now that the cmake setting `-Werror=dev` is set since commit 6a13a6106e3c1ebe95ba6430184d6260a7b942bd for the CI, guix and the dev cmake preset, it could make sense to notify developers about any warnings.
So do that with a single `AUTHOR_WARNING`.
This can be tested by introducing a bug, like:
```diff
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6017775fa7..5610e03c66 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -589,7 +589,7 @@ set(Python3_FIND_FRAMEWORK LAST CACHE STRING "")
# improves compatibility with Python version managers that use shims.
set(Python3_FIND_UNVERSIONED_NAMES FIRST CACHE STRING "")
mark_as_advanced(Python3_FIND_FRAMEWORK Python3_FIND_UNVERSIONED_NAMES)
-find_package(Python3 3.10 COMPONENTS Interpreter)
+find_package(Python3 3.210 COMPONENTS Interpreter)
if(NOT TARGET Python3::Interpreter)
list(APPEND configure_warnings
"Minimum required Python not found."
```
Fixes https://github.com/bitcoin/bitcoin/issues/31476.
ACKs for top commit:
l0rinc:
ACK fa6497ba71e9573d341c1c051af09b3ec2fc8d74
purpleKarrot:
ACK fa6497ba71e9573d341c1c051af09b3ec2fc8d74
stickies-v:
ACK fa6497ba71e9573d341c1c051af09b3ec2fc8d74
hebasto:
ACK fa6497ba71e9573d341c1c051af09b3ec2fc8d74, this effectively allows us to use `-Werror=dev` to automatically catch any warning.
sedited:
ACK fa6497ba71e9573d341c1c051af09b3ec2fc8d74
Tree-SHA512: df3b7fa88451527d6a950bd6ebe46e96d1d2f6447c2b53cbe26c6ece0b63a41663a0accfc0ee20c03bc40328363d46f0e7ad88aab1be9b383ad7ff2621363a30
2f8f2e900118c98699db91075bd1b72e6460c6b1 validation: remove ConnectTrace wrapper class (stickies-v)
b83de7f28e71f0b10a999327395198fbcb02f44b validation: remove sentinel block from ConnectTrace (stickies-v)
Pull request description:
The sentinel pattern in `ConnectTrace` has been unnecessary since conflicted transaction tracking was removed in 5613f9842b4000fed088b8cf7b99674c328d15e1. Without that tracking `ConnectTrace` is a trivial wrapper around `std::vector`, so it seems better to just replace it with the vector directly.
Also modernize/update naming along the way, renaming `PerBlockConnectTrace` to `ConnectedBlock`
Refactor, no behaviour change.
ACKs for top commit:
HowHsu:
ACK 2f8f2e900118c98699db91075bd1b72e6460c6b1
sedited:
ACK 2f8f2e900118c98699db91075bd1b72e6460c6b1
w0xlt:
reACK 2f8f2e9001
Tree-SHA512: 0045fcdc1178a160e31ef9d44dcd5fddd21c30c53ed06e84beacddb0b73e7b8120fee874256d1b9ceae45da65164a2e5531992bd374f8d57b6a8455a5354fe57
b19caeea098f92a7f72aaeee49573358f4b153a3 doc: add release note for #31560 (named pipe support for `dumptxoutset` RPC) (Sebastian Falbesoner)
61a5460d0d6cd174d395c51333def798fe7442fe test: add test for utxo-to-sqlite conversion using named pipe (Sebastian Falbesoner)
2e8072edbeb20a8c05c0dbd06ca105bc4dd07b96 rpc: support writing UTXO set dump (`dumptxoutset`) to a named pipe (Sebastian Falbesoner)
Pull request description:
This PR slightly modifies the `dumptxoutset` RPC to allow writing the UTXO set dump into a [named pipe](https://askubuntu.com/a/449192), so that the output data can be consumed by another process, see #31373. Taking use of this with the utxo-to-sqlite.py tool (introduced in #27432), creating an UTXO set in SQLite3 format is possible on the fly. E.g. for signet:
```
$ mkfifo /tmp/utxo_fifo && ./build/bin/bitcoin-cli -signet dumptxoutset /tmp/utxo_fifo latest &
$ ./contrib/utxo-tools/utxo_to_sqlite.py /tmp/utxo_fifo ./utxo.sqlite
UTXO Snapshot for Signet at block hash 000000012711f0a4e741be4a22792982..., contains 61848352 coins
1048576 coins converted [1.70%], 2.800s passed since start
....
....
60817408 coins converted [98.33%], 159.598s passed since start
{
"coins_written": 61848352,
"base_hash": "000000012711f0a4e741be4a22792982370f51326db20fca955c7d45da97f768",
"base_height": 294305,
"path": "/tmp/utxo_fifo",
"txoutset_hash": "34ae7fe7af33f58d4b83e00ecfc3b9605d927f154e7a94401226922f8e3f534e",
"nchaintx": 28760852
}
TOTAL: 61848352 coins written to ./utxo.sqlite, snapshot height is 294305.
```
Note that the `dumptxoutset` RPC calculates an UTXO set hash as a first step before any data is emitted, so especially on mainnet it takes quite a while until the conversion starts and something is happening visibly.
ACKs for top commit:
ajtowns:
utACK b19caeea098f92a7f72aaeee49573358f4b153a3
sedited:
Re-ACK b19caeea098f92a7f72aaeee49573358f4b153a3
Tree-SHA512: 7101563d0dba15439cdef8c8fb535f8593d5a779ff04208e2d72382a3f99072db8eac3651d1b3fe72c5e1f03e164efb281c3030d45d0723b943ebbbcf2a841d6
faae981d354e13de96442159d604a831b6c3a71e lint: Temporarily revert to vulture==2.14 (MarcoFalke)
Pull request description:
To work around https://github.com/bitcoin/bitcoin/issues/34810
ACKs for top commit:
fanquake:
ACK faae981d354e13de96442159d604a831b6c3a71e
Tree-SHA512: 80b8250379e672e62090e7853d342b93befcb33c9f6e86fb39776e34029f0952ad4fb6d22747c387a3503b2b9cb3f894ef6e5f7606058647a46037918e1b35b0
745ad941dedaa0f4e2a5fdda58a3bf683d672196 p2p: remove m_starting_height field from node state (only show once in debug log) (Sebastian Falbesoner)
b267efcdaf854b6b50424d6a6af7cac8ba458390 rpc, net: completely remove `startingheight` field of `getpeerinfo` RPC (Sebastian Falbesoner)
Pull request description:
This PR completely removes the `startingheight` field in the `getpeerinfo` RPC, previously deprecated in v31.0 (see #34197). The second commit goes one step further: the only remaining usage for a `Peer`s `m_starting_height` field is for printing P2P debug messages. Considering that the information is untrusted and not deemed useful (see discussion #33990), remove the field and only print the starting height information once at reception of the `VERSION` message (suggested by ajtowns in https://github.com/bitcoin/bitcoin/pull/33990#issuecomment-3621363780), without saving it.
ACKs for top commit:
stickies-v:
re-ACK 745ad941dedaa0f4e2a5fdda58a3bf683d672196, no changes except for addressing minor merge conflict
w0xlt:
ACK 745ad941dedaa0f4e2a5fdda58a3bf683d672196
sedited:
Re-ACK 745ad941dedaa0f4e2a5fdda58a3bf683d672196
Tree-SHA512: 09d30f34ea564e884e5d5c0e56cd66f99d6127ba8cc779f1e7ab29ea4fe8603835d306e343ee3a0038b1c07245f429012375034518135c915f543ba175d6dc45
ec4ec91d59b25badf212bf0aee0ad5c1eead7ff1 kernel: doc: explain return value for `btck_WriteBytes` callback (Sebastian Falbesoner)
Pull request description:
Note that this is the only callback type with a non-void return type. Probably it would make sense to document the parameters of callbacks as well on the long-term (Doxygen style?), but this IMHO the most critical missing documentation, where likely other bitcoinkernel users working with the C API could trip over too.
Background: I've been working on btck bindings for [Zig](https://ziglang.org/) for a while [1]. Overall I found the C API header `bitcoinkernel.h` very well-documented and to a large degree self-explanatory, but for implementing a `btck_WriteBytes` callback (needed for various `btck_..._to_bytes` serialization functions) I had to look into the [kernel implementation](e98d36715e/src/kernel/bitcoinkernel.cpp (L86)) to figure out what return value is expected.
[1] still not public and put on hold for the last few months due to lib(std)c++ build system / linking issues (needing ugly workarounds, hopefully improved with Zig's upcoming 0.16 release); now continued, hopefully ready to be published in a first presentable version soon(tm)
ACKs for top commit:
sedited:
ACK ec4ec91d59b25badf212bf0aee0ad5c1eead7ff1
alexanderwiederin:
ACK ec4ec91d59
stickies-v:
ACK ec4ec91d59b25badf212bf0aee0ad5c1eead7ff1
Tree-SHA512: aeea68474a04e0bc4d2421a0539a0bd717b0526f58c7379848a8bb8cb75a0fdf5c94bfcc226f28dd09b89c5fc368921d1d875893b86680e7c51d7b98f4ab749d
fadaa7db335d1b1bd2674c51d89d6c0820ddb887 ci: Bump GHA actions versions (MarcoFalke)
Pull request description:
Looks like this was forgotten in the prior pull https://github.com/bitcoin/bitcoin/pull/34344#issuecomment-3769438233
ACKs for top commit:
fanquake:
ACK fadaa7db335d1b1bd2674c51d89d6c0820ddb887
Tree-SHA512: bc3bc5b7ba275d7c48bd34fa0eaaac05942212369fc87be338b31c024ca736550a5ff64766c78912f9705e9f95ffedeae1eddfe2f95828350c7672ab2069713c
This allows external tooling (e.g. converters) to consume the output
directly, rather than having to write the dump to disk first and then
read it from there again.
Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
