Commit Graph

44742 Commits

Author SHA1 Message Date
Hennadii Stepanov
938208d91a build: Resolve @rpath in macdeployqtplus
Extend `getFrameworks()` with an optional `rpath` parameter to replace
`@rpath` in dependency paths. This fixes resolution of framework
dependencies when using Homebrew's `qt@6` package.
2025-04-16 13:16:17 +01:00
Eval EXEC
bcaa23a2b7 torcontrol: Fix addrOnion outdate comment
Signed-off-by: Eval EXEC <execvy@gmail.com>
2025-04-16 19:59:52 +08:00
Brandon Odiwuor
a4041c77f0 test: Handle empty string returned by CLI as None in RPC tests 2025-04-16 14:18:48 +03:00
Sjors Provoost
abe43dfadd doc: release note for #27826 2025-04-15 08:04:50 -04:00
Sjors Provoost
f9fa28788e Use LogBlockHeader for compact blocks
The only behavior change is that the block height is now added to the log message.
2025-04-15 08:04:50 -04:00
Sjors Provoost
bad7c91479 Log which peer sent us a header
This also supports -logips.
2025-04-15 08:04:50 -04:00
Sjors Provoost
9d3e39c29c Log block header in net_processing
Previously ChainstateManager::AcceptBlockHeader would log when it
saw a new header. This commit moves logging to the call site(s) in
net_processing. The next commits will then log which peer sent it
and whether it was part of a compact block.

This commit changes behavior:
- when multiple headers are received in a single message, only the
  last one is logged
- if any of the headers are invalid, the valid ones are not logged

This happens because net_processing calls ProcessNewBlockHeaders
with multiple headers, which then calls AcceptBlockHeader one
header at a time.

Additionally:
- when the header is received via a compact block, there's no more
  duplicate log (a later commit also unifies logging code paths)
2025-04-15 08:04:49 -04:00
merge-script
cdc32994fe Merge bitcoin/bitcoin#32272: [doc] archive 29.0 release notes
12dc507c4a [doc] archive 29.0 release notes (glozow)

Pull request description:

ACKs for top commit:
  achow101:
    ACK 12dc507c4a
  janb84:
    ACK [12dc507](12dc507c4a)

Tree-SHA512: 61aa6c1892d70c53ad4600d17dd8bc23bd181cff48aaa432272ce5dfb4f9b24e6f558629513ba731060b2748877f50b2731c3dc7f3fc437a76f9922a0d26c7cc
2025-04-15 11:07:52 +01:00
MarcoFalke
faeb1babe2 ci: refactor: Use version id over version codename consistently
Also, remove the minor version from the comment, because it does not
matter and may even change at any time.
2025-04-15 09:43:59 +02:00
glozow
12dc507c4a [doc] archive 29.0 release notes 2025-04-14 21:02:18 -04:00
Ava Chow
acee5c59e6 descriptors: Have GetPrivKey fill keys directly
Instead of GetPrivKey returning a key and having the caller fill the
FlatSigningProvider, have GetPrivKey take the FlatSigningProvider and
fill it by itself. This will be necessary for descriptors such as
musig() where there are private keys that need to be added to the
FlatSigningProvider but do not directly appear in any resulting scripts.

GetPrivKey is now changed to void as the caller no longer cares whether
it succeeds or fails.
2025-04-14 16:32:01 -07:00
Ava Chow
4b0303197e descriptors: Move FlatSigningProvider pubkey filling to GetPubKey
Instead of MakeScripts inconsistently filling the output
FlatSigningProvider with the pubkeys involved, just do it in GetPubKey.
2025-04-14 16:32:01 -07:00
Ava Chow
25a3b9b0f5 descriptors: Have GetPubKey fill origins directly
Instead of having ExpandHelper fill in the origins in the
FlatSigningProvider output, have GetPubKey do it by itself. This reduces
the extra variables needed in order to track and set origins in
ExpandHelper.

