0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-24 12:41:41 -05:00
Commit graph

27480 commits

Author SHA1 Message Date
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
ismaelsadeeq
c8e2eeeffb
chain: uniformly use SettingsAction enum in settings methods 2024-09-08 20:37:45 +01:00
Hennadii Stepanov
5c80192ff6
test: Drop no longer needed workarounds
`ctest` skips "no test cases matching filter" tests gracefully.
2024-09-08 09:05:39 +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
Hennadii Stepanov
f03c942095
build, test: Add missed log options 2024-09-06 21:59:51 +01:00
TheCharlatan
0dd16d7118
build: Add a pkg-config file for libbitcoinkernel 2024-09-06 21:35:07 +02:00
TheCharlatan
45be32f838
build: Produce a usable static kernel library
Since the move to cmake, the kernel static library that is installed
after a cmake --install build is unusable. It lacks symbols for the
internal libraries, besides those defined in the kernel library target.

This is because cmake, unlike the libtool archiver, does not combine
multiple static libraries into a single static library on installation.
This is likely an intentional design choice, since there were a bunch of
caveats to the way libtool calculated these libraries.

Fix this problem by installing all the required libraries. The user must
then link all of them along with the bitcoin kernel library.
2024-09-06 21:19:27 +02:00
stickies-v
43cd83b0c7
test: move uint256_tests/operator_with_self to arith_uint256_tests
move/formatting-only change.

These tests do not cover uint256, so move them to the appropriate
test suite. Additionally, apply clang-format suggestions.
2024-09-06 17:36:28 +02:00
stickies-v
c6c994cb2b
test: remove test-only uint160S
uint160S is a test-only function, and testing input that
is not allowed in uint160::FromHex() is superfluous.

Tests that can't use uint160::FromHex() because they use input
with non-hex digit characters are
a) modified by dropping the non-hex digit characters if that
provides useful test coverage.
b) dropped if the test without non-hex digit characters does
not provide useful test coverage, e.g. because it is now
duplicated.
2024-09-06 17:36:27 +02:00
stickies-v
62cc4656e2
test: remove test-only uint256S
uint256S was previously deprecated for being unsafe. All non-test
usage has already been removed in earlier commits.

1. Tests now use uint256::FromHex() or other constructors wherever
possible without further modification.
2. Tests that can't use uint256::FromHex() because they use input
with non-hex digit characters are
  a) modified by dropping the non-hex digit characters if that
     provides useful test coverage.
  b) dropped if the test without non-hex digit characters does
     not provide useful test coverage, e.g. because it is now
     duplicated.

Additionally, use BOOST_CHECK_EQUAL where relevant on touched lines
to make error messages more readable.
2024-09-06 17:36:18 +02:00
stickies-v
adc00ad728
test: remove test-only arith_uint256S
Tests that are solely testing constructing from a hex string
are dropped, others are modified to use a uint256 constructor
or the arith_uint256 uint64_t constructor.

Since an arith_uint256 can not be constructed from a string
directly, we need to ensure that test coverage on
UintToArith256(uint256::FromHex()) is not reduced.
uint256::FromHex() already has good test coverage, but
the test coverage on UintToArith256() and ArithToUint256()
is increased in this commit by upgrading the `conversion`
test case.

Moreover, since `uint256.h` does not have any dependencies
on `arith_uint256.h`, the conversion tests are moved to
`arith_uint256_tests.cpp` so the dependency can be cleaned
up entirely in a future commit.
2024-09-06 17:36:18 +02:00
Hennadii Stepanov
7b04fabe2d
build: Introduce "Kernel" installation component
This change enables building and installing only `libbitcoinkernel`,
without the need to disable other targets during the project build
system generation.
2024-09-06 15:58:55 +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
brunoerg
f482d0e366 fuzz: reduce number of iterations in crypto_aeadchacha20poly1305 target
By reducing the number of iterations we improve the performance of
this target and may increase coverage.
2024-09-05 17:56:54 -03:00
ismaelsadeeq
1e9e735670
chain: move new settings safely in overwriteRwSetting 2024-09-05 21:18:17 +01:00
Ava Chow
7f472e9bcd
Merge bitcoin/bitcoin#30821: build: work around issue with Boost <= 1.80 and Clang >= 18
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
ismaelsadeeq
1c409004c8
test: remove wallet context from write_wallet_settings_concurrently 2024-09-05 20:32:20 +01:00
Sergi Delgado Segura
d4c7c4009d init: error out if -maxconnections is negative 2024-09-05 11:43:46 -04:00
Sergi Delgado Segura
c773649481 init: improves file descriptors accounting and docs
The current logic for file descriptor accounting is pretty convoluted and hard
to follow. This is partially caused by the lack of documentation plus non-intuitive
variable naming (which was more intuitive when fewer things were accounted for, but
hasn't aged well). This has led to this accounting being error-prone and hard to maintain
(as shown in he previous commit).

Redefine some of the constants, plus document what are we accounting for so this can be
extended more easily

Remove FreeBSD workaround to #2695
2024-09-05 11:43:46 -04:00
Sergi Delgado Segura
29008a7ff4 init: fixes fd accounting regarding poll/select
We are computing our file descriptors limits based on whether we use
poll or select. However, we are taking that into account only partially
(subtracting from fd_max in one case, but from nFD later on). Moreover,
nBind is also only accounted for partially.

Simplify and fix this logic
2024-09-05 11:43:46 -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
Sebastian Falbesoner
58499b00d0 refactor: move SignSignature helpers to test utils
These helpers haven't been used in production code since segwit was
merged more than eight years ago (see commit 605e8473, PR #8149),
so it seems appropriate to move them to the test utils module.

Can be reviewed via `--color-moved=dimmed-zebra`.
2024-09-04 21:06:09 +02: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
stickies-v
f51b237723
refactor: rpc: use uint256::FromHex for ParseHashV
uint256S() is deprecated for being unsafe, and will be removed
in a future commit.
2024-09-04 16:39:55 +01: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