5cb1241814 feefrac: avoid integer overflow in temporary (Pieter Wuille)
Pull request description:
In `FeeFrac::Div(__int128 n, int32_t d, bool round_down)` in src/util/feefrac.h, the following line computes the result:
```c++
return quot + (mod > 0) - (mod && round_down);
```
The function can only be called under conditions where the result is in range, and thus doesn't involve any integer overflow. However, the intermediary result computed by just `quot + (mod > 0)` may still overflow if it's going to be corrected by the `- (mod && round_down)` that follows.
Fix this by balancing the two correction steps with each other first:
```c++
return quot + ((mod > 0) - (mod && round_down));
```
Fixes#32294.
ACKs for top commit:
l0rinc:
Tested ACK 5cb1241814
maflcko:
lgtm ACK 5cb1241814
achow101:
ACK 5cb1241814
Tree-SHA512: 9daaccdf9acd7652d53b52cad2dc12872558265e863acdde2d6015f885cb87c0505f9bd5be5499fc0a0eded29bec719643f6af1fbc3604518143985094226c95
e261eb8d50 tests: Add BIP 373 test vectors (Ava Chow)
26370c68d0 rpc: Include MuSig2 fields in decodepsbt (Ava Chow)
ff3d460898 psbt: Implement un/ser of musig2 fields (Ava Chow)
Pull request description:
Implements un/serialization of MuSig2 PSBT fields and prepares PSBT to be able to sign for MuSig2 inputs.
Split from #29675
ACKs for top commit:
fjahr:
re-ACK e261eb8d50
theStack:
re-ACK e261eb8d50
rkrux:
tACK e261eb8d50
Tree-SHA512: bb852ad074978847ac4dc656332025e2d4d1025d4283537b89618c7cadd61a8ecd2eff24779b8a014bc8d7b431125060449768192fa05ad0577f29e3c64b2374
2835216ec0 txgraph: make GroupClusters use partition numbers directly (optimization) (Pieter Wuille)
c72c8d5d45 txgraph: compare sequence numbers instead of Cluster* (bugfix) (Pieter Wuille)
Pull request description:
Part of cluster mempool: #30289
The implicit transaction ordering for transactions in a TxGraphImpl is defined by:
1. higher chunk feerate first
2. lower Cluster* object pointer first
3. lower position within cluster linearization first.
Number (2) is not deterministic, as it intricately depends on the heap allocation algorithm. Fix this by giving each Cluster a unique `uint64_t m_sequence` value, and sorting by those instead.
The second commit then uses this new approach to optimize GroupClusters a bit more, avoiding some repeated checks and dereferences, by making a local copy of the involved sequence numbers.
Thanks to @dergoegge for pointing this out.
ACKs for top commit:
instagibbs:
reACK 2835216ec0
marcofleon:
ACK 2835216ec0
glozow:
utACK 2835216ec0
Tree-SHA512: d772a55b9ed620159b934a42a39fca7f900d4aa89c099a280a0c61ea0bd7c4fc39b388281ffc775064ea77b0b17263871b4c9763aa71c710a79287d5eb2cd4b4
fa6a007b8e fuzz: Avoid integer sanitizer warnings in policy_estimator target (MarcoFalke)
Pull request description:
It seems odd to write a fuzz target to trigger integer sanitizer warnings in `CBlockPolicyEstimator::processBlockTx` and then suppress them. If the scenario can happen in reality, the code should be properly fixed to handle the cases. If not, it seems better to fix the fuzz target to not trigger meaningless traces.
Do that here by keeping track of the current height and limiting mempool entries to at most this entry height.
ACKs for top commit:
brunoerg:
ACK fa6a007b8e
dergoegge:
utACK fa6a007b8e
Tree-SHA512: 2092017dc309fb095fe5d43cfb76efb691795f303d567ee919be2b5cac19a944293636229903dc4d1e8b9fe5daf9dc3058544321eff1735f91f804c3baa36cd0
9ccee9cd02 doc: Document WITH_EXTERNAL_LIBMULTIPROCESS build option better (Ryan Ofsky)
Pull request description:
This includes a cmake documentation change suggested https://github.com/bitcoin/bitcoin/pull/31741#discussion_r2039716492 and another change to mention the option in markdown documentation
ACKs for top commit:
hebasto:
ACK 9ccee9cd02, changes look good.
TheCharlatan:
ACK 9ccee9cd02
Tree-SHA512: c9103b001b970ac57afedc6dc384091f5661975d569573e93003cbd7df1891c54cefb06d7296eac5b9a5c57251803dcab2bd3b26c9d81aa476c62f211dcb3d6e
27f11217ca ci: drop -priority-level from bench in win cross CI (fanquake)
Pull request description:
So there's at least one CI sanity checking all benchmarks.
Related to #32277.
ACKs for top commit:
l0rinc:
utACK 27f11217ca
hebasto:
ACK 27f11217ca.
mabu44:
utACK 27f11217ca
Tree-SHA512: 4853584bf9db418f6e31aa0f558d08bc45479d672b193e1d25a25907f82fb225bc4388321f8f23286cd9fd9168c7546c713829607eb0cf5e3c62b98e88f8e68b
faeb1babe2 ci: refactor: Use version id over version codename consistently (MarcoFalke)
fae322a43a ci: Slim down lint image (MarcoFalke)
3333273a8f ci: Bump lint imagefile FROM base (MarcoFalke)
Pull request description:
Currently, the lint_test_runner is built and installed into the lint CI image. This is problematic, because it triggers a full image build on every change to its source code. Doing a build of the lint test_runner on every run is easier and faster.
ACKs for top commit:
l0rinc:
ACK faeb1babe2
janb84:
Re- ACK [faeb1ba](faeb1babe2)
Tree-SHA512: 39103e61ec2587096213bc1ce55b80087f6f03775592827d8c96a366453b798570d912690bf96fde4685798e5fc8ee2695ce851f473b4c8782d1a4c50c65a594
7912cd4125 bench: Fix WalletMigration benchmark (pablomartin4btc)
Pull request description:
The keys and scripts created for the Legacy Wallet needed to be persisted in order for the migration to work properly.
Fixes#32277.
ACKs for top commit:
achow101:
ACK 7912cd4125
davidgumberg:
Tested ACK 7912cd4125
furszy:
utACK 7912cd4125
Tree-SHA512: fe7b8e0a80d4d030ad3fd6446717ee09a260ab2bd6140bc817bdca52d233e3af8a8fed2d754743ca2ba022f7d2c8615a36b5070991d12942c13835e8f72e359f
7a93544cdc doc: Fix and clarify description of ZMQ message format (Jiri Jakes)
Pull request description:
This change stresses that all ZMQ messages share the same structure and that they differ only in the format of the bodies. Previously this was not clear.
Further it removes the notion of endianness of 32-byte hashes, as it was misleading, and replaces it with the term 'reversed byte order' (as opposed to natural or normal byte order produced by hashing functions).
Additionally, it states that ZMQ 32-byte hashes are in the same format as in RPC. Previously it incorrectly stated that the two were in different formats.
[Rendered](https://github.com/jirijakes/bitcoin/blob/zmq-doc/doc/zmq.md).
Fixes#31856.
ACKs for top commit:
w0xlt:
Code review ACK 7a93544cdc
achow101:
ACK 7a93544cdc
ryanofsky:
Code review ACK 7a93544cdc. Nice changes. Documentation seems less repetitive and easier to understand now
Tree-SHA512: 8c5ab047c5fd9b5b6910d691b725886d7743dfd01510735b46e43d01c2d0d25ec52d79d71ec75dbeb142e96a88ad503d69ee14b971e3cdaeb8fd85e5292a8c21
a4041c77f0 test: Handle empty string returned by CLI as None in RPC tests (Brandon Odiwuor)
Pull request description:
Partially Fixes https://github.com/bitcoin/bitcoin/issues/32264
Some tests are failing when `bitcoin-cli` returns an empty string. This change treats an empty response as `None`. See https://github.com/bitcoin/bitcoin/issues/32264#issuecomment-2807616694
This fixes the error for:
- feature_bip68_sequence.py
- feature_nulldummy.py
- feature_signet.py
- mining_mainnet.py
- rpc_scanblocks.py
- rpc_scantxoutset.py
- wallet_descriptor.py --descriptors
ACKs for top commit:
maflcko:
lgtm ACK a4041c77f0
achow101:
ACK a4041c77f0
pablomartin4btc:
ACK a4041c77f0
mzumsande:
ACK a4041c77f0
Tree-SHA512: 2f1a416a18e0b3eebdb014c2e2e8dadf1d46b15c231cb61f577d47f5e551994ab0e2aeb7c179c01be7c1f07ebc03476236d29cf2d04c358ffb1fae985aa385c9
32dcec269b rpc: update RPC help of `createpsbt` (rkrux)
931117a46f rpc: update the doc for `data` field in `outputs` argument (rkrux)
8134a6b5d4 rpc: add cli example for `walletcreatefundedpsbt` RPC (rkrux)
Pull request description:
### add cli example for `walletcreatefundedpsbt` and `createpsbt` RPCs
The only example present earlier was one that creates an OP_RETURN output. This
lack of examples has discouraged me earlier to use this RPC. Adding an example
that creates PSBT sending bitcoin to address, a scenario that is much more common.
### rpc: update the doc for `data` field in `outputs` argument
It was not evident to me that this field creates an `OP_RETURN` output until
I read the code and tried it out. Thus, making the doc explicitly mention it.
This affects docs of the following RPCs:
`bumpfee`, `psbtbumpfee`, `send`, `walletcreatefundedpsbt`, `createpsbt`,
and `createrawtransaction`
ACKs for top commit:
sipa:
utACK 32dcec269b
1440000bytes:
utACK 32dcec269b
achow101:
ACK 32dcec269b
ryanofsky:
Concept ACK 32dcec269b. These seem like helpful clarifications, but I did not look into the details
Tree-SHA512: f994488ba7d52d00960fc52064bb419cf548e29822fe23d6ee0452fdf514dd93f089145eddb32b8086a7918cf8cf33a4c3f16bfcb7948f3c9d5afd95e8d3a1cb
7749d929a0 Remove support for RNDR/RNDRRS for aarch64 on Linux (laanwj)
Pull request description:
This hardware feature is
- Rarely supported on SoCs (and broken on like half of the chips that support it in the first place) (#31817). It is not clear if, or how, the brokenness will be worked around in the kernel, but working around it in user space seems the wrong thing to do, this is not the place to maintain special workarounds for specific hardware (which despite that, was attempted in #31826, but had to be reverted in #31908 due to other problems).
- Apparently not compiled into the release binary anymore (https://github.com/bitcoin/bitcoin/issues/31817#issuecomment-2795885962). Did check this at the time, but a build system change must have caused this, and went undetected.
- Hard to test in CI (as well as manually), due to unavailability of hardware.
Better to remove it.
This reverts commit aee5404e02 from #26839.
Closes#31817.
ACKs for top commit:
sipa:
utACK 7749d929a0
davidgumberg:
utACK 7749d929a0
achow101:
ACK 7749d929a0
w0xlt:
utACK 7749d929a0
Tree-SHA512: d243ad7f745fb46f711f24b6983d9ea1d94e5d8ee60959229bafdba5caa210a60801a1c2cb5b558a0e72f365371b32285aee9a8d0cd24a60589adc7b03dd6a44
05117e6e17 rpc: clarify longpoll behavior (Sjors Provoost)
5315278e7c Have createNewBlock() wait for a tip (Sjors Provoost)
64a2795fd4 rpc: handle shutdown during long poll and wait methods (Sjors Provoost)
a3bf43343f rpc: drop unneeded IsRPCRunning() guards (Sjors Provoost)
f9cf8bd0ab Handle negative timeout for waitTipChanged() (Sjors Provoost)
Pull request description:
This PR prevents Mining interface methods from sometimes crashing when called during startup before a tip is connected. It also makes other improvements like making more RPC methods usable from the GUI. Specifically this PR:
- Adds an `Assume` check to disallow passing negative timeout values to `Mining::waitTipChanged`
- Makes `waitfornewblock`, `waitforblock` and `waitforblockheight` RPC methods usable from the GUI when `-server=1` is not set.
- Changes `Mining::waitTipChanged` to return `optional<BlockRef>` instead of `BlockRef` and return `nullopt` instead of crashing if there is a timeout or if the node is shut down before a tip is connected.
- Changes `Mining::waitTipChanged` to not time out before a tip is connected, so it is convenient and safe to call during startup, and only returns `nullopt` on early shutdowns.
- Changes `Mining::createNewBlock` to block and wait for a tip to be connected if it is called on startup instead of crashing. Also documents that it will return null on early shutdowns.
This allows `waitNext()` (added in https://github.com/bitcoin/bitcoin/pull/31283) to safely assume `TipBlock()` isn't `null`, not even during a scenario of early shutdown.
Finally this PR clarifies long poll behaviour, mostly by adding code comments, but also through an early `break`.
ACKs for top commit:
achow101:
ACK 05117e6e17
ryanofsky:
Code review ACK 05117e6e17, just updated a commit message since last review
TheCharlatan:
ACK 05117e6e17
vasild:
ACK 05117e6e17
Tree-SHA512: 277c285a6e73dfff88fd379298190b264254996f98b93c91c062986ab35c2aa5e1fbfec4cd71d7b29dc2d68e33f252b5cfc501345f54939d6bd78599b71fec04
fa21f83d29 ci: Use G++ in valgrind tasks (MarcoFalke)
fabd05bf65 refactor: Fix net_processing iwyu includes (MarcoFalke)
fa1622db20 refactor: Make node_id a const& in RemoveBlockRequest (MarcoFalke)
Pull request description:
Currently, `valgrind` is not usable on a default build with GCC. Specifically, `p2p_compactblocks.py --valgrind` gives a false-positive in `RemoveBlockRequest` when comparing `node_id` with `from_peer`. According to the upstream bug report, this happens because both symbols are on the stack and the compiler can more aggressively optimize the compare (order). See https://bugs.kde.org/show_bug.cgi?id=472329#c7
It is possible to work around this bug by pulling at least one value from the stack. For example, by making `from_peer` a `const` reference. Alternatively, by replacing `auto [node_id, list_it]` with `const auto& [node_id, list_it]`, which is done here.
I think this workaround is acceptable, because it does not look like valgrind can trivially fix this. The alternative would be to add a (temporary?) suppression.
Fixes https://github.com/bitcoin/bitcoin/issues/27741
Also, fix iwyu includes, while touching this module.
Also, switch the CI valgrind scripts to use G++.
ACKs for top commit:
achow101:
ACK fa21f83d29
TheCharlatan:
ACK fa21f83d29
darosior:
utACK fa21f83d29
ryanofsky:
Code review ACK fa21f83d29. Code changes all look good but I'm a little confused about purpose of the third commit, so left a question about that
Tree-SHA512: 7b92cdafd525a5ac53ae2c1a7a92e599bc9b5fd5d315a694b493cd5079ac323d884393b57aa18581b7789247a588c9a27d47698de25b340bc76fc9f1dd1850b4
The obfuscation (XOR) operations are currently done byte-by-byte during serialization. Buffering the reads will enable batching the obfuscation operations later.
Different operating systems handle file caching differently, so reading larger batches (and processing them from memory) is measurably faster, likely because of fewer native fread calls and reduced lock contention.
Note that `ReadRawBlock` doesn't need buffering since it already reads the whole block directly.
Unlike `ReadBlockUndo`, the new `ReadBlock` implementation delegates to `ReadRawBlock`, which uses more memory than a buffered alternative but results in slightly simpler code and a small performance increase (~0.4%). This approach also clearly documents that `ReadRawBlock` is a logical subset of `ReadBlock` functionality.
The current implementation, which iterates over a fixed-size buffer, provides a more general alternative to Cory Fields' solution of reading the entire block size in advance.
Buffer sizes were selected based on benchmarking to ensure the buffered reader produces performance similar to reading the whole block into memory. Smaller buffers were slower, while larger ones showed diminishing returns.
------
> macOS Sequoia 15.3.1
> C++ compiler .......................... Clang 19.1.7
> cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ && cmake --build build -j$(nproc) && build/bin/bench_bitcoin -filter='ReadBlockBench' -min-time=10000
Before:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 2,271,441.67 | 440.25 | 0.1% | 11.00 | `ReadBlockBench`
After:
| ns/op | op/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 1,738,971.29 | 575.05 | 0.2% | 10.97 | `ReadBlockBench`
------
> Ubuntu 24.04.2 LTS
> C++ compiler .......................... GNU 13.3.0
> cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ && cmake --build build -j$(nproc) && build/bin/bench_bitcoin -filter='ReadBlockBench' -min-time=20000
Before:
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 6,895,987.11 | 145.01 | 0.0% | 71,055,269.86 | 23,977,374.37 | 2.963 | 5,074,828.78 | 0.4% | 22.00 | `ReadBlockBench`
After:
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 5,771,882.71 | 173.25 | 0.0% | 65,741,889.82 | 20,453,232.33 | 3.214 | 3,971,321.75 | 0.3% | 22.01 | `ReadBlockBench`
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: Martin Leitner-Ankerl <martin.ankerl@gmail.com>
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
Made every OpenBlockFile#fReadOnly value explicit.
Replaced hard-coded values in ReadRawBlock with STORAGE_HEADER_BYTES.
Changed `STORAGE_HEADER_BYTES` and `UNDO_DATA_DISK_OVERHEAD` to `uint32_t` to avoid casts.
Also added `LIFETIMEBOUND` to the `AutoFile` parameter of `BufferedFile`, which stores a reference to the underlying `AutoFile`, allowing Clang to emit warnings if the referenced `AutoFile` might be destroyed while `BufferedFile` still exists.
Without this attribute, code with lifetime violations wouldn't trigger compiler warnings.
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Renames the constant to be less verbose and better reflect its purpose:
it represents the size of the storage header that precedes serialized block data on disk,
not to be confused with a block's own header.
-BEGIN VERIFY SCRIPT-
git grep -q "STORAGE_HEADER_BYTES" $(git ls-files) && echo "Error: Target name STORAGE_HEADER_BYTES already exists in the codebase" && exit 1
sed -i 's/BLOCK_SERIALIZATION_HEADER_SIZE/STORAGE_HEADER_BYTES/g' $(git grep -l 'BLOCK_SERIALIZATION_HEADER_SIZE')
-END VERIFY SCRIPT-
`AutoFile{OpenUndoFile(pos)}` was still in scope when `FlushUndoFile(pos.nFile)` was called, which could lead to file handle conflicts or other unexpected behavior.
Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Reorganized error handling in block-related operations by grouping related operations together within the same scope.
In `ReadBlockUndo()` and `ReadBlock()`, moved all deserialization operations, comments and checksum verification inside a single try/catch block for cleaner error handling.
In `WriteBlockUndo()`, consolidated hash calculation and data writing operations within a common block to better express their logical relationship.
8fe001d597 doc: Updates how to reproduce fuzz CI failure locally (Sergi Delgado Segura)
Pull request description:
The current version of the doc does not explain how to reproduce a recent fuzzing CI failure (not yet part of the corpora). Add instructions on how to manually create a crash file based on a report.
ACKs for top commit:
maflcko:
lgtm ACK 8fe001d597
glozow:
ACK 8fe001d597
Tree-SHA512: 7436d71a30bbbffc34770027f1deeacca2de528d8d1b333431d6070c2ba779ecfcdaf25dc791d2154ba4dd37824d06aed2695a8412d7ca1f29e5bd1796d42aeb
Currently, the lint_test_runner is built and installed into the lint CI
image. This is problematic, because it triggers a full image build on
every change to its source code. Doing a build of the lint test_runner
on every run is easier and faster.
babb9f5db6 depends: remove non-native libmultiprocess build (Cory Fields)
5d105fb8c3 depends: Switch libmultiprocess packages to use local git subtree (Ryan Ofsky)
9b35518d2f depends, moveonly: split up int_get_build_id function (Ryan Ofsky)
2d373e2707 lint: Add exclusions for libmultiprocess subtree (Ryan Ofsky)
e88ab394c1 doc: Update documentation to explain libmultiprocess subtree (Ryan Ofsky)
d4bc563982 cmake: Fix clang-tidy "no input files" errors (Ryan Ofsky)
abdf3cb645 cmake: Fix warnings from boost headers (Ryan Ofsky)
8532fcb1c3 cmake: Fix ctest mptest "Unable to find executable" errors (Ryan Ofsky)
d597ab1dee cmake: Support building with libmultiprocess subtree (Ryan Ofsky)
69f0d4adb7 scripted-diff: s/WITH_MULTIPROCESS/ENABLE_IPC/ in cmake (Ryan Ofsky)
a2f28e4be9 Squashed 'src/ipc/libmultiprocess/' content from commit 35944ffd23fa (Ryan Ofsky)
d6244f85c5 depends: Update libmultiprocess library to simplify cmake subtree build (Ryan Ofsky)
Pull request description:
This adds the [libmultiprocess](https://github.com/chaincodelabs/libmultiprocess) library and code generator as a subtree in `src/ipc/libmultiprocess` and allows it to be built with the cmake `-DENABLE_IPC` option, which is disabled by default.
This PR does not entirely remove the depends system [libmultiprocess package](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/native_libmultiprocess.mk) because the package is useful when cross compiling. (A cross-compiling cmake build cannot easily build and run a native code generation tool.) However, it does update the depends package to build from the new git subtree, instead of being downloaded separately from github, so the same sources are used to build both the runtime library and the code generator.
This PR includes the following manual changes (not created automatically with `git subtree add`) which just update the build system and documentation:
- [`d6244f85c509` depends: Update libmultiprocess library to simplify cmake subtree build](d6244f85c5)
- [`69f0d4adb72c` scripted-diff: s/WITH_MULTIPROCESS/ENABLE_IPC/ in cmake](69f0d4adb7)
- [`d597ab1dee6b` cmake: Support building with libmultiprocess subtree](d597ab1dee)
- [`8532fcb1c30d` cmake: Fix ctest mptest "Unable to find executable" errors](8532fcb1c3)
- [`abdf3cb6456f` cmake: Fix warnings from boost headers](abdf3cb645)
- [`d4bc5639829f` cmake: Fix clang-tidy "no input files" errors](d4bc563982)
- [`e88ab394c163` doc: Update documentation to explain libmultiprocess subtree](e88ab394c1)
- [`2d373e27071f` lint: Add exclusions for libmultiprocess subtree](2d373e2707)
- [`9b35518d2f3f` depends, moveonly: split up int_get_build_id function](9b35518d2f)
- [`5d105fb8c3ff` depends: Switch libmultiprocess packages to use local git subtree](5d105fb8c3)
- [`babb9f5db641` depends: remove non-native libmultiprocess build](babb9f5db6)
---
Previous minisketch subtree PR #23114 may be useful for comparison
Instructions for subtree verification can be found:
- https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#subtrees
- https://github.com/bitcoin/bitcoin/tree/master/test/lint#git-subtree-checksh
TL&DR:
```sh
git remote add --fetch libmultiprocess https://github.com/chaincodelabs/libmultiprocess.git
test/lint/git-subtree-check.sh -r src/ipc/libmultiprocess
```
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
Sjors:
re-ACK babb9f5db6
TheCharlatan:
tACK babb9f5db6
vasild:
ACK babb9f5db6
Tree-SHA512: 43d4eecca5aab63e55c613de935965666eaced327f9fe859a0e9c9b85f7685dc16c5c8d6e03e09ca998628c5d468633f4f743529930b037049abe8e0101e0143
This hardware feature is
- rarely supported on SoCs (and broken on like half of the chips that support it in the first place) (#31817)
- apparently not compiled into the release binary (https://github.com/bitcoin/bitcoin/issues/31817#issuecomment-2795885962)
- hard to test in CI, due to unavailable of hardware
Better to remove it.
This reverts commit aee5404e02.
Closes#31817.
ff0194a7ce miniscript: convert non-critical asserts to CHECK_NONFATAL (Antoine Poinsot)
Pull request description:
The Miniscript code contains assertions to prevent ending up in an insane state or prevent UB, but also to enforce logical invariants. For the latter it is not necessary to crash the program if they are broken. Raising an exception suffices, especially as this code is often called through the RPC interface which can in turn handle the exception and the user can report it to developers.
This revives #28678 from Pieter Wuille.
ACKs for top commit:
hodlinator:
ACK ff0194a7ce
TheCharlatan:
ACK ff0194a7ce
brunoerg:
code review ACK ff0194a7ce
Tree-SHA512: 8ed8f7b494e46ecf7cdebe75120cd0ffe543b6bc289bf882dac631fe2ec2cae590d5f7bc2316e52db085791694b136dffbc71c40c1e16886fa53ab00bd8cabd0
ec81a72b36 net: Add randomized prefix to Tor stream isolation credentials (laanwj)
c47f81e8ac net: Rename `_randomize_credentials` Proxy parameter to `tor_stream_isolation` (laanwj)
Pull request description:
Add a class TorsStreamIsolationCredentialsGenerator that generates unique credentials based on a randomly generated session prefix and an atomic counter. Use this in `ConnectThroughProxy` instead of a simple atomic int counter.
This makes sure that different launches of the application won't share the same credentials, and thus circuits, even in edge cases.
Example with `-debug=proxy`:
```
2025-03-31T16:30:27Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-0:0afb2da441f5c105-0
2025-03-31T16:30:31Z [proxy] SOCKS5 sending proxy authentication 0afb2da441f5c105-1:0afb2da441f5c105-1
```
Thanks to hodlinator in https://github.com/bitcoin/bitcoin/pull/32166#discussion_r2020973352 for the idea.
ACKs for top commit:
hodlinator:
re-ACK ec81a72b36
jonatack:
ACK ec81a72b36
danielabrozzoni:
tACK ec81a72b36
Tree-SHA512: 195f5885fade77545977b91bdc41394234ae575679cb61631341df443fd8482cd74650104e323c7dbfff7826b10ad61692cca1284d6810f84500a3488f46597a
The current version of the doc does not explain how to reproduce a recent fuzzing CI failure
(not yet part of the corpora). Add instructions on how to manually create a crash file based
on a report.
faa3ce3199 fuzz: Avoid influence on the global RNG from peerman m_rng (MarcoFalke)
faf4c1b6fc fuzz: Disable unused validation interface and scheduler in p2p_headers_presync (MarcoFalke)
fafaca6cbc fuzz: Avoid setting the mock-time twice (MarcoFalke)
fad22149f4 refactor: Use MockableSteadyClock in ReportHeadersPresync (MarcoFalke)
fa9c38794e test: Introduce MockableSteadyClock::mock_time_point and ElapseSteady helper (MarcoFalke)
faf2d512c5 fuzz: Move global node id counter along with other global state (MarcoFalke)
fa98455e4b fuzz: Set ignore_incoming_txs in p2p_headers_presync (MarcoFalke)
faf2e238fb fuzz: Shuffle files before testing them (MarcoFalke)
Pull request description:
This should make the `p2p_headers_presync` fuzz target more deterministic.
Tracking issue: https://github.com/bitcoin/bitcoin/issues/29018.
The first commits adds an `ElapseSteady` helper and type aliases. The second commit uses those helpers in `ReportHeadersPresync` and in the fuzz target to increase determinism.
### Testing
It can be tested via (setting 32 parallel threads):
```
cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../b-c-qa-assets/fuzz_corpora/ p2p_headers_presync 32
```
The failing diff is contained in the commit messages, if applicable.
ACKs for top commit:
Crypt-iQ:
tACK faa3ce3199
janb84:
Re-ACK [faa3ce3](faa3ce3199)
marcofleon:
ACK faa3ce3199
Tree-SHA512: 7e2e0ddf3b4e818300373d6906384df57a87f1eeb507fa43de1ba88cf03c8e6752a26b6e91bfb3ee26a21efcaf1d0d9eaf70d311d1637b671965ef4cb96e6b59
868816d962 refactor: Remove SetHexDeprecated (marcofleon)
6b63218ec2 qt: Update SetHexDeprecated to FromHex (marcofleon)
Pull request description:
This is part of https://github.com/bitcoin/bitcoin/pull/32189. I'm separating this out because it's not immediately obvious that it's just a refactor. `SetHexDeprecated()` doesn't do any correctness checks on the input, while `FromHex()` does, so it's theoretically possible that there's a behavior change.
Replaces `uint256::SetHexDeprecated()` calls with `Txid::FromHex()` in four locations:
- `TransactionTableModel::updateTransaction`
- `TransactionView::contextualMenu`
- `TransactionView::abandonTx`
- `TransactionView::bumpFee`
The input strings in these cases aren't user input, so they should only be valid hex strings from `GetHex()` (through `TransactionRecord::getTxHash()`). These conversions should be safe without additional checks.
ACKs for top commit:
laanwj:
Code review ACK 868816d962
w0xlt:
Code review ACK 868816d962
BrandonOdiwuor:
Code Review ACK 868816d962
TheCharlatan:
ACK 868816d962
hebasto:
ACK 868816d962, I have reviewed the code and it looks OK.
Tree-SHA512: 121f149dcc7358231d0327cb3212ec96486a88410174d3c74ab8cbd61bad35185bc0a9740d534492b714811f72a6736bc7ac6eeae590c0ea1365c61cc791da37