Also changes GetPubKey to return a std::optional<CPubKey> rather than
using a bool and output parameters.
2025-04-14 16:32:01 -07:00
Ava Chow
6268bde0af descriptor: Remove unused parent_info from BIP32PUbKeyProvider::GetPubKey 2025-04-14 16:32:01 -07:00
Ava Chow
0ff072caa1 wallet, rpc: Only allow keypool import from single key descriptors
Legacy wallets should only import keys to the keypool if they came in a
single key descriptor. Instead of relying on assumptions about the
descriptor based on how many pubkeys show up after expanding the
descriptor, explicitly mark descriptors as being single key type and use
that for the check.
2025-04-14 16:32:01 -07:00
Ava Chow
99a4ddf5ab Merge bitcoin/bitcoin#31785: Have createNewBlock() wait for tip, make rpc handle shutdown during long poll and wait methods
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
2025-04-14 14:39:57 -07:00
Ava Chow
22770ce8cb Merge bitcoin/bitcoin#31282: refactor: Make node_id a const& in RemoveBlockRequest
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
2025-04-14 14:22:56 -07:00
Ryan Ofsky
9ccee9cd02 doc: Document WITH_EXTERNAL_LIBMULTIPROCESS build option better
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

Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
2025-04-14 16:32:37 -04:00
Lőrinc
8d801e3efb optimization: bulk serialization writes in WriteBlockUndo and WriteBlock
Similarly to the serialization reads optimization, buffered writes will enable batched XOR calculations.
This is especially beneficial since the current implementation requires copying the write input's `std::span` to perform obfuscation.
Batching allows us to apply XOR operations on the internal buffer instead, reducing unnecessary data copying and improving performance.

------

> 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='WriteBlockBench' -min-time=10000

Before:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|        5,149,564.31 |              194.19 |    0.8% |     10.95 | `WriteBlockBench`

After:

|               ns/op |                op/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|        2,990,564.63 |              334.39 |    1.5% |     11.27 | `WriteBlockBench`

------

> 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='WriteBlockBench' -min-time=20000

Before:

|               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
|        5,152,973.58 |              194.06 |    2.2% |   19,350,886.41 |    8,784,539.75 |  2.203 |   3,079,335.21 |    0.4% |     23.18 | `WriteBlockBench`

After:

|               ns/op |                op/s |    err% |          ins/op |          cyc/op |    IPC |         bra/op |   miss% |     total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
|        4,145,681.13 |              241.21 |    4.0% |   15,337,596.85 |    5,732,186.47 |  2.676 |   2,239,662.64 |    0.1% |     23.94 | `WriteBlockBench`

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
2025-04-14 12:04:06 +02:00
Lőrinc
520965e293 optimization: bulk serialization reads in UndoRead, ReadBlock
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>
2025-04-14 12:04:06 +02:00
merge-script
5116655980 Merge bitcoin/bitcoin#32255: miniscript: Correct off-by-one assert guards (#31727 follow-up)
3693e4d6ee miniscript: Correct off-by-one assert guards (Hodlinator)

Pull request description:

  First instances discovered by darosior in https://github.com/bitcoin/bitcoin/pull/31727#issuecomment-2619342125.

ACKs for top commit:
  maflcko:
    lgtm ACK 3693e4d6ee
  sipa:
    ACK 3693e4d6ee
  l0rinc:
    Tested ACK 3693e4d6ee

