Commit graph

44088 commits

Author SHA1 Message Date
merge-script
9491676438
Merge bitcoin/bitcoin#31157: Cleanups to port mapping module post UPnP drop
70398ae05b mapport: make ProcessPCP void (Antoine Poinsot)
9e6cba2988 mapport: remove unnecessary 'g_mapport_enabled' (Antoine Poinsot)
8fb45fcda0 mapport: remove unnecessary 'g_mapport_current' variable (Antoine Poinsot)
1b223cb19b mapport: merge DispatchMapPort into StartMapPort (Antoine Poinsot)
9bd936fa34 mapport: drop unnecessary function (Antoine Poinsot)
2a6536ceda mapport: rename 'use_pcp' to 'enable' (Antoine Poinsot)
c4e82b854c mapport: make 'enabled' and 'current' bool (Antoine Poinsot)

Pull request description:

  Followup to #31130, this does a couple cleanups to `src/mapport.*` to clarify the logic now that there is a single protocol option for port mapping.

ACKs for top commit:
  laanwj:
    Code review ACK 70398ae05b
  TheCharlatan:
    ACK 70398ae05b

Tree-SHA512: d9a3ab4fcd59a7cf4872415c40cc7ac3a98dfc5aa25e195d4df880bb588bac286c30c3471e9d9499de379a75f45dcd0a82019eba3cb9f342004ae1482d0ba075
2025-02-14 11:15:53 +01:00
merge-script
109bfe9573
Merge bitcoin/bitcoin#31857: depends: avoid an unset CMAKE_OBJDUMP
2434aeab62 depends: avoid an unset CMAKE_OBJDUMP (fanquake)

Pull request description:

  Similar to #31840, currently our Linux toolchain file contains:
  ```bash
  set(CMAKE_AR "aarch64-linux-gnu-ar")
  set(CMAKE_RANLIB "aarch64-linux-gnu-ranlib")
  set(CMAKE_STRIP "aarch64-linux-gnu-strip")
  set(CMAKE_OBJCOPY "aarch64-linux-gnu-objcopy")
  set(CMAKE_OBJDUMP "")
  ```

  `objdump` is currently only used for the macOS cross build, where it's `llvm-objdump`, but we should be consistent in producing a toolchain file that points to actual tools, rather than leaving variables unset.

ACKs for top commit:
  hebasto:
    ACK 2434aeab62.
  theuni:
    utACK 2434aeab62

Tree-SHA512: 65f6b7b9cae79e9c0784c108709139125e52d8f2818afbea5f719bc1b6dc338b870abbdfcb174ae541c0027a7ac07cb56012735b7a37b58b9a6e55a48c0257cf
2025-02-14 10:49:09 +01:00
merge-script
14d1d8e212
Merge bitcoin/bitcoin#31758: test: deduplicates p2p_tx_download constants
0a02e7fdea test: deduplicates p2p_tx_download constants (Sergi Delgado Segura)

Pull request description:

  Some of the networking constants defined in p2p_tx_download.py are more generally defined in p2p.py

ACKs for top commit:
  i-am-yuvi:
    re-ACK 0a02e7fdea
  maflcko:
    review ACK 0a02e7fdea 🔖
  danielabrozzoni:
    re-ACK 0a02e7fdea
  tdb3:
    re ACK 0a02e7fdea

Tree-SHA512: 05fc114a32b6b42a7c57563a38f1a8921e0bb224c4b124ae9d395c3a1105ae6e9cdfc62f603f4f2dee55cef5f6a6ed400d328740ad84fbd3093c5e0f3fb2982a
2025-02-14 10:47:18 +01:00
MarcoFalke
fa3e409c9a
contrib: Add deterministic-fuzz-coverage 2025-02-14 10:37:33 +01:00
Ava Chow
2549fc6fd1
Merge bitcoin/bitcoin#31768: test: check scanning field from getwalletinfo
bb0879ddab test: check `scanning` field from `getwalletinfo` (brunoerg)

Pull request description:

  During a rescan, check that `getwalletinfo` returns properly information (the scanning field) about it.

ACKs for top commit:
  maflcko:
    lgtm ACK bb0879ddab
  arejula27:
    ACK [`bb0879d`](bb0879ddab)
  achow101:
    ACK bb0879ddab
  BrandonOdiwuor:
    Code Review ACK bb0879ddab
  Prabhat1308:
    re-ACK [`bb0879d`](bb0879ddab)

Tree-SHA512: 9bca1c1e813bf4f61a5621bdc0a5f5c2bcfb388ffe9dfacb821bf6954f6e0880140d72258dc93ab6b84efb54f55c682a17aebd42f6559d6cfac9998e6bc4e5b9
2025-02-13 16:25:01 -08:00
Cory Fields
9b033bebb1 cmake: rename Kernel component to bitcoinkernel for consistency 2025-02-13 18:25:58 +00:00
Cory Fields
2e0c92558e cmake: add and use install_binary_component
Add a separate component for each binary for fine-grained installation options.

