Commit graph

26953 commits

Author SHA1 Message Date
Ava Chow
df3f63ccfa
Merge bitcoin/bitcoin#30509: multiprocess: Add -ipcbind option to bitcoin-node
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 13 native, x86_64, 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
30073e6b3a multiprocess: Add -ipcbind option to bitcoin-node (Russell Yanofsky)
73fe7d7230 multiprocess: Add unit tests for connect, serve, and listen functions (Ryan Ofsky)
955d4077aa multiprocess: Add IPC connectAddress and listenAddress methods (Russell Yanofsky)
4da20434d4 depends: Update libmultiprocess library for CustomMessage function and ThreadContext bugfix (Ryan Ofsky)

Pull request description:

  Add `-ipcbind` option to `bitcoin-node` to make it listen on a unix socket and accept connections from other processes. The default socket path is `<datadir>/node.sock`, but this can be customized.

  This option lets potential wallet, gui, index, and mining processes connect to the node and control it. See examples in #19460, #19461, and #30437.

  Motivation for this PR, in combination with #30510, is be able to release a bitcoin core node binary that can generate block templates for a separate Stratum v2 mining service, like the one being implemented in https://github.com/Sjors/bitcoin/pull/48, that connects over IPC.

  Other things to know about this PR:

  - While the `-ipcbind` option lets other processes to connect to the `bitcoin-node` process, the only thing they can actually do after connecting is call methods on the [`Init`](https://github.com/bitcoin/bitcoin/blob/master/src/ipc/capnp/init.capnp#L17-L20) interface which is currently very limited and doesn't do much. But PRs [#30510](https://github.com/bitcoin/bitcoin/pull/30510), [#29409](https://github.com/bitcoin/bitcoin/pull/29409), and [#10102](https://github.com/bitcoin/bitcoin/pull/10102) expand the `Init` interface to expose mining, wallet, and gui functionality respectively.

  - This PR is not needed for [#10102](https://github.com/bitcoin/bitcoin/pull/10102), which runs GUI, node, and wallet code in different processes, because [#10102](https://github.com/bitcoin/bitcoin/pull/10102) does not use unix sockets or allow outside processes to connect to existing processes. [#10102](https://github.com/bitcoin/bitcoin/pull/10102) lets parent and child processes communicate over internal socketpairs, not externally accessible sockets.

  ---

  This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).

ACKs for top commit:
  achow101:
    ACK 30073e6b3a
  TheCharlatan:
    Re-ACK 30073e6b3a
  itornaza:
    Code review ACK 30073e6b3a

Tree-SHA512: 2b766e60535f57352e8afda9c3748a32acb5a57b2827371b48ba865fa9aa1df00f340732654f2e300c6823dbc6f3e14377fca87e4e959e613fe85a6d2312d9c8
2024-09-09 17:14:15 -04:00
Ava Chow
712a2b5453
Merge bitcoin/bitcoin#30817: test: Add coverage for dumptxoutset failure robustness
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 13 native, x86_64, 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
c2b779da4e refactor: Manage dumptxoutset RAII classes with std::optional (Fabian Jahr)
4b5bf335ad test: Add coverage for failing dumptxoutset behavior (Fabian Jahr)

Pull request description:

  This adds a test that checks that network activity is not suspended if dumptxoutset fails in the middle of its process which is implemented with the `NetworkDisable` RAII class. I would have liked to add coverage for the `TemporaryRollback` RAII class but that seems a lot more tricky since the failure needs to happen at some point after the rollback and on the scale of our test chain here I couldn't find a way to do it yet. This was requested by pablomartin4btc here: https://github.com/bitcoin/bitcoin/pull/30808#pullrequestreview-2280450117. To test the test you can comment out the content of the destructor of `NetworkDisable`.

  It also addresses the feedback by ryanofsky to use `std::optional` instead of `std::unique_ptr` for the management of the RAII object: https://github.com/bitcoin/bitcoin/pull/30808#discussion_r1744149228

ACKs for top commit:
  achow101:
    ACK c2b779da4e
  pablomartin4btc:
    cr & tACK c2b779da4e
  tdb3:
    ACK c2b779da4e
  BrandonOdiwuor:
    Code Review ACK c2b779da4e
  theStack:
    ACK c2b779da4e

Tree-SHA512: 9556e75014a2599bb870b70faf887608b332f2312626333f771d4ec11c04f863a2cf17e223ec473d4e8b0c9e8008394a4e0c321561f7ef3a2eec713dcfaea58a
2024-09-09 13:02:51 -04:00
Ava Chow
fb52023ee6
Merge bitcoin/bitcoin#30684: init: fix init fatal error on invalid negated option value
ee47ca29d6 init: fix fatal error on '-wallet' negated option value (furszy)

Pull request description:

  Currently, if users provide a double negated value such as '-nowallet=0' or a non-boolean
  convertible value to a negated option such as '-nowallet=not_a_boolean', the initialization
  process results in a fatal error, causing an unclean shutdown and displaying a poorly
  descriptive error message:
  "JSON value of type bool is not of expected type string." (On bitcoind. The GUI
  does not display any error msg - upcoming PR -).

  This PR fixes the issue by ensuring that only string values are returned in the
  the "wallet" settings list, failing otherwise. It also improves the clarity of the
  returned error message.

  Note:
  This bug was introduced in https://github.com/bitcoin/bitcoin/pull/22217. Where the `GetArgs("-wallet")` call was
  replaced by `GetSettingsList("-wallet")`.

ACKs for top commit:
  achow101:
    ACK ee47ca29d6
  ryanofsky:
    Code review ACK ee47ca29d6, just adding the suggested test since last review
  TheCharlatan:
    ACK ee47ca29d6
  ismaelsadeeq:
    Tested ACK ee47ca29d6

Tree-SHA512: 5f01076f74a048019bb70791160f0accc2db7a457d969cb23687bed81ccbbdec1dda68311e7c6e2dd56250e23e8d926d4066e5014b2a99a2fc202e24ed264fbd
2024-09-09 12:44:29 -04:00
Ava Chow
746f88000e
Merge bitcoin/bitcoin#30401: fix: increase consistency of rpcauth parsing
27c976d11a fix: increase consistency of rpcauth parsing (tdb3)
2ad3689512 test: add norpcauth test (tdb3)
67df0dec1a test: blank rpcauth CLI interaction (tdb3)
ecc98ccff2 test: add cases for blank rpcauth (tdb3)

Pull request description:

  The current `rpcauth` parsing behavior is inconsistent and unintuitive (see https://github.com/bitcoin/bitcoin/pull/29141#issuecomment-1972085251 and additional details below).
  The current behavior inconsistently treats empty `rpcauth` as an error (or not) depending on the location within CLI/bitcoin.conf and the location of adjacent valid `rpcauth` params.

  Empty `rpcauth` is now consistently treated as an error and prevents bitcoind from starting.
  Continuation of the upforgrabs PR #29141.

  ### Additional details:
  Current `rpcauth` behavior is nonsensical:

   - If an empty `rpcauth` argument was specified as the last command line argument, it would cause all other `rpcauth` arguments to be ignored.
   - If an empty `rpcauth` argument was specified on the command line followed by any nonempty `rpcauth` argument, it would cause an error.
   - If an empty `rpcauth=` line was specified after non-empty rpcauth line in the config file it would cause an error.
   - If an empty `rpcauth=` line in a config file was first it would cause other rpcauth entries in the config file to be ignored, unless there were `-rpcauth` command line arguments and the last one was nonempty, in which case it would cause an error.

  New behavior is simple:
   - If an empty rpcauth config line or command line argument is used it will cause an error

ACKs for top commit:
  naiyoma:
    Tested ACK  [27c976d11a)
  achow101:
    ACK 27c976d11a
  ryanofsky:
    Code review ACK 27c976d11a. Since last review commit message was just tweaked to clarify previous behavior.

Tree-SHA512: af2e9dd60d1ad030409ae2c3805ab139c7435327823d9f8bbeede815f376cb696a5929b08a6e8c8b5f7278ed49cfb231789f9041bd57f1f03ec96501b669da5b
2024-09-09 12:29:17 -04:00
merge-script
df86a4f333
Merge bitcoin/bitcoin#30845: Update libsecp256k1 subtree to latest master
611562806c Squashed 'src/secp256k1/' changes from 642c885b61..2f2ccc4695 (Hennadii Stepanov)

Pull request description:

  This PR updates the libsecp256k1 subtree to 2f2ccc4695, which includes the following changes:
  - https://github.com/bitcoin-core/secp256k1/pull/1577
  - https://github.com/bitcoin-core/secp256k1/pull/1578
  - https://github.com/bitcoin-core/secp256k1/pull/1583
  - https://github.com/bitcoin-core/secp256k1/pull/1586
  - https://github.com/bitcoin-core/secp256k1/pull/1600

  The latter is required for https://github.com/bitcoin/bitcoin/pull/30791.

ACKs for top commit:
  l0rinc:
    utACK ff54395de4
  real-or-random:
    utACK ff54395de4 no blockers from libsecp256k1 side, and these commits touch only build/docs/tests and are thus particularly harmless
  fanquake:
    ACK ff54395de4

Tree-SHA512: 77cf1ba9aa24efdcf01e99850ea8bed54f847307a3c98c10289c9b7fd9e70c9219f28bee0f00ef7d69979d95a0ddac1e937d3d183ebc9d9b8e6cce0db1d507c9
2024-09-09 15:28:06 +01:00
merge-script
ba84c2774d
Merge bitcoin/bitcoin#30823: cmake: add USE_SOURCE_PERMISSIONS to all configure_file() usage
1f054eca4e cmake: add USE_SOURCE_PERMISSIONS to all configure_file usage (fanquake)

Pull request description:

  `USE_SOURCE_PERMISSIONS` is the default, so this should not change behaviour. However, being explicit makes it clear what we are doing.

  Related to #30815.

  See https://cmake.org/cmake/help/latest/command/configure_file.html#options.

ACKs for top commit:
  hebasto:
    ACK 1f054eca4e.
  TheCharlatan:
    ACK 1f054eca4e

Tree-SHA512: efed91b8aa0813100304ee58e169bbf5cfbb7db465ec4f7e6cbbae6053f09a36757bf96b4d1cb9ddf4c1cab0ceb3ab18805ebefa122535518ffb501c9b489d3d
2024-09-09 10:39:36 +01:00
Hennadii Stepanov
ff54395de4
Update secp256k1 subtree to latest master 2024-09-07 18:15:41 +01:00
Hennadii Stepanov
611562806c Squashed 'src/secp256k1/' changes from 642c885b61..2f2ccc4695
2f2ccc4695 Merge bitcoin-core/secp256k1#1600: cmake: Introduce `SECP256K1_APPEND_LDFLAGS` variable
421ed1b46f cmake: Introduce `SECP256K1_APPEND_LDFLAGS` variable
1988855079 Merge bitcoin-core/secp256k1#1586: fix: remove duplicate 'the' from header file comment
b307614401 Merge bitcoin-core/secp256k1#1583: ci: Bump GCC_SNAPSHOT_MAJOR to 15
fa67b6752d refactor: Use array initialization for unterminated strings
9b0f37bff1 fix: remove duplicate 'the' from header file comment
e34b476730 ci: Bump GCC_SNAPSHOT_MAJOR to 15
3fdf146bad Merge bitcoin-core/secp256k1#1578: ci: Silent Homebrew's noisy reinstall warnings
f8c1b0e0e6 Merge bitcoin-core/secp256k1#1577: release cleanup: bump version after 0.5.1
7057d3c9af ci: Silent Homebrew's noisy reinstall warnings
c3e40d75db release cleanup: bump version after 0.5.1

git-subtree-dir: src/secp256k1
git-subtree-split: 2f2ccc469540fde6495959cec061e95aab033148
2024-09-07 18:12:35 +01:00
MarcoFalke
fa9d7d5d20
test: Work around boost compilation error 2024-09-06 15:57:33 +02:00
MarcoFalke
fa3ecdf778
Revert "build: work around issue with Boost <= 1.80 and Clang >= 18"
This reverts commit cd062d6684.
2024-09-06 15:57:00 +02:00
Russell Yanofsky
30073e6b3a multiprocess: Add -ipcbind option to bitcoin-node
Add `-ipcbind` option to `bitcoin-node` to listen on an IPC socket and accept
connections from other processes. In the future, there will be an `-ipcconnect`
option added to `bitcoin-wallet` and `bitcoin-node` to allow wallet and gui
processes to connect to the node and access it.

Example usage:

    src/bitcoin-node -regtest -debug -ipcbind=unix
    src/bitcoin-wallet -regtest -ipcconnect=unix info
    src/bitcoin-gui -regtest -ipcconnect=unix
    src/bitcoin-mine -regtest -ipcconnect=unix
2024-09-06 09:08:10 -04:00
Ryan Ofsky
73fe7d7230 multiprocess: Add unit tests for connect, serve, and listen functions 2024-09-06 09:08:10 -04:00
Russell Yanofsky
955d4077aa multiprocess: Add IPC connectAddress and listenAddress methods
Allow listening on and connecting to unix sockets.
2024-09-06 09:08:10 -04:00
Hennadii Stepanov
c07fdd6546
fuzz: Don't compile BDB-specific code on MSVC in wallet_bdb_parser.cpp 2024-09-06 12:19:27 +01:00
fanquake
1f054eca4e
cmake: add USE_SOURCE_PERMISSIONS to all configure_file usage
`USE_SOURCE_PERMISSIONS` is the default, so this should not change
behaviour. However, being explicit makes it clear what we are doing.

Related to #30815.

See
https://cmake.org/cmake/help/latest/command/configure_file.html#options.
2024-09-06 10:52:19 +01:00
merge-script
118b55c462
Merge bitcoin/bitcoin#30790: bench: Remove redundant logging benchmarks
fadbcd51fc bench: Remove redundant logging benchmarks (MarcoFalke)
fa8dd952e2 bench: Use LogInfo instead of the deprecated alias LogPrintf (MarcoFalke)

Pull request description:

  `LogPrint*ThreadNames` is redundant with `LogWith(out)ThreadNames`,
  because they all measure toggling the thread names (and check that it
  has no effect on performance).

  Fix it by removing the redundant ones. This also allows to drop a deprecated logging alias.

ACKs for top commit:
  stickies-v:
    ACK fadbcd51fc

Tree-SHA512: 4fe137f374aa4ee1aa0e1da4a1f9839c0e52c23dbb93198ecafee98de39d311cc47304bba4191f3807aa00c51b1eae543e3f270f03d341c84910e5e341a1d475
2024-09-06 09:50:19 +01:00
merge-script
c0cbe26a86
Merge bitcoin/bitcoin#30748: test: Pin and document TEST_DIR_PATH_ELEMENT, SeedRand::FIXED_SEED
fa84f9decd test: Pin and document TEST_DIR_PATH_ELEMENT (MarcoFalke)
2222f7a874 test: Rename SeedRand::SEED to FIXED_SEED for clarity (MarcoFalke)

Pull request description:

  Two small test changes:

  * A refactor to update the name and documentation around `SeedRand::FIXED_SEED`.
  * A change to extract and document `TEST_DIR_PATH_ELEMENT`, and to change its value to better match the `TMPDIR_PREFIX` in functional tests. The value previously included `PACKAGE_NAME`, which is cute, but doesn't explain why it was used (to include a space). So just use `test_common bitcoin` to achieve the same with less effort.

ACKs for top commit:
  hodlinator:
    ACK fa84f9decd
  ryanofsky:
    Code review ACK fa84f9decd

Tree-SHA512: eb35d6598bb08f9b996e3a4762d8f26b2441c0ca00780798e473015af735dfc9997120895a922b94d4b6ada45adadba4a686e9cf9c285ddf688848e764c64840
2024-09-06 09:42:02 +01:00
Ava Chow
7f472e9bcd
Merge bitcoin/bitcoin#30821: build: work around issue with Boost <= 1.80 and Clang >= 18
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 13 native, x86_64, 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
cd062d6684 build: work around issue with Boost <= 1.80 and Clang >= 18 (fanquake)

Pull request description:

  Our current minimum supported Boost is `1.73.0`. However, when compiling with Boost `1.74.0` (Debian Stable), using Clang `18`, compilation fails with:
  ```bash
  In file included from /usr/include/boost/mpl/integral_c.hpp:32:
  /usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'udt_builtin_mixture_enum' [-Wenum-constexpr-conversion]
     73 |     typedef AUX_WRAPPER_INST( BOOST_MPL_AUX_STATIC_CAST(AUX_WRAPPER_VALUE_TYPE, (value - 1)) ) prior;
        |                               ^
  /usr/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 'BOOST_MPL_AUX_STATIC_CAST'
     24 | #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast<T>(expr)
        |                                               ^
  In file included from ../../../src/test/validation_chainstatemanager_tests.cpp:8:
  In file included from ../../../src/node/chainstatemanager_args.h:9:
  In file included from ../../../src/validation.h:28:
  In file included from ../../../src/txmempool.h:26:
  In file included from /usr/include/boost/multi_index/hashed_index.hpp:38:
  In file included from /usr/include/boost/multi_index/detail/node_handle.hpp:22:
  In file included from /usr/include/boost/multi_index_container_fwd.hpp:18:
  In file included from /usr/include/boost/multi_index/indexed_by.hpp:17:
  In file included from /usr/include/boost/mpl/vector.hpp:36:
  In file included from /usr/include/boost/mpl/vector/vector20.hpp:18:
  In file included from /usr/include/boost/mpl/vector/vector10.hpp:18:
  In file included from /usr/include/boost/mpl/vector/vector0.hpp:24:
  In file included from /usr/include/boost/mpl/vector/aux_/clear.hpp:18:
  In file included from /usr/include/boost/mpl/vector/aux_/vector0.hpp:22:
  In file included from /usr/include/boost/mpl/vector/aux_/iterator.hpp:19:
  In file included from /usr/include/boost/mpl/plus.hpp:19:
  In file included from /usr/include/boost/mpl/aux_/arithmetic_op.hpp:17:
  In file included from /usr/include/boost/mpl/integral_c.hpp:32:
  /usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'int_float_mixture_enum' [-Wenum-constexpr-conversion]
  /usr/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 'BOOST_MPL_AUX_STATIC_CAST'
     24 | #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast<T>(expr)
        |                                               ^
  2 errors generated.
  ```

  Work around this issue by ignoring this diagnostic for this include. I did attempt to just downgrade the error into a warning, but that did not seem to work. Not a huge fan of inline warning/issue suppression, but this seems like the cleanest thing to do here (and easy to backport to `28.x`).

  Can be tested with something like:
  ```bash
  docker pull debian:bookworm
  docker run -it debian:bookworm /bin/bash

  apt update &&  apt install ccache cmake git pkg-config libboost-dev libevent-dev python3 libsqlite3-dev lsb-release wget software-properties-common gnupg
  git clone https://github.com/bitcoin/bitcoin

  wget https://apt.llvm.org/llvm.sh
  chmod +x llvm.sh
  ./llvm.sh 18

  cd bitcoin
  cmake -B build -DCMAKE_C_COMPILER=clang-18 -DCMAKE_CXX_COMPILER=clang++-18
  cmake --build build -j17
  <snip>
  In file included from /usr/include/boost/mpl/integral_c.hpp:32:
  /usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'int_float_mixture_enum' [-Wenum-constexpr-conversion]
  /usr/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 'BOOST_MPL_AUX_STATIC_CAST'
     24 | #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast<T>(expr)
        |                                               ^
  2 errors generated.

  Apply the patch

  cmake --build build -j17
  ctest --test-dir build -j17
  ```

  Fixes #30751.

ACKs for top commit:
  achow101:
    ACK cd062d6684
  hebasto:
    ACK cd062d6684, tested on Fedora 40 using the downloaded [Boost 1.74](https://archives.boost.io/release/1.74.0/source/) and commands as follows:

Tree-SHA512: 13e5b3a544496ed2a6529ad45d03a2d872ebf41caaa06d0eec23a639d678ae1c55d73f2d4b164a4cc9e2c163264e736cd85eae90fde8089ca999cd810b16ecb5
2024-09-05 15:39:53 -04:00
fanquake
cd062d6684
build: work around issue with Boost <= 1.80 and Clang >= 18
Our current minimum supported Boost is `1.73.0`. However, when compiling
with Boost `1.74.0` (Debian Stable), using Clang `18`, compilation fails
with:
```bash
In file included from /usr/include/boost/mpl/integral_c.hpp:32:
/usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'udt_builtin_mixture_enum' [-Wenum-constexpr-conversion]
   73 |     typedef AUX_WRAPPER_INST( BOOST_MPL_AUX_STATIC_CAST(AUX_WRAPPER_VALUE_TYPE, (value - 1)) ) prior;
      |                               ^
/usr/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 'BOOST_MPL_AUX_STATIC_CAST'
   24 | #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast<T>(expr)
      |                                               ^
In file included from ../../../src/test/validation_chainstatemanager_tests.cpp:8:
In file included from ../../../src/node/chainstatemanager_args.h:9:
In file included from ../../../src/validation.h:28:
In file included from ../../../src/txmempool.h:26:
In file included from /usr/include/boost/multi_index/hashed_index.hpp:38:
In file included from /usr/include/boost/multi_index/detail/node_handle.hpp:22:
In file included from /usr/include/boost/multi_index_container_fwd.hpp:18:
In file included from /usr/include/boost/multi_index/indexed_by.hpp:17:
In file included from /usr/include/boost/mpl/vector.hpp:36:
In file included from /usr/include/boost/mpl/vector/vector20.hpp:18:
In file included from /usr/include/boost/mpl/vector/vector10.hpp:18:
In file included from /usr/include/boost/mpl/vector/vector0.hpp:24:
In file included from /usr/include/boost/mpl/vector/aux_/clear.hpp:18:
In file included from /usr/include/boost/mpl/vector/aux_/vector0.hpp:22:
In file included from /usr/include/boost/mpl/vector/aux_/iterator.hpp:19:
In file included from /usr/include/boost/mpl/plus.hpp:19:
In file included from /usr/include/boost/mpl/aux_/arithmetic_op.hpp:17:
In file included from /usr/include/boost/mpl/integral_c.hpp:32:
/usr/include/boost/mpl/aux_/integral_wrapper.hpp:73:31: error: integer value -1 is outside the valid range of values [0, 3] for the enumeration type 'int_float_mixture_enum' [-Wenum-constexpr-conversion]
/usr/include/boost/mpl/aux_/static_cast.hpp:24:47: note: expanded from macro 'BOOST_MPL_AUX_STATIC_CAST'
   24 | #   define BOOST_MPL_AUX_STATIC_CAST(T, expr) static_cast<T>(expr)
      |                                               ^
2 errors generated.
```

Work around this issue by ignoring this diagnostic for this include.
I did attempt to just downgrade the error into a warning, but that did
not seem to work.

See https://github.com/bitcoin/bitcoin/issues/30751 for further
discussion.
2024-09-05 14:45:21 +01:00
Hennadii Stepanov
d6a1b94ffd
Merge bitcoin-core/gui#834: qt, build: remove unneeded Q_IMPORT_PLUGIN macro calls
7346b01092 qt, build: remove unneeded `Q_IMPORT_PLUGIN` macro calls (Sebastian Falbesoner)

Pull request description:

  After the recent full removal of Autotools (PR [#30664](https://github.com/bitcoin/bitcoin/pull/30664)), these macros are not needed anymore in the .cpp files according to the TODO in qt's CMakeLists.txt. Tested building on OpenBSD 7.5, where the XCB plugin was still imported according to the debug log:
  ```
  2024-09-02T21:13:27Z Bitcoin Core version v28.99.0-7346b0109208 (release build)
  2024-09-02T21:13:27Z Qt 5.15.12 (dynamic), plugin=xcb
  2024-09-02T21:13:27Z No static plugins.
  2024-09-02T21:13:27Z Style: fusion / QFusionStyle
  2024-09-02T21:13:27Z System: OpenBSD 7.5, x86_64-little_endian-lp64
  ```

ACKs for top commit:
  hebasto:
    ACK 7346b01092.

Tree-SHA512: ffa033fc6e0412a99d2c167044cc7af89512a731172d6911db71434f5353e811802c268d853a76d3732e9da954444cf6c39a852aeb25938c435826e117a16fca
2024-09-05 14:28:52 +01:00
merge-script
6852d1d487
Merge bitcoin/bitcoin#30796: test: Use std::span and std::string_view for raw data
faecca9a85 test: Use span for raw data (MarcoFalke)
fac973647d test: Use string_view for json_tests (MarcoFalke)

Pull request description:

  The build system converts raw data into a C++ header file for tests.

  This change modernizes the code to use the convenience wrappers `std::span` and `std::string_view`, so that redundant copies can be avoided.

ACKs for top commit:
  fjahr:
    re-ACK faecca9a85
  TheCharlatan:
    ACK faecca9a85
  stickies-v:
    ACK faecca9a85
  hebasto:
    ACK faecca9a85, I have reviewed the code and the generated headers.

Tree-SHA512: 1f4951c54aff11ba27c41fb70f2821bdb79e06ca0abae734b970bd0d64dda9d8cced824a891fd51b3e9d4e5715ee9eb49ed5d369010a45eca7c3bec9f8641235
2024-09-05 13:46:22 +01:00
MarcoFalke
faecca9a85
test: Use span for raw data
This change allows to drop brittle sizeof calls in favor of the
std::span::size method.

Other improvements include:

* Use of a namespace to mark test and bench data
* Use of the modern std::byte
* Drop of a no longer used std::vector copy and the bench/data module
2024-09-05 10:57:19 +02:00
Fabian Jahr
c2b779da4e
refactor: Manage dumptxoutset RAII classes with std::optional
Also removes unused UniValue error variable.
2024-09-05 10:30:35 +02:00
MarcoFalke
fadbcd51fc
bench: Remove redundant logging benchmarks
LogPrint*ThreadNames is redundant with LogWith(out)ThreadNames, because
they all measure toggling the thread names (and check that it has no
effect on performance).

This also allows to remove unused and deprecated macros.
2024-09-05 07:17:22 +02:00
MarcoFalke
fa8dd952e2
bench: Use LogInfo instead of the deprecated alias LogPrintf 2024-09-05 07:17:07 +02:00
Ava Chow
5373aa30e2
Merge bitcoin/bitcoin#30788: test: fixing failing system_tests/run_command under some Locales
ae48a22a3d test: fixing failing system_tests/run_command under some Locales (Jadi)

Pull request description:

  the run_command test under system_tests fails if the locale is anything
  other than English ones because results such as "No such file or directory"
  will be different under Non-English locales.

  On the old version, a `ls nonexistingfile` was used to generate the error
  output which is not ideal. In the current version we are using a Python one-liner
  to generate a non 0 zero return value and "err" on stderr and check the
  expected value against this.

  fixes https://github.com/bitcoin/bitcoin/issues/30608

ACKs for top commit:
  maflcko:
    review ACK ae48a22a3d
  achow101:
    ACK ae48a22a3d
  hebasto:
    ACK ae48a22a3d, tested on Ubuntu 24.04 by switching locale.

Tree-SHA512: af7522ddcd786fa4a6832c8336ca89d8ff05f49ff963cbe1a96653b0edf29e0f950a032f23d742b16b3895e90cf5117b5f6a95464268dec67039df166d7d8639
2024-09-04 15:28:41 -04:00
Ava Chow
3210d87dfc
Merge bitcoin/bitcoin#29043: fuzz: make FuzzedDataProvider usage deterministic
01960c53c7 fuzz: make FuzzedDataProvider usage deterministic (Martin Leitner-Ankerl)

Pull request description:

  There exist many usages of `fuzzed_data_provider` where it is evaluated directly in the function call.
  Unfortunately, [the order of evaluation of function arguments is unspecified](https://en.cppreference.com/w/cpp/language/eval_order), and a simple example shows that it can differ e.g. between clang++ and g++: https://godbolt.org/z/jooMezWWY

  When the evaluation order is not consistent, the same fuzzing/random input will produce different output, which is bad for coverage/reproducibility. This PR fixes all these cases I have found where unspecified evaluation order could be a problem.

  Finding these has been manual work; I grepped the sourcecode for these patterns, and looked at each usage individually. So there is a chance I missed some.

  * `fuzzed_data_provider`
  * `.Consume`
  * `>Consume`
  * `.rand`

  I first discovered this in https://github.com/bitcoin/bitcoin/pull/29013#discussion_r1420236394. Note that there is a possibility that due to this fix the evaluation order is now different in many cases than when the fuzzing corpus has been created. If that is the case, the fuzzing corpus will have worse coverage than before.

  Update: In list-initialization the order of evaluation is well defined, so e.g. usages in `initializer_list` or constructors that use `{...}` is ok.

ACKs for top commit:
  achow101:
    ACK 01960c53c7
  vasild:
    ACK 01960c53c7
  ismaelsadeeq:
    ACK 01960c53c7

Tree-SHA512: e56d087f6f4bf79c90b972a5f0c6908d1784b3cfbb8130b6b450d5ca7d116c5a791df506b869a23bce930b2a6977558e1fb5115bb4e061969cc40f568077a1ad
2024-09-04 15:04:53 -04:00
Ava Chow
81276540d3
Merge bitcoin/bitcoin#30148: cli: restrict multiple exclusive argument usage in bitcoin-cli
c8e6771af0 test: restrict multiple CLI arguments (naiyoma)
8838c4f171 common/args.h: automate check for multiple cli commands (naiyoma)

Pull request description:

  This PR is a continuation of the validation suggested [here](https://github.com/bitcoin/bitcoin/pull/27815) to ensure that only one Request Handler can be specified at a time.

ACKs for top commit:
  stratospher:
    reACK c8e6771.
  achow101:
    ACK c8e6771af0
  tdb3:
    cr re ACK c8e6771af0

Tree-SHA512: f4fe036fee342a54f1a7ac702ac35c40bf3d420fde6ab16313a75327292d5ee5c8ece1be9f852a13fcf73da1148b143b37b4894e294052abdde6eefb2e8c6f3f
2024-09-04 14:47:38 -04:00
Ava Chow
cb65ac469a
Merge bitcoin/bitcoin#29605: net: Favor peers from addrman over fetching seednodes
6eeb188d40 test: adds seednode functional tests (Sergi Delgado Segura)
3270f0adad net: Favor peers from addrman over fetching seednodes (Sergi Delgado Segura)

Pull request description:

  This is a follow-up of #28016 motivated by https://github.com/bitcoin/bitcoin/pull/28016#pullrequestreview-1913140932 and https://github.com/bitcoin/bitcoin/pull/28016#issuecomment-1984448937.

  The current behavior of seednode fetching is pretty eager: we do it as the first step under `ThreadOpenNetworkConnections` even if some peers may be queryable from our addrman. This poses two potential issues:

  - First, if permanently set (e.g. running with seednode in a config file) we'd be signaling such seed every time we restart our node
  - Second, we will be giving the seed node way too much influence over our addrman, populating the latter with data from the former even when unnecessary

  This changes the behavior to only add seednodes to `m_addr_fetch` if our addrman is empty, or little by little after we've spent some time trying addresses from our addrman. Also, seednodes are added to `m_addr_fetch` in random order, to avoid signaling the same node in case more than one seed is added and we happen to try them over multiple restarts

ACKs for top commit:
  achow101:
    ACK 6eeb188d40
  cbergqvist:
    ACK 6eeb188d40
  itornaza:
    Tested ACK 6eeb188d40
  tdb3:
    ACK 6eeb188d40

Tree-SHA512: b04445412f22018852d6bef4d3f1e88425ee6ddb434f61dcffa9e0c41b8e31f8c56f83858d5c7686289c86dc4c9476c437df15ea61a47082e2bb2e073cc62f15
2024-09-04 13:15:08 -04:00
Ava Chow
b8d2f58e06
Merge bitcoin/bitcoin#30808: rpc: dumptxoutset height parameter follow-ups (29553)
a3108a7c56 rpc: Manage dumptxoutset rollback with RAII class (Fabian Jahr)
c5eaae3b89 doc: Add -rpcclienttimeout=0 to loadtxoutset examples (Fabian Jahr)
598b9bba5a rpc: Don't re-enable previously disabled network after dumptxoutset (Fabian Jahr)

Pull request description:

  First, this addresses two left-over comments in #29553:

  - When running `dumptxoutset` the network gets disabled in the beginning and then re-enabled at the end. The network would be re-enabled even if the user had already disabled the network themself before running `dumptxoutset`. The network is now not re-enabled anymore since that might not be what the user wants.
  - The `-rpcclienttimeout=0` option is added to `loadtxoutset` examples in documentation

  Additionally, pablomartin4btc notified me that he found his node stuck at the invalidated height after some late testing after #29553 was merged. We could not find the actual source of the issue since his logs got lost. However, it seems likely that some kind of disruption stopped the process before the node could roll forward again. We fixed this issue for network disablement with a RAII class previously and it seems logical that this can happen the same way for the rollback part so I suggest to also fix it the same way.

  An example to reproduce the issue described above as I think it happened: Remove the `!` in the following line in `PrepareUTXOSnapshot()` to simulate an issue occurring during `GetUTXOStats()`.

  ```
  if (!maybe_stats) {
  ```

  This leaves the node in the following state on master:

  ```
  $ build/src/bitcoin-cli -rpcclienttimeout=0 -named dumptxoutset utxo-859750.dat rollback=859750
  error code: -32603
  error message:
  Unable to read UTXO set
  $ build/src/bitcoin-cli getchaintips
  [
    {
      "height": 859762,
      "hash": "00000000000000000002ec7a0fcca3aeca5b35545b52eb925766670aacc704ad",
      "branchlen": 12,
      "status": "headers-only"
    },
    {
      "height": 859750,
      "hash": "0000000000000000000010897b6b88a18f9478050200d8d048013c58bfd6229e",
      "branchlen": 0,
      "status": "active"
    },
  ```

  (Note that the first tip is `headers-only` and not `invalid` only because I started `dumptxoutset` before my node had fully synced to the tip. pablomartin4btc saw it as `invalid`.)

ACKs for top commit:
  maflcko:
    re-ACK a3108a7c56 🐸
  achow101:
    ACK a3108a7c56
  pablomartin4btc:
    cr ACK a3108a7c56

Tree-SHA512: d2ab32f62de2253312e27d7d753ec0995da3fe7a22ffc3d6c7cfa3b68a4a144c59210aa82b7a704c2a29c3b2aad6ea74972e3e8bb979ee4b7082a20f4bfddc9c
2024-09-04 11:40:26 -04:00
glozow
f66011e88f
Merge bitcoin/bitcoin#30784: test: add check that too large txs aren't put into orphanage
66d13c8702 test: add check that large txs aren't put into orphanage (Sebastian Falbesoner)
ed7d224666 test: add `BulkTransaction` helper to unit test transaction utils (Sebastian Falbesoner)

Pull request description:

  This PR adds test coverage for the following check in `TxOrphanage::AddTx`, where large orphan txs are ignored in order to avoid memory exhaustion attacks:
  5abb9b1af4/src/txorphanage.cpp (L22-L34)
  Note that this code-path isn't reachable under normal circumstances, as txs larger than `MAX_STANDARD_TX_WEIGHT` are already rejected earlier in the course of doing the mempool standardness checks (see `MemPoolAccept::PreChecks` -> `IsStandardTx` -> `reason = "tx-size";`), so this is only relevant if tx standardness rules are disabled via `-acceptnonstdtxns=1`. The ignore path is checked ~~by asserting the debug log, which is ugly, but as far as I know there is currently no way to access the orphanage entries from the outside~~ via unit test that checks the return value of `AddTx`. As an alternative to adding test coverage, one might consider removing this check altogether (or replacing it with an `Assume`), as it's redundant as explained above.

ACKs for top commit:
  maflcko:
    review ACK 66d13c8702
  glozow:
    ACK 66d13c8702
  tdb3:
    re-ACK 66d13c8702

Tree-SHA512: 88e8254ab5fca70c387a5992649ea6a704a65162999be972cc86bd74fc26c5f0f1e13e04856708d07ad5524cb77c0918e19663db92b3593e842469dfe04af6a1
2024-09-04 10:20:33 -04:00
Fabian Jahr
a3108a7c56
rpc: Manage dumptxoutset rollback with RAII class 2024-09-04 16:04:17 +02:00
Fabian Jahr
c5eaae3b89
doc: Add -rpcclienttimeout=0 to loadtxoutset examples 2024-09-04 15:49:04 +02:00
Fabian Jahr
598b9bba5a
rpc: Don't re-enable previously disabled network after dumptxoutset
Also fixes a typo in the RPC help text.
2024-09-04 15:49:03 +02:00
merge-script
ab317ad2ef
Merge bitcoin/bitcoin#30804: fuzz: Rename fuzz_seed_corpus to fuzz_corpora
8888beea8d scripted-diff: fuzz: Rename fuzz_seed_corpus to fuzz_corpora (MarcoFalke)

Pull request description:

  Now that cmake was a breaking change for all fuzz scripts, it seems fine to bundle it with another breaking change to rename the fuzz corpora directory, as discussed and approved in https://github.com/bitcoin-core/qa-assets/issues/200:

  * The word "seed" in the old name doesn't really apply. In reality it is a collection of fuzz input seeds, as well as fuzz inputs.
  * The rename will also allow in the future (when there is a need and desire) to provide a minimal set of possibly hand-crafted or otherwise non-fuzz-generated fuzz seed inputs to some fuzz targets (and possibly store them in a separate folder and validate that their format is still accurate and matches the fuzz target code).
  * Finally, "corpus" is renamed to corpora, to clarify that the folder holds the fuzz inputs for several fuzz targets.

ACKs for top commit:
  brunoerg:
    ACK 8888beea8d
  marcofleon:
    ACK 8888beea8d

Tree-SHA512: abc693ca5d946850f04b6349e2a98f8fbc2ba9991be5a025bc0f357e341cbe7510f2f5f0e47b997d07136736d818df361270f372b8fb70860995a0605ca81e4d
2024-09-04 14:04:27 +01:00
merge-script
4835bba2cb
Merge bitcoin/bitcoin#30802: doc: Clarify libbitcoin_consensus in design/libraries.md
fa78ed83be doc: Clarify libbitcoin_consensus in design/libraries.md (MarcoFalke)

Pull request description:

  Now that the shared library has been removed in commit 80f8b92f4f, update the documentation to drop the no-longer applicable prefix "Stable...".

ACKs for top commit:
  hebasto:
    ACK fa78ed83be.
  fanquake:
    ACK fa78ed83be

Tree-SHA512: d7b946d50f734c0474ff6155a655a2bb873f76e071bfeeca1dd42ea5fdd32bc1e45129826bb54e3f111265d19c2aba2d02cb77ad7663f9fc40c8c875e5fddda2
2024-09-04 10:13:56 +01:00
Jadi
ae48a22a3d test: fixing failing system_tests/run_command under some Locales
the run_command test under system_tests fails if the locale is anything
other than English ones because results such as "No such file or directory"
will be different under Non-English locales.

On the old version, a `ls nonexistingfile` was used to generate the error
output which is not ideal. In the current version we are using a Python one-liner
to generate a non 0 zero return value and "err" on stderr and check the
expected value against this.

fixes #30608
2024-09-04 09:30:05 +03:30
Ava Chow
94c307b3c0
Merge bitcoin/bitcoin#30675: http: set TCP_NODELAY when creating HTTP server
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 13 native, x86_64, 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
03d49d0f25 http: set TCP_NODELAY when creating HTTP server (Roman Zeyde)

Pull request description:

  Otherwise, the default HTTP server config may result in high latency, due to Nagle's algorithm (on the server) and delayed ACK (on the client):

  [1] https://www.extrahop.com/blog/tcp-nodelay-nagle-quickack-best-practices
  [2] https://eklitzke.org/the-caveats-of-tcp-nodelay

  Without the fix, fetching a small block takes ~40ms (when connection keep-alive is enabled):
  ```
  $ ab -k -c 1 -n 100 http://localhost:8332/rest/block/00000000000002b5898f7cdc80d9c84e9747bc6b9388cc989971d443f05713ee.bin

  Server Software:
  Server Hostname:        localhost
  Server Port:            8332

  Document Path:          /rest/block/00000000000002b5898f7cdc80d9c84e9747bc6b9388cc989971d443f05713ee.bin
  Document Length:        25086 bytes

  Concurrency Level:      1
  Time taken for tests:   4.075 seconds
  Complete requests:      100
  Failed requests:        0
  Keep-Alive requests:    100
  Total transferred:      2519200 bytes
  HTML transferred:       2508600 bytes
  Requests per second:    24.54 [#/sec] (mean)
  Time per request:       40.747 [ms] (mean)
  Time per request:       40.747 [ms] (mean, across all concurrent requests)
  Transfer rate:          603.76 [Kbytes/sec] received

  Connection Times (ms)
                min  mean[+/-sd] median   max
  Connect:        0    0   0.0      0       0
  Processing:     0   41   4.1     41      42
  Waiting:        0    0   0.1      0       1
  Total:          0   41   4.1     41      42

  Percentage of the requests served within a certain time (ms)
    50%     41
    66%     41
    75%     41
    80%     41
    90%     42
    95%     42
    98%     42
    99%     42
   100%     42 (longest request)
  ```

  With the fix, it takes ~0.2ms:
  ```
  $ ab -k -c 1 -n 1000 http://localhost:8332/rest/block/00000000000002b5898f7cdc80d9c84e9747bc6b9388cc989971d443f05713ee.bin

  Benchmarking localhost (be patient)
  Completed 100 requests
  Completed 200 requests
  Completed 300 requests
  Completed 400 requests
  Completed 500 requests
  Completed 600 requests
  Completed 700 requests
  Completed 800 requests
  Completed 900 requests
  Completed 1000 requests
  Finished 1000 requests

  Server Software:
  Server Hostname:        localhost
  Server Port:            8332

  Document Path:          /rest/block/00000000000002b5898f7cdc80d9c84e9747bc6b9388cc989971d443f05713ee.bin
  Document Length:        25086 bytes

  Concurrency Level:      1
  Time taken for tests:   0.194 seconds
  Complete requests:      1000
  Failed requests:        0
  Keep-Alive requests:    1000
  Total transferred:      25192000 bytes
  HTML transferred:       25086000 bytes
  Requests per second:    5147.05 [#/sec] (mean)
  Time per request:       0.194 [ms] (mean)
  Time per request:       0.194 [ms] (mean, across all concurrent requests)
  Transfer rate:          126625.50 [Kbytes/sec] received

  Connection Times (ms)
                min  mean[+/-sd] median   max
  Connect:        0    0   0.0      0       0
  Processing:     0    0   0.0      0       0
  Waiting:        0    0   0.0      0       0
  Total:          0    0   0.0      0       0

  Percentage of the requests served within a certain time (ms)
    50%      0
    66%      0
    75%      0
    80%      0
    90%      0
    95%      0
    98%      0
    99%      0
   100%      0 (longest request)
  ```

ACKs for top commit:
  achow101:
    ACK 03d49d0f25
  theStack:
    re-ACK 03d49d0f25
  tdb3:
    ACK 03d49d0f25

Tree-SHA512: bbf3d78b8521f569430850ec4315a75711303547df1a3de213a4ad34c9700105e374e0a649352fd05f8e4badb5b59debd3720e1c5d392c5113d7816648f7fcaa
2024-09-03 17:27:50 -04:00
Ava Chow
27e89bc2f5
Merge bitcoin/bitcoin#26619: log: expand BCLog::LogFlags (categories) to 64 bits
b31a0cd037 log: expand BCLog::LogFlags (categories) to 64 bits (Larry Ruane)

Pull request description:

  Increase the maximum number of logging categories from 32 to 64.

  We're currently using 29 of the 32 available logging categories (there are only 3 remaining). It would be good to increase the limit soon; the fourth PR to be merged that adds a new logging category will be blocked until something like this is done.

  This PR also adds a `TEST` category that uses the new range (`1ULL << 63`) in case there's a hidden assumption somewhere that the `BCLog::LogFlags` type is 32 bits. (Also added a test for this test category.) It also provides an example showing that the expression must be `1ULL << <shift>` for shift value 31 and beyond.

ACKs for top commit:
  achow101:
    ACK b31a0cd037
  vasild:
    ACK b31a0cd037
  ryanofsky:
    Code review ACK b31a0cd037, just dropping mask_bit constant since last review. I still think
  theStack:
    Code-review ACK b31a0cd037

Tree-SHA512: de422dbeb479848d370aed42d415f42461457ab0eda62b245dc7ff9f0e111626e7d4c0d62ff13082ec664d05fbb0db04c71eb4b6f22eb8f19198826a67c4035e
2024-09-03 16:33:49 -04:00
Sebastian Falbesoner
66d13c8702 test: add check that large txs aren't put into orphanage 2024-09-03 22:23:48 +02:00
Sebastian Falbesoner
ed7d224666 test: add BulkTransaction helper to unit test transaction utils
The padding method used matches the one used in MiniWallet,
`MiniWallet._bulk_tx`.
2024-09-03 22:20:01 +02:00
Ava Chow
d4b5553849
Merge bitcoin/bitcoin#30742: kernel: Use spans instead of vectors for passing block headers to validation functions
a2955f0979 validation: Use span for ImportBlocks paths (TheCharlatan)
20515ea3f5 validation: Use span for CalculateClaimedHeadersWork (TheCharlatan)
52575e96e7 validation: Use span for ProcessNewBlockHeaders (TheCharlatan)

Pull request description:

  Makes it friendlier for potential future users of the kernel library if they do not store the headers in a std::vector, but can guarantee contiguous memory.

  Take this opportunity to also change the argument of ImportBlocks previously taking a `std::vector` to a `std::span`.

ACKs for top commit:
  stickies-v:
    re-ACK a2955f0979 - no changes except further walking the ~file~ path of modernizing variable names.
  maflcko:
    ACK a2955f0979 🕑
  achow101:
    ACK a2955f0979
  danielabrozzoni:
    ACK a2955f0979

Tree-SHA512: 8b07f4ad26e270b65600d1968cd78847b85caca5bfbb83fd9860389f26656b1d9a40b85e0990339f50403d18cedcd2456990054f3b8b0bedce943e50222d2709
2024-09-03 15:40:40 -04:00
Ava Chow
fa5fc71199
Merge bitcoin/bitcoin#29553: assumeutxo: Add dumptxoutset height param, remove shell scripts
94b0adcc37 rpc, refactor: Prevent potential race conditions in dumptxoutset (Fabian Jahr)
e868a6e070 doc: Improve assumeutxo guide and add more docs/comments (Fabian Jahr)
b29c21fc92 assumeutxo: Remove devtools/utxo_snapshot.sh (Fabian Jahr)
20a1c77aa7 contrib: Remove test_utxo_snapshots.sh (Fabian Jahr)
8426850352 test: Test for dumptxoutset at specific height (Fabian Jahr)
993cafe7e4 RPC: Add type parameter to dumptxoutset (Fabian Jahr)
fccf4f91d2 RPC: Extract ReconsiderBlock helper (Fabian Jahr)
446ce51c21 RPC: Extract InvalidateBlock helper (Fabian Jahr)

Pull request description:

  This adds a height parameter to the `dumptxoutset` RPC. This internalizes the workflow that was previously done by scripts: roll back the chain to the height we actually want the snapshot from, create the snapshot, roll forward to the real tip again.

  The nice thing about internalizing this functionality is that we can write tests for the code and it gives us more options to make the functionality robust. The shell scripts we have so far will be more cumbersome to maintain in the long run, especially since we will only notice later when we have broken them. I think it's safe to remove these `test_utxo_snapshots.sh` as well when we have this option in `dumptxoutset` because we have also added some good additional functional test coverage for this functionality.

ACKs for top commit:
  Sjors:
    re-utACK 94b0adcc37
  achow101:
    ACK 94b0adcc37
  mzumsande:
    ACK 94b0adcc37
  pablomartin4btc:
    re-ACK 94b0adcc37

Tree-SHA512: a4c9af5f687d1ca7bfb579a36f363882823386b5fa80c05de531b05a2782b5da6ff5baf3ada4bca8f32f63975d86f1948175abed9affe51fc958472b5f838dab
2024-09-03 15:30:45 -04:00
MarcoFalke
8888beea8d
scripted-diff: fuzz: Rename fuzz_seed_corpus to fuzz_corpora
-BEGIN VERIFY SCRIPT-
 ren() { sed -i "s:\<$1\>:$2:g" $( git grep -l "$1" ) ; }
 ren fuzz_seed_corpus     fuzz_corpora
 ren FUZZ_SEED_CORPUS_DIR FUZZ_CORPORA_DIR
-END VERIFY SCRIPT-
2024-09-03 20:40:35 +02:00
MarcoFalke
fa78ed83be
doc: Clarify libbitcoin_consensus in design/libraries.md 2024-09-03 19:35:43 +02:00
MarcoFalke
fac973647d
test: Use string_view for json_tests
This avoids a static constructor of the global std::string, and rules
out possibly expensive and implicit copies of the string completely.
2024-09-03 16:06:20 +02:00
merge-script
9cb9651d92
Merge bitcoin/bitcoin#30778: build: Fix linking for fuzz target when building with MSan
Some checks are pending
CI / test each commit (push) Waiting to run
CI / macOS 13 native, x86_64, 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
787dfaf084 ci: Do not override `-g -O1` set in `MSAN_FLAGS` (Hennadii Stepanov)
26c460aa8b build: Fix linking for `fuzz` target when building with MSan (Hennadii Stepanov)

Pull request description:

  The first commit fixes https://github.com/bitcoin/bitcoin/issues/30760.

  The second commit:
  1. Preserves `-g -O1` set in `MSAN_FLAGS`. Since configuration-specific flags override general flags, these are set to empty strings. A similar approach is used in the OSS-Fuzz repository.
  2. Sets the "Debug" build configuration when depends are built with `DEBUG=1`, ensuring that `linux_debug_CPPFLAGS` from depends are passed to the main build system.

ACKs for top commit:
  maflcko:
    review-only ACK 787dfaf084
  fanquake:
    ACK 787dfaf084 - as a follow up it would be good to:

Tree-SHA512: c324390d1dbda30f82025d8482ddb0cfa0395f9ba225a2ddce05a123c65e0622a6a1d5f0fa03f09e21d62792431cf3da5c49e41a3ac7f7a958d0392a0430f29c
2024-09-03 12:15:28 +01:00
merge-script
4c526f575c
Merge bitcoin/bitcoin#30741: doc: update documentation and scripts related to build directories
6a68343ffb doc: Prepend 'build/' to binary paths under 'src/' in docs (Lőrinc)
91b3bc2b9c doc: Update documentation generation example in developer-notes.md (Lőrinc)

Pull request description:

  In [the other readmes](6ce50fd9d0/src/test/README.md (L19)) we've provided a default build directory instead, unified the `developer-notes.md` to specify it explicitly.

  In the next commit I've used this default to go over each reference to our binaries and changed their in-source references to the build directory.
  Some of these changes were in example outputs - I haven't validated that the outputs are still the same.
  I haven't modified the build folders in the devtools.

ACKs for top commit:
  maflcko:
    review ACK 6a68343ffb
  pablomartin4btc:
    ACK 6a68343ffb
  fanquake:
    ACK 6a68343ffb - we still need to followup with other scripts/devtools, and likely unify what we are doing in some way, but this is an improvement.

Tree-SHA512: 905d9c68cafe1e405e98d6aa089d7a36a34c9e03403df5c67ac2c9a98cfa54a0305b647cb92247dcb9f49e9b509a8ba88367392b95618c67059684c67b6c36fb
2024-09-03 10:31:00 +01:00
Hennadii Stepanov
26c460aa8b
build: Fix linking for fuzz target when building with MSan 2024-09-02 23:18:16 +01:00
Sebastian Falbesoner
7346b01092 qt, build: remove unneeded Q_IMPORT_PLUGIN macro calls
After the recent full removal of Autotools (PR #30664), these
macros are not needed anymore in the .cpp files according to the
TODO in qt's CMakeLists.txt. Tested building on OpenBSD 7.5, where
the XCB plugin was still imported according to the debug log.
2024-09-02 23:01:30 +02:00