Commit graph

27437 commits

Author SHA1 Message Date
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
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
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
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
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
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
James O'Beirne
811f76f3a5 rpc: add getdescriptoractivity 2024-11-26 20:47:08 -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
Sebastian Falbesoner
61e18dec30 doc: ephemeral policy: add missing closing double quote 2024-11-25 21:05:11 +01:00
Lőrinc
3305972f7b refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors 2024-11-25 20:11:54 +01:00
Lőrinc
11f3bc229c refactor: Reserve vectors in fuzz tests
* Since the main LIMITED_WHILE stated `outpoints.size() < 200'000`, I've presized outpoints accordingly.
* `tx_mut.vin` and `tx_mut.vout` weren't caught by the clang-tidy, but addressed them anyway.
2024-11-25 20:09:44 +01:00
Lőrinc
152fefe7a2 refactor: Preallocate PrevectorFillVector(In)Direct without vector resize
The prevector benchmarks were likely not trying to measure vector resize performance.
2024-11-25 20:09:44 +01:00
Lőrinc
a774c7a339 refactor: Fix remaining clang-tidy performance-inefficient-vector errors 2024-11-25 20:09:44 +01:00
glozow
f7144b24be
Merge bitcoin/bitcoin#31279: policy: ephemeral dust followups
Some checks are pending
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
CI / test each commit (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
466e4df3fb assert_mempool_contents: assert not duplicates expected (Greg Sanders)
ea5db2f269 functional: only generate required blocks for test (Greg Sanders)
d033acb608 fuzz: package_eval: let fuzzer run out input in main tx creation loop (Greg Sanders)
ba35a570c5 CheckEphemeralSpends: return boolean, and set child state and txid outparams (Greg Sanders)
cf0cee1617 func: add note about lack of 1P1C propagation in tree submitpackage (Greg Sanders)
8424290304 unit test: ephemeral_tests is using a dust relay rate, not minrelay (Greg Sanders)
d9cfa5fc4e CheckEphemeralSpends: no need to iterate inputs if no parent dust (Greg Sanders)
87b26e3dc0 func: rename test_free_relay to test_no_minrelay_fee (Greg Sanders)
e5709a4a41 func: slight elaboration on submitpackage restriction (Greg Sanders)
08e969bd10 RPC: only enforce dust rules on priority when standardness active (Greg Sanders)
ca050d12e7 unit test: adapt to changing MAX_DUST_OUTPUTS_PER_TX (Greg Sanders)
7c3490169c fuzz: package_eval: move last_tx inside txn ctor (Greg Sanders)
445eaed182 fuzz: use optional status instead of should_rbf_eph_spend (Greg Sanders)
4dfdf615b9 fuzz: remove unused TransactionsDelta validation interface (Greg Sanders)
09ce926e4a func: cleanup reorg test comment (Greg Sanders)
768a0c1889 func: cleanup test_dustrelay comments (Greg Sanders)
bedca1cb66 fuzz: Directly place transactions in vector (Greg Sanders)
c041ad6ecc fuzz: explain package eval coin tracking better (Greg Sanders)
bc0d98ea61 fuzz: remove dangling reference to GetEntry (Greg Sanders)
15b6cbf07f unit test: make dust index less magical (Greg Sanders)
5fbcfd12b8 unit test: assert txid returned on CheckEphemeralSpends failures (Greg Sanders)
ef94d84b4e bench: remove unnecessary CMTxn constructors (Greg Sanders)
c5c10fd317 ephemeral policy doxygen cleanup (Greg Sanders)
dd9044b8d4 ephemeral policy: IWYU (Greg Sanders)
c6859ce2de Move+rename GetDustIndexes -> GetDust (Greg Sanders)
62016b3230 Use std::ranges for ephemeral policy checks (Greg Sanders)
3ed930a1f4 Have HasDust and PreCheckValidEphemeralTx take CTransaction (Greg Sanders)
04a614bf9a Rename CheckValidEphemeralTx to PreCheckEphemeralTx (Greg Sanders)
cbf1a47d60 CheckEphemeralSpends: only compute txid of tx when needed (Greg Sanders)

Pull request description:

  Follow-up to https://github.com/bitcoin/bitcoin/pull/30239

  Here are the parent PR's comments that should be addressed by this PR:

  https://github.com/bitcoin/bitcoin/pull/30239/files#r1834529646
  https://github.com/bitcoin/bitcoin/pull/30239/files#r1831247308
  https://github.com/bitcoin/bitcoin/pull/30239/files#r1832622481
  https://github.com/bitcoin/bitcoin/pull/30239/files#r1831195216
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1835805164
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1835805164
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834639096
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834624976
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834619709
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834610434
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834504436
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834500036
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832985488
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830929809
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832376920
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832755799
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832492686
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832980576
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832784278
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1837989979
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830996993
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830997947
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830012890
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830037288
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830977092
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832622481
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834726168
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832453654
  https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1848488226

ACKs for top commit:
  naumenkogs:
    ACK 466e4df3fb
  hodlinator:
    ACK 466e4df3fb
  theStack:
    lgtm ACK 466e4df3fb
  glozow:
    utACK 466e4df3fb

Tree-SHA512: 89106f695755c238b84e0996b89446c0733e10a94c867f656d516d26697d2efe38dfc332188b8589a0a26a3d2bd2c88c6ab70c108e187ce5bfcb91bbf3fb0391
2024-11-25 13:47:44 -05:00
TheCharlatan
c288c790cd
interpreter: Use the same type for SignatureHash in the definition
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*)'
2024-11-25 15:36:05 +01:00
Hennadii Stepanov
6f4128e3a8
cmake, qt: Use absolute paths for includes in MOC-generated files
This change resolves build issues that occur when the source or build
directory is symlinked.
2024-11-24 15:14:14 +00:00
Hennadii Stepanov
ab5c63edcc
cmake: Build secp256k1 only when required 2024-11-23 15:31:39 +00:00
Hennadii Stepanov
76a3a540a4
cmake: Ensure script correctness when no targets are specified 2024-11-23 15:17:18 +00:00
merge-script
17834bd197
Merge bitcoin/bitcoin#31333: fuzz: Implement G_TEST_GET_FULL_NAME
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 / Win64 native, VS 2022 (push) Waiting to run
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
92d3d691f0 fuzz: Implement G_TEST_GET_FULL_NAME (Hodlinator)

