build: Drop ZeroMQ patch for glibc < 2.12 (Hennadii Stepanov) 079df96 build: Drop ZeroMQ patch for Mingw-w64 < 4.0 (Hennadii Stepanov) #3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
b35341b Update ci.yml (Coder) Pull request description: Release notes:https://github.com/actions/download-artifact/releases/tag/v5.0.0 Change: uses: actions/download-artifact@v4 -> uses: actions/download-artifact@v5 ACKs for top commit: maflcko: lgtm ACK b35341b willcl-ark: ACK b35341b hebasto: ACK b35341b, I have reviewed the code and it looks OK. Tree-SHA512: f82dd0fe3ca8d431b9ff6ef9f23a4f2e92a1463c6f55fbe9b46b9e13750d311bd2aa915a8570f76600363b3a1ccbf394c95216cfac0f6db30846d9be7ec7c4cf
ceeb53a ci: Properly include $FILE_ENV in DEPENDS_HASH (Ava Chow) Pull request description: $FILE_ENV has a full relative path already, prepending with ci/test/ results in a non-existent path which means that DEPENDS_HASH was not actually committing to the test's environment file. ACKs for top commit: maflcko: lgtm ACK ceeb53a Tree-SHA512: 80a7a23676ff8bf2f48a7d3c5897217f11d7d4d4f8a54897d2b7c42689585d2d63e45fad2b8f4c442111f128a87eeb6edeac2b25c79862e6bc035eeb1ebc7f4e
a1226bc doc: how to update a subtree (Sjors Provoost) Pull request description: We have instructions on how to verify a subtree update, but not on how to perform one. ACKs for top commit: yuvicc: ACK a1226bc achow101: ACK a1226bc janb84: ACK a1226bc furszy: ACK a1226bc Tree-SHA512: ba3ccc56a9f1c7f461e0db9699612e1fd64b7c72bfd1dae63d4cb830db416871a493820d3a7924c19b6ce353fc20c5fe07578b053dec6ea68273a007cbebc512
This reverts commit 53372f2.
…ce tarball" e4335a3 Revert "depends: Update URL for `qrencode` package source tarball" (Ava Chow) a89a822 Revert "depends: Use hash instead of file name for package download stamp" (Ava Chow) Pull request description: The new URL breaks CI on the current release branches, see #33494 (comment). The old URL also no longer exists so the tarball is fetched from the depends sources cache that we host, but the original tarball has already been overwritten on there. We will need to manually reinstate the original tarball. ACKs for top commit: m3dwards: utACK e4335a3 maflcko: review ACK e4335a3 💤 glozow: ACK e4335a3 Tree-SHA512: a5028342d77b4768daaec8688acd364795d683aed2bea0407c7827d44f814a97d50cc3b30c2de2a8296a2b212115fe1e76c57685a74e93387fc57afdabb93bd2
Simplify and improve the logic for calculating pindexLastCommonBlock, in order to calculate nWindowEnd better. The previous logic would not take into account when the chain tip had moved forward, so that FindNextBlocks could iterate over many blocks already downloaded and connected, which could result in blocks not being requested for download that should have been requested, and peers being wrongly marked as staller. It also removes extra logic from commit 49d569c for the situation right after a snapshot was loaded: After snapshot loading, our tip becomes the snapshot block. For peers that have the most-work chain, which inlcludes the snapshot, our tip is an ancestor of the peer's best block, hence the general advancement logic will move pindexLastCommonBlock from any pre-snapshot position to the snapshot height automatically. Co-authored-by: stringintech <stringintech@gmail.com> Co-authored-by: Pieter Wuille <pieter@wuille.net>
getpeerinfo provides a list of blocks that are inflight, which can be used instead.
This test (which would fail without the previous commit) checks that after the stalling block was received, we don't incorrectly mark another peer as a staller immediately.
0f01e15 Squashed 'src/ipc/libmultiprocess/' changes from 47d79db8a552..a4f929696490 (Ryan Ofsky) Pull request description: Includes: - bitcoin-core/libmultiprocess#213 - bitcoin-core/libmultiprocess#214 - bitcoin-core/libmultiprocess#221 - bitcoin-core/libmultiprocess#220 - bitcoin-core/libmultiprocess#222 - bitcoin-core/libmultiprocess#224 The change bitcoin-core/libmultiprocess#220 is needed to support #33517 and fix poor performance in some cases caused by slow logging. The changes can be verified by running `test/lint/git-subtree-check.sh src/ipc/libmultiprocess` as described in [developer notes](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees) and [lint instructions](https://github.com/bitcoin/bitcoin/tree/master/test/lint#git-subtree-checksh) ACKs for top commit: Sjors: utACK eda91b0 theuni: utACK eda91b0. Tree-SHA512: 43c2f47bb95f56181f3ce8cf41380e83b1c00b363a7c732d735a9115ed251fa2c2c9bd096d9be011e47503047a740b2e05c9a79d7e4170a4de9c20ad0de3e501
It makes more sense to have a higher OS version than one that is not supported
f656752 doc: bump the template macOS version (kevkevinpal) Pull request description: Motivated by #33489 (comment) The minimum version of MacOS for this repo is now 14 and above so it makes sense to update the issue template to reflect that. We are now using a higher version but since it is just a bug template, there is no need to put the lowest version we support. ACKs for top commit: maflcko: lgtm ACK f656752 l0rinc: ACK f656752 janb84: ACK f656752 Tree-SHA512: 701b161bda25245996c94b6d2119b5cc85a34917551dcf8c92ffacf3aa80fa7fe84bb3497edc7e600c5b2443de13a6f6107fc7289721e585b16c4972d07a796c
This uses the constructors recently added upstream.
Without this change, logging (even if unused) may account for a substantial portion of bitcoin-node's and/or client's runtime cpu usage, due to libmultiprocess's expensive message serialization. This (along with some recent upstream changes) avoids the overhead by opting out of log handling for messages that we're not interested in. Info, Warning, and Error are logged unconditionally to match our behavior elsewhere. See BCLog::Logger::GetCategoryLogLevel .
This reduces per-Cluster memory usage by making Clusters not aware of their own level. Instead, track it either in calling code, or infer it based on the transactions in them.
Co-Authored-By: Lőrinc <pap.lorinc@gmail.com>
This adds 4 functions to Cluster to help implement Merge() and Split() without needing access to the internals of the other Cluster. This is a preparation for a follow-up that will make Clusters a virtual class whose internals are abstracted away.
This adds a specialized Cluster implementation for singleton clusters, saving a significant amount of memory by avoiding the need for m_depgraph, m_mapping, and m_linearization, and their overheads.
fa1e8d8 refactor: Add missing include in bitcoinkernel_wrapper.h (MarcoFalke) Pull request description: Otherwise, the compilation may fail with: ``` /home/admin/actions-runner/_work/_temp/src/kernel/bitcoinkernel_wrapper.h:271:14: error: no type named 'exception_ptr' in namespace 'std'; did you mean 'exception'? 271 | std::exception_ptr exception; | ~~~~~^~~~~~~~~~~~~ | exception /cxx_build/include/c++/v1/__exception/exception.h:72:33: note: 'exception' declared here 72 | class _LIBCPP_EXPORTED_FROM_ABI exception { | ^ In file included from /home/admin/actions-runner/_work/_temp/src/bitcoin-chainstate.cpp:1: /home/admin/actions-runner/_work/_temp/src/kernel/bitcoinkernel_wrapper.h:284:35: error: no member named 'current_exception' in namespace 'std' 284 | data.exception = std::current_exception(); | ^~~~~~~~~~~~~~~~~ /home/admin/actions-runner/_work/_temp/src/kernel/bitcoinkernel_wrapper.h:290:14: error: no member named 'rethrow_exception' in namespace 'std' 290 | std::rethrow_exception(user_data.exception); | ^~~~~~~~~~~~~~~~~ /home/admin/actions-runner/_work/_temp/src/kernel/bitcoinkernel_wrapper.h:273:65: error: no viable conversion from 'std::nullptr_t' to 'std::exception' 273 | UserData user_data = UserData{.bytes = &bytes, .exception = nullptr}; | ^~~~~~~ /home/admin/actions-runner/_work/_temp/src/kernel/bitcoinkernel_wrapper.h:733:16: note: in instantiation of function template specialization 'btck::write_bytes<btck_Block>' requested here 733 | return write_bytes(get(), btck_block_to_bytes); | ^ /cxx_build/include/c++/v1/__exception/exception.h:75:25: note: candidate constructor not viable: no known conversion from 'std::nullptr_t' to 'const exception &' for 1st argument 75 | _LIBCPP_HIDE_FROM_ABI exception(const exception&) _NOEXCEPT = default; | ^ ~~~~~~~~~~~~~~~~ 4 errors generated. ACKs for top commit: TheCharlatan: ACK fa1e8d8 hebasto: ACK fa1e8d8. yuvicc: ACK fa1e8d8 Tree-SHA512: c0127678db5913402c92b7602d159faae26539dc33f6159abd909b33746dd4626b8cbb6a86d8ccd3c9c83e06956fe55fb721a034480498d0cd87349aceea51f9
Points a reader to Developer Notes which explains to compile with llvm source based coverage instrumentation
dcb56fd interfaces: add interruptWait method (ismaelsadeeq) Pull request description: This is an attempt to fix #33575 see the issue for background and the usefulness of this feature. This PR uses one of the suggested approaches: adding a new `interruptWaitNext()` method to the mining interface. It introduces a new boolean variable, `m_interrupt_wait`, which is set to `false` when the thread starts waiting. The `interruptWaitNext()` method wakes the thread and sets `m_interrupt_wait` to `true`. Whenever the thread wakes up, it checks whether the wait was aborted; if so, it simply set ` m_interrupt_wait ` to false and return`nullptr`. This PR also adds a functional test for the new method. The test uses `asyncio` to spawn two tasks and attempts to ensure that the wait is executed before the interrupt by using an event monitor. It adds a 0.1-second buffer to ensure the wait has started executing. If that buffer elapses without `waitNext` executing, the test will fail because a transaction is created after the buffer. ACKs for top commit: furszy: Code ACK dcb56fd ryanofsky: Code review ACK dcb56fd, just tweaking semantics slightly since last review so if an `interruptWait` call is made shortly after a `waitNext` call it will reliably cause the `waitNext` call to return right away without blocking, even if the `waitNext` call had not begun to execute or wait yet. Sjors: tACK dcb56fd TheCharlatan: ACK dcb56fd Tree-SHA512: a03f049e1f303b174a9e5d125733b6583dfd8effa12e7b6c37bd9b2cff9541100f5f4514e80f89005c44a57d7e47804afe87aa5fdb6831f3b0cd9b01d83e42be
fa6db67 ci: [refactor] Extract build_dir constant in ci-test-each-commit-exec.py (MarcoFalke) fa95e6c ci: Use cmake --preset=dev-mode in test-each-commit task (MarcoFalke) Pull request description: Using the preset should reduce the bloat and need to maintain several places to list the same cmake cache variables. The only difference should be that `bitcoin-chainstate (experimental)` will be enabled, which seems fast and in line with the goal of the CI task. * Before: https://github.com/bitcoin/bitcoin/actions/runs/19174075826/job/54814118651#step:8:315 * After: (this pull) https://github.com/bitcoin/bitcoin/actions/runs/19190748069/job/54864837086#step:7:324 ```diff bitcoin-tx .......................... ON bitcoin-util ........................ ON bitcoin-wallet ...................... ON - bitcoin-chainstate (experimental) ... OFF + bitcoin-chainstate (experimental) ... ON libbitcoinkernel (experimental) ..... ON kernel-test (experimental) .......... ON Optional features: ACKs for top commit: hebasto: ACK fa6db67, I have reviewed the code and it looks OK. Tree-SHA512: 61a78de7bcbf42bd266cb035f354862f5d1e1235acd2a81041e3a68a4d3ab4703fa2cfc993f28e4dacaa74e3cccc9ef568d5d4526605ce5a00bcd7c347b97121
dee7eec doc: mention coverage build in quickstart section (frankomosh) Pull request description: Adds a single comment in the libFuzzer quick-start that links to the Developer Notes coverage section. No build flags are changed or shown. ACKs for top commit: janb84: ACK dee7eec dergoegge: ACK dee7eec Tree-SHA512: 2fe5ffb6c3d06f75694646473c29b4cc9fe571f4659631ec174d444a14716771308eedeb7acab3bef7f62e9bfa8ed0462da0163b214cccdc6a9ad63bbf66d2a0
0698c6b doc: Correct `pkgin` command usage on NetBSD (Hennadii Stepanov) Pull request description: When using `pkgin` on NetBSD, the `install` command must be specified. ACKs for top commit: fanquake: ACK 0698c6b Tree-SHA512: 840fc1621d6fa9ad43501a3691a31cffd66c1ac8d34167f7ab0fe33e1a395198c241b3c31f3d0ebc314e28c0edb6055cc2ca3deba6408dcbd14390fd679a4803
3672420 scripted-diff: Remove obsolete comment (Hennadii Stepanov) Pull request description: The removed comment become obsolete after #32697 and #32881. ACKs for top commit: l0rinc: code review ACK 3672420 maflcko: lgtm ACK 3672420 rkrux: crACK 3672420 Tree-SHA512: b880f71ce9aa8288c635eb216c86d5424227a27bfb524443255d48193afca05e65b092fb1b481f3b850f7b8d6cef0f64d275d173c580023b8b5fa19966356d52
fad6efd refactor: Use STR_INTERNAL_BUG macro where possible (MarcoFalke) fada379 doc: Remove unused bugprone-lambda-function-name suppression (MarcoFalke) fae1d99 refactor: Use const reference to std::source_location (MarcoFalke) fa5fbcd util: Allow Assert() in contexts without __func__ (MarcoFalke) Pull request description: Without this, compile warnings could be hit about `__func__` being only valid inside functions. ``` warning: predefined identifier is only valid inside function [-Wpredefined-identifier-outside-function] note: expanded from macro Assert 115 | #define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val) | ^ ``` Ref #32740 (comment) This also introduces a slight behaviour change, because `std::source_location::function_name` usually includes the entire function signature instead of just the name. ACKs for top commit: l0rinc: Code review ACK fad6efd stickies-v: ACK fad6efd hodlinator: re-ACK fad6efd Tree-SHA512: e78a2d812d5ae22e45c93db1661dafbcd22ef209b3d8d8d5f2ac514e92fd19a17c3f0a5db2ef5e7748aa2083b10c0465326eb36812e6a80e238972facd2c7e98
It does not make sense to use a pointer, when a reference is more
appropriate, especially given that nullptr has been ruled out.
This is also allows to remove the CI workaround to avoid warnings:
```
C++ compiler .......................... GNU 13.0.0, /bin/x86_64-w64-mingw32-g++-posix
...
/ci_container_base/src/test/blockmanager_tests.cpp: In member function ‘void blockmanager_tests::blockmanager_scan_unlink_already_pruned_files::test_method()’:
/ci_container_base/src/test/blockmanager_tests.cpp:63:17: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
63 | const auto& chainman = Assert(m_node.chainman);
| ^~~~~~~~
In file included from /ci_container_base/src/streams.h:13,
from /ci_container_base/src/dbwrapper.h:11,
from /ci_container_base/src/node/blockstorage.h:10,
from /ci_container_base/src/test/blockmanager_tests.cpp:8:
/ci_container_base/src/util/check.h:116:49: note: the temporary was destroyed at the end of the full expression ‘inline_assertion_check<true, std::unique_ptr<ChainstateManager>&>(((blockmanager_tests::blockmanager_scan_unlink_already_pruned_files*)this)->blockmanager_tests::blockmanager_scan_unlink_already_pruned_files::<anonymous>.TestChain100Setup::<anonymous>.TestingSetup::<anonymous>.ChainTestingSetup::<anonymous>.BasicTestingSetup::m_node.node::NodeContext::chainman, std::source_location{(& *.Lsrc_loc27)}, std::basic_string_view<char>(((const char*)"m_node.chainman")))’
116 | #define Assert(val) inline_assertion_check<true>(val, std::source_location::current(), #val)
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/ci_container_base/src/test/blockmanager_tests.cpp:63:28: note: in expansion of macro ‘Assert’
63 | const auto& chainman = Assert(m_node.chainman);
| ^~~~~~
cc1plus: all warnings being treated as errors
gmake[2]: Leaving directory '/ci_container_base/ci/scratch/build-x86_64-w64-mingw32'
gmake[2]: *** [src/test/CMakeFiles/test_bitcoin.dir/build.make:382: src/test/CMakeFiles/test_bitcoin.dir/blockmanager_tests.cpp.obj] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:1810: src/test/CMakeFiles/test_bitcoin.dir/all] Error 2
gmake[1]: Leaving directory '/ci_container_base/ci/scratch/build-x86_64-w64-mingw32'
gmake: *** [Makefile:146: all] Error 2
```
This false-positive warning is also fixed in later GCC versions.
See also https://godbolt.org/z/fjc6be65M
It is equivalent to calling btck_chain_get_by_height(0).
It is equivalent to calling btck_chain_get_by_height with the height obtained from btck_chain_get_height. In neither case do we provide guarantees that the returned block index still corresponds to the actual tip.
4543a3b Squashed 'src/minisketch/' changes from ea8f66b1ea..d1bd01e189 (Hennadii Stepanov) Pull request description: This PR updates the `minisketch` subtree to latest upstream, which includes: - bitcoin-core/minisketch#75 - bitcoin-core/minisketch#98 ACKs for top commit: fanquake: ACK c235aa4 Tree-SHA512: 856fb8b7dc2e743c9c67164023bf53faf8766079aeccc82a30c8b90c85920b31977b6a8b26e51e5485b20e445a3ca6ff806e701a53e95f70181ea30055e3528c
…address is found 79b4c27 Bugfix: QA: rpc_bind: Skip nonloopback test if no such address is found (Luke Dashjr) Pull request description: Without this, I get: ``` 2025-09-19T03:14:05.157000Z TestFramework (INFO): PRNG seed is: 3218602557639511064 2025-09-19T03:14:05.158000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin-test/a 2025-09-19T03:14:05.158000Z TestFramework (INFO): Check for ipv6 2025-09-19T03:14:05.158000Z TestFramework (INFO): Check for non-loopback interface 2025-09-19T03:14:05.158000Z TestFramework (INFO): Bind test for [] 2025-09-19T03:14:05.516000Z TestFramework (INFO): Bind test for [] 2025-09-19T03:14:05.871000Z TestFramework (INFO): Bind test for ['[::1]'] 2025-09-19T03:14:06.227000Z TestFramework (INFO): Bind test for ['127.0.0.1', '[::1]'] 2025-09-19T03:14:06.583000Z TestFramework (INFO): Using interface None for testing 2025-09-19T03:14:06.583000Z TestFramework (INFO): Bind test for [None] 2025-09-19T03:14:06.583000Z TestFramework (ERROR): Unexpected exception Traceback (most recent call last): File "/Bitcoin/bitcoin/workingtree/test/functional/test_framework/test_framework.py", line 135, in main self.run_test() ~~~~~~~~~~~~~^^ File "/Bitcoin/bitcoin/workingtree/test/functional/rpc_bind.py", line 126, in run_test self._run_nonloopback_tests() ~~~~~~~~~~~~~~~~~~~~~~~~~~~^^ File "/Bitcoin/bitcoin/workingtree/test/functional/rpc_bind.py", line 157, in _run_nonloopback_tests self.run_bind_test([self.non_loopback_ip], self.non_loopback_ip, [self.non_loopback_ip], ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [(self.non_loopback_ip, self.defaultport)]) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ File "/Bitcoin/bitcoin/workingtree/test/functional/rpc_bind.py", line 38, in run_bind_test expected = [(addr_to_hex(addr), port) for (addr, port) in expected] ~~~~~~~~~~~^^^^^^ File "/Bitcoin/bitcoin/workingtree/test/functional/test_framework/netutil.py", line 132, in addr_to_hex if '.' in addr: # IPv4 ^^^^^^^^^^^ TypeError: argument of type 'NoneType' is not iterable ``` ACKs for top commit: maflcko: review ACK 79b4c27 🏑 theStack: Tested ACK 79b4c27 Tree-SHA512: 2a723d9bc5d1d50a8321a4f8a8cac3da3125d373ea71e6cc9d03de07307008f58970e361490d4c34530a6a976cb078b62d0ef09b7fb321ca1cfb9249a70d99a5
…uring block replay 1fc7a81 log: reduce excessive messages during block replay (Lőrinc) Pull request description: ### Summary After an incomplete reindex the blocks will need to be replayed. This results in excessive `Rolling back` and `Rolling forward` messages which quickly triggers the recently introduced log rate limiter. Change the logging strategy to: - Add single `LogInfo` messages showing the full range being replayed for both rollback and roll forward; - Log progress at `LogInfo` level only every 10,000 blocks to track the long operations. ### Reproducer: * Start a normal ibd, stop after some progress * Do a reindex, stop before it finishes * Restart the node normally without specifying the reindex parameter It should start rolling the blocks forward. Before this change the excessive logging would show: ``` [*] Rolling forward 000000002f4f55aecfccc911076dc3f73ac0288c83dc1d79db0a026441031d40 (46245) [*] Rolling forward 0000000017ffcf34c8eac010c529670ba6745ea59cf1edf7b820928e3b40acf6 (46246) ``` After the change it shows: ``` Replaying blocks Rolling forward to 00000000000000001034012d7e4facaf16ca747ea94b8ea66743086cfe298ef8 (326223 to 340991) Rolling forward 00000000000000000faabab19f17c0178c754dbed023e6c871dcaf74159c5f02 (330000) Rolling forward 00000000000000000d9b2508615d569e18f00c034d71474fc44a43af8d4a5003 (340000) ... Rolled forward to 00000000000000001034012d7e4facaf16ca747ea94b8ea66743086cfe298ef8 ``` (similarly to rolling back) ACKs for top commit: Crypt-iQ: crACK 1fc7a81 stickies-v: ACK 1fc7a81 achow101: ACK 1fc7a81 vasild: ACK 1fc7a81 hodlinator: Concept ACK 1fc7a81 Tree-SHA512: 44ed1da8336de5a3d937e11a13e6f1789064e23eb70640a1c406fbb0074255344268f6eb6b06f036ca8d22bfeb4bdea319c3085a2139d848f6d36a4f8352b76a
…ng blocks 01cc20f test: improve coverage for a resolved stalling situation (Martin Zumsande) 9af6daf test: remove magic number when checking for blocks that have arrived (Martin Zumsande) 3069d66 p2p: During block download, adjust pindexLastCommonBlock better (Martin Zumsande) Pull request description: As described in #32179, `pindexLastCommonBlock` is updated later than necessary in master. In case of a linear chain with no forks, it can be moved forward at the beginning of `FindNextBlocksToDownload`, so that the updated value can be used to better estimate `nWindowEnd`. This helps the node to request all blocks from peers within the correct 1024-block-window and avoids peers being incorrectly marked as stallers. I also changed `p2p_ibd_stalling.py` to cover the situation after a resolved situation, making sure that no additional peers are marked for stalling. Fixes #32179 ACKs for top commit: Crypt-iQ: crACK 01cc20f stringintech: re-ACK 01cc20f achow101: ACK 01cc20f sipa: utACK 01cc20f Tree-SHA512: a97f7a7ef5ded538ee35576e04b3fbcdd46a6d0189c7ba3abacc6e0d81e800aac5b0c2d2565d0462ef6fd4acc751989f577fd6adfd450171a7d6ab26f437df32
…et gettransaction response 060bb55 rpc: add decoded tx details to gettransaction with extra wallet fields (Matthew Zipkin) ad1c3bd [move only] move DecodeTxDoc() to a common util file for sharing (Matthew Zipkin) d633db5 rpc: add "ischange: true" in wallet gettransaction decoded tx output (Matthew Zipkin) Pull request description: This change is motivated by external RBF clients like https://github.com/CardCoins/additive-rbf-batcher/. It saves the user a redundant re-looping of tx outputs, calling `getaddressinfo` on each one, looking for the change output in order to adjust the fee. The field `"ischange"` only appears when `gettransaction` is called on a wallet, and is either `true` or not present at all. I chose not to include `ischange: false` because it is confusing to see that on *received* transactions. Example of the new field: ``` "vout": [ { "value": 1.00000000, "n": 0, "scriptPubKey": { "asm": "0 5483235e05c76273b3b50af62519738781aff021", "desc": "addr(bcrt1q2jpjxhs9ca388va4ptmz2xtns7q6lupppkw7wu)#d42g84j6", "hex": "00145483235e05c76273b3b50af62519738781aff021", "address": "bcrt1q2jpjxhs9ca388va4ptmz2xtns7q6lupppkw7wu", "type": "witness_v0_keyhash" } }, { "value": 198.99859000, "n": 1, "scriptPubKey": { "asm": "0 870ab1ab58632b05a417d5295f4038500e407592", "desc": "addr(bcrt1qsu9tr26cvv4stfqh65547spc2q8yqavj7fnlju)#tgapemkv", "hex": "0014870ab1ab58632b05a417d5295f4038500e407592", "address": "bcrt1qsu9tr26cvv4stfqh65547spc2q8yqavj7fnlju", "type": "witness_v0_keyhash" }, "ischange": true } ] ``` ACKs for top commit: furszy: ACK [060bb55](060bb55) maflcko: review ACK 060bb55 🌛 achow101: ACK 060bb55 rkrux: lgtm ACK 060bb55 Tree-SHA512: aae4854d2bb4e9a7bc1152691ea90e594e8da8a63c9c7fda72a504fb6a7e54ae274ed5fa98d35d270e0829cc8f8d2fd35a5fc9735c252a10aa42cc22828930e7
…` write methods 743abbc refactor: inline constant return value of `BlockTreeDB::WriteBatchSync` and `BlockManager::WriteBlockIndexDB` and `BlockTreeDB::WriteFlag` (Lőrinc) e030240 refactor: inline constant return value of `CDBWrapper::Erase` and `BlockTreeDB::WriteReindexing` (Lőrinc) cdab948 refactor: inline constant return value of `CDBWrapper::Write` (Lőrinc) d1847cf refactor: inline constant return value of `TxIndex::DB::WriteTxs` (Lőrinc) 50b63a5 refactor: inline constant return value of `CDBWrapper::WriteBatch` (Lőrinc) Pull request description: Related to #31144 (comment) ### Summary `WriteBatch` always returns `true` - the errors are handled by throwing `dbwrapper_error` instead. ### Context This boolean return value of the `Write` methods is confusing because it's inconsistent with `CDBWrapper::Read`, which catches exceptions and returns a boolean to indicate success/failure. It's bad that `Read` returns and `Write` throws - but it's a lot worse that `Write` advertises a return value when it actually communicates errors through exceptions. ### Solution This PR removes the constant return values from write methods and inlines `true` at their call sites. Many upstream methods had boolean return values only because they were propagating these constants - those have been cleaned up as well. Methods that returned a constant `true` value that now return `void`: - `CDBWrapper::WriteBatch`, `CDBWrapper::Write`, `CDBWrapper::Erase` - `TxIndex::DB::WriteTxs` - `BlockTreeDB::WriteReindexing`, `BlockTreeDB::WriteBatchSync`, `BlockTreeDB::WriteFlag` - `BlockManager::WriteBlockIndexDB` ### Note `CCoinsView::BatchWrite` (and transitively `CCoinsViewCache::Flush` & `CCoinsViewCache::Sync`) were intentionally not changed here. While all implementations return `true`, the base `CCoinsView::BatchWrite` returns `false`. Changing this would cause `coins_view` tests to fail with: > terminating due to uncaught exception of type std::logic_error: Not all unspent flagged entries were cleared We can fix that in a follow-up PR. ACKs for top commit: achow101: ACK 743abbc janb84: ACK 743abbc TheCharlatan: ACK 743abbc sipa: ACK 743abbc Tree-SHA512: b2a550bff066216f1958d2dd9a7ef6a9949de518cc636f8ab9c670e0b7a330c1eb8c838e458a8629acb8ac980cea6616955cd84436a7b8ab9096f6d648073b1e
66978a1 kernel: remove btck_chain_get_tip (stickies-v) 4dd7e6d kernel: remove btck_chain_get_genesis (stickies-v) Pull request description: Removes `btck_chain_get_genesis` and `btck_chain_get_tip`. They are trivially replaced with `btck_chain_get_by_height` (as indicated in the updated `bitcoinkernel_wrapper.h`), so I think it makes sense to trim the interface. For `btck_chain_get_tip`: on `master` we don't provide any guarantees that the returned block index still corresponds to the actual tip, so the extra call doesn't seem like a regression to me. ACKs for top commit: TheCharlatan: ACK 66978a1 janb84: ACK 66978a1 Tree-SHA512: f583fbb7f2e3f8f23afb57732b2cbe9e1d550bfc43c9a2619895ee30c27f5f3c5cd9e4ecb7e05b1f6ab9e11c368596ec9b733d67e06cfafb12326d88e8e4dd7d
f06c6e1 guix: build for Linux HOSTS with -static-libgcc (fanquake) 1bdf469 guix: patch store paths out of libunwind (fanquake) 078a72c guix: move static-libc++ into CMAKE_EXE_LINKER_FLAGS flags (fanquake) Pull request description: Build release binaries with `-static-libgcc`. Would avoid future issues like #33178. ACKs for top commit: willcl-ark: ACK f06c6e1 hebasto: ACK f06c6e1. janb84: Concept ACK f06c6e1 Tree-SHA512: 79409d9044fe7a339ea8090ca0e70e1305816aa3225b41ca6e4f2fec37650206ab5a78c1b2495a27a0c6c0dd6d5f86bd696101d2d1c5ecc72c630dc34e55f7dc
There is no-longer a minimum required libgcc version, after #33181.
7a4901c test, refactor: Fix `-Warray-bounds` warning (Hennadii Stepanov) faf2759 test: [refactor] Use reference over ptr to chainman (MarcoFalke) Pull request description: Just some minor test-only refactor commits to fix GCC false positive warnings, along with making the test code easier to read and understand: * First change requested in #33785 (comment) * Second change requested in commit 3b135a8 Those changes are required in a bunch of pulls touching the CI system, so merging them allows to drop them in all pulls. ACKs for top commit: l0rinc: ACK 7a4901c hebasto: ACK 7a4901c, I have reviewed the code and it looks OK. Tree-SHA512: 64dca52ec7b25078bf489e2d8b43e449f4968fbac14a09c66a60cdc75b513588403665f248368820694a6f72c4f7f465589d9306355239cffe35c38111929eff
169f93d depends: drop qtbase_avoid_native_float16 qt patch (fanquake) Pull request description: There is no-longer a minimum required / max supported libgcc version, after #33181. ACKs for top commit: laanwj: Code review ACK 169f93d hebasto: ACK 169f93d. Tree-SHA512: 1cb3639742d1466ae4355f99bea08afd1dab89a03b10aa7c0e04c8ec18e7654913028155badbfa67fdfa39764f6e04e7a0b5d007b0b3af8606425641db01f1e3
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
31 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
build: Drop ZeroMQ patch for glibc < 2.12 (Hennadii Stepanov)
079df96 build: Drop ZeroMQ patch for Mingw-w64 < 4.0 (Hennadii Stepanov)