Commit graph

43513 commits

Author SHA1 Message Date
Pieter Wuille
1ac1c33f3f [checkqueue] support user-defined return type through std::optional
The check type function now needs to return a std::optional<R> for some type R,
and the check queue overall will return std::nullopt if all individual checks
return that, or one of the non-nullopt values if there is at least one.

For most tests, we use R=int, but for the actual validation code, we make it return
the ScriptError.
2024-12-02 16:25:13 -05:00
Ryan Ofsky
ebe4cac38b
Merge bitcoin/bitcoin#30991: test: enable running independent functional test sub-tests
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
409d0d6293 test: enable running individual independent functional test methods (ismaelsadeeq)

Pull request description:

  - Some test methods in the functional test framework are independent and do not require any prior context or setup in `run_test`.
  - This commit adds a new option for running these specific methods within a test file, allowing them to be executed individually without running the entire test suite.
  - Using this option reduces the time you need to wait before the test you are interested in starts executing.
  - The functionality added by this PR can be achieved manually by commenting out code, but having a pragmatic option to do this is more convenient.

  Note: Running test methods that require arguments or context will fail.

  **Example Usage**:
  ```zsh
  build/test/functional/feature_reindex.py --test_methods continue_reindex_after_shutdown
  ```

  ```zsh
  build/test/functional/feature_config_args.py --test_methods test_log_buffer test_args_log test_connect_with_seednode
  ```

ACKs for top commit:
  maflcko:
    review ACK 409d0d6293
  rkrux:
    reACK 409d0d6293
  ryanofsky:
    Code review ACK 409d0d6293. This seems like a good step towards making it easy to run independent tests quickly. I think ideally there would be some naming convention or @ annotation added to test methods that can run independently, so the test framework could provide more functionality like being able to list test methods, being able to show command lines to quickly reproduce problems when tests fails, and calling test methods automatically instead of requiring individual tests to call them. But these ideas are all compatible with the new `--test_methods` option

Tree-SHA512: b0daac7c3b322e6fd9b946962335d8279e8cb004ff76f502c8d597b9c4b0073840945be198a79d44c5aaa64bda421429829d5c84ceeb8c6139eb6ed079a35878
2024-12-02 11:45:32 -05:00
merge-script
1927674100
Merge bitcoin/bitcoin#31387: doc: Use more precise anchor link to codesigning docs
19f49c7489 doc: Use more precise anchor link to codesigning docs (Jeremy Rand)

Pull request description:

  The "Codesigning" section is what users presumably are looking for when they follow this link.

ACKs for top commit:
  fanquake:
    ACK 19f49c7489

Tree-SHA512: 0e25cf0d7160db7d564d67d3e3ac614f9bd209b2399414f1278fa01cfc1ff827aa8311f7c1c2666924d5ac2dc23fe9bc258b80ed8025d5b8d5b11bcf1d12b28c
2024-12-02 14:09:54 +00:00
MarcoFalke
faf70cc994
Remove wallet::ParseISO8601DateTime, use ParseISO8601DateTime instead 2024-12-02 15:09:31 +01:00
merge-script
e043618d44
Merge bitcoin/bitcoin#31396: test: simple reordering to reduce run time
62f6d9e1a4 test: simple ordering optimization to reduce runtime (tdb3)

Pull request description:

  Noticed in #31371 that the position of `mempool_ephemeral_dust` within `BASE_SCRIPTS` was lengthening total test runtime. Instead of moving only that test, looked for others to move to reduce runtime.

  This is a quick optimization that was found to reduce overall functional test runtime of up to around 20% (depending on jobs and machine characteristics). Since it seems like test ordering could be done in many different ways, with many variables, and bike shedding could creep in, a relatively straightforward approach was taken for now that minimized changes to test_runner.

ACKs for top commit:
  maflcko:
    lgtm ACK 62f6d9e1a4
  TheCharlatan:
    ACK 62f6d9e1a4

Tree-SHA512: 6f93fbe4de3fce202383d9f84aa0e96961af3de3c02b8cab73589339d701f32c5e1b57a191eeebf4b06b5cd7a82617f63f24110732940be1a5a4d9237813a570
2024-12-02 14:08:58 +00:00
Lőrinc
50cce20013 test, refactor: Compact ccoins_access and ccoins_spend
Also added an extra check for `AccessCoin` in `CheckAccessCoin` to make sure its result is also validated.
2024-12-02 14:49:45 +01:00
Lőrinc
0a159f0914 test, refactor: Remove remaining unbounded flags from coins_tests 2024-12-02 14:49:45 +01:00
Lőrinc
c0b4b2c1ee test: Validate error messages on fail
The `ccoins_add` and `ccoins_write` tests check the actual exception error messages now instead of just that they fail for the given parameters.
This enables us testing different exceptions in a more fine-grained way in later changes.
2024-12-02 14:49:43 +01:00
Lőrinc
d5f8d607ab test: Group values and states in tests into CoinEntry wrappers
Note: that this commit affects the test order, but doesn't change its behavior or coverage otherwise.
2024-12-02 14:49:36 +01:00
merge-script
a25b892ab1
Merge bitcoin/bitcoin#31386: doc: Use more precise anchor links to Xcode SDK extraction
8bf1b3039c doc: Use more precise anchor links to Xcode SDK extraction (Jeremy Rand)