Tree-SHA512: a41302bb9349d5ad2daf89431c67fdbe80f91690e1759d01529acd1b2fa5d99bad3383da684ee62c00584f7696cd3f87efff084c74edf52eb7cd88d60ee99829
2025-04-14 10:59:07 +01:00
Lőrinc
056cb3c0d2 refactor: clear up blockstorage/streams in preparation for optimization
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>
2025-04-14 11:57:14 +02:00
Lőrinc
67fcc64802 log: unify error messages for (read/write)[undo]block
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
2025-04-13 23:44:46 +02:00
Lőrinc
a4de160492 scripted-diff: shorten BLOCK_SERIALIZATION_HEADER_SIZE constant
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-
2025-04-13 23:44:46 +02:00
Lőrinc
6640dd52c9 Narrow scope of undofile write to avoid possible resource management issue
`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>
2025-04-13 23:44:46 +02:00
Lőrinc
3197155f91 refactor: collect block read operations into try block
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.
2025-04-13 23:44:44 +02:00
Hodlinator
3693e4d6ee miniscript: Correct off-by-one assert guards
Co-authored-by: Antoine Poinsot <darosior@protonmail.com>
2025-04-12 09:46:56 +02:00
merge-script
817edfb21e Merge bitcoin/bitcoin#32245: doc: Updates how to reproduce fuzz CI failure locally
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
2025-04-11 16:34:41 -04:00
MarcoFalke
fae322a43a ci: Slim down lint image
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.
2025-04-11 18:56:46 +02:00
MarcoFalke
3333273a8f ci: Bump lint imagefile FROM base
According to https://wiki.debian.org/DebianReleases#Production_Releases,
the current image will be EOL in about one year, so it seems fine to
slowly bump it to something more recent.
2025-04-11 18:55:57 +02:00
Pieter Wuille
2835216ec0 txgraph: make GroupClusters use partition numbers directly (optimization) 2025-04-11 10:43:56 -04:00
Pieter Wuille
c72c8d5d45 txgraph: compare sequence numbers instead of Cluster* (bugfix)
This makes fuzz testing more deterministic, by avoiding the (arbitrary) pointer
value ordering in comparing transactions.
2025-04-11 10:43:34 -04:00
merge-script
b2bb27f40c Merge bitcoin/bitcoin#31741: multiprocess: Add libmultiprocess git subtree
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
2025-04-11 13:40:31 +01:00
laanwj
7749d929a0 Remove support for RNDR/RNDRRS for aarch64 on Linux
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.
2025-04-11 08:11:41 +02:00
merge-script
a4fd565191 Merge bitcoin/bitcoin#31727: miniscript: convert non-critical asserts to CHECK_NONFATAL
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
2025-04-10 14:06:21 -04:00
merge-script
e364e6b509 Merge bitcoin/bitcoin#32176: net: Prevent accidental circuit sharing when using Tor stream isolation
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
2025-04-10 12:42:34 -04:00
Sergi Delgado Segura
8fe001d597 doc: Updates how to reproduce fuzz CI failure locally
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.
2025-04-10 11:10:41 -04:00
glozow
c58ae197a3 Merge bitcoin/bitcoin#32198: fuzz: Make p2p_headers_presync more deterministic
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
2025-04-10 11:08:11 -04:00
Hennadii Stepanov
e1dfa4faeb Merge bitcoin/bitcoin#32237: qt: Update SetHexDeprecated to FromHex
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
2025-04-10 12:30:14 +01:00
Ava Chow
b8cefeb221 Merge bitcoin/bitcoin#32149: wallet, migration: Fix empty wallet crash
0f602c5693 wallet, migration: Fix crash on empty wallet (pablomartin4btc)
42c13141b5 wallet, refactor: Decouple into HasLegacyRecords() (pablomartin4btc)

Pull request description:

  Same as with a blank wallet (#28976), wallets with no legacy records (i.e. empty, non-blank, watch-only wallet) do not require to be migrated.

  Steps to reproduce the issue:

  1.- `createwallet "empty_wo_noblank_legacy_wallet" true false "" false false`
  2.- `migratewallet`

  ```
  wallet/wallet.cpp:4071 GetDescriptorsForLegacy: Assertion `legacy_spkm' failed.
  Aborted (core dumped)
  ```

ACKs for top commit:
  davidgumberg:
    untested reACK 0f602c5693
  fjahr:
    re-ACK 0f602c5693
  achow101:
    ACK 0f602c5693
  furszy:
    ACK 0f602c5693
  BrandonOdiwuor:
    Code Review ACK 0f602c5693

Tree-SHA512: 796c3f0b1946281097f7ffc3563bc79f879e80a98237012535cc530a4a2239fd2d71a17b4f54e30258886dc9f0b83206d7a5d50312e4fc6d0abe4f559fbe07ec
2025-04-09 17:50:48 -07:00
glozow
874da961d0 Merge bitcoin/bitcoin#32240: feefrac test: avoid integer overflow (bugfix)
a2bc330da8 feefrac test: avoid integer overflow (bugfix) (Pieter Wuille)

Pull request description:

  The `feefrac_mul_div` fuzz test fails after #30535 with the following (base64) input: `Nb6Fc/97AACAAAD/ewAAgAAAAIAAAACAAAAAoA==` (see https://cirrus-ci.com/task/5240029192126464?logs=ci#L3353).

  This is caused by an internal multiplication inside `CFeeRate` that *just* exceeds the limit of the `int64_t` type. Fix that by tightening the bounds slightly further.

ACKs for top commit:
  sr-gi:
    utACK a2bc330da8
  instagibbs:
    ACK a2bc330da8
  glozow:
    ACK a2bc330da8, was able to reproduce + verify this fix

Tree-SHA512: cfbcdc8becfd518f4349ddc00c9af3ed0a23bb9534af71cc21df167d7038e5967127e5d97c4b3e8aeff6bf071c4f630c32ffaf81d8ec227954d21fdcbe205333
2025-04-09 14:26:55 -04:00
MarcoFalke
faa3ce3199 fuzz: Avoid influence on the global RNG from peerman m_rng
This should avoid the remaining non-determistic code coverage paths.

