Starting with Python 3.11, Pythons gzip might delegate to zlib.
Depending on the OS, i.e Ubuntu vs Fedora, the underlying zlib
implementation might differ, resulting in different output.
For now, or until a better solution exists, disable compression. This
results in the SDK increasing in size to ~157mb. Which is not
unreasonable, to regain determinism (and would be significantly worse
without the previous commit).
See: https://docs.python.org/3/library/gzip.html#gzip.compress
Co-authored-by: stickies-v <stickies-v@protonmail.com>
fa0fee44a8 ci: Remove redundant busybox option (MarcoFalke)
fa102ec69f doc: Shorten ci name (MarcoFalke)
2222223780 doc: Remove bash -c wrapper (MarcoFalke)
Pull request description:
The option was fine, but now that there is a dedicated Alpine Linux task, which uses BusyBox, it seems redundant.
(See: `ci/test/00_setup_env_native_alpine_musl.sh`)
So remove the `USE_BUSY_BOX` option, along with the `BINS_SCRATCH_DIR` env var.
Also includes two small ci-doc fixups.
ACKs for top commit:
fanquake:
ACK fa0fee44a8
Tree-SHA512: abad7888cbf5833cc322d133187f74653c232c03b8949ad06dd1ea5ace14127871322fe682268ebec7720963615c3f113c7660a2055bf48bbb9ff9a730221d7e
c34bc01b2f doc: clarify and cleanup macOS fuzzing notes (dergoegge)
Pull request description:
* Remove or consolidate macOS notes sprinkled throughout the doc into dedicated section
* Note that support for fuzzing on macOS is not maintained
Closes#33731
ACKs for top commit:
frankomosh:
ACK c34bc01
janb84:
Concept ACK c34bc01b2f
darosior:
reACK c34bc01b2f
brunoerg:
ACK c34bc01b2f
rkrux:
ACK c34bc01b2f
ismaelsadeeq:
reACK c34bc01b2f
Tree-SHA512: 76a79a102b4941b31bd05b65aa37beb0f48d8bb7b07f4ec2fa55be7e6b4880ce7031fe9a7764984dbd879bf6238dc61689006a9225deee39b8b759722078b808
ade0397f59 txgraph: drop move assignment operator (Anthony Towns)
Pull request description:
This removes the only place where move-assignment of `TxGraph::Ref` is used (in tests), and drops supports for it.
Suggested in https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2518940184
ACKs for top commit:
l0rinc:
reACK ade0397f59
instagibbs:
ACK ade0397f59
Tree-SHA512: 0f49e454d0d44817278cbd3fbb8fce89c64c6f6c6d852bea26c728b9f1b6827a0f2b8731ac2031150af92b0ec479c2fe4ece01256fb3b6b2bf941f16c0e2c541
096924d39d kernel: add btck_block_tree_entry_equals (stickies-v)
Pull request description:
`BlockTreeEntry` objects are often compared. This happens frequently in our own codebase and seems likely to be the case for clients, too. Users can already work around this by comparing based on block hash (and optionally height as belt-and-suspenders), but I think this should be part of the interface for performance and consistency reasons.
Note: perhaps this is too ad-hoc, and we should extend this PR to add the operator for more types? `BlockTreeEntry` is the main one I've needed this for in developing `py-bitcoinkernel`, though.
ACKs for top commit:
maflcko:
review ACK 096924d39d📓
TheCharlatan:
ACK 096924d39d
yuvicc:
Code Review ACK 096924d39d
Tree-SHA512: a0c08c01ab6c855aec4e2b2b898e9550493cd4cf8c6e1fe9e4fe5039d0d9ef3bffb2f2ab0454c7cc449b9deedd7889f5fd7b5f100fa706a855023af4adb803c6
fa7e222a23 clang-format: Set Bitcoin Core IncludeCategories (MarcoFalke)
Pull request description:
Replace the default llvm include categories with the ones specific to Bitcoin Core.
Ref: https://releases.llvm.org/17.0.1/tools/clang/docs/ClangFormatStyleOptions.html#includecategories
Also, format a file as example. To test this, the diff in src/test needs
to be reverted. Also `IncludeBlocks: Regroup` needs to be set. Then
`clang-format -i src/test/blockchain_tests.cpp` should recreate the
diff.
```diff
diff --git a/src/.clang-format b/src/.clang-format
index 15335fe9ae..579079095f 100644
--- a/src/.clang-format
+++ b/src/.clang-format
@@ -99,3 +99,3 @@ IfMacros:
- KJ_IF_MAYBE
-IncludeBlocks: Preserve
+IncludeBlocks: Regroup
IncludeCategories:
```
ACKs for top commit:
l0rinc:
ACK fa7e222a23
Tree-SHA512: f500060858898c48f709e8fecfe4c41fc05645e2ff3b1504cc82ce354129642dd658a2311fd038363068ed682d016c32b56d935783c8084591ac5152b413fa2c
fa7ea497c3 ci: Run GUI unit tests in cross-Windows task (MarcoFalke)
Pull request description:
Most users of the cross-compiled releases for Windows will most likely pick the GUI, so running the cross-compiled GUI unit tests on a real Windows seems desirable.
ACKs for top commit:
fanquake:
ACK fa7ea497c3 - didn't test.
Tree-SHA512: 6826474295ecf0170e45a644100887d5e53899b4a3eb49635e5cc62c774b512b383bc0b2de15ac854a527da0779b5ab8f52aae9f817c7f2971a0cbfc68c027b7
8343a9ffcc test: add `-alertnotify` test for large work invalid chain warning (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for the `LARGE_WORK_INVALID_CHAIN` fork warning, checked with the `-alertnotify` option:
https://github.com/bitcoin/bitcoin/blob/ead849c9f177a3a175a22b35fa864b4b37fb9934/src/validation.cpp#L2033-L2040
Found that this is missing during review of #32587. The test works by first creating a bunch of invalid blocks, that are first announced by headers and then submitted fully in reverse (invalid tip first), in order to set `m_best_invalid` to that value, finally leading to the best chain / invalid chain gap of >= 6 blocks. I'd be curious if there are other (more realistic?) ways to test this. One simple alternative is just to call `invalidateblock` twice (once at the tip, once at the base of the invalid chain).
Note that the written warning doesn't include the exclamation mark, as it is removed via `SanitizeString` in the `AlertNotify` function.
ACKs for top commit:
brunoerg:
reACK 8343a9ffcc
mzumsande:
re-ACK 8343a9ffcc
Tree-SHA512: d81e9ce7622026498cad5cdcdb867a22068670983737502888c72c72209ca6ff183e77d7429f758765a42c25cda439e01f795884864ac6fe6ff258a98d0bbcbc
fa9537cde1 ci: Use latest Xcode that the minimum macOS version allows (MarcoFalke)
Pull request description:
Changing the CI policy to use the *latest* Xcode (instead of the *earliest*), allowed by the Bitcoin Core minimum supported macOS version, makes sense: While this may require the developer or user to install a later security point-release on macOS, this should generally be fine and it is even expected that users run the latest supported security release of their operating system. Also, in practise, this often doesn't result in a visible change anyway: This specific change from Xcode 16.0 to 16.2 does not change any behavior of the Bitcoin Core CI, because there are no C++-related changes in those point releases.
ACKs for top commit:
l0rinc:
ACK fa9537cde1
vasild:
ACK fa9537cde1
fanquake:
ACK https://github.com/bitcoin/bitcoin/pull/33932/commits/fa9537cde10120b12c96061cbc3f79a7680f9d64 - seems fine.
Tree-SHA512: ca0e6c4fc396cad7569d04d7a648bc5318cce171f2fa941d8b00a30cf49b054e70a0133738533c744bccd65baf9129ba527b1950b15e10c3f7953a24ef359977
17cf9ff7ef Use cluster size limit for -maxmempool bound, and allow -maxmempool=0 in general (Suhas Daftuar)
315e43e5d8 Sanity check `GetFeerateDiagram()` in CTxMemPool::check() (Suhas Daftuar)
de2e9a24c4 test: extend package rbf functional test to larger clusters (Suhas Daftuar)
4ef4ddb504 doc: update policy/packages.md for new package acceptance logic (Suhas Daftuar)
79f73ad713 Add check that GetSortedScoreWithTopology() agrees with CompareMiningScoreWithTopology() (Suhas Daftuar)
a86ac11768 Update comments for CTxMemPool class (Suhas Daftuar)
9567eaa66d Invoke TxGraph::DoWork() at appropriate times (Suhas Daftuar)
6c5c44f774 test: add functional test for new cluster mempool RPCs (Suhas Daftuar)
72f60c877e doc: Update mempool_replacements.md to reflect feerate diagram checks (Suhas Daftuar)
21693f031a Expose cluster information via rpc (Suhas Daftuar)
72e74e0d42 fuzz: try to add more code coverage for mempool fuzzing (Suhas Daftuar)
f107417490 bench: add more mempool benchmarks (Suhas Daftuar)
7976eb1ae7 Avoid violating mempool policy limits in tests (Suhas Daftuar)
84de685cf7 Stop tracking parents/children outside of txgraph (Suhas Daftuar)
88672e205b Rewrite GatherClusters to use the txgraph implementation (Suhas Daftuar)
1ca4f01090 Fix miniminer_tests to work with cluster limits (Suhas Daftuar)
1902111e0f Eliminate CheckPackageLimits, which no longer does anything (Suhas Daftuar)
3a646ec462 Rework RBF and TRUC validation (Suhas Daftuar)
19b8479868 Make getting parents/children a function of the mempool, not a mempool entry (Suhas Daftuar)
5560913e51 Rework truc_policy to use descendants, not children (Suhas Daftuar)
a4458d6c40 Use txgraph to calculate descendants (Suhas Daftuar)
c8b6f70d64 Use txgraph to calculate ancestors (Suhas Daftuar)
241a3e666b Simplify ancestor calculation functions (Suhas Daftuar)
b9cec7f0a1 Make removeConflicts private (Suhas Daftuar)
0402e6c780 Remove unused limits from CalculateMemPoolAncestors (Suhas Daftuar)
08be765ac2 Remove mempool logic designed to maintain ancestor/descendant state (Suhas Daftuar)
fc4e3e6bc1 Remove unused members from CTxMemPoolEntry (Suhas Daftuar)
ff3b398d12 mempool: eliminate accessors to mempool entry ancestor/descendant cached state (Suhas Daftuar)
b9a2039f51 Eliminate use of cached ancestor data in miniminer_tests and truc_policy (Suhas Daftuar)
ba09fc9774 mempool: Remove unused function CalculateDescendantMaximum (Suhas Daftuar)
8e49477e86 wallet: Replace max descendant count with cluster_count (Suhas Daftuar)
e031085fd4 Eliminate Single-Conflict RBF Carve Out (Suhas Daftuar)
cf3ab8e1d0 Stop enforcing descendant size/count limits (Suhas Daftuar)
89ae38f489 test: remove rbf carveout test from mempool_limit.py (Suhas Daftuar)
c0bd04d18f Calculate descendant information for mempool RPC output on-the-fly (Suhas Daftuar)
bdcefb8a8b Use mempool/txgraph to determine if a tx has descendants (Suhas Daftuar)
69e1eaa6ed Add test case for cluster size limits to TRUC logic (Suhas Daftuar)
9cda64b86c Stop enforcing ancestor size/count limits (Suhas Daftuar)
1f93227a84 Remove dependency on cached ancestor data in mini-miner (Suhas Daftuar)
9fbe0a4ac2 rpc: Calculate ancestor data from scratch for mempool rpc calls (Suhas Daftuar)
7961496dda Reimplement GetTransactionAncestry() to not rely on cached data (Suhas Daftuar)
feceaa42e8 Remove CTxMemPool::GetSortedDepthAndScore (Suhas Daftuar)
21b5cea588 Use cluster linearization for transaction relay sort order (Suhas Daftuar)
6445aa7d97 Remove the ancestor and descendant indices from the mempool (Suhas Daftuar)
216e693729 Implement new RBF logic for cluster mempool (Suhas Daftuar)
ff8f115dec policy: Remove CPFP carveout rule (Suhas Daftuar)
c3f1afc934 test: rewrite PopulateMempool to not violate mempool policy (cluster size) limits (Suhas Daftuar)
47ab32fdb1 Select transactions for blocks based on chunk feerate (Suhas Daftuar)
dec138d1dd fuzz: remove comparison between mini_miner block construction and miner (Suhas Daftuar)
6c2bceb200 bench: rewrite ComplexMemPool to not create oversized clusters (Suhas Daftuar)
1ad4590f63 Limit mempool size based on chunk feerate (Suhas Daftuar)
b11c89cab2 Rework miner_tests to not require large cluster limit (Suhas Daftuar)
95a8297d48 Check cluster limits when using -walletrejectlongchains (Suhas Daftuar)
95762e6759 Do not allow mempool clusters to exceed configured limits (Suhas Daftuar)
edb3e7cdf6 [test] rework/delete feature_rbf tests requiring large clusters (glozow)
435fd56711 test: update feature_rbf.py replacement test (Suhas Daftuar)
34e32985e8 Add new (unused) limits for cluster size/count (Suhas Daftuar)
838d7e3553 Add transactions to txgraph, but without cluster dependencies (Suhas Daftuar)
d5ed9cb3eb Add accessor for sigops-adjusted weight (Suhas Daftuar)
1bf3b51396 Add sigops adjusted weight calculator (Suhas Daftuar)
c18c68a950 Create a txgraph inside CTxMemPool (Suhas Daftuar)
29a94d5b2f Make CTxMemPoolEntry derive from TxGraph::Ref (Suhas Daftuar)
92b0079fe3 Allow moving CTxMemPoolEntry objects, disallow copying (Suhas Daftuar)
6c73e47448 mempool: Store iterators into mapTx in mapNextTx (Suhas Daftuar)
51430680ec Allow moving an Epoch::Marker (Suhas Daftuar)
Pull request description:
[Reopening #28676 here as a new PR, because GitHub is slow to load the page making it hard to scroll through and see comments. Also, that PR was originally opened with a prototype implementation which has changed significantly with the introduction of `TxGraph`.]
This is an implementation of the [cluster mempool proposal](https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393).
This branch implements the following observable behavior changes:
- Maintains a partitioning of the mempool into connected clusters (via the `txgraph` class), which are limited in vsize to 101 kvB by default, and limited in count to 64 by default.
- Each cluster is sorted ("linearized") to try to optimize for selecting highest-feerate-subsets of a cluster first
- Transaction selection for mining is updated to use the cluster linearizations, selecting highest feerate "chunks" first for inclusion in a block template.
- Mempool eviction is updated to use the cluster linearizations, selecting lowest feerate "chunks" first for removal.
- The RBF rules are updated to: (a) drop the requirement that no new inputs are introduced; (b) change the feerate requirement to instead check that the feerate diagram of the mempool will strictly improve; (c) replace the direct conflicts limit with a directly-conflicting-clusters limit.
- The CPFP carveout rule is eliminated (it doesn't make sense in a cluster-limited mempool)
- The ancestor and descendant limits are no longer enforced.
- New cluster count/cluster vsize limits are now enforced instead.
- Transaction relay now uses chunk feerate comparisons to determine the order that newly received transactions are announced to peers.
Additionally, the cached ancestor and descendant data are dropped from the mempool, along with the multi_index indices that were maintained to sort the mempool by ancestor and descendant feerates. For compatibility (eg with wallet behavior or RPCs exposing this), this information is now calculated dynamically instead.
ACKs for top commit:
instagibbs:
reACK 17cf9ff7ef
glozow:
reACK 17cf9ff7ef
sipa:
ACK 17cf9ff7ef
Tree-SHA512: bbde46d913d56f8d9c0426cb0a6c4fa80b01b0a4c2299500769921f886082fb4f51f1694e0ee1bc318c52e1976d7ebed8134a64eda0b8044f3a708c04938eee7
55555db055 doc: Add missing --platform=linux to docker build command (MarcoFalke)
fa0ce4c148 ci: Re-enable LINT_CI_SANITY_CHECK_COMMIT_SIG (MarcoFalke)
faa0973de2 ci: [refactor] Rename CIRRUS_PR env var to LINT_CI_IS_PR (MarcoFalke)
fa1dacaebe ci: Move lint exec snippet to stand-alone py file (MarcoFalke)
Pull request description:
The sanity check to check the last few merge commit signatures on the main branch was accidentally and silently disabled while moving from the `cirrus-ci.com` platform to the GHA platform.
So fix that by re-enabling it.
Also, contains a few other lint cleanup commits.
ACKs for top commit:
janb84:
re ACK 55555db055
willcl-ark:
ACK 55555db055
Tree-SHA512: e623dc88035ee4d1c6a8efa5fad33c35cface87f54e78c7ebfe5d468d28d8d8097150344d276f90f8ed52a89e61609ce95380476ea0151b50f73ad5919233933
99d012ec80 refactor: return reference instead of pointer (Andrew Toth)
f743e6c5dd refactor: add missing LIFETIMEBOUND annotation for parameter (Andrew Toth)
141117f5e8 refactor: remove incorrect LIFETIMEBOUND annotations (Andrew Toth)
Pull request description:
The [developer-notes say](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#lifetimebound):
> You can use the attribute by adding a `LIFETIMEBOUND`
annotation defined in `src/attributes.h`; please grep the codebase for examples.
While grepping, I found an incorrect usage of the `LIFETIMEBOUND` annotation on `BlockManager::CheckBlockDataAvailability`. This could be misleading about usage for other greppers. As I was looking, I also noticed a missing `LIFETIMEBOUND` on `BlockManager::GetFirstBlock`. While looking more closely at that method, it should return a reference instead of a pointer. The only reason to return a pointer is if it can be null.
ACKs for top commit:
maflcko:
review ACK 99d012ec80💧
l0rinc:
ACK 99d012ec80
stickies-v:
ACK 99d012ec80
optout21:
ACK 99d012ec80
vasild:
ACK 99d012ec80
Tree-SHA512: d6c56ee223d6614d52ee6cf5cd66e787125c98c6ae37705a17e51a6e15774e260ac55b3d60f2fc818132e766ad98dd94232d6c8829119f628498e9d0d2bd977f
552eb90071 doc: CI - Describe qemu-user-static usage (Hodlinator)
2afbbddee5 doc: CI - Clarify how important `env -i` is and why (Hodlinator)
Pull request description:
Should at least partially fix#31199
ACKs for top commit:
maflcko:
lgtm ACK 552eb90071
janb84:
ACK 552eb90071
Tree-SHA512: 45807a61d805646384c8162501f432537b7e655aa01434766ffb90ea47da9532387a76fcccac7fe208ad77f4ea5573f60b9be09e1235b9493eaa8795e1d7fbdd
2578e6fc0f test: Fix race condition in IPC interface block propagation test (Fabian Jahr)
Pull request description:
CI failed on this condition here: https://github.com/bitcoin/bitcoin/actions/runs/19395398994/job/55494696022?pr=33878#step:9:3983
The check was added not too long ago in https://github.com/bitcoin/bitcoin/pull/33745 and the fix here switches the check to the node which actually produces the block. There are also some comments added to make the checks easier so understand.
Closes#33884
ACKs for top commit:
Sjors:
re-utACK 2578e6fc0f
maflcko:
lgtm ACK 2578e6fc0f
Tree-SHA512: bfb7ae44aede50a00d4096e1a9922f9b8df31ce4242e12863e329d0d1e714d8cb46c852f694c32314e4bd26b524535e3a6967b7c57861a9b00cf09831a950b99
fa411f938e ci: Consistenly only cache on the default branch (MarcoFalke)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/33685
The general idea for caches is to only save them on pushes to the default branch, because the cache is limited in size and time that the only benefit of the cache can be to speed up pull requests against the default branch.
Backport pull requests to older branches don't benefit from caches, because usually they will be running into a cache miss anyway. Also, they would cause the cache size to overflow and lead to cache misses down the line.
So fix it by consistently applying cache saves only on the default branch.
For reference, the same is already done for the composite action in this repo: https://github.com/bitcoin/bitcoin/blob/2444488f6ad32dcbbed51a73cd4f59ff3a239e32/.github/actions/save-caches/action.yml#L15
ACKs for top commit:
hebasto:
ACK fa411f938e.
willcl-ark:
ACK fa411f938e
Tree-SHA512: 59d3de4201b596e5f7eb1915c4bd5ded80bcd4df217f8f5d62d92fa8977a77e2c4c8602b17755b33ff0dfa87f2896e7c1c8f4da8e6a77c111f2a42ec9cf78ffd
7f318e1dd0 test: Add better coverage for Autofile size() (Fabian Jahr)
b7af960eb8 refactor: Add AutoFile::size (Fabian Jahr)
ec0f75862e refactor: Modernize logging in util/asmap.cpp (Fabian Jahr)
606a251e0a tests: add unit test vectors for asmap interpreter (Pieter Wuille)
Pull request description:
This contains some commits from #28792 that can be easily reviewed and merged independently. I hope splitting this change off can make this part move a bit faster and reduce frequency of needed rebases for #28792.
The commits in order:
- Add additional unit test vectors to the asmap interpreter (written by sipa). This helps to ensure that the further refactors in #28792 don't change behavior.
- Modernizes the logging in `util/asmap.cpp`, I added this while touching the rest of the file all over anyway.
- Adds an `AutoFile::size` helper function with some additional test coverage in a separate commit
ACKs for top commit:
maflcko:
review ACK 7f318e1dd0🏀
hodlinator:
tACK 7f318e1dd0
laanwj:
Code review ACK 7f318e1dd0
Tree-SHA512: 45156b74e4bd9278a7ec24521dfdafe4dab1ba3384243c7d589ef17e16ca374ee2af7178c86b7229e80ca262dbe78c4d456d80a6ee742ec31d2ab5243dac8b57
4d893c0f46 net: Remove unused `local_socket_bytes` variable in `CConnman::GetAddresses()` (WakeTrainDev)
Pull request description:
The local_socket_bytes variable was never used. Removed it to clean up dead code.
ACKs for top commit:
mzumsande:
ACK 4d893c0f46
theStack:
ACK 4d893c0f46
Tree-SHA512: f423bcf975aa2602464fcb96db323cbd6007a7491ddbe119f1d20e890c883dd351a55976151c5d25f5d26267b0efe1f0836fbd65e540c920dac931ed8d67846a
fa95353902 ci: Run macos tasks in a git archive, not git checkout (MarcoFalke)
faf99ae379 refactor: Avoid -W*-whitespace in git archive (MarcoFalke)
Pull request description:
Otherwise, compilation with GCC-15+ will warn about it:
```
src/clientversion.cpp:33:79: error: trailing whitespace [-Werror=trailing-whitespace=]
33 | //! git will put "#define GIT_COMMIT_ID ..." on the next line inside archives.
```
Follow-up to https://github.com/bitcoin/bitcoin/pull/32482#issuecomment-3522280482
Can be tested via `git archive --output=/tmp/a.tar HEAD`
ACKs for top commit:
fanquake:
ACK fa95353902
Tree-SHA512: 73940ffc0fd83db557275bd5e993a3c47c5397682a1188447c48e077ead597ba0fc3e5ef9da7b746746ff04a26022ce35ac10768888bbd4707f25b799af43e45
2594d5a189 build: Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings (Henry Romp)
Pull request description:
Remove CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings that are no longer needed after reordering the Guix build script to perform binary checks after installation.
This PR also removes the unused CMake maintenance targets (`check-security` and `check-symbols`) and updates the Guix security checks to include binaries in the `libexec/` directory (added in PR #31679).
ACKs for top commit:
purpleKarrot:
ACK 2594d5a189
hebasto:
ACK 2594d5a189.
Tree-SHA512: ed451a298f5aae05c177b0033b092faaa7536caeaa3d84da9b8b611e2aa905e1dd337e57aef0efd69ce6ce6ac0cf77dc57adf175079b95bf53dd96d5d0c8118b
c29eaeeaf9 doc: Update NetBSD Build Guide (Hennadii Stepanov)
Pull request description:
The `py310-zmq` binary package is not available by default on NetBSD 10.1. It has been updated to `py313-zmq`, and the `python310` package is updated accordingly.
See: https://ftp.netbsd.org/pub/pkgsrc/current/pkgsrc/index-all.html.
ACKs for top commit:
fanquake:
ACK c29eaeeaf9
Tree-SHA512: 6924a974d6ed494609c789cc3f28cf173af3a37b940520ad7b169eff87e30af8346fec07e46f8bffe14a329060c41ac46d46b91910a00994cdf8a7ace8391d1c
3e9aca6f1b depends: drop qtbase-moc-ignore-gcc-macro.patch qt patch (fanquake)
0da5a82700 depends: drop unused qt patch (fanquake)
Pull request description:
Drop one patch that was already unused, and one that compilation succeeds without.
ACKs for top commit:
TheCharlatan:
ACK 3e9aca6f1b
Tree-SHA512: 4416348c80d8af8530d46d4f5a02a1170f7a4e2fc8ef88cffb8888fa913ed86d1bef10efb437434ebcdac1b1ed23a3669c1ba654cf6f4395dc0a73192fe0024f
b0a3887154 scripted-diff: fix leftover references to `policy/fees.h` (ismaelsadeeq)
Pull request description:
Fixes#33863
ryanofsky wrote
> I still see some references to the src/policy/fees.h file removed by this PR:
```
$ git grep -n policy/fees.h
src/wallet/rpc/spend.cpp:206: * @param[in] conf_target UniValue integer; confirmation target in blocks, values between 1 and 1008 are valid per policy/fees.h;
test/functional/rpc_estimatefee.py:39: # max value of 1008 per src/policy/fees.h
test/functional/rpc_psbt.py:604: assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", # max value of 1008 per src/policy/fees.h
test/functional/wallet_basic.py:337: assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", # max value of 1008 per src/policy/fees.h
test/functional/wallet_fundrawtransaction.py:851: assert_raises_rpc_error(-8, "Invalid conf_target, must be between 1 and 1008", # max value of 1008 per src/policy/fees.h
test/functional/wallet_send.py:315: expect_error=(-8, "Invalid conf_target, must be between 1 and 1008")) # max value of 1008 per src/policy/fees.h
```
This is fixed in this PR by running a script that searches for what he greps and replaces it with the right reference.
```
git grep -l "policy\/fees\.h" | xargs sed -i "s/policy\/fees.h/policy\/fees\/block_policy_estimator.h/g"
```
ACKs for top commit:
kevkevinpal:
ACK [b0a3887](https://github.com/bitcoin/bitcoin/pull/33864/commits/b0a38871546dfcdd3a578c1ae4c28a88b6ee32d5)
janb84:
ACK b0a3887154
rkrux:
lgtm ACK b0a3887154
Tree-SHA512: e24f2aaf18fcfb0ae047a53ed209135a644ff08f5a8bc162c1522be3f99d7d01d550fc2e73d8db5fec7b748902daf68e61e7a5624f5913b9824feba5641fc78c
a3ac59a431 ci: Enable experimental kernel stuff in ASan task (MarcoFalke)
5b89956eeb kernel: Allow null arguments for serialized data (TheCharlatan)
Pull request description:
An empty span constructed from an empty vector may have a null data pointer depending on the implementation. Remove the BITCOINKERNEL_ARG_NONNULL requirement for these arguments and instead handle such null arguments in the implementation.
Also cherry-picked from #33845 to show that CI task passing now.
ACKs for top commit:
yuvicc:
Code review ACK a3ac59a431
maflcko:
review ACK a3ac59a431🥈
laanwj:
code review ACK a3ac59a431
Tree-SHA512: 629e463796f2f057df5be8e8981a45751c578ed0021be731c1d57fe849a539fe38b0a445914b0fc48f32f0408ad6d566984bd7f3a68797fcfdf1c6889e316a08
40dcbf580d build: add -Wtrailing-whitespace=any (fanquake)
d7659cd7e6 build: add -Wleading-whitespace=spaces (fanquake)
d86650220a cmake: Disable `-Wtrailing-whitespace` warnings for RCC-generated files (Hennadii Stepanov)
aabc5ca6ed cmake: Switch from AUTORCC to `qt6_add_resources` (Hennadii Stepanov)
25ae14c339 subprocess: replace tab with space (fanquake)
0c2b9dadd5 scripted-diff: remove whitespace in sha256_sse4.cpp (fanquake)
4da084fbc9 scripted-diff: change whitespace to spaces in univalue (fanquake)
e6caf150b3 ci: add moreutils to lint job (fanquake)
Pull request description:
GCC 15 now has options to turn leading & trailing whitespace into compile failures: https://gcc.gnu.org/gcc-15/changes.html#c-family. Fix the few cases of leading tabs, and trailing whitespace, and then enable `-Wleading-whitespace` and `-Wtrailing-whitespace`.
We currently get PRs that are opened with various whitespace, i.e #33822, so turning that into compile-time failure where possible, seems useful, to avoid a CI roundtrip.
ACKs for top commit:
ajtowns:
utACK 40dcbf580d
hebasto:
re-ACK 40dcbf580d.
Tree-SHA512: a128001ab2abb41cd6d249dcf46be4167ebd608d6b0f1452212a3ec9a383747bea623ab0382ec7bc0ac7a232a47cca5174e1cd73d4eda6751aa3cb2365ad2ede
fa9f29a4a7 doc: Recommend latest Debian stable or Ubuntu LTS (MarcoFalke)
fa1711ee0d doc: Add GCC-12 min release notes (MarcoFalke)
faa8be75c9 ci: Enable experimental kernel stuff in G++-12 task (previous releases) (MarcoFalke)
fabce97b30 test: Remove gccbug_90348 test case (MarcoFalke)
fa3854e432 test: Remove unused fs::create_directories test (MarcoFalke)
fa9dacdbde util: [refactor] Remove unused create_directories workaround (MarcoFalke)
fa807f78ae build: Bump g++ minimum supported version to 12 (MarcoFalke)
Pull request description:
All supported operating systems that previously came with at least g++-11, also come with at least g++-12, so bumping the minimum should be fine.
For reference:
* https://packages.ubuntu.com/jammy/g++-12
* https://packages.ubuntu.com/noble/g++ (g++-13)
* https://packages.debian.org/bookworm/g++ (g++-12)
* FreeBSD Ports ship a recent GCC
* RHEL-based 8, and 9 ship with g++-14 via appstream (`dnf install gcc-toolset-14` -> `/opt/rh/gcc-toolset-14/`)
* RHEL-based 10 ships with g++ (14 by default)
* OpenSuse Leap and Tumbleweed ship with g++ 15 https://software.opensuse.org/package/gcc15-c++
Obviously, downloading pre-compiled releases or compiling previous release branches is unaffected by this change.
ACKs for top commit:
janb84:
re-ACK fa9f29a4a7
TheCharlatan:
Re-ACK fa9f29a4a7
hebasto:
ACK fa9f29a4a7.
Tree-SHA512: ce14ecf78ccfe4f221dcbc9147dcfc00c0512b23a6fcda5ba71b62b4f5d39a5139f083d035113f189bfbd396d485e1ebc626a9a16b6fa0b74fd95aed2041c841
7a4901c902 test, refactor: Fix `-Warray-bounds` warning (Hennadii Stepanov)
faf2759c8c 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 https://github.com/bitcoin/bitcoin/pull/33785#discussion_r2510727269
* Second change requested in commit 3b135a8fc4451c93b3ea50b3f4621e0d19f35daf
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 7a4901c902
hebasto:
ACK 7a4901c902, I have reviewed the code and it looks OK.
Tree-SHA512: 64dca52ec7b25078bf489e2d8b43e449f4968fbac14a09c66a60cdc75b513588403665f248368820694a6f72c4f7f465589d9306355239cffe35c38111929eff
f06c6e1898 guix: build for Linux HOSTS with -static-libgcc (fanquake)
1bdf4695b0 guix: patch store paths out of libunwind (fanquake)
078a72c35f 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 https://github.com/bitcoin/bitcoin/pull/33178.
ACKs for top commit:
willcl-ark:
ACK f06c6e1898
hebasto:
ACK f06c6e1898.
janb84:
Concept ACK f06c6e1898
Tree-SHA512: 79409d9044fe7a339ea8090ca0e70e1305816aa3225b41ca6e4f2fec37650206ab5a78c1b2495a27a0c6c0dd6d5f86bd696101d2d1c5ecc72c630dc34e55f7dc
66978a1a95 kernel: remove btck_chain_get_tip (stickies-v)
4dd7e6dc48 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 66978a1a95
janb84:
ACK 66978a1a95
Tree-SHA512: f583fbb7f2e3f8f23afb57732b2cbe9e1d550bfc43c9a2619895ee30c27f5f3c5cd9e4ecb7e05b1f6ab9e11c368596ec9b733d67e06cfafb12326d88e8e4dd7d
79b4c276e7 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 79b4c276e7🏑
theStack:
Tested ACK 79b4c276e7
Tree-SHA512: 2a723d9bc5d1d50a8321a4f8a8cac3da3125d373ea71e6cc9d03de07307008f58970e361490d4c34530a6a976cb078b62d0ef09b7fb321ca1cfb9249a70d99a5
4543a3bde2 Squashed 'src/minisketch/' changes from ea8f66b1ea..d1bd01e189 (Hennadii Stepanov)
Pull request description:
This PR updates the `minisketch` subtree to latest upstream, which includes:
- https://github.com/bitcoin-core/minisketch/pull/75
- https://github.com/bitcoin-core/minisketch/pull/98
ACKs for top commit:
fanquake:
ACK c235aa468b
Tree-SHA512: 856fb8b7dc2e743c9c67164023bf53faf8766079aeccc82a30c8b90c85920b31977b6a8b26e51e5485b20e445a3ca6ff806e701a53e95f70181ea30055e3528c
fad6efd3be refactor: Use STR_INTERNAL_BUG macro where possible (MarcoFalke)
fada379589 doc: Remove unused bugprone-lambda-function-name suppression (MarcoFalke)
fae1d99651 refactor: Use const reference to std::source_location (MarcoFalke)
fa5fbcd615 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 https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2486258473
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 fad6efd3be
stickies-v:
ACK fad6efd3be
hodlinator:
re-ACK fad6efd3be
Tree-SHA512: e78a2d812d5ae22e45c93db1661dafbcd22ef209b3d8d8d5f2ac514e92fd19a17c3f0a5db2ef5e7748aa2083b10c0465326eb36812e6a80e238972facd2c7e98
0698c6b494 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 0698c6b494
Tree-SHA512: 840fc1621d6fa9ad43501a3691a31cffd66c1ac8d34167f7ab0fe33e1a395198c241b3c31f3d0ebc314e28c0edb6055cc2ca3deba6408dcbd14390fd679a4803
dee7eec643 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 dee7eec643
dergoegge:
ACK dee7eec643
Tree-SHA512: 2fe5ffb6c3d06f75694646473c29b4cc9fe571f4659631ec174d444a14716771308eedeb7acab3bef7f62e9bfa8ed0462da0163b214cccdc6a9ad63bbf66d2a0
fa6db67369 ci: [refactor] Extract build_dir constant in ci-test-each-commit-exec.py (MarcoFalke)
fa95e6cdc1 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 fa6db67369, I have reviewed the code and it looks OK.
Tree-SHA512: 61a78de7bcbf42bd266cb035f354862f5d1e1235acd2a81041e3a68a4d3ab4703fa2cfc993f28e4dacaa74e3cccc9ef568d5d4526605ce5a00bcd7c347b97121
dcb56fd4cb 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 dcb56fd4cb
ryanofsky:
Code review ACK dcb56fd4cb, 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 dcb56fd4cb
TheCharlatan:
ACK dcb56fd4cb
Tree-SHA512: a03f049e1f303b174a9e5d125733b6583dfd8effa12e7b6c37bd9b2cff9541100f5f4514e80f89005c44a57d7e47804afe87aa5fdb6831f3b0cd9b01d83e42be
ec8516ceb7 test: remove obsolete `get_{key,multisig}` helpers from wallet_util.py (Sebastian Falbesoner)
Pull request description:
This small cleanup PR is a late follow-up to #31250 (commit c847dee148). These helpers are unused and wouldn't work anymore, as they call a legacy wallet RPC (`dumpprivkey`). They were only ever used for testing the `importmulti` RPC, which also doesn't exist anymore. Functional tests that need to create key pairs and derive various output script types from them can use `get_generate_key` (introduced in #16528, commit f193ea889d) instead, without involving the node.
ACKs for top commit:
rkrux:
crACK ec8516ceb7
brunoerg:
code review ACK ec8516ceb7
Tree-SHA512: cab3701f1a8fbcff0eecea4cfdc632ffac226afd2eefe3c9274a84ee1bb71fb231a57cd0876025c714be257a249157b048b67e309b3734442c425d85cf481cf6
2bd155e6ee test: move create_malleated_version() to messages.py for reuse (Vasil Dimov)
Pull request description:
Move `create_malleated_version()` from `p2p_orphan_handling.py` to `test_framework/messages.py` so that it can be reused by other tests.
---
This is part of [#29415 Broadcast own transactions only via short-lived Tor or I2P connections](https://github.com/bitcoin/bitcoin/pull/29415). Putting it in its own PR to reduce the size of #29415 and because it does not depend on the other commits from there.
ACKs for top commit:
maflcko:
review ACK 2bd155e6ee🍨
l0rinc:
ACK 2bd155e6ee
brunoerg:
ACK 2bd155e6ee
w0xlt:
Code Review ACK https://github.com/bitcoin/bitcoin/pull/33793/commits/2bd155e6ee7e3cabd76083ac921b34bb45d98769
pablomartin4btc:
cr ACK 2bd155e6ee
rkrux:
crACK 2bd155e6ee
Tree-SHA512: 566bd204ebf8b7e1b06513fd86fd5a3bb8097c5e875e50758f886abdc405c02717554334b10eb77e72400d0361924824b655e558b1f06e3064d1c837252e04af
9577daa3b8 doc: Add cmake help option in Windows build instructions (frankomosh)
Pull request description:
Follow-up to #33088.
Adds `cmake -B build -LH` documentation to Windows build guides, similar to Unix build documentation.
Based on the suggestion and example provided by stickies-v in #33088, with minor adjustment to match existing indented code block format in `build-windows.md`.
Tested for:
- WSL Ubuntu with mingw-w64 cross-compilation
- Windows 11 with Visual Studio 2022 (MSVC)
ACKs for top commit:
waketraindev:
ACK 9577daa
stickies-v:
ACK 9577daa3b8
hebasto:
ACK 9577daa3b8.
pablomartin4btc:
ACK 9577daa3b8
Tree-SHA512: a9c08944aff962a61d0111317b79cbb44368e85d0255daf8d3c229c6532cf8c11046fe6bba81025ef994d92b4324247ec75f5bdaf4a89b3ef403c5be37e63bd8
038849e2e0 clang-tidy: Remove no longer needed NOLINT (Hennadii Stepanov)
Pull request description:
From https://github.com/bitcoin/bitcoin/pull/33714/files#r2491476516:
> Actually, the `NOLINT` was fixed and can be removed? You've confirmed that it is undeclared on the listed platforms, so it can't be hit by `readability-redundant-declaration`
ACKs for top commit:
maflcko:
lgtm ACK 038849e2e0
l0rinc:
I wanted to ask the same on the original PR but forgot - ACK 038849e2e0
Tree-SHA512: c0b24235425e80baeac3158c7169122364f31140367bc289430d34f01cd38f9f6a3931319f6fe4e1dc86bc4d87e21a5b4b8a2263c199e8083593f89ce592a177
5c41fa2918 guix: disable libsanitizer in Linux GCC build (fanquake)
Pull request description:
This causes issues when building against newer glibcs (i.e 2.42), and isn't needed in any case.
```bash
../../../../gcc-14.3.0/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cpp:483:31: error: invalid application of ‘sizeof’ to incomplete type ‘__sanitizer::termio’
483 | unsigned struct_termio_sz = sizeof(struct termio);
| ^~~~~~~~~~~~~~~~~~~~~
```
Extracted from #25573.
ACKs for top commit:
maflcko:
lgtm ACK 5c41fa2918
hebasto:
ACK 5c41fa2918.
willcl-ark:
utACK 5c41fa2918
Tree-SHA512: a2a7b1a72155d47a1f2a1f3270d7a8255fad432c2d5d77f139e489634a3abb0ce1152c3c19fd7df629fa17c53ddb80eb1807dd195b7b7455d12d49a09c7b92dc
This causes issues when building against newer glibcs (i.e 2.42), and isn't needed
in any case.
```bash
../../../../gcc-14.3.0/libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cpp:483:31: error: invalid application of ‘sizeof’ to incomplete type ‘__sanitizer::termio’
483 | unsigned struct_termio_sz = sizeof(struct termio);
| ^~~~~~~~~~~~~~~~~~~~~
```
Extracted from #25573.
6c7a34f3b0 kernel: Add Purpose section to header documentation (TheCharlatan)
7e9f00bcc1 kernel: Allowing reducing exports (TheCharlatan)
7990463b10 kernel: Add pure kernel bitcoin-chainstate (TheCharlatan)
36ec9a3ea2 Kernel: Add functions for working with outpoints (TheCharlatan)
5eec7fa96a kernel: Add block hash type and block tree utility functions to C header (TheCharlatan)
f5d5d1213c kernel: Add function to read block undo data from disk to C header (TheCharlatan)
09d0f62638 kernel: Add functions to read block from disk to C header (TheCharlatan)
a263a4caf2 kernel: Add function for copying block data to C header (TheCharlatan)
b30e15f432 kernel: Add functions for the block validation state to C header (TheCharlatan)
aa262da7bc kernel: Add validation interface to C header (TheCharlatan)
d27e27758d kernel: Add interrupt function to C header (TheCharlatan)
1976b13be9 kernel: Add import blocks function to C header (TheCharlatan)
a747ca1f51 kernel: Add chainstate load options for in-memory dbs in C header (TheCharlatan)
070e77732c kernel: Add options for reindexing in C header (TheCharlatan)
ad80abc73d kernel: Add block validation to C header (TheCharlatan)
cb1590b05e kernel: Add chainstate loading when instantiating a ChainstateManager (TheCharlatan)
e2c1bd3d71 kernel: Add chainstate manager option for setting worker threads (TheCharlatan)
65571c36a2 kernel: Add chainstate manager object to C header (TheCharlatan)
c62f657ba3 kernel: Add notifications context option to C header (TheCharlatan)
9e1bac4585 kernel: Add chain params context option to C header (TheCharlatan)
337ea860df kernel: Add kernel library context object (TheCharlatan)
28d679bad9 kernel: Add logging to kernel library C header (TheCharlatan)
2cf136dec4 kernel: Introduce initial kernel C header API (TheCharlatan)
Pull request description:
This is a first attempt at introducing a C header for the libbitcoinkernel library that may be used by external applications for interfacing with Bitcoin Core's validation logic. It currently is limited to operations on blocks. This is a conscious choice, since it already offers a lot of powerful functionality, but sits just on the cusp of still being reviewable scope-wise while giving some pointers on how the rest of the API could look like.
The current design was informed by the development of some tools using the C header:
* A re-implementation (part of this pull request) of [bitcoin-chainstate](https://github.com/bitcoin/bitcoin/blob/master/src/bitcoin-chainstate.cpp).
* A re-implementation of the python [block linearize](https://github.com/bitcoin/bitcoin/tree/master/contrib/linearize) scripts: https://github.com/TheCharlatan/bitcoin/tree/kernelLinearize
* A silent payment scanner: https://github.com/josibake/silent-payments-scanner
* An electrs index builder: https://github.com/josibake/electrs/commits/electrs-kernel-integration
* A rust bitcoin node: https://github.com/TheCharlatan/kernel-node
* A reindexer: https://github.com/TheCharlatan/bitcoin/tree/kernelApi_Reindexer
The library has also been used by other developers already:
* A historical block analysis tool: https://github.com/ismaelsadeeq/mining-analysis
* A swiftsync hints generator: https://github.com/theStack/swiftsync-hints-gen
* Fast script validation in floresta: https://github.com/vinteumorg/Floresta/pull/456
* A swiftsync node implementation: https://github.com/2140-dev/swiftsync/tree/master/node
Next to the C++ header also made available in this pull request, bindings for other languages are available here:
* Rust: https://github.com/TheCharlatan/rust-bitcoinkernel
* Python: https://github.com/stickies-v/py-bitcoinkernel
* Go: https://github.com/stringintech/go-bitcoinkernel
* Java: https://github.com/yuvicc/java-bitcoinkernel
The rust bindings include unit and fuzz tests for the API.
The header currently exposes logic for enabling the following functionality:
* Feature-parity with the now deprecated libbitcoin-consensus
* Optimized sha256 implementations that were not available to previous users of libbitcoin-consensus thanks to a static kernel context
* Full support for logging as well as control over categories and severity
* Feature parity with the existing experimental bitcoin-chainstate
* Traversing the block index as well as using block index entries for reading block and undo data.
* Running the chainstate in memory
* Reindexing (both full and chainstate-only)
* Interrupting long-running functions
The pull request introduces a new kernel-only test binary that purely relies on the kernel C header and the C++ standard library. This is intentionally done to show its capabilities without relying on other code inside the project. This may be relaxed to include some of the existing utilities, or even be merged into the existing test suite.
The complete docs for the API as well as some usage examples are hosted on [thecharlatan.ch/kernel-docs](https://thecharlatan.ch/kernel-docs/index.html). The docs are generated from the following repository (which also holds the examples): [github.com/TheCharlatan/kernel-docs](https://github.com/TheCharlatan/kernel-docs).
#### How can I review this PR?
Scrutinize the commit messages, run the tests, write your own little applications using the library, let your favorite code sanitizer loose on it, hook it up to your fuzzing infrastructure, profile the difference between the existing bitcoin-chainstate and the bitcoin-chainstate introduced here, be nitty on the documentation, police the C interface, opine on your own API design philosophy.
To get a feeling for the API, read through the tests, or one of the examples.
To configure this PR for making the shared library and the bitcoin-chainstate and test_kernel utilities available:
```
cmake -B build -DBUILD_KERNEL_LIB=ON -DBUILD_UTIL_CHAINSTATE=ON
```
Once compiled the library is part of the build artifacts that can be installed with:
```
cmake --install build
```
#### Why a C header (and not a C++ header)
* Shipping a shared library with a C++ header is hard, because of name mangling and an unstable ABI.
* Mature and well-supported tooling for integrating C exists for nearly every popular language.
* C offers a reasonably stable ABI
Also see https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2285719575.
#### What about versioning?
The header and library are still experimental and I would expect this to remain so for some time, so best not to worry about versioning yet.
#### Potential future additions
In future, the C header could be expanded to support (some of these have been roughly implemented):
* Handling transactions, block headers, coins cache, utxo set, meta data, and the mempool
* Adapters for an abstract coins store
* Adapters for an abstract block store
* Adapters for an abstract block tree store
* Allocators and buffers for more efficient memory usage
* An "[io-less](https://sans-io.readthedocs.io/how-to-sans-io.html)" interface
* Hooks for an external mempool, or external policy rules
#### Current drawbacks
* For external applications to read the block index of an existing Bitcoin Core node, Bitcoin Core needs to shut down first, since leveldb does not support reading across multiple processes. Other than migrating away from leveldb, there does not seem to be a solution for this problem. Such a migration is implemented in #32427.
* The fatal error handling through the notifications is awkward. This is partly improved through #29642.
* Handling shared pointers in the interfaces is unfortunate. They make ownership and freeing of the resources fuzzy and poison the interfaces with additional types and complexity. However, they seem to be an artifact of the current code that interfaces with the validation engine. The validation engine itself does not seem to make extensive use of these shared pointers.
* If multiple instances of the same type of objects are used, there is no mechanism for distinguishing the log messages produced by each of them. A potential solution is #30342.
* The background leveldb compaction thread may not finish in time leading to a non-clean exit. There seems to be nothing we can do about this, outside of patching leveldb.
ACKs for top commit:
alexanderwiederin:
re-ACK https://github.com/bitcoin/bitcoin/commit/6c7a34f3b0bd39ef7a1520aac56e12f78e5cc969
stringintech:
re-ACK 6c7a34f
laanwj:
Code review ACK 6c7a34f3b0
ismaelsadeeq:
reACK 6c7a34f3b0👾
fanquake:
ACK 6c7a34f3b0 - soon we'll be running bitcoin (kernel)
Tree-SHA512: ffe7d4581facb7017d06da8b685b81f4b5e4840576e878bb6845595021730eab808d8f9780ed0eb0d2b57f2647c85dcb36b6325180caaac469eaf339f7258030
52b1595850 depends: disable builtin variables (fanquake)
8b5a28fa78 depends: disable builtin rules and suffixes. (Dmitry Goncharov)
Pull request description:
This picks up #22126. Previously, this was more complicated to do, as depends packages (upnp, natpmp) used the rules being disabled. Those packages have since been removed.
When there is no rule to build a target in the makefile, make looks for a builtin rule. When `-r` is specified make no longer performs this lookup.
E.g. the following in an excerpt from `make -d` output. Here, make looks for a rule to build `all`.
```bash
Considering target file 'all'.
File 'all' does not exist.
Looking for an implicit rule for 'all'.
Trying pattern rule with stem 'all'.
Trying implicit prerequisite 'all.o'.
Trying pattern rule with stem 'all'.
Trying implicit prerequisite 'all.c'.
Trying pattern rule with stem 'all'.
Trying implicit prerequisite 'all.cc'.
Trying pattern rule with stem 'all'.
Trying implicit prerequisite 'all.C'.
Trying pattern rule with stem 'all'.
Trying implicit prerequisite 'all.cpp'.
Trying pattern rule with stem 'all'.
Trying implicit prerequisite 'all.p'.
Trying pattern rule with stem 'all'.
Trying implicit prerequisite 'all.f'.
...
```
Many more lines like this are omitted.
Because this build system does not use make builtin rules or suffixes, there is no benefit in having builtin rules enabled.
There are 2 benefits in having builtin rules disabled.
1. Improves performance by eliminating redundant lookups.
2. Simplifies troubleshooting by reducing the output of make `-d` or make `-p`.
Also see: https://www.gnu.org/software/make/manual/make.html#index-_002d_002dno_002dbuiltin_002drules.
ACKs for top commit:
hebasto:
re-ACK 52b1595850.
Tree-SHA512: 8b5fab5f8b01096942801d34edc47511e794d7eaf628ea78f8d2e020b83c91ed430f397dfb2b735d5fe957cdf5f10b0942fde6396f031f3ed9a773e0e6a6fabc
1e6e32fa8a ci: run native fuzz with MSAN job (fanquake)
3784d15bcd ci: use LLVM libcxx 21.1.5 (fanquake)
Pull request description:
I think this job should exist in this repo (not just qa-assets), if the alternative is double-handling changes to the interpreter. #32998 made changes which were then re-changed in #33600, to work around a false positive.
The unchached runtime of this job with `-lg` is `~32m`, with `-md` it's `~43m`.
Timeout is set to 150m, as the slow GHA runners were close to hitting a 120m limit.
ACKs for top commit:
maflcko:
lgtm ACK 1e6e32fa8a
dergoegge:
utACK 1e6e32fa8a
Tree-SHA512: afd4cb0039f4f49ddc23f5553a5bf6d5ceffbc12d91acd6890d5cc40c30b7421b23d04f305983d94c862daa6fc07535b1331d7fa2a8ebfe9f19c20d83d95c692
5d784bebaf clang-tidy: Disable `ArrayBound` check in src/ipc and src/test (Hennadii Stepanov)
5efdb0ef30 ci: Update Clang in "tidy" job (Hennadii Stepanov)
Pull request description:
This PR:
1. Updates to [IWYU 0.25](https://github.com/include-what-you-use/include-what-you-use/releases/tag/0.25), which is compatible with Clang 21.
2. Fixes new "modernize-use-default-member-init" warnings. The warning in `interpreter.cpp` is a [false positive](https://github.com/llvm/llvm-project/issues/160394), so it has been suppressed.
ACKs for top commit:
maflcko:
review ACK 5d784bebaf🎒
ryanofsky:
Code review ACK 5d784bebaf, just adding clang version comment since last review.
Tree-SHA512: a1d853675ec064170ee0f1cd16be6a900676588d4a1e7b5def8733933b140ba1a9520ec6f6a42bf7638b2ff7cf2fe4d5866d407f68b677b49d2bd68ff345f735
81e5c8385b test: cover invalid codesep positions for signature in taproot (Greg Sanders)
Pull request description:
There is some basic coverage, but I felt like adding some boundary conditions where the only issue is the codesep value would be nice.
ACKs for top commit:
ajtowns:
ACK 81e5c8385b
TheCharlatan:
ACK 81e5c8385b
Tree-SHA512: de74895c3bb49854987654720ebcefea2f47c4a55ba6ab4a52878f6a9a0bd8b3085afa3485101610327fa8d35c3d074542f58540e126460bd4bea918cb0054ee
fa9d0f994b ci: gha: Set debug_pull_request_number_str annotation (MarcoFalke)
Pull request description:
GitHub Actions does not offer any way to determine the pull request number in a machine readable way from the checks API. See https://github.com/bitcoin/bitcoin/issues/27178#issuecomment-1503475232.
However, the pull request number can be useful for external tools to act on CI results.
Fix that by using a check run annotation for a single task named `debug_pull_request_number_str`.
This should re-enable the 'CI Failed' labelling mechanism via https://github.com/maflcko/DrahtBot/commit/1f24cc1ab9be8ad35fbb3a44aaa073bf669a7685.
ACKs for top commit:
l0rinc:
code review ACK fa9d0f994b
willcl-ark:
ACK fa9d0f994b
Tree-SHA512: d872b81afeaef603006bb65f18acafdff2771acf2b70af4ab6b46167b0826e96b1ac434bba2020833107922eaf1e73f59a50782a535ba04ea16921f1828d42ca
51093d6ae1 test: resolve symlinks in which result for capnp (David Gumberg)
Pull request description:
On Fedora, `/bin/` and `/usr/bin` are symlinked, and on one of my boxes (although I could not reproduce this behavior in a docker container), `/bin` comes before `/usr/bin` in `$PATH`, so `which capnp` reports `/bin/capnp`, and `capnp_dir` is set to `/include`, and the test fails:
```console
$ ./build/test/functional/interface_ipc.py
2025-10-30T20:43:43.753812Z TestFramework (INFO): PRNG seed is: 8370468257027235753
2025-10-30T20:43:43.754163Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_b9kjzj2a
terminate called after throwing an instance of 'kj::ExceptionImpl'
what(): mp/proxy.capnp:6: failed: Import failed: /capnp/c++.capnp
Aborted (core dumped)
```
This changes the functional test to resolve any symlinks in the `capnp` binary path reported by `which`.
ACKs for top commit:
TheCharlatan:
utACK 51093d6ae1
ryanofsky:
Code review ACK 51093d6ae1
Tree-SHA512: 17a3e16c3ef50d19e65c18bd12636f287b41e54fc14629e2eb6efb8f9532af7e0e0d404e4e234eeba92473b7ae18d97144a953d28523670308e78e4c4fbb7137
78d4d36730 test: Format strings in `*.rs` (rustaceanrob)
Pull request description:
`format!` strings may contain variables within the string representation. This is a lint as of a recent `rustc` nightly version.
ACKs for top commit:
maflcko:
lgtm ACK 78d4d36730
TheCharlatan:
ACK 78d4d36730
rkrux:
crACK 78d4d36730
Tree-SHA512: d6da94682dfa35964be4d7bba323847bae040dcec921e3d4ee2f25400751fa3af40fafe27805c2d6587d00a8ff54cc6af22ca46bf8911f13a200e73e77daa019
facf8b771a ci: Add missing python3-dev package for riscv64 (MarcoFalke)
Pull request description:
This is required to compile the pip wheels on native riscv64.
ACKs for top commit:
fanquake:
ACK facf8b771a
Tree-SHA512: 7305deda4f2a7c2be5a82f4fcbc110f20a154374d98442e56d50175edda7f37a68b8e4cc1d84fc1fbc69ec1cc28559bbe795cc553fae8bd2e5effc36b0e534a2
fa4b52bd16 fuzz: refactor memcpy to std::ranges::copy to work around ubsan warn (MarcoFalke)
Pull request description:
Using std::ranges::copy from the C++ standard library has a few benefits here:
* It has the additional benefit of being a bit more type safe and document the byte cast explicitly.
* The compiler will likely optimize it to the same asm, but performance doesn't really matter here anyway.
* It has defined semantics for empty source ranges.
Fixes https://github.com/bitcoin/bitcoin/issues/33643
ACKs for top commit:
marcofleon:
tACK fa4b52bd16
dergoegge:
utACK fa4b52bd16
Tree-SHA512: 04fcf096e3cfc526e996c9313ec6e0a4d12c382fa19cb846b51564d33de2f0ef78a588fc6a936da0c76ca8bc9d9db4a824c36d99413db4f538a98239864d48f0
66667d6512 test: Use same rpc timeout for authproxy and cli (MarcoFalke)
Pull request description:
It seems odd to use different timeouts (and timeout factors) depending on whether the Python RPC proxy is used, or the bitcoin rpc command line interface.
Fix it by using the same timeout.
This can be tested by introducing a timeout error and checking it happens with and without `--usecli` after the exact same time.
Example timeout error:
```diff
diff --git a/test/functional/mining_template_verification.py b/test/functional/mining_template_verification.py
index de0833c596..e0f93a2b1e 100755
--- a/test/functional/mining_template_verification.py
+++ b/test/functional/mining_template_verification.py
@@ -173,7 +173,7 @@ class MiningTemplateVerificationTest(BitcoinTestFramework):
self.log.info("Submitting this block should succeed")
assert_equal(node.submitblock(block.serialize().hex()), None)
- node.waitforblockheight(2)
+ node.waitforblockheight(200000)
def transaction_test(self, node, block_0_height, tx):
self.log.info("make block template with a transaction")
```
Example cmd: `./bld-cmake/test/functional/mining_template_verification.py --timeout-factor=0.1 --usecli`.
ACKs for top commit:
brunoerg:
ACK 66667d6512
stickies-v:
tACK 66667d6512
Tree-SHA512: c8c21d8b9fb60ab192e3bbd45b317b96a40e10bf03704148613ac3cbdaae4abc2c03c4afbd504309ea0958201267c0d2a4bc5b40aa020917175c47e080ffe292
5fa81e239a test: add valid tx test with minimum-sized ECDSA signature (8 bytes DER-encoded) (Sebastian Falbesoner)
Pull request description:
Currently in our tests, all ECDSA signatures passing verification have sizes of 69 bytes and above (that's the DER-encoded size, i.e. counted without the sighash flag byte) [1]. This PR adds test coverage for the minimum-sized valid case of 8 bytes, by taking an interesting testnet transaction that I stumbled upon:
https://mempool.space/testnet/tx/c6c232a36395fa338da458b86ff1327395a9afc28c5d2daa4273e410089fd433
Note that this is a very obscure construction that only works because the public key used isn't contained in the locking script, but calculated and provided later at spending time (see https://bitcointalk.org/index.php?topic=1729534.msg17309060#msg17309060 for an explainer), to match the message (sighash) and picked signature. So this doesn't represent a use-case that really makes sense in practice, but it can still appear in a block (not in mempool though, due to `SCRIPT_VERIFY_CONST_SCRIPTCODE`), and having test-coverage seems useful.
Can be tested with same patch below (tests crash with the condition `>= 9`, but pass with `>= 8`).
[1] this can be verified by applying the following patch and running the tests:
```diff
diff --git a/src/pubkey.cpp b/src/pubkey.cpp
index a4ca9a170a..bee0caa603 100644
--- a/src/pubkey.cpp
+++ b/src/pubkey.cpp
@@ -288,7 +288,9 @@ bool CPubKey::Verify(const uint256 &hash, const std::vector<unsigned char>& vchS
/* libsecp256k1's ECDSA verification requires lower-S signatures, which have
* not historically been enforced in Bitcoin, so normalize them first. */
secp256k1_ecdsa_signature_normalize(secp256k1_context_static, &sig, &sig);
- return secp256k1_ecdsa_verify(secp256k1_context_static, &sig, hash.begin(), &pubkey);
+ bool ret = secp256k1_ecdsa_verify(secp256k1_context_static, &sig, hash.begin(), &pubkey);
+ if (ret) assert(vchSig.size() >= 69);
+ return ret;
}
```
ACKs for top commit:
ajtowns:
ACK 5fa81e239a lgtm
fjahr:
tACK 5fa81e239a
real-or-random:
utACK https://github.com/bitcoin/bitcoin/pull/32924/commits/5fa81e239a39d161a6d5aba7bcc7e1f22a5be777 interesting case
Tree-SHA512: d1f0612fdb71c9238ca0420f574f6f246e60dbd11970b23f21d082c759a89ff98a13b12a1f6266f14f20539ec437b7ab79322082278da32984ddfee2d8893356
fanquake