Pull request description:

  The "SDK Extraction" section is what users presumably are looking for when they follow these links.

ACKs for top commit:
  fanquake:
    ACK 8bf1b3039c

Tree-SHA512: 38669a6b171aa102bb80f5b3a343bd6a067c6921c454f6d18087c5add8016eea2ba8196036f9968f0a9b7df1f642c96ff6c657338c32e775beb04038497cde1f
2024-12-02 13:41:22 +00:00
merge-script
eb646111cd
Merge bitcoin/bitcoin#31383: test: Add missing node.setmocktime(self.mocktime) to p2p_ibd_stalling.py
faa16ed4b9 test: Add missing node.setmocktime(self.mocktime) to p2p_ibd_stalling.py (MarcoFalke)

Pull request description:

  This was forgotten by myself in commit fa5b58ea01.

  This time, there is a diff to test, which fails on current master and passes with this pull request.

  ```diff
  diff --git a/src/net_processing.cpp b/src/net_processing.cpp
  index e503a68382..16438ebd08 100644
  --- a/src/net_processing.cpp
  +++ b/src/net_processing.cpp
  @@ -112,9 +112,9 @@ static_assert(MAX_BLOCKTXN_DEPTH <= MIN_BLOCKS_TO_KEEP, "MAX_BLOCKTXN_DEPTH too
    *  want to make this a per-peer adaptive value at some point. */
   static const unsigned int BLOCK_DOWNLOAD_WINDOW = 1024;
   /** Block download timeout base, expressed in multiples of the block interval (i.e. 10 min) */
  -static constexpr double BLOCK_DOWNLOAD_TIMEOUT_BASE = 1;
  +static constexpr double BLOCK_DOWNLOAD_TIMEOUT_BASE = .05; // 30 sec
   /** Additional block download timeout per parallel downloading peer (i.e. 5 min) */
  -static constexpr double BLOCK_DOWNLOAD_TIMEOUT_PER_PEER = 0.5;
  +static constexpr double BLOCK_DOWNLOAD_TIMEOUT_PER_PEER = 0.;
   /** Maximum number of headers to announce when relaying blocks with headers message.*/
   static const unsigned int MAX_BLOCKS_TO_ANNOUNCE = 8;
   /** Minimum blocks required to signal NODE_NETWORK_LIMITED */
  diff --git a/test/functional/p2p_ibd_stalling.py b/test/functional/p2p_ibd_stalling.py
  index fa07873929..f8cdd8998c 100755
  --- a/test/functional/p2p_ibd_stalling.py
  +++ b/test/functional/p2p_ibd_stalling.py
  @@ -82,6 +82,7 @@ class P2PIBDStallingTest(BitcoinTestFramework):
           # Need to wait until 1023 blocks are received - the magic total bytes number is a workaround in lack of an rpc
           # returning the number of downloaded (but not connected) blocks.
           bytes_recv = 172761 if not self.options.v2transport else 169692
  +        time.sleep(31);
           self.wait_until(lambda: self.total_bytes_recv_for_blocks() == bytes_recv)

           self.all_sync_send_with_ping(peers)

ACKs for top commit:
  brunoerg:
    ACK faa16ed4b9

Tree-SHA512: 5a670e2dcf828ac83b721a3e20d897744cca50080b0583a8460a0d0c7bf2c2c988cf7e35f688dde6a3349f1c21cc83a16ea5242ed06a59d59a04130416690737
2024-12-02 13:34:18 +00:00
Lőrinc
ca74aa7490 test, refactor: Migrate GetCoinsMapEntry to return MaybeCoin 2024-12-02 14:24:10 +01:00
Lőrinc
15aaa81c38 coins, refactor: Remove direct GetFlags access
We don't need so much access to the internals of CCoinsCacheEntry, since many tests are just exercising invalid combinations this way.
This implies that `AddFlags` has private access now.
2024-12-02 13:52:34 +01:00
Lőrinc
6b733699cf coins, refactor: Assume state after SetClean in AddFlags to prevent dangling pointers
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-12-02 13:52:34 +01:00
Lőrinc
fc8c282022 coins, refactor: Make AddFlags, SetDirty, SetFresh static
This makes the `Assume(&self.second == this)` check redundant

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-12-02 13:52:32 +01:00
Lőrinc
cd0498eabc coins, refactor: Split up AddFlags to remove invalid states
CCoinsCacheEntry provided general access to its internal flags state, even though in reality it could only be clean, fresh, dirty or fresh|dirty.

After it got dirtied we couldn't set the state back to clean by AddFlags(0) - tests were explicitly checking against that.

This commit cleans up the public interface to make this distinction cleaner and invalid behavior impossible instead of just checked by tests.
This includes the removal of redundant `inline` qualifiers (we're inside a struct).
Also renamed `self` to `pair` to simplify the upcoming commits.

Also modernized `EmplaceCoinInternalDANGER` since it was already modified.

Co-authored-by: Andrew Toth <andrewstoth@gmail.com>
2024-12-02 13:48:04 +01:00
merge-script
6cd95de2e0
Merge bitcoin/bitcoin#31395: build: Set shared linker flags in toolchain file
a8e04704f9 build: Set shared linker flags in toolchain file (TheCharlatan)

Pull request description:

  These are required when cross-compiling shared libraries such as the kernel library.

  This was discovered after attempting to cross-compile the kernel library and running into the following error:
  ```
  [100%] Linking CXX shared library libbitcoinkernel.dylib
  /usr/bin/ld: unrecognised emulation mode: llvm
  Supported emulations: elf_x86_64 elf32_x86_64 elf_i386 elf_iamcu i386pep i386pe
  clang++: error: linker command failed with exit code 1 (use -v to see invocation)
  gmake[3]: *** [src/kernel/CMakeFiles/bitcoinkernel.dir/build.make:1209: src/kernel/libbitcoinkernel.dylib] Error 1
  gmake[2]: *** [CMakeFiles/Makefile2:1175: src/kernel/CMakeFiles/bitcoinkernel.dir/all] Error 2
  gmake[1]: *** [CMakeFiles/Makefile2:1182: src/kernel/CMakeFiles/bitcoinkernel.dir/rule] Error 2
  gmake: *** [Makefile:569: bitcoinkernel] Error 2
  ```

ACKs for top commit:
  hebasto:
    ACK a8e04704f9, tested on Ubuntu 24.04:
  tdb3:
    ACK a8e04704f9

Tree-SHA512: a896f366348647888085fdbe66ff3d21484ca3b7cba0ba5390adb55bbcfe3ec09d4f81dd514ecf360cd0d31dcc8311108c4b772cecd5ea719d1c07faef285ecf
2024-12-02 12:13:05 +00:00
merge-script
abeebccc48
Merge bitcoin/bitcoin#31357: cmake: Improve build script correctness
ab5c63edcc cmake: Build `secp256k1` only when required (Hennadii Stepanov)
76a3a540a4 cmake: Ensure script correctness when no targets are specified (Hennadii Stepanov)

Pull request description:

  When no build targets are specified, it is reasonable to expect the configuration step to succeed and produce a build system that does not build any targets.

  This PR updates the code to ensure this behaviour:
  ```
  $ cmake -B build -G "Ninja" -DBUILD_DAEMON=OFF -DBUILD_CLI=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DENABLE_WALLET=OFF -DBUILD_TESTS=OFF
  $ cmake --build build
  ninja: no work to do.
  ```

ACKs for top commit:
  TheCharlatan:
    ACK ab5c63edcc
  tdb3:
    light test ACK ab5c63edcc

Tree-SHA512: 1b13f406c58b02768d9ba831413aeae1d7e03659e7101de8e598f906ba220f479ac06707965c96a14468ce4ba49011a1ab9adee9cee34ab1e8622f690b94dad8
2024-12-02 10:52:36 +00:00
merge-script
4c9b13841c
Merge bitcoin/bitcoin#31402: doc: correct libfuzzer-nosan preset flag
16b140f225 doc: correct libfuzzer-nosan preset flag (Niklas Gögge)

Pull request description:

  `--prefix` is not the correct option for using a preset (it's not an option at all).

ACKs for top commit:
  maflcko:
    lgtm ACK 16b140f225

Tree-SHA512: 8c5fad4f8573bd9ef972447b2847ede61a3b6af9650a599f6ff7e90a2c009e4422715164261b424c08170c9e179cce241a3ca31ddc234f446316f24fc2c353b1
2024-12-02 10:31:43 +00:00
merge-script
da4f4fac8d
Merge bitcoin/bitcoin#31361: cmake, qt: Use absolute paths for includes in MOC-generated files
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
6f4128e3a8 cmake, qt: Use absolute paths for includes in MOC-generated files (Hennadii Stepanov)

Pull request description:

  Fixes https://github.com/bitcoin/bitcoin/issues/31145.

  From the `moc --help` output:
  ```
    -p <path>                         Path prefix for included file.
  ```

ACKs for top commit:
  laanwj:
    Tested ACK 6f4128e3a8
  willcl-ark:
    tACK 6f4128e3a8
  TheCharlatan:
    ACK 6f4128e3a8
  BrandonOdiwuor:
    ACK 6f4128e3a8

Tree-SHA512: f313698dec8976a7e0b3f26e6fd34ec041c253ccd75bdc0d7b272c3f786d77e83c35bc4607112960c65a378ec139f30a5187ce74498dce8b99a7349994846dee
2024-12-02 10:28:56 +00:00
Niklas Gögge
16b140f225
doc: correct libfuzzer-nosan preset flag 2024-12-02 10:27:14 +00:00
merge-script
097c66f614
Merge bitcoin/bitcoin#30039: dbwrapper: Bump LevelDB max file size to 32 MiB to avoid system slowdown from high disk cache flush rate
b73d331937 dbwrapper: Bump max file size to 32 MiB (Maciej S. Szmigiero)

Pull request description:

  The default max file size for LevelDB is 2 MiB, which results in the LevelDB compaction code generating ~4 disk cache flushes per second when syncing with the Bitcoin network.
  These disk cache flushes are triggered by `fdatasync()` syscall issued by the LevelDB compaction code when reaching the max file size.

  If the database is on a HDD this flush rate brings the whole system to a crawl.
  It also results in very slow throughput since 2 MiB * 4 flushes per second is about 8 MiB / second max throughput, while even an old HDD can pull 100 - 200 MiB / second streaming throughput.

  Increase the max file size for LevelDB to 128 MiB instead so the flush rate drops to about 1 flush / 2 seconds and the system no longer gets so sluggish.

  The max file size value chosen also matches the `MAX_BLOCKFILE_SIZE` file size setting already used by the block storage.

ACKs for top commit:
  l0rinc:
    ACK b73d331937
  davidgumberg:
    ACK b73d331937
  andrewtoth:
    ACK b73d331937
  TheCharlatan:
    ACK b73d331937
  willcl-ark:
    ACK b73d331937
  tdb3:
    ACK b73d331937
  laanwj:
    ACK b73d331937

Tree-SHA512: 5d8fb9ad1ea643fb3e42a9c59f6fc90cc5cc3b82c06d9b8d59de3a5a926fabaeb78efb51b608b1e7925f49d82dfcbd5b72c552993879789f33201efe57c278f3
2024-12-02 10:22:53 +00:00
merge-script
68daaea0e4
Merge bitcoin/bitcoin#31390: Remove src/config directory
935973b315 Remove `src/config` directory (Hennadii Stepanov)

Pull request description:

  The `src/config` directory has not been used since the migration to CMake, which disables in-source builds.

ACKs for top commit:
  TheCharlatan:
    ACK 935973b315
  BrandonOdiwuor:
    ACK 935973b315

Tree-SHA512: cc5b405e39387673fa2fd1e96680295b6eb3dc49a5f9a4d288580b8ea83efba04c96132811ca2cec14bcca509dbaf20c390cd47dabeea2a6ebc973e364e7a43b
2024-12-02 10:07:18 +00:00
merge-script
14f162dc5c
Merge bitcoin/bitcoin#31399: ci, macos: Install pkgconf Homebrew's package
e2f2698395 ci, macos: Install `pkgconf` Homebrew's package (Hennadii Stepanov)

Pull request description:

  The updated GHA image [`20241125.556`](https://github.com/actions/runner-images/releases/tag/macos-14-arm64%2F20241125.556) is now fully [deployed](https://github.com/actions/runner-images/blob/main/README.md#available-images), enabling the installation of `pkgconf` as documented in [`doc/build-osx.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md#3-install-required-dependencies).