Without this patch, the tool would report a diff (only when running
without libFuzzer):

cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32
2025-04-09 20:06:56 +02:00
MarcoFalke
faf4c1b6fc fuzz: Disable unused validation interface and scheduler in p2p_headers_presync
This may also avoid non-determinism in the scheduler thread.
2025-04-09 20:05:56 +02:00
MarcoFalke
fafaca6cbc fuzz: Avoid setting the mock-time twice
It should be sufficient to set it once. Especially, if the dynamic value
is only used by ResetAndInitialize.

This also avoids non-determistic code paths, when ResetAndInitialize may
re-initialize m_next_inv_to_inbounds.

Without this patch, the tool would report a diff:

cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32

...
- 1126|      3|        m_next_inv_to_inbounds = now + m_rng.rand_exp_duration(average_interval);
- 1127|      3|    }
+ 1126|     10|        m_next_inv_to_inbounds = now + m_rng.rand_exp_duration(average_interval);
+ 1127|     10|    }
  1128|    491|    return m_next_inv_to_inbounds;
...
2025-04-09 20:05:39 +02:00
MarcoFalke
fad22149f4 refactor: Use MockableSteadyClock in ReportHeadersPresync
This allows the clock to be mockable in tests. Also, replace cs_main
with GetMutex() while touching this function.

Also, use the ElapseSteady test helper in the p2p_headers_presync fuzz
target to make it more deterministic.

The m_last_presync_update variable is a global that is not reset in
ResetAndInitialize. However, it is only used for logging, so completely
disable it for now.

Without this patch, the tool would report a diff:

cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32

...
  4468|     81|        auto now = std::chrono::steady_clock::now();
  4469|     81|        if (now < m_last_presync_update + std::chrono::milliseconds{250}) return;
-                                                                                        ^80
+                                                                                        ^79
...
2025-04-09 20:05:36 +02:00
MarcoFalke
fa9c38794e test: Introduce MockableSteadyClock::mock_time_point and ElapseSteady helper
This refactor clarifies that the MockableSteadyClock::mock_time_point
has millisecond precision by defining a type an using it.

Moreover, a ElapseSteady helper is added which can be re-used easily.
2025-04-09 20:05:17 +02:00
MarcoFalke
faf2d512c5 fuzz: Move global node id counter along with other global state
The global m_headers_presync_stats is not reset in ResetAndInitialize.
This may lead to non-determinism.

Fix it by incrementing the global node id counter instead.

Without this patch, the tool would report a diff:

cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32

...
  2587|  3.73k|            if (best_it == m_headers_presync_stats.end()) {
   ------------------
-  |  Branch (2587:17): [True: 80, False: 3.65k]
+  |  Branch (2587:17): [True: 73, False: 3.66k]
   ------------------
...
2025-04-09 20:04:49 +02:00
MarcoFalke
fa98455e4b fuzz: Set ignore_incoming_txs in p2p_headers_presync
This avoids non-determistic code paths.

Without this patch, the tool would report a diff:

cargo run --manifest-path ./contrib/devtools/deterministic-fuzz-coverage/Cargo.toml -- $PWD/bld-cmake/ $PWD/../qa-assets/fuzz_corpora/ p2p_headers_presync 32

...
- 5371|    393|        peer.m_next_send_feefilter = current_time + m_rng.randrange<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY);
- 5372|    393|    }
+ 5371|    396|        peer.m_next_send_feefilter = current_time + m_rng.randrange<std::chrono::microseconds>(MAX_FEEFILTER_CHANGE_DELAY);
+ 5372|    396|    }
  5373|  16.2k|}
...
2025-04-09 20:04:46 +02:00
MarcoFalke
faf2e238fb fuzz: Shuffle files before testing them
When iterating over all fuzz input files in a folder, the order should
not matter.

However, shuffling may be useful to detect non-determinism.

Thus, shuffle in fuzz.cpp, when using neither libFuzzer, nor AFL.

Also, shuffle in the deterministic-fuzz-coverage tool, when using
libFuzzer.
2025-04-09 20:04:38 +02:00
marcofleon
868816d962 refactor: Remove SetHexDeprecated
After replacing all instances of `SetHexDeprecated` in the GUI,
remove it entirely and reimplement the behavior in `FromHex`.
2025-04-09 15:59:59 +01:00