Pull request description:

  Catching up to bench & unit tests. Makes for more orderly paths for fuzz tests using `BasicTestingSetup`.

  ### Before
  ```
  /tmp/test_common bitcoin/0748ae43ef8fa80703bc/regtest/blocks/xor.dat
  ```
  ### After
  ```
  /tmp/test_common bitcoin/tx_pool_standard/f18b3744625e0600eb0c/regtest/blocks/xor.dat
  ```

ACKs for top commit:
  kevkevinpal:
    ACK [92d3d69](92d3d691f0)
  furszy:
    utACK 92d3d691f0
  tdb3:
    ACK 92d3d691f0
  dergoegge:
    utACK 92d3d691f0
  brunoerg:
    code review ACK 92d3d691f0

Tree-SHA512: 5e83970b111232adece10f79e3a43d0c3c49ab635763e2a4b420f1336cbb8fee94aab751264ddec01ac8363166636e3b29cfe3b2969fc28c8dd6b31bda351950
2024-11-21 13:57:17 +00:00
Greg Sanders
d033acb608 fuzz: package_eval: let fuzzer run out input in main tx creation loop 2024-11-20 13:49:41 -05:00
Greg Sanders
ba35a570c5 CheckEphemeralSpends: return boolean, and set child state and txid outparams 2024-11-20 13:49:41 -05:00
Greg Sanders
8424290304 unit test: ephemeral_tests is using a dust relay rate, not minrelay 2024-11-20 13:49:41 -05:00
Greg Sanders
d9cfa5fc4e CheckEphemeralSpends: no need to iterate inputs if no parent dust 2024-11-20 13:49:41 -05:00
Greg Sanders
08e969bd10 RPC: only enforce dust rules on priority when standardness active 2024-11-20 13:49:41 -05:00
Greg Sanders
ca050d12e7 unit test: adapt to changing MAX_DUST_OUTPUTS_PER_TX 2024-11-20 13:49:41 -05:00
Greg Sanders
7c3490169c fuzz: package_eval: move last_tx inside txn ctor 2024-11-20 13:49:41 -05:00
Greg Sanders
445eaed182 fuzz: use optional status instead of should_rbf_eph_spend 2024-11-20 13:49:41 -05:00
Greg Sanders
4dfdf615b9 fuzz: remove unused TransactionsDelta validation interface 2024-11-20 13:44:49 -05:00
Greg Sanders
bedca1cb66 fuzz: Directly place transactions in vector 2024-11-20 13:44:49 -05:00
Greg Sanders
c041ad6ecc fuzz: explain package eval coin tracking better 2024-11-20 13:44:49 -05:00
Greg Sanders
bc0d98ea61 fuzz: remove dangling reference to GetEntry 2024-11-20 13:44:49 -05:00
Greg Sanders
15b6cbf07f unit test: make dust index less magical 2024-11-20 13:44:49 -05:00
Greg Sanders
5fbcfd12b8 unit test: assert txid returned on CheckEphemeralSpends failures
Simplify nullopt checks
2024-11-20 12:49:03 -05:00
Greg Sanders
ef94d84b4e bench: remove unnecessary CMTxn constructors 2024-11-20 12:49:03 -05:00
Greg Sanders
c5c10fd317 ephemeral policy doxygen cleanup 2024-11-20 12:49:03 -05:00
Greg Sanders
dd9044b8d4 ephemeral policy: IWYU 2024-11-20 12:49:03 -05:00
Greg Sanders
c6859ce2de Move+rename GetDustIndexes -> GetDust
Use to replace HasDust and where appropraite
2024-11-20 12:48:03 -05:00
Ryan Ofsky
22ef95dbe3
Merge bitcoin/bitcoin#31288: Add destroy to BlockTemplate schema
Some checks are pending
CI / ASan + LSan + UBSan + integer, no depends, USDT (push) Waiting to run
CI / test each commit (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
9aa50152c1 Add destroy to BlockTemplate schema (Sjors Provoost)

Pull request description:

  This ensures that if a client no longer needs a block template, the node can clear its memory as soon as possible.

  A block template may hold on to transactions that are no longer in the mempool, so this can be significant.

  This has a trivial silent merge conflict with #31283 because it also used the number `@9` (gaps are not allowed). I'll rebase whichever is merged last.

ACKs for top commit:
  TheCharlatan:
    Re-ACK 9aa50152c1
  ryanofsky:
    Code review ACK 9aa50152c1

Tree-SHA512: 393571b4455969cba71c7572feaeff4503738205331ab198b9181c156c75eb65933a8e5ceff66fc06d1efb8f2528bdb254e5eee7df75735b912526de1e7b166d
2024-11-20 10:45:43 -05:00
Hodlinator
92d3d691f0
fuzz: Implement G_TEST_GET_FULL_NAME
When BasicTestingSetup is used in fuzz-tests it will now create test directories containing the fuzz target names. Example:
/tmp/test_common bitcoin/tx_package_eval/153d7906294f7d0606a7/

This is already implemented for bench and unit tests.
2024-11-20 15:08:33 +01:00
glozow
f34fe0806a
Merge bitcoin/bitcoin#31122: cluster mempool: Implement changeset interface for mempool
5736d1ddac tracing: pass if replaced by tx/pkg to tracepoint (0xb10c)
a4ec07f194 doc: add comments for CTxMemPool::ChangeSet (Suhas Daftuar)
83f814b1d1 Remove m_all_conflicts from SubPackageState (Suhas Daftuar)
d3c8e7dfb6 Ensure that we don't add duplicate transactions in rbf fuzz tests (Suhas Daftuar)
d7dc9fd2f7 Move CalculateChunksForRBF() to the mempool changeset (Suhas Daftuar)
284a1d33f1 Move prioritisation into changeset (Suhas Daftuar)
446b08b599 Don't distinguish between direct conflicts and all conflicts when doing cluster-size-2-rbf checks (Suhas Daftuar)
b53041021a Duplicate transactions are not permitted within a changeset (Suhas Daftuar)
b447416fdd Public mempool removal methods Assume() no changeset is outstanding (Suhas Daftuar)
2b30f4d36c Make RemoveStaged() private (Suhas Daftuar)
18829194ca Enforce that there is only one changeset at a time (Suhas Daftuar)
7fb62f7db6 Apply mempool changeset transactions directly into the mempool (Suhas Daftuar)
34b6c5833d Clean up FinalizeSubpackage to avoid workspace-specific information (Suhas Daftuar)
57983b8add Move LimitMempoolSize to take place outside FinalizeSubpackage (Suhas Daftuar)
01e145b975 Move changeset from workspace to subpackage (Suhas Daftuar)
802214c083 Introduce mempool changesets (Suhas Daftuar)
87d92fa340 test: Add unit test coverage of package rbf + prioritisetransaction (Suhas Daftuar)
15d982f91e Add package hash to package-rbf log message (Suhas Daftuar)

Pull request description:

  part of cluster mempool: #30289

  It became clear while working on cluster mempool that it would be helpful for transaction validation if we could consider a full set of proposed changes to the mempool -- consisting of a set of transactions to add, and a set of transactions (ie conflicts) to simultaneously remove -- and perform calculations on what the mempool would look like if the proposed changes were to be applied.  Two specific examples of where we'd like to do this:

  - Determining if ancestor/descendant/TRUC limits would be violated (in the future, cluster limits) if either a single transaction or a package of transactions were to be accepted
  - Determining if an RBF would make the mempool "better", however that idea is defined, both in the single transaction and package of transaction cases

  In preparation for cluster mempool, I have pulled this reworking of the mempool interface out of #28676 so it can be reviewed on its own.  I have not re-implemented ancestor/descendant limits to be run through the changeset, since with cluster mempool those limits will be going away, so this seems like wasted effort.  However, I have rebased #28676 on top of this branch so reviewers can see what the new mempool interface could look like in the cluster mempool setting.

  There are some minor behavior changes here, which I believe are inconsequential:
  - In the package validation setting, transactions would be added to the mempool before the `ConsensusScriptChecks()` are run. In theory, `ConsensusScriptChecks()` should always pass if the `PolicyScriptChecks()` have passed and it's just a belt-and-suspenders for us, but if somehow they were to diverge then there could be some small behavior change from adding transactions and then removing them, versus never adding them at all.
  - The error reporting on `CheckConflictTopology()` has slightly changed due to no longer distinguishing between direct conflicts and indirect conflicts. I believe this should be entirely inconsequential because there shouldn't be a logical difference between those two ideas from the perspective of this function, but I did have to update some error strings in some tests.
  - Because, in a package setting, RBFs now happen as part of the entire package being accepted, the logging has changed slightly because we do not know which transaction specifically evicted a given removed transaction.
    -  Specifically, the "package hash" is now used to reference the set of transactions that are being accepted, rather than any single txid. The log message relating to package RBF that happen in the `TXPACKAGES` category has been updated as well to include the package hash, so that it's possible to see which specific set of transactions are being referenced by that package hash.
    - Relatedly, the tracepoint logging in the package rbf case has been updated as well to reference the package hash, rather than a transaction hash.

ACKs for top commit:
  naumenkogs:
    ACK 5736d1ddac
  instagibbs:
    ACK 5736d1ddac
  ismaelsadeeq:
    reACK 5736d1ddac
  glozow:
    ACK 5736d1ddac

Tree-SHA512: 21810872e082920d337c89ac406085aa71c5f8e5151ab07aedf41e6601f60a909b22fbf462ef3b735d5d5881e9b76142c53957158e674dd5dfe6f6aabbdf630b
2024-11-20 07:48:29 -05:00
fanquake
7d3703dec3
doc: add copyright header to p2p_headers_presync 2024-11-20 11:48:53 +00:00
merge-script
116b8c5573
Merge bitcoin/bitcoin#31213: fuzz: Fix difficulty target generation in p2p_headers_presync
a6ca8f3243 fuzz: Fix difficulty target generation in p2p_headers_presync (marcofleon)
fa327c77e3 util: Add ConsumeArithUInt256InRange fuzzing helper (marcofleon)

Pull request description:

  In the `p2p_headers_presync` fuzz target, this assertion failed:
  ```
   assert(total_work < chainman.MinimumChainWork());
   ```
  Input that triggered the failure: [p2ppresync_crash.txt](https://github.com/user-attachments/files/17620203/p2ppresync_crash.txt)

  The test previously used `ConsumeIntegralInRange` to generate header difficulty targets within a hardcoded range. The fuzzer found specific values in that range that correspond to very low thresholds due to how [`SetCompact`][setcompact-link] works. The total work of a long enough test chain ended up exceeding `MinimumChainWork`.

  Fix this by adding a new `ConsumeArithUInt256InRange` helper function and use it in the fuzz test to generate target values within the originally intended range. The target is then converted to an `nBits` value using `GetCompact()`.

  For some more context, see https://github.com/bitcoin/bitcoin/pull/30918.

  [setcompact-link]: 6463117a29/src/arith_uint256.h (L251-L271)

ACKs for top commit:
  instagibbs:
    ACK a6ca8f3243
  dergoegge:
    Code review ACK a6ca8f3243
  brunoerg:
    code review ACK a6ca8f3243

Tree-SHA512: 92013d9d37bd3f11992ee678ba9745196efbdc4d773fd14994116629260bea46ffc9fa3923d443af7b623d39c6211900ce98a349c62ad1976e12312c37ef9df0
2024-11-20 11:46:22 +00:00
merge-script
e122309958
Merge bitcoin/bitcoin#31317: test: Revert to random path element
faaaf59f71 test: Make g_rng_temp_path rand, not dependent on SeedRandomForTest (MarcoFalke)
fa80b08fef test: Revert to random path element (MarcoFalke)

Pull request description:

  The randomness in the path element is required to allow a single fuzz test to run in parallel. Previous releases used a uint256 random value, but 10 random bytes should be sufficient as well, while avoiding a `MAX_PATH` violation on Windows.

  The issue was introduced by myself, by suggesting to use the current time in https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1835351305.

ACKs for top commit:
  kevkevinpal:
    reACK faaaf59f71
  hodlinator:
    ACK faaaf59f71
  tdb3:
    re ACK faaaf59f71
  dergoegge:
    ACK faaaf59f71

Tree-SHA512: f12256c8b353618291030f71bf36eab97a25ffeaa28e36a5f2c6718dfc1fbbc8548c71475edec53d59026f2a779a05778db83f0530dd3e1d1faf6e4fc0ee7d70
2024-11-20 10:10:54 +00:00
MarcoFalke
faaaf59f71
test: Make g_rng_temp_path rand, not dependent on SeedRandomForTest 2024-11-19 17:31:59 +01:00
James O'Beirne
25fe087de5 rpc: move-only: move ScriptPubKeyDoc to utils 2024-11-18 15:22:44 -05:00
MarcoFalke
fa80b08fef
test: Revert to random path element 2024-11-18 14:21:15 +01:00