ACKs for top commit:
  TheCharlatan:
    Lgtm ACK e2f2698395

Tree-SHA512: 9c9a3e9a3998097ed029aede61879c0e0ddd30e6bf1c36927887d7c38f3af13580efaf0fee0d75fdf344c5e92242530d4454560c62e75ba3f81c7f99d112e3f5
2024-12-02 10:04:31 +00:00
MarcoFalke
2222aecd5f
util: Implement ParseISO8601DateTime based on C++20 2024-12-02 08:21:32 +01:00
Hennadii Stepanov
e2f2698395
ci, macos: Install pkgconf Homebrew's package
The updated GHA image `20241125.556` is now fully deployed, enabling the
installation of `pkgconf` as documented in `doc/build-osx.md`.
2024-12-01 10:58:15 +00:00
Maciej S. Szmigiero
b73d331937 dbwrapper: Bump max file size to 32 MiB
The default max file size for LevelDB is 2 MiB, which results in the
LevelDB compaction code generating ~4 disk cache flushes per second when
syncing with the Bitcoin network.
These disk cache flushes are triggered by fdatasync() syscall issued by the
LevelDB compaction code when reaching the max file size.

If the database is on a HDD this flush rate brings the whole system to a
crawl.
It also results in very slow throughput since 2 MiB * 4 flushes per second
is about 8 MiB / second max throughput, while even an old HDD can pull
100 - 200 MiB / second streaming throughput.

Increase the max file size for LevelDB to 32 MiB instead so the flush rate
drops significantly and the system no longer gets so sluggish.

The new max file size value chosen is a compromise between the one that
works best for HDD and SSD performance, as determined by benchmarks done by
various people.
2024-11-30 20:19:08 +01:00
tdb3
62f6d9e1a4
test: simple ordering optimization to reduce runtime 2024-11-30 12:31:16 -05:00
TheCharlatan
a8e04704f9
build: Set shared linker flags in toolchain file
These are required when cross-compiling shared libraries such as the
kernel library.
2024-11-30 11:31:27 +01:00
glozow
dbc8ba12f3
Merge bitcoin/bitcoin#31371: doc, test: more ephemeral dust follow-ups
Some checks failed
CI / test each commit (push) Has been cancelled
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Has been cancelled
CI / macOS 14 native, arm64, fuzz (push) Has been cancelled
CI / Win64 native, VS 2022 (push) Has been cancelled
CI / Win64 native fuzz, VS 2022 (push) Has been cancelled
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Has been cancelled
160799d913 test: refactor: introduce `create_ephemeral_dust_package` helper (Sebastian Falbesoner)
61e18dec30 doc: ephemeral policy: add missing closing double quote (Sebastian Falbesoner)

Pull request description:

  This small PR contains ephemeral dust follow-ups mentioned in #30329 that were not tackled in the first follow-up PR #31279:

  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1828577696
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825279952

  Happy to add more if I missed some or anyone has concrete commits to add.

ACKs for top commit:
  rkrux:
    tACK 160799d913
  instagibbs:
    ACK 160799d913
  tdb3:
    Code review ACK 160799d913