Also install the man pages for only for the targets enabled.
2025-02-13 18:14:41 +00:00
Ryan Ofsky
a85e8c0e61 doc: Add some general documentation about negated options 2025-02-13 12:30:15 -05:00
glozow
96d30ed4f9
Merge bitcoin/bitcoin#31495: wallet: Utilize IsMine() and CanProvide() in migration to cover edge cases
af76664b12 test: Test migration of a solvable script with no privkeys (Ava Chow)
17f01b0795 test: Test migration of taproot output scripts (Ava Chow)
1eb9a2a39f test: Test migration of miniscript in legacy wallets (Ava Chow)
e8c3efc7d8 wallet migration: Determine Solvables with CanProvide (Ava Chow)
fa1b7cd6e2 migration: Skip descriptors which do not parse (Ava Chow)
440ea1ab63 legacy spkm: use IsMine() to extract watched output scripts (Ava Chow)
b777e84cd7 legacy spkm: Move CanProvide to LegacyDataSPKM (Ava Chow)
b1ab927bbf tests: Test migration of additional P2WSH scripts (Ava Chow)
c39b3cfcd1 test: Extra verification that migratewallet migrates (Ava Chow)

Pull request description:

  The legacy wallet `IsMine()` is essentially a black box that would tell us whether the wallet is watching an output script. In order to migrate legacy wallets to descriptor wallets, we need to be able to compute all of the output scripts that a legacy wallet would watch. The original approach for this was to understand `IsMine()` and write a function which would be its inverse. This was partially done in the original migration code, and attempted to be completed in #30328. However, further analysis of `IsMine()` has continued to reveal additional edge cases which make writing an inverse function increasingly difficult to verify correctness.

  This PR instead changes migration to utilize `IsMine()` to produce the output scripts by first computing a superset of all of the output scripts that `IsMine()` would watch and testing each script against `IsMine()` to filter for the ones that actually are watched. The superset is constructed by computing all possible output scripts for the keys and scripts in the wallet - for keys, every key could be a P2PK, P2PKH, P2WPKH, and P2SH-P2WPKH; for scripts, every script could be an output script, the redeemScript of a P2SH, the witnessScript of a P2WSH, and the witnessScript of a P2SH-P2WSH.

  Additionally, the legacy wallet can contain scripts that are redeemScripts and witnessScripts, while not watching for any output script utilizing that script. These are known as solvable scripts and are migrated to a separate "solvables" wallet. The previous approach to identifying these solvables was similar to identifying output scripts - finding known solvable conditions and computing the scripts. However, this also can miss scripts, so the solvables are now identified in a manner similar to the output scripts but using the function `CanProvide()`. Using the same superset as before, all output scripts which are `ISMINE_NO` are put through `CanProvide()` which will perform a dummy signing and then a key lookup to determine whether the legacy wallet could provide any solving data for the output script. The scripts that pass will have their descriptors inferred and the script included in the solvables wallet.

  The main downside of this approach is that `IsMine()` and `CanProvide()` can no longer be deleted. They will need to be refactored to be migration only code instead in #28710.

  Lastly, I've added 2 test cases for the edge cases that prompted this change of approach. In particular, miniscript witnessScripts and `rawtr()` output scripts are  solvable and signable in a legacy wallet, although never `ISMINE_SPENDABLE`.

ACKs for top commit:
  sipa:
    Code review ACK af76664b12d8611b606a7e755a103a20542ee539; I did not review the tests in detail.
  brunoerg:
    code review ACK af76664b12
  rkrux:
    ACK af76664b12

Tree-SHA512: 7f58a90de6f38fe9801fb6c2a520627072c8d66358652ad0872ff59deb678a82664b99babcfd874288bebcb1487d099a77821f03ae063c2b4cbf2d316e77d141
2025-02-13 12:30:15 -05:00
Ryan Ofsky
490c8fa178 doc: Add release notes summarizing negated option behavior changes. 2025-02-13 12:30:15 -05:00
Ryan Ofsky
458ef0a11b refactor: Avoid using IsArgSet() on -connect list option
This commit does not change behavior, it just changes code to handle -noconnect
values explicitly with IsArgNegated() instead of implicitly with IsArgSet(),
and adds comments to make it clear what behavior is intended when -noconnect is
specified.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
752ab9c3c6 test: Add test to make sure -noconnect disables -dnsseed and -listen by default
Make sure -noconnect has same effect as -connect for disabling DNS seeding and
listening by default, and warning about -dnsseed being ignored with the -proxy
setting.

Initial implementation of https://github.com/bitcoin/bitcoin/pull/30529
accidentally broke this behavior, so having coverage may be useful to make sure
it does not break again.

Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>
2025-02-13 13:30:15 -04:00
Ryan Ofsky
3c2920ec98 refactor: Avoid using IsArgSet() on -signetseednode and -signetchallenge list options
This commit does not change behavior because negation of -signetseednode and
-signetchallenge parameters has been disallowed since these were introduced in
#18267, so calling IsArgSet() is equivalent to checking if GetArgs() returns a
non-empty list.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
d05668922a refactor: Avoid using IsArgSet() on -debug, -loglevel, and -vbparams list options
This commit does not change behavior, it just drops unnecessary IsArgSet()
calls for -debug, -loglevel, and -vbparams options. The calls are unnecessary
because GetArgs() already returns empty arrays if these arguments are not
specified.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
3d1e8ca53a Normalize inconsistent -noexternalip behavior
Treat specifying -noexternalip the same as not specifying -externalip, instead
of causing it to soft-set the -discover default to false.

Before this change, was -noexternalip basically an undocumented synonym for
-nodiscover.

After this change, specifying -noexternalip just clears previously specifed
-externalip options, restoring default behavior as if they were not were
specified.

The previous -noexternalip behavior wasn't neccessarily bad, but it was
undocumented, redundant with the -nodiscover option, and inconsistent with
behavior of other list options.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
ecd590d4c1 Normalize inconsistent -noonlynet behavior
Treat specifying -noonlynet the same as not specifying -onlynet, instead of
marking all networks unreachable.

Before this change, specifying -noonlynet cleared list of reachable networks
and did not allow connecting to any network. It was basically an undocumented
synonym for -noconnect.

After this change, specifying -nononlynet just clears previously specifed
-onlynet options and allows connecting to all networks, restoring default
behavior as if no -onlynet options were specified.

Before this change, there was no way to restore default behavior once an
-onlynet option was specified. So for example, if a config file specifed
onlynet settings, they couldn't be reset on the command line without disabling
the entire config file.

The previous -noonlynet behavior wasn't neccessarily bad, but it was
undocumented, redundant with the -noconnect option, inconsistent with behavior
of other list options, and inconsistent with being able to use the command line
to selectively override config options. It was also probably unintended,
arising from use of the IsArgSet() method and its interaction with negated
options.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
5544a19f86 Fix nonsensical bitcoin-cli -norpcwallet behavior
Treat specifying -norpcwallet the same as not specifying any -rpcwallet option,
instead of treating it like -rpcwallet=0 with 0 as the wallet name.

This restores previous behavior before 743077544b
from https://github.com/bitcoin/bitcoin/pull/18594, which inadvertently changed
it.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
6e8e7f433f Fix nonsensical -noasmap behavior
Instead of failing with "fread failed: iostream error" error when -noasmap is
specified, just don't load an asmap file.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
b6ab350806 Fix nonsensical -notest behavior
Treat specifying -notest exactly the same as not specifying any
-test value, instead of complaining that it must be used with -regtest.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
6768389917 Fix nonsensical -norpcwhitelist behavior
Treat specifying -norpcwhitelist the same as not specifying -rpcwhitelist,
instead of behaving almost the same but flipping the default
-rpcwhitelistdefault value.

This is confusing because before this change if -norpcwhitelist was specified
it would block users from calling any RPC methods.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
e03409c70f Fix nonsensical -norpcbind and -norpcallowip behavior
Treat specifying -norpcbind and -norpcallowip the same as not specifying
-rpcbind or -rpcallowip, instead of failing to bind to localhost and failing to
show warnings.

Also add code comment to clarify what intent of existing code is.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
40c4899bc2 Fix nonsensical -nobind and -nowhitebind behavior
Treat specifying -nobind and -nowhitebind the same as not specifying -bind and
-whitebind values instead of causing them to soft-set -listen=1.
2025-02-13 12:30:15 -05:00
Ryan Ofsky
5453e66fd9 Fix nonsensical -noseednode behavior
Treat specifying -noseednode the same as not specifying any -seednode value,
instead of enabling the seed node timeout and log messages, and waiting longer
to add other seeds.
2025-02-13 12:30:15 -05:00
merge-script
c242fa5be3
Merge bitcoin/bitcoin#31858: chore: remove redundant word
4c62b37fcd chore: remove redundant word (tianzedavid)

Pull request description:

  Remove redundant word

  For https://github.com/bitcoin/bitcoin/pull/31855#issuecomment-2656392782

ACKs for top commit:
  maflcko:
    lgtm ACK 4c62b37fcd
  delta1:
    ACK 4c62b37fcd

Tree-SHA512: b6af8aa158be37977d7016435aafb07542e38db863111b86369cba29bab25c9dd0075892dbe559d4d72c6aa33fcb08a7881a41d1a124d0bfc2c18aea696a0b57
2025-02-13 15:33:43 +01:00
tianzedavid
4c62b37fcd chore: remove redundant word
Signed-off-by: tianzedavid <cuitianze@aliyun.com>
2025-02-13 22:09:55 +08:00
Ryan Ofsky
251ea7367c
Merge bitcoin/bitcoin#31767: logging: Ensure -debug=0/none behaves consistently with -nodebug
7afeaa2469 test: `-debug=0` and `-debug=none` behave similarly to `-nodebug` (Daniela Brozzoni)
a8fedb36a7 logging: Ensure -debug=0/none behaves consistently with -nodebug (Daniela Brozzoni)
d39d521d86 test: `-nodebug` clears previously set debug options (Daniela Brozzoni)

Pull request description:

  Previously, -nodebug cleared all prior -debug configurations in the command line while allowing subsequent debug options to be applied.
  However, -debug=0 and -debug=none completely disabled debugging, even for categories specified afterward.

  This commit ensures consistency by making -debug=0 and -debug=none behave like -nodebug: they now clear previously set debug configurations but do not disable debugging for categories specified later.

  See https://github.com/bitcoin/bitcoin/pull/30529#discussion_r1930956563

ACKs for top commit:
  hodlinator:
    re-ACK 7afeaa2469
  ryanofsky:
    Code review ACK 7afeaa2469. Nicely implemented change with test and release notes, and I like how the test is implemented as the first commit.
  maflcko:
    review ACK 7afeaa2469 👡

Tree-SHA512: c69b17ff10da6c88636bd01918366dd408832e70f2d0a7b951e9619089e89c39282db70398ba2542d3aa69a2fe6b6a0a01638b3225aff79d234d84d3067f2caa
2025-02-13 08:40:12 -05:00
fanquake
2434aeab62
depends: avoid an unset CMAKE_OBJDUMP
Similar to #31840, currently our Linux toolchain file contains:
```bash
set(CMAKE_AR "aarch64-linux-gnu-ar")
set(CMAKE_RANLIB "aarch64-linux-gnu-ranlib")
set(CMAKE_STRIP "aarch64-linux-gnu-strip")
set(CMAKE_OBJCOPY "aarch64-linux-gnu-objcopy")
set(CMAKE_OBJDUMP "")
```

`objdump` is currently only used for the macOS cross build, where it's
`llvm-objdump`, but we should be consistent in producing a toolchain
file that points to actual tools, rather than leaving variables unset.
2025-02-13 13:02:53 +01:00
merge-script
a5b0a441f8
Merge bitcoin/bitcoin#31855: chore: remove redundant word
033acdf03d chore: remove redundant word (tianzedavid)

Pull request description:

  Remove redundant word
  For https://github.com/ElementsProject/elements/pull/1407

ACKs for top commit:
  maflcko:
    lgtm ACK 033acdf03d
  delta1:
    ACK 033acdf03d

Tree-SHA512: ec9c9951c0153fa598d7733cfba287217c4a3389090a6d106d9144ac9cb1c13cf854f660fe187dad992b6ae1b252c01f1764af5bba5bdf8703b978d04cdb190f
2025-02-13 13:01:47 +01:00
Vasil Dimov
cd4bfaee10
net: reduce CAddress usage to CService or CNetAddr
* `CConnman::CalculateKeyedNetGroup()` needs `CNetAddr`, not `CAddress`,
  thus change its argument.

* Both callers of `CConnman::CreateNodeFromAcceptedSocket()` create a
  dummy `CAddress` from `CService`, so use `CService` instead.

* `GetBindAddress()` only needs to return `CService`.

* `CNode::addrBind` only needs to be `CService`.
2025-02-13 12:38:55 +01:00
tianzedavid
033acdf03d chore: remove redundant word
Signed-off-by: tianzedavid <cuitianze@aliyun.com>
2025-02-13 18:55:31 +08:00
merge-script
55cf39e4c5
Merge bitcoin/bitcoin#31722: cmake: Copy cov_tool_wrapper.sh.in to the build tree
e3c0152769 cmake: Copy `cov_tool_wrapper.sh.in` to the build tree (Hennadii Stepanov)

Pull request description:

  This PR ensures that `cov_tool_wrapper.sh.in` is available when invoking the `Coverage.cmake` script from any directory.

  Here is an example of usage on Ubuntu 24.10 with the default GCC 14.2.0:
  ```
  $ cmake -B build -DCMAKE_BUILD_TYPE=Coverage -DCMAKE_CXX_FLAGS="-fprofile-update=atomic" -DCMAKE_C_FLAGS="-fprofile-update=atomic"
  $ cmake --build build -j $(nproc)
  $ cd ..
  $ cmake -DJOBS=$(nproc) -DLCOV_OPTS="--ignore-errors inconsistent,inconsistent" -P bitcoin/build/Coverage.cmake
  ```

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

ACKs for top commit:
  theuni:
    utACK e3c0152769

Tree-SHA512: ccfc8e893567f199d9b05ea3916cac06fca89c5892cc7492d5251c331c35408222fd918ed08017515e2dfca10970ae3f633b3917bfb7037db539559e71d7f711
2025-02-13 10:34:28 +01:00
David Gumberg
c3fa043ae5 doc: build: Fix instructions for msvc gui builds
If the instructions are followed as-is, and "Developer
(PowerShell|Command Prompt) for VS 2022" is used to execute the
suggested build commands, the root directory of vcpkg (e.g. in VS 2022
Community edition: `C:\Program Files\Microsoft Visual
Studio\2022\Community\VC\vcpkg`), is too long, and when vcpkg attempts
to build any of the QT packages, it will fail because of build steps
that require path lengths greater than Windows' `MAX_PATH` 260 character
limit. This can be avoided without needing to move the vcpkg root dir by
setting `--x-buildtrees-root` to a short path, like `C:\vcpkg`.
2025-02-12 12:53:59 -08:00
merge-script
048ef98626
Merge bitcoin/bitcoin#31840: depends: add missing Darwin objcopy
3edaf0b428 depends: add missing Darwin objcopy (fanquake)

Pull request description:

  Our CMake toolchain for a Darwin cross build currently contains:
  ```bash
  set(CMAKE_AR "/usr/bin/llvm-ar")
  set(CMAKE_RANLIB "/usr/bin/llvm-ranlib")
  set(CMAKE_STRIP "/usr/bin/llvm-strip")
  set(CMAKE_OBJCOPY "arm64-apple-darwin-objcopy")
  set(CMAKE_OBJDUMP "/usr/bin/llvm-objdump")
  ```

  `objcopy` isn't currently used for the Darwin build (only for Linux and splitting the debug symbols), but we shouldn't be producing a toolchain file that refers to nonexistent tools.

ACKs for top commit:
  laanwj:
    Code review ACK 3edaf0b428
  theuni:
    utACK 3edaf0b428

Tree-SHA512: b74deb9f3f053c37d03505e698419d4a14131131f12a042dab698a81f2ad76b71fd55c1d1afd5f5822cc50fdaad5acdab15a8b20626c56f705179add1165449f
2025-02-12 18:50:58 +01:00
Antoine Poinsot
c73b59d47f fuzz: implement targets for PCP and NAT-PMP port mapping requests 2025-02-12 11:39:37 -05:00
Antoine Poinsot
1695c8ab5b fuzz: in FuzzedSock::GetSockName(), return a random-length name
ConsumeData() will always try to return a name as long as the requested size. It is more useful, and
closer to how `getsockname` would actually behave in reality, to return a random length name
instead.

This was hindering coverage in the PCP fuzz target as the addr len was set to the size of the
sockaddr_in struct and would exhaust all the provided data from the fuzzer.

Thanks to Marco Fleon for suggesting this.

Co-Authored-by: marcofleon <marleo23@proton.me>
2025-02-12 11:39:37 -05:00
merge-script
713bf66b1f
Merge bitcoin/bitcoin#31500: depends: Fix compiling libevent package on NetBSD
f89f16846e depends: Fix compiling `libevent` package on NetBSD (Hennadii Stepanov)

Pull request description:

  Libevent [introduced](https://github.com/libevent/libevent/pull/909) the [`typeof`](https://gcc.gnu.org/onlinedocs/gcc/Typeof.html) C language extension in the NetBSD-specific code, which was pulled into our depends in https://github.com/bitcoin/bitcoin/pull/21991.

  However, GCC [states](https://gcc.gnu.org/onlinedocs/gcc/Alternate-Keywords.html):
  > the various `-std` options disable certain keywords.

  Due to our use of b042c4f053/depends/hosts/netbsd.mk (L1)

  the `typeof` keyword is disabled, resulting in a compilation error:
  ```
  $ gmake -C depends libevent CC=/usr/pkg/gcc14/bin/gcc CXX=/usr/pkg/gcc14/bin/g++
  <snip>
  [ 37%] Building C object CMakeFiles/event_core_static.dir/kqueue.c.o
  /home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/libevent/2.1.12-stable-ca6b96ec97c/kqueue.c: In function 'kq_setup_kevent':
  /home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/libevent/2.1.12-stable-ca6b96ec97c/kqueue.c:56:27: error: implicit declaration of function 'typeof' [-Wimplicit-function-declaration]
     56 | #define INT_TO_UDATA(x) ((typeof(((struct kevent *)0)->udata))(intptr_t)(x))
        |                           ^~~~~~
  /home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/libevent/2.1.12-stable-ca6b96ec97c/kqueue.c:190:30: note: in expansion of macro 'INT_TO_UDATA'
    190 |                 out->udata = INT_TO_UDATA(ADD_UDATA);
        |                              ^~~~~~~~~~~~
  /home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/libevent/2.1.12-stable-ca6b96ec97c/kqueue.c:56:64: error: expected expression before 'intptr_t'
     56 | #define INT_TO_UDATA(x) ((typeof(((struct kevent *)0)->udata))(intptr_t)(x))
        |                                                                ^~~~~~~~
  /home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/libevent/2.1.12-stable-ca6b96ec97c/kqueue.c:190:30: note: in expansion of macro 'INT_TO_UDATA'
    190 |                 out->udata = INT_TO_UDATA(ADD_UDATA);
        |                              ^~~~~~~~~~~~
  /home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/libevent/2.1.12-stable-ca6b96ec97c/kqueue.c:56:27: error: called object is not a function or function pointer
     56 | #define INT_TO_UDATA(x) ((typeof(((struct kevent *)0)->udata))(intptr_t)(x))
        |                          ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/libevent/2.1.12-stable-ca6b96ec97c/kqueue.c:190:30: note: in expansion of macro 'INT_TO_UDATA'
    190 |                 out->udata = INT_TO_UDATA(ADD_UDATA);
        |                              ^~~~~~~~~~~~
  gmake[3]: *** [CMakeFiles/event_core_static.dir/build.make:328: CMakeFiles/event_core_static.dir/kqueue.c.o] Error 1
  <snip>
  ```

  This PR resolves this issue by following GCC's [recommendation](https://gcc.gnu.org/onlinedocs/gcc/Typeof.html):
  > write `__typeof__` instead of `typeof`.

ACKs for top commit:
  fanquake:
    ACK f89f16846e

Tree-SHA512: c0d2e535408db120535781f8518c616b0f5a39b1c6babb2a74e8e0565348aaf00b0f5a93cac0af7cf6d6bf028d5d58763fe71b3969ed9c7059fa7c3dca9d084c
2025-02-12 16:38:24 +01:00
Antoine Poinsot
0d472c1953 fuzz: never return an uninitialized sockaddr in FuzzedSock::GetSockName
The fuzz provider's `ConsumeData` may return less data than necessary
to fill the sockaddr struct and still return success. Fix this to avoid
the caller using uninitialized memory.
2025-02-12 10:31:43 -05:00
Antoine Poinsot
39b7e2b590 fuzz: add steady clock mocking to FuzzedSock 2025-02-12 10:31:43 -05:00
Antoine Poinsot
6fe1c35c05 pcp: make NAT-PMP error codes uint16_t
They are defined as being 16 bits in the RFC and correctly parsed in the code
which may result in an implicit conversion from uint16_t to uint8_t.
2025-02-12 10:31:43 -05:00
Antoine Poinsot
01906ce912 pcp: make the ToString method const 2025-02-12 10:31:43 -05:00
0xb10c
a0b66b4bff
Revert "test: Disable known broken USDT test for now"
This reverts commit faed533743.

This commit worked around a lifetime issue likely caused by using
bpf_usdt_readarg_p(). Since we don't use bpf_usdt_readarg_p() anymore
this commit can be reverted.

See the discussion in https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2500962838
2025-02-12 16:29:00 +01:00
0xb10c
ec47ba349d
contrib: don't use bpf_usdt_readarg_p 2025-02-12 16:28:28 +01:00
0xb10c
35ae6ff60f
test: don't use bpf_usdt_readarg_p
Instead of using the undocumented bcc helper bpf_usdt_readarg_p(),
use bpf_usdt_readarg() [1] and bpf_probe_read_user{_str}() [2, 3] as
documented in the bcc USDT reference guide [1].

Note that the bpf_probe_read_user() documentation says the following:

> For safety, all user address space memory reads must pass through bpf_probe_read_user().

It's assumed that using bpf_usdt_readarg_p() caused a lifetime issue.
See https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2286505348
With bpf_usdt_readarg() and bpf_probe_read_user(), this doesn't seem
to be a problem anymore. See https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-2528671656

[1]: https://github.com/iovisor/bcc/blob/master/docs/reference_guide.md#6-usdt-probes
[2]: https://github.com/iovisor/bcc/blob/master/docs/reference_guide.md#10-bpf_probe_read_user
[3]: https://github.com/iovisor/bcc/blob/master/docs/reference_guide.md#11-bpf_probe_read_user_str
2025-02-12 16:21:35 +01:00
merge-script
ede388d03d
Merge bitcoin/bitcoin#30911: build: simplify by flattening the dependency graph
12fa9511b5 build: simplify dependency graph (Cory Fields)
c4e498300c build: avoid unnecessary dependencies on generated headers (Cory Fields)

Pull request description:

  These changes speed up my build (default config/options/targets) by roughly 10%. I suspect the difference may be more significant in other build configs.

  Before:
  > $ time cmake --build build -j24
  > real3m26.932s

  After:
  > $ time cmake --build build -j24
  > real3m7.556s

  Generally they allow for jobservers (either `make -jX` or `ninja`) to be better utilized. This can be verified using `top` while building and looking at the number of compiles running at any given time before/after these changes. Before, it's easy to observe periods of stalling when only one or two compiles are happening. After these changes, the compiler process count should mostly match the number of jobs given (`-jX`) until it falls off at the end.

  ---

  The first commit sets [DEPENDS_EXPLICIT_ONLY](https://cmake.org/cmake/help/latest/command/add_custom_command.html#command:add_custom_command) for commands which generate our test header files. Without this option, `test_bitcoin`'s generated headers won't be built until all of its other dependencies have been built. This introduces a significant stall in the build, though currently only Ninja benefits from this being set, and only CMake >= 3.27 understands it.

  Example from a generated `build.ninja`:

  Before:

  > \# Custom command for src/test/data/base58_encode_decode.json.h
  >
  > build src/test/data/base58_encode_decode.json.h | ${cmake_ninja_workdir}src/test/data/base58_encode_decode.json.h: CUSTOM_COMMAND /home/cory/dev/bitcoin/src/test/data/base58_encode_decode.json /home/cory/dev/bitcoin/cmake/script/GenerateHeaderFromJson.cmake || libcrc32c.a libcrc32c_sse42.a libleveldb.a libminisketch.a minisketch_clmul src/bitcoin_clientversion src/crypto/libbitcoin_crypto.a src/crypto/libbitcoin_crypto_avx2.a src/crypto/libbitcoin_crypto_sse41.a src/crypto/libbitcoin_crypto_x86_shani.a src/generate_build_info src/libbitcoin_cli.a src/libbitcoin_common.a src/libbitcoin_consensus.a src/libbitcoin_node.a src/secp256k1/src/libsecp256k1.a src/secp256k1/src/secp256k1_precomputed src/test/util/libtest_util.a src/univalue/libunivalue.a src/util/libbitcoin_util.a src/wallet/libbitcoin_wallet.a src/zmq/libbitcoin_zmq.a

  After:

  > \# Custom command for src/test/data/base58_encode_decode.json.h
  >
  > build src/test/data/base58_encode_decode.json.h | ${cmake_ninja_workdir}src/test/data/base58_encode_decode.json.h: CUSTOM_COMMAND /home/cory/dev/bitcoin/src/test/data/base58_encode_decode.json /home/cory/dev/bitcoin/cmake/script/GenerateHeaderFromJson.cmake

  ---

  The second commit is more significant. It sets [CMAKE_OPTIMIZE_DEPENDENCIES](https://cmake.org/cmake/help/latest/prop_tgt/OPTIMIZE_DEPENDENCIES.html) globally, which allows the objects of static libs to be built in parallel when one lib depends on the other. This can be set as a per-lib property, ~but I don't see any need for that as we don't currently have any edge-cases where this wouldn't be ok. If those should arise, we could always disable on a per-lib basis~.

  Edit: turns out this triggers an [upstream bug](https://gitlab.kitware.com/cmake/cmake/-/issues/24058), which I guess can be considered an edge-case until fixed in CMake. I've added 2 per-lib opt-outs as a result.

  Example:

  Before:

  > \# Link the static library src/libbitcoin_cli.a
  >
  > build src/libbitcoin_cli.a: CXX_STATIC_LIBRARY_LINKER__bitcoin_cli_RelWithDebInfo src/CMakeFiles/bitcoin_cli.dir/compat/stdin.cpp.o src/CMakeFiles/bitcoin_cli.dir/rpc/client.cpp.o || src/univalue/libunivalue.a

  After:

  > \# Link the static library src/libbitcoin_cli.a
  >
  > build src/libbitcoin_cli.a: CXX_STATIC_LIBRARY_LINKER__bitcoin_cli_RelWithDebInfo src/CMakeFiles/bitcoin_cli.dir/compat/stdin.cpp.o src/CMakeFiles/bitcoin_cli.dir/rpc/client.cpp.o
  >

ACKs for top commit:
  l0rinc:
    utACK 12fa9511b5
  hebasto:
    ACK 12fa9511b5.

Tree-SHA512: f85f507e70cdc06acd07542161d9f9b8edf9ba866f08c8ef17aaaed770fa11530a27521c4413456d863463a6e77d4d6983fa623a64e17bbd602c2bc70aacc112
2025-02-12 16:02:57 +01:00
merge-script
534414ca9d
Merge bitcoin/bitcoin#31678: ci: Skip read-write of default env vars
fa952acdb6 ci: Skip read-write of default env vars (MarcoFalke)

Pull request description:

  If they remain unset, they use the default anyway. Except for `USER`, but this seems unused anyway.

  Can be checked via:

  ```
  sh-5.2# touch /tmp/empty_env
  sh-5.2# podman run --rm --env-file /tmp/empty_env 'ubuntu:24.04' env
  PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
  container=podman
  HOME=/root
  HOSTNAME=19ece5c9e052

ACKs for top commit:
  0xB10C:
    ACK fa952acdb6
  Prabhat1308:
    utACK [fa952ac](fa952acdb6)

Tree-SHA512: fe0c173b23cfda3025306303a44ffe32ecc57c2e0e1a2376594696f9887ed22f5105da84e898e790041bf15a4aa42a365fba016710ad269d439dda691977be90
2025-02-12 15:54:16 +01:00
merge-script
87ce116058
Merge bitcoin/bitcoin#31846: test: Remove stale gettime test
fa3a4eafa1 test: Remove stale gettime test (MarcoFalke)

Pull request description:

  The `gettime` test is stale:

  * It was added to sanity check the `time` implementation in the mingw toolchain to catch a 32-bit vs 64-bit mismatch in commit eaafa23cbd. However, since commit 0000a63689, `std::chrono::system_clock` is used.
  * Even though `system_clock` may also return incorrect values, such an error should affect *all* `GetTime<>` calls (not only the second-precision ones). (I expect such an error to lead to a signed integer overflow in the normal nanosecond precision, so it should be caught by ubsan or by the `assert(ret > 0s)`. If not, the error should be apparent on startup in the debug log.)

  So remove it for now. An alternative would be to extend the test to cover `time` again, and adjust the comment to say that the test should be fixed along with the block header timestamp. Since that timestamp can't grow beyond 2106 anyway, see the `_test_y2106` functional test.

ACKs for top commit:
  l0rinc:
    ACK fa3a4eafa1
  laanwj:
    ACK fa3a4eafa1

Tree-SHA512: fd485e74962b659ee23ba2952d284fa9d6cfb9d9844a5e70013c8ead495ed77f5b784d5ca3ba0b30c492a5d27b2e81f9e1e0dbc530af7da1494789ac5e055b99
2025-02-12 15:02:26 +01:00
MarcoFalke
fa3a4eafa1
test: Remove stale gettime test 2025-02-12 12:16:20 +01:00
merge-script
42251e00e8
Merge bitcoin/bitcoin#30584: depends: Make default host and build comparable
b28917be36 depends: Make default `host` and `build` comparable (Hennadii Stepanov)

Pull request description:

  To detect cross-compiling, the host and build platforms are compared. The `build` variable is always an output of `config.sub`, but the `host` is not. This can lead to false results. For example, on OpenBSD:
   - host=amd64-unknown-openbsd7.5
   - build=x86_64-unknown-openbsd7.5

  This PR sets the default value of the `host` variable to the value of `build`, ensuring cross-compiling won't be triggered when the `HOST` variable is not set.

  This PR fixes needless triggering of cross-compiling for CMake-built packages in depends on OpenBSD due to this code:eb85cacd29/depends/funcs.mk (L193-L197)

  No changes in Guix build.

ACKs for top commit:
  laanwj:
    Concept and code review ACK b28917be36
  theuni:
    utACK b28917be36.

Tree-SHA512: 8c5835cb8b739355b71f7cb161b350ad8b038a00e6b1def36354ba228cea3dcb9883df3c9a8e79d7d0143241f6f054129fe90772b1b2579702db51237f9d66ff
2025-02-12 11:18:48 +01:00
merge-script
0b6ed342b5
Merge bitcoin/bitcoin#31711: build: set build type and per-build-type flags as early as possible
56a9b847bb build: set build type and per-build-type flags as early as possible (Cory Fields)
f605f7a9c2 build: refactor: set debug definitions in main CMakeLists (Cory Fields)

Pull request description:

  This ensures that most compiler tests are not run with the wrong build type's flags. The initial c++ checks are an exception to that because many internal CMake variables are unset until a language is selected, so it's problematic to change our build type before that.

  The difference can be seen in `build/CMakeFiles/CMakeConfigureLog.yaml`. Before, `Debug` was used for many of the earlly checks. After this PR, it's only the first 2 checks.

ACKs for top commit:
  hebasto:
    ACK 56a9b847bb.

Tree-SHA512: 87947352d6d4fd08554515822cb13634ed3be33fcda2af817c22ef943b1d0856ceb39311ddc01b8221397528fdc09f630dc7e74fc92f5a4a073f09c4ae493596
2025-02-12 10:41:25 +01:00
merge-script
a44ccedcc2
Merge bitcoin/bitcoin#31818: guix: remove test-security/symbol-check scripts
76c090145e guix: remove test-security/symbol-check scripts (fanquake)

Pull request description:

  These scripts are becoming more of nuisance, than a value-add; particularly since we've been building releases using Guix. Adding new (release bin) tests can be harder, because it requires constructing a failing test, which is becoming less easy, e.g trying to disable a feature or protection that has been built into the compiler/toolchain by default.

  In the pre-Guix days, these were valuable to sanity-check the environment, because we were pulling that pre-built from Ubuntu, with little control. At this point, it's less clear what these scripts are (sanity) checking.

  Note that these also weren't completely ported to CMake (#31698), see also #31715 which contains other fixes that would be needed for these test-tests, to accomodate future changes.

ACKs for top commit:
  hebasto:
    ACK 76c090145e.
  theuni:
    utACK 76c090145e

Tree-SHA512: 99b5e7c0645c6966a45b17f411b5bee61df23c64d8258cce0ad9cdea4c3af4d4db32ca5fd80d0df2a3a30ba873eb772cc0d5901c345ff7f0eea13fcb971443b4
2025-02-12 09:47:52 +01:00