Tree-SHA512: e9a80c6733f1e7fe9e834d81b404f6e8ef7a61fe986f61b3dcdbda1a0bc547145fc279ec02f54361df56cb4e62a6fedaa0f3991c6e084c3a703ed1b1bfbdbe4e
2024-11-29 08:33:49 -05:00
Hennadii Stepanov
935973b315
Remove src/config directory
The `src/config` directory has not been used since the migration to
CMake, which disables in-source builds.
2024-11-28 11:53:23 +00:00
Jeremy Rand
19f49c7489
doc: Use more precise anchor link to codesigning docs
The "Codesigning" section is what users presumably are looking for when
they follow this link.
2024-11-28 05:48:30 +00:00
Jeremy Rand
8bf1b3039c
doc: Use more precise anchor links to Xcode SDK extraction
The "SDK Extraction" section is what users presumably are looking for
when they follow these links.
2024-11-28 05:36:25 +00:00
Ava Chow
7590e93bc7
Merge bitcoin/bitcoin#30986: contrib: skip missing binaries in gen-manpages
Some checks failed
CI / test each commit (push) Has been cancelled
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Has been cancelled
CI / macOS 14 native, arm64, fuzz (push) Has been cancelled
CI / Win64 native, VS 2022 (push) Has been cancelled
CI / Win64 native fuzz, VS 2022 (push) Has been cancelled
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Has been cancelled
ee6185372f gen-manpages: Prompt error if no binaries are found (Andre)
299e2220e9 gen-manpages: implement --skip-missing-binaries (Andre Alves)

Pull request description:

  Instead of stopping the execution of gen-manpages.py when a binary is not found, continue generating manpages for the available binaries and skip the missing ones.

  A new argument, `--skip-missing-binaries`, has been added to enable this behavior.

  ```sh
  ➜  bitcoin git:(fix-gen-manpages) ✗ ./contrib/devtools/gen-manpages.py --help
  usage: gen-manpages.py [-h] [-s]

  options:
    -h, --help            show this help message and exit
    -s, --skip-missing-binaries
                          skip generation for binaries that are not found

  ```

  closes #30985

  This PR also includes an error prompt if no binaries are found in the build path.

ACKs for top commit:
  achow101:
    ACK ee6185372f
  laanwj:
    re-ACK ee6185372f

Tree-SHA512: af4a0a5e26e508a51ab63f8aa9f98a6d6af9d7682a16791d8a6a61d49e44cb0147453f628ad5910f65d4efa6e3c7b6605c007259c23230b54888845bfaeb050c
2024-11-27 12:34:38 -05:00
Ava Chow
b2af068825
Merge bitcoin/bitcoin#30708: rpc: add getdescriptoractivity
37a5c5d836 doc: update descriptors.md for getdescriptoractivity (James O'Beirne)
ee3ce6a4f4 test: rpc: add no address case for getdescriptoractivity (James O'Beirne)
811f76f3a5 rpc: add getdescriptoractivity (James O'Beirne)
25fe087de5 rpc: move-only: move ScriptPubKeyDoc to utils (James O'Beirne)

Pull request description:

  The RPC command `scanblocks` provides a useful way to get a set of blockhashes that have activity relevant to a set of descriptors (`relevant_blocks`). However actually extracting the activity from those blocks is left as an exercise to the end user.

  This process involves not only generating the (potentially ranged) set of scripts for the descriptor set on the client side (maybe via `deriveaddresses`), but then the user must retrieve each block's contents one-by-one using `getblock <hash>`, which is transmitted over a network link. And that's all before they perform the actual search over block content. There's even more work required to incorporate unconfirmed transactions.

  This PR introduces an RPC `getdescriptoractivity` that [dovetails](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-08-16#1046393;) with `scanblocks` output, handling the process described above. Users specify the blockhashes (perhaps from `relevant_blocks`) and a set of descriptors; they are then given all spend/receive activity in that set of blocks.

  This is a very useful tool when implementing lightweight wallets that want neither to require a third-party indexer like electrs, nor the overhead of creating and managing watch-only wallets in Core. This allows Core to be more easily used in a "stateless" manner by wallets, with potentially many nodes interchangeably acting as backends.

  ### Example usage

  ```
  % ./src/bitcoin-cli scanblocks start \
      '["addr(bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t)"]' \
      857263
  {
    "from_height": 857263,
    "to_height": 858263,
    "relevant_blocks": [
      "00000000000000000002bc5cc78f5b0913a5230a8f4b0d5060bc9a60900a5a88",
      "00000000000000000001c5291ed6a40c06d3db5c8fb738567654b24a14b24ecb"
    ],
    "completed": true
  }

  % ./src/bitcoin-cli getdescriptoractivity \
      '["00000000000000000002bc5cc78f5b0913a5230a8f4b0d5060bc9a60900a5a88", "00000000000000000001c5291ed6a40c06d3db5c8fb738567654b24a14b24ecb"]' \
      '["addr(bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t)"]'
  {
    "activity": [
      {
        "type": "receive",
        "amount": 0.00002900,
        "blockhash": "00000000000000000002bc5cc78f5b0913a5230a8f4b0d5060bc9a60900a5a88",
        "height": 857907,
        "txid": "c9d34f202c1f66d80cae76f305350f5fdde910b97cf6ae6bf79f5bcf2a337d06",
        "vout": 254,
        "output_spk": {
          "asm": "1 7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b",
          "desc": "rawtr(7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b)#yewcd80j",
          "hex": "51207e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b",
          "address": "bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t",
          "type": "witness_v1_taproot"
        }
      },
      {
        "type": "spend",
        "amount": 0.00002900,
        "blockhash": "00000000000000000001c5291ed6a40c06d3db5c8fb738567654b24a14b24ecb",
        "height": 858260,
        "spend_txid": "7f61d1b248d4ee46376f9c6df272f63fbb0c17039381fb23ca5d90473b823c36",
        "spend_vin": 0,
        "prevout_txid": "c9d34f202c1f66d80cae76f305350f5fdde910b97cf6ae6bf79f5bcf2a337d06",
        "prevout_vout": 254,
        "prevout_spk": {
          "asm": "1 7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b",
          "desc": "rawtr(7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b)#yewcd80j",
          "hex": "51207e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b",
          "address": "bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t",
          "type": "witness_v1_taproot"
        }
      }
    ]
  }
  ```

ACKs for top commit:
  instagibbs:
    reACK 37a5c5d836
  achow101:
    ACK 37a5c5d836
  tdb3:
    Code review and light retest ACK 37a5c5d836
  rkrux:
    re-ACK 37a5c5d836

Tree-SHA512: 04aa51e329c6c2ed72464b9886281d5ebd7511a8a8e184ea81249033a4dad535a12829b1010afc2da79b344ea8b5ab8ed47e426d0bf2eb78ab395d20b1da8dbb
2024-11-27 12:23:35 -05:00
merge-script
144f98db85
Merge bitcoin/bitcoin#31337: build: Fix coverage builds
Some checks are pending
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / test each commit (push) Waiting to run
01a7298818 build: Avoid using the `-ffile-prefix-map` compiler option (Hennadii Stepanov)

Pull request description:

  This PR follows up on https://github.com/bitcoin/bitcoin/pull/30811, which inadvertently broke coverage builds:
  1. For GCC. See https://github.com/bitcoin/bitcoin/pull/31337#issuecomment-2490598011.
  2. For [Clang's source-based code coverage](https://clang.llvm.org/docs/SourceBasedCodeCoverage.html) in the OSS-Fuzz environment due to its use of other options and a third party script. See https://issues.oss-fuzz.com/issues/379122777.

  The root cause of this regression is that the `-ffile-prefix-map` option implicitly applies:
  -  [`-fprofile-prefix-map`](https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fprofile-prefix-map) when using GCC.
  - [`-fcoverage-prefix-map`](https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fcoverage-prefix-map) when using Clang. ~This behaviour is not explicitly documented~ See 994c544c18.

  With this PR, only the `-fdebug-prefix-map` and `-fmacro-prefix-map` options are applied.

  **Note for reviewers:** Please ensure that https://github.com/bitcoin/bitcoin/issues/30799 is not reintroduced.

ACKs for top commit:
  maflcko:
    review ACK 01a7298818
  dergoegge:
    tACK 01a7298818

Tree-SHA512: 70b11d769d2653c1528ffe1d430b79f20b061037bcb1db6addb261aa2c7be20c4bc4328ccff919adb72e81aa2e65b885c970cce3837aeeb3ca0a98b32b83d2e3
2024-11-27 14:21:52 +00:00
MarcoFalke
faa16ed4b9
test: Add missing node.setmocktime(self.mocktime) to p2p_ibd_stalling.py
This was forgotten by myself in commit fa5b58ea01
2024-11-27 14:28:52 +01:00
merge-script
efdb49afb9
Merge bitcoin/bitcoin#31323: guix: swap moreutils for just sponge
e8f50c5deb guix: swap moreutils for just sponge (fanquake)

Pull request description:

  Switch to building the only `moreutils` utility we actually need (`sponge`). This results in having less unused stuff in the Guix environment (i.e all the other `moreutils` utilities), and, the dependency graph is simplified. i.e we no-longer have a dependency on `perl`, `docbook` etc, for this package.

  Current `moreutils` dependency graph:
  ![moreutils](https://github.com/user-attachments/assets/b91a8609-1434-4094-ad12-93332737ef0f)

  In the Guix env, `chronic`, `combine`, `errno`, `ifdata`, `ifne`, `isutf8`, `lckdo`, `mispipe`, `parallel`, `pee`, `ts`, `vidir`, `vipe` & `zrun` (plus their `*.real` variants) are removed.

ACKs for top commit:
  hebasto:
    ACK e8f50c5deb.
  TheCharlatan:
    Re-ACK e8f50c5deb

Tree-SHA512: 3687ec4a821ff79c26ee839d2af879166edb7e179287a9574eca8fbf34bed1fea8fcdad822a2140d0a0089e1820f3fef29a6100e0e8da788896e1f7bac5ec3e6
2024-11-27 10:46:54 +00:00
James O'Beirne
37a5c5d836 doc: update descriptors.md for getdescriptoractivity 2024-11-26 20:47:11 -05:00
James O'Beirne
ee3ce6a4f4 test: rpc: add no address case for getdescriptoractivity
Co-authored-by: Greg Sanders <gsanders87@gmail.com>
2024-11-26 20:47:11 -05:00
James O'Beirne
811f76f3a5 rpc: add getdescriptoractivity 2024-11-26 20:47:08 -05:00
Andre
ee6185372f gen-manpages: Prompt error if no binaries are found 2024-11-26 21:35:50 -03:00
Ava Chow
70e20ea024
Merge bitcoin/bitcoin#31172: build: increase minimum supported Windows to 10.0
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 14 native, arm64, no depends, sqlite only, gui (push) Waiting to run
CI / macOS 14 native, arm64, fuzz (push) Waiting to run
CI / Win64 native, VS 2022 (push) Waiting to run
CI / Win64 native fuzz, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
ee1128ead8 doc: update stack-clash-protection comment re mingw-w64 (fanquake)
bf47448f15 test: drop check for Windows < 10 (fanquake)
35b898c47f release: target Windows 10 or later (fanquake)
398754e70b depends: target Windows 10 when building for mingw-w64 (fanquake)

Pull request description:

  Follows up to https://github.com/bitcoin/bitcoin/pull/31048#discussion_r1803165670.

  We definitely cannot claim that Bitcoin Core is "supported and extensively tested on" on Windows 7.

  Note that #30997 is also increasing the minimum required Windows version (for the GUI) to 10.

ACKs for top commit:
  hodlinator:
    cr-ACK ee1128ead8
  davidgumberg:
    ACK ee1128ead8
  achow101:
    ACK ee1128ead8
  hebasto:
    re-ACK ee1128ead8, only rebased, a commit message and a comment have been amended since my recent [review](https://github.com/bitcoin/bitcoin/pull/31172#pullrequestreview-2415452160).
  TheCharlatan:
    ACK ee1128ead8

Tree-SHA512: 245e0bac3d63414d919a1948661fef4ff79359faaacaf19d64abd91cc62e822797fb1cf3379e340bfdf9a85c0b88fd99a90eda450dd4218b6213ab78aefb1374
2024-11-26 17:47:29 -05:00
Ava Chow
733317ba94
Merge bitcoin/bitcoin#31364: refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors
3305972f7b refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors (Lőrinc)

Pull request description:

  A follow-up of https://github.com/bitcoin/bitcoin/pull/31305.

  The `clang-tidy` check can be run via:
  ```bash
  cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_FOR_FUZZING=ON && cmake --build build -j$(nproc)

  run-clang-tidy -quiet -p build -j $(nproc) -checks='-*,performance-unnecessary-copy-initialization' | grep -v 'clang-tidy'
  ```

ACKs for top commit:
  maflcko:
    review ACK 3305972f7b 🏀
  achow101:
    ACK 3305972f7b
  theuni:
    ACK the much more constrained 3305972f7b.
  hebasto:
    ACK 3305972f7b, tested with clang 19.1.5 + clang-tidy.

Tree-SHA512: 64dc3b35f33b7ac064ebf9e56e9f0ceca5d26681a1379dcd2168987960020fe1a282ec4de8c353c82ddf0a534a4866b607fc691e690010c6cea78887045897fb
2024-11-26 17:00:01 -05:00
Ava Chow
5a4bc5c036
Merge bitcoin/bitcoin#31305: refactor: Fix remaining clang-tidy performance-inefficient-vector errors
11f3bc229c refactor: Reserve vectors in fuzz tests (Lőrinc)
152fefe7a2 refactor: Preallocate PrevectorFillVector(In)Direct without vector resize (Lőrinc)
a774c7a339 refactor: Fix remaining clang-tidy performance-inefficient-vector errors (Lőrinc)

Pull request description:

  PR inspired by https://github.com/bitcoin/bitcoin/pull/29608#issuecomment-2437847307 (and https://github.com/bitcoin/bitcoin/pull/29458, https://github.com/bitcoin/bitcoin/pull/29606, https://github.com/bitcoin/bitcoin/pull/29607, https://github.com/bitcoin/bitcoin/pull/30093).

  The `clang-tidy` check can be run via:
  ```bash
  cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_FOR_FUZZING=ON && cmake --build build -j$(nproc)

  run-clang-tidy -quiet -p build -j $(nproc) -checks='-*,performance-inefficient-vector-operation' | grep -v 'clang-tidy'
  ```
  which revealed 3 tests and 1 prod warning (+ fuzz and benching, found by hebasto).
  Even though the tests aren't performance critical, getting rid of these warnings (for which the checks were already enabled via https://github.com/bitcoin/bitcoin/blob/master/src/.clang-tidy#L18, see below), the fix was quite simple.

  <details>
  <summary>clang-tidy -list-checks</summary>

  ```bash
  cd src && clang-tidy -list-checks | grep 'vector'
      performance-inefficient-vector-operation
  ```

  </details>

  <details>
  <summary>Output before the change</summary>

  ```
  src/test/rpc_tests.cpp:434:9: error: 'emplace_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors]
    433 |     for (int64_t i = 0; i < 100; i++) {
    434 |         feerates.emplace_back(1 ,1);
        |         ^

  src/test/checkqueue_tests.cpp:366:13: error: 'emplace_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors]
    365 |         for (size_t i = 0; i < 3; ++i) {
    366 |             tg.emplace_back(
        |             ^

  src/test/cuckoocache_tests.cpp:231:9: error: 'emplace_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors]
    228 |     for (uint32_t x = 0; x < 3; ++x)
    229 |         /** Each thread is emplaced with x copy-by-value
    230 |         */
    231 |         threads.emplace_back([&, x] {
        |         ^

  src/rpc/output_script.cpp:127:17: error: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors]
    126 |             for (unsigned int i = 0; i < keys.size(); ++i) {
    127 |                 pubkeys.push_back(HexToPubKey(keys[i].get_str()));
        |                 ^
  ```

  And the fuzz and benchmarks, noticed by hebasto: https://github.com/bitcoin/bitcoin/pull/31305#issuecomment-2483124499

  </details>

ACKs for top commit:
  maflcko:
    review ACK 11f3bc229c 🎦
  achow101:
    ACK 11f3bc229c
  theuni:
    ACK 11f3bc229c
  hebasto:
    ACK 11f3bc229c, tested with clang 19.1.5 + clang-tidy.

Tree-SHA512: 41691c19f35c63b922a95407617a54f9bff1af3f95f99d15642064f321df038aeb1ae5f061f854ed913f69036807cc28fa6222b2ff4c24ef43b909027fa0f9b3
2024-11-26 14:58:44 -05:00
Ava Chow
28fd0bc731
Merge bitcoin/bitcoin#31365: interpreter: Use the same type for SignatureHash in the definition
c288c790cd interpreter: Use the same type for SignatureHash in the definition (TheCharlatan)

Pull request description:

  This was missed during the original PR switching the nHashType argument to int32_t in SignatureHash in bc52cda1f3.

  The problem was discovered after running into a linker error when attempting to link this code as a static library using the header as a declaration with a riscv32 bare metal toolchain. The compiler would error with:
  ```
  /opt/riscv-ilp32/lib/gcc/riscv32-unknown-elf/13.2.0/../../../../riscv32-unknown-elf/bin/ld: build_kernel_riscv/src/libbitcoin_consensus.a(interpreter.cpp.o): in function `GenericTransactionSignatureChecker<CTransaction>::CheckECDSASignature(std::vector<unsigned char, std::allocator<unsigned char> > const&, std::vector<unsigned char, std::allocator<unsigned char> > const&, CScript const&, SigVersion) const':
  /home/user/bitcoin/build_kernel_riscv/./script/interpreter.cpp:2043:(.text._ZNK34GenericTransactionSignatureCheckerI12CTransactionE19CheckECDSASignatureERKSt6vectorIhSaIhEES6_RK7CScript10SigVersion[_ZNK34GenericTransactionSignatureCheckerI12CTransactionE19CheckECDSASignatureERKSt6vectorIhSaIhEES6_RK7CScript10SigVersion]+0xee): undefined reference to `uint256 SignatureHash<CTransaction>(CScript const&, CTransaction const&, unsigned int, int, long long const&, SigVersion, PrecomputedTransactionData const*)'
  ```

  With this patch it is possible to link against the static consensus library and produce a fully static executable.

ACKs for top commit:
  l0rinc:
    ACK c288c790cd
  maflcko:
    review ACK c288c790cd 🐺
  achow101:
    ACK c288c790cd
  theuni:
    Obvious fix ACK c288c790cd.
  BrandonOdiwuor:
    Code Review ACK c288c790cd

Tree-SHA512: 74f283637f0a9cd0cab65d3502f2f8fc4fb983c7672f24e7a76ba2eb6e53b4a81cca0aacb610ef39ac0a454305be594ab440a697ae3718987bf5dbcbc7146a31
2024-11-26 13:45:41 -05:00
Ava Chow
72ab35a6d0
Merge bitcoin/bitcoin#31221: ci: Split out native fuzz jobs for macOS and windows (take 2)
b031b7910d [ci] Split out native fuzz jobs for macOS and windows (dergoegge)

Pull request description:

  Split out two new CI jobs (for native macOS and windows) that run the fuzz tests on the qa-assets input corpora.

  In both jobs the fuzz binary is built with `-DBUILD_FOR_FUZZING` to enable `Assume` assertions as well as `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION`.

ACKs for top commit:
  maflcko:
    re-lgtm ACK b031b7910d
  achow101:
    ACK b031b7910d
  hebasto:
    ACK b031b7910d.

Tree-SHA512: 7d0dc5a9cb299f6f4596dd9a5526b6aaf82efc6eba308bdc9d8b0a45f79dea87204fb6cd4b2ea2a1bd952466b2e958d64021999296d110d7a83c1667f4de51fe
2024-11-26 13:29:59 -05:00
Sjors Provoost
06443b8f28
net: clarify if we ever sent or received from peer 2024-11-26 14:01:36 +01:00
Sjors Provoost
1d01ad4d73
net: add LogIP() helper, use in net_processing 2024-11-26 13:22:55 +01:00
Sjors Provoost
937ef9eb40
net_processing: use CNode::DisconnectMsg helper
This is not a pure refactor:
1. It slightly changes the log messages, as reflected in the test changes
2. It adds the IP address to all disconnect logging (when fLogIPs is set)
2024-11-26 13:22:55 +01:00