Commit Graph

42220 Commits

Author SHA1 Message Date
merge-script
338bc2cd26
Merge bitcoin/bitcoin#30454: build: Introduce CMake-based build system
41051290ab cmake: Ignore build subdirectories within source directory (Hennadii Stepanov)
6ce50fd9d0 doc: Update for CMake-based build system (Hennadii Stepanov)
9730288a0c ci: Migrate CI scripts to CMake (Hennadii Stepanov)
c360837ca5 cmake, lint: Adjust `lint_includes_build_config` (Hennadii Stepanov)
3885441ee0 cmake: Add presets for native Windows builds (Hennadii Stepanov)
7681746b20 cmake: Add vcpkg manifest file (Hennadii Stepanov)
8b6f1c4353 cmake: Add `Coverage` and `CoverageFuzz` scripts (Hennadii Stepanov)
65bdbc1ff2 cmake: Add `docs` build target (Hennadii Stepanov)
fb75ebbc33 cmake: Add compiler diagnostic flags (Hennadii Stepanov)
e821f0a37a cmake: Migrate Guix build scripts to CMake (Hennadii Stepanov)
747adb6ffe cmake: Add `Maintenance` module (Hennadii Stepanov)
1f60b30df0 cmake: Add `APPEND_{CPP,C,CXX,LD}FLAGS` cache variables (Hennadii Stepanov)
2b43c45b13 cmake: Add `AddWindowsResources` module (Hennadii Stepanov)
973a3b0c5d cmake: Implement `install` build target (Hennadii Stepanov)
84ac35cfd4 cmake: Add cross-compiling support (Hennadii Stepanov)
0d01c228a7 build: Generate `toolchain.cmake` in depends (Hennadii Stepanov)
91a799247d depends: Add host-specific `cmake_system_version` variables (Hennadii Stepanov)
9b31209b4c depends: Rename `cmake_system` -> `cmake_system_name` (Hennadii Stepanov)
4a5208a81d Revert "build, qt: Do not install *.prl files" (Hennadii Stepanov)
6522af62af depends: Amend handling flags environment variables (Hennadii Stepanov)
90cec4d251 cmake: Add `MULTIPROCESS` option (Hennadii Stepanov)
bb1a450dcb cmake: Build `bitcoin-chainstate` executable (Hennadii Stepanov)
aed38ea58c cmake: Build `bitcoinkernel` library (Hennadii Stepanov)
975d67369b cmake: Build `test_bitcoin-qt` executable (Hennadii Stepanov)
10fcc668a3 cmake: Add `WITH_DBUS` option (Hennadii Stepanov)
5bb5a4bc75 cmake: Add `libqrencode` optional package support (Hennadii Stepanov)
57a6e2ef4a cmake: Build `bitcoin-qt` executable (Hennadii Stepanov)
30f642952c cmake: Add `WERROR` option (Hennadii Stepanov)
c98d4a4c34 cmake: Add `REDUCE_EXPORTS` option (Hennadii Stepanov)
a01cb6e63f cmake: Add `HARDENING` option (Hennadii Stepanov)
a8a2e364ac cmake: Add Python-based tests (Hennadii Stepanov)
3d85379570 cmake: Add fuzzing options (Hennadii Stepanov)
908530e312 cmake: Add `SANITIZERS` option (Hennadii Stepanov)
8bb0e85631 cmake: Build `bench_bitcoin` executable (Hennadii Stepanov)
801735163a cmake: Add external signer support (Hennadii Stepanov)
353e0c9e96 cmake: Add `systemtap-sdt` optional package support (Hennadii Stepanov)
d2fda82b49 cmake: Add `libzmq` optional package support (Hennadii Stepanov)
ae7b39a0e1 cmake: Add `libminiupnpc` optional package support (Hennadii Stepanov)
6480e1dcdb cmake: Add `libnatpmp` optional package support (Hennadii Stepanov)
e73e9304a1 cmake: Build `bitcoin-util` executable (Hennadii Stepanov)
027c6d7caa cmake: Build `bitcoin-tx` executable (Hennadii Stepanov)
d10c5c34c3 cmake: Add wallet functionality (Hennadii Stepanov)
ab2e99b0d9 cmake: Create test suite for `ctest` (Hennadii Stepanov)
959370bd76 cmake: Build `test_bitcoin` executable (Hennadii Stepanov)
b27bf9700d cmake: Build `bitcoin-cli` executable (Hennadii Stepanov)
a9813df826 cmake: Build `bitcoind` executable (Hennadii Stepanov)
97829ce2d5 cmake: Add `FindLibevent` module (Hennadii Stepanov)
3118e40c61 cmake: Build `bitcoin_consensus` library (Hennadii Stepanov)
809a2f1929 cmake: Build `bitcoin_util` static library (Hennadii Stepanov)
0a9a521a70 cmake: Build `bitcoin_crypto` library (Hennadii Stepanov)
958971f476 cmake: Build `univalue` static library (Hennadii Stepanov)
752747fda8 cmake: Generate `obj/build.h` header (Hennadii Stepanov)
1f0a78edf3 cmake: Build `minisketch` static library (Hennadii Stepanov)
12bfbc8154 cmake: Build `leveldb` static library (Hennadii Stepanov)
51985c5304 cmake: Build `crc32c` static library (Hennadii Stepanov)
db7a198f29 cmake: Build `secp256k1` subtree (Hennadii Stepanov)
dbb7ed14e8 cmake: Add `ccache` support (Hennadii Stepanov)
cedfdf6c72 cmake: Redefine/adjust per-configuration flags (Hennadii Stepanov)
b6b5e732c8 cmake: Add global compiler and linker flags (Hennadii Stepanov)
f98327931b cmake: Add `TryAppendLinkerFlag` module (Hennadii Stepanov)
4a0af29697 cmake: Add `TryAppendCXXFlags` module (Hennadii Stepanov)
35cffc497d cmake: Add POSIX threads support (Hennadii Stepanov)
fd72d00ffe cmake: Add position independent code support (Hennadii Stepanov)
07069e2bb0 cmake: Add introspection module (Hennadii Stepanov)
27d687fc1f cmake: Add `config/bitcoin-config.h` support (Hennadii Stepanov)
fe5cdace5f cmake: Print compiler and linker flags in summary (Hennadii Stepanov)
70683884c5 cmake: Introduce interface libraries to encapsulate common flags (Hennadii Stepanov)
a2317e27b7 cmake: Add root `CMakeLists.txt` file (Hennadii Stepanov)

Pull request description:

  This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.

  ML announcement: https://groups.google.com/g/bitcoindev/c/hgKkfQWzrTo

  As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.

  This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of contributors, including (in alphabetical order):
  - [**achow101**](https://github.com/achow101)
  - [**fanquake**](https://github.com/fanquake)
  - [**maflcko**](https://github.com/maflcko)
  - [**m3dwards**](https://github.com/m3dwards)
  - [**pablomartin4btc**](https://github.com/pablomartin4btc)
  - [**real-or-random**](https://github.com/real-or-random)
  - [**ryanofsky**](https://github.com/ryanofsky)
  - [**sipsorcery**](https://github.com/sipsorcery)
  - [**TheCharlatan**](https://github.com/TheCharlatan)
  - [**theStack**](https://github.com/theStack)
  - [**theuni**](https://github.com/theuni)
  - [**vasild**](https://github.com/vasild)

  Reviewing in a separate staging repo was suggested in https://github.com/bitcoin/bitcoin/pull/27060#issuecomment-1431798320.

  The accompanying changes to the OSS-Fuzz project are available in https://github.com/hebasto/oss-fuzz/pull/8.

  Please refer to the [build options parity table](https://gist.github.com/hebasto/2ef97d3a726bfce08ded9df07f7dab5e). The "auto" value is no longer available; non-default values must be specified explicitly. Additionally, the new default values have been chosen to suit the everyday build experience for the majority of developers.

  System requirements for using the CMake-based build system:
  - CMake >= 3.22 (if not available in your system's repository, it can be downloaded from https://cmake.org/download/)
  - a build tool of your choice:
  - any Make (GNU Make is no longer a requirement); GNU Make is still required to build depends
  - Ninja (https://ninja-build.org/)
  - MSBuild
  - Xcode

  A note for Windows users: The default installation of the latest version of MSVC 17.10.4 includes both CMake 3.28.3 and the vcpkg package manager).

  ---

  We, the build system developers, kindly ask reviewers to refrain from making suggestions that are not directly related to the migration process or can be implemented separately. Bugs in the scripts and errors in the updated documentation should be the focus of this PR. Please be advised that comments not aligned with this PR's goal may be ignored.

  Thank you all for your understanding.

ACKs for top commit:
  maflcko:
    review ACK 41051290ab 🐥
  sipsorcery:
    ACK 41051290ab.
  vasild:
    ACK 41051290ab
  TheCharlatan:
    ACK 41051290ab
  pablomartin4btc:
    tACK 41051290ab
  i-am-yuvi:
    tACK [`4105129`](41051290ab)
  theuni:
    ACK 41051290ab.
  fanquake:
    ACK 41051290ab

Tree-SHA512: 6c1445054436c6c00ad63bfa0f19d64091a2b25c9bd694f85bf2218ac358ffb774d6c000685b3ca1e9b50401babed989fa2a0694b774c211d226bfd1944c9b39
2024-08-28 10:51:24 +01:00
merge-script
80f00cafde
Merge bitcoin/bitcoin#29071: refactor: Remove Span operator==, Use std::ranges::equal
fad0cf6f26 refactor: Use std::ranges::equal in GetNetworkForMagic (MarcoFalke)
fadf0a7e15 refactor: Remove Span operator==, Use std::ranges::equal (MarcoFalke)

Pull request description:

  `std::span` removed the comparison operators, so it makes sense to remove them for the `Span` "backport" as well. Using `std::ranges::equal` also has the benefit that some `Span` temporary constructions can now be dropped.

  This is required to move from `Span` toward `std::span`.

ACKs for top commit:
  hodlinator:
    Untested Code Review re-ACK fad0cf6
  stickies-v:
    ACK fad0cf6f26
  TheCharlatan:
    ACK fad0cf6f26

Tree-SHA512: 5b9d1826ceac2aabae2295bc89893dd23ac3a1cc0d41988331cdbdc21be531aa91795d5273819f349f79648c6c4f30ed31af6e7a3816153e92080061b92ffe00
2024-08-28 10:34:47 +01:00
merge-script
128ade02e4
Merge bitcoin/bitcoin#30725: fuzz: fix timeout in crypto_fschacha20poly1305
8dec4e1294 fuzz: fix timeout in `crypto_fschacha20poly1305` (brunoerg)

Pull request description:

  Fixes #30505

  This PR fixes a timeout in `crypto_fschacha20poly1305` by reducing the number of iterations. I left it running for a while and noticed it speeds up the target and do not impact coverage.

ACKs for top commit:
  maflcko:
    lgtm ACK 8dec4e1294
  stratospher:
    ACK 8dec4e1. saw similar coverage stats (these are from different machines, saw more similar from same machine).

Tree-SHA512: 66cd689f10f09cf880c1f2c77aa481336a95a7e10b655d156b2dc9cd38165670000019a6fb3775485ccb5cce248939a52a31ebd755a6befed6274cc841d21d97
2024-08-28 09:42:43 +01:00
merge-script
f4a10911c7
Merge bitcoin/bitcoin#30721: refactor: Testnet4 - Replace uint256S("str") -> uint256{"str"}
49f9b645ea refactor: Testnet4 - Replace uint256S("str") -> uint256{"str"} (Hodlinator)

Pull request description:

  Ran scripted-diff from 2d9d752e4f:
  ```
  sed -i --regexp-extended -e 's/\buint256S\("(0x)?([^"]{64})"\)/uint256{"\2"}/g' $(git grep -l uint256S)
  ```

  Follow-up to Testnet4 introduction #29775 which overlapped with work on `uint256` `consteval` ctor #30560 (the latter includes the scripted-diff commit).

  Going forward `uint256{}` should be used for constants instead of `uint256S()`.

ACKs for top commit:
  maflcko:
    review-ACK 49f9b645ea 🐮
  fjahr:
    ACK 49f9b645ea

Tree-SHA512: 94fe5d9f1fb85e9ce5c3c4c5e4c31667e8cbb55ee691a4b5b3ae4172ccac38230281071023663965917f188b4c19bdf67afd4e3cdf69d89e97c65faea88af833
2024-08-28 09:42:20 +01:00
Ava Chow
2c7a4231db
Merge bitcoin/bitcoin#30569: node: reduce unsafe uint256S usage
18d65d2772 test: use uint256::FromUserHex for RANDOM_CTX_SEED (stickies-v)
6819e5a329 node: use uint256::FromUserHex for -assumevalid parsing (stickies-v)
2e58fdb544 util: remove unused IsHexNumber (stickies-v)
8a44d7d3c1 node: use uint256::FromUserHex for -minimumchainwork parsing (stickies-v)
70e2c87737 refactor: add uint256::FromUserHex helper (stickies-v)
85b7cbfcbe test: unittest chainstatemanager_args (stickies-v)

Pull request description:

  Since fad2991ba0, `uint256S` has been [deprecated](fad2991ba0 (diff-800776e2dda39116e889839f69409571a5d397de048a141da7e4003bc099e3e2R138)) because it is less robust than the `base_blob::FromHex()` introduced in [the same PR](https://github.com/bitcoin/bitcoin/pull/30482). Specifically, it tries to recover from length-mismatches, recover from untrimmed whitespace, 0x-prefix and garbage at the end, instead of simply requiring exactly 64 hex-only characters. _(see also #30532)_

  This PR carves out the few `uint256S` callsites that may potentially prove a bit more controversial to change because they deal with user input and backwards incompatible behaviour change.

  The main behaviour change introduced in this PR is:
  - `-minimumchainwork` will raise an error when input is longer than 64 hex digits
  - `-assumevalid` will raise an error when input contains invalid hex characters, or when it is longer than 64 hex digits
  - test: the optional RANDOM_CTX_SEED env var will now cause tests to abort when it contains invalid hex characters, or when it is longer than 64 hex digits

  After this PR, the remaining work to remove `uint256S` completely is almost entirely mechanical and/or test related. I will open that PR once #30560 is merged because it builds on that.

ACKs for top commit:
  hodlinator:
    re-ACK 18d65d2772
  l0rinc:
    ACK 18d65d2772
  achow101:
    ACK 18d65d2772
  ryanofsky:
    Code review ACK 18d65d2772. Very nice change that cleans up the API, adds checking for invalid values, makes parsing of values more consistent, and adds test coverage.

Tree-SHA512: ec118ea3d56e1dfbc4c79acdbfc797f65c4d2107b0ca9577c848b4ab9b7cb8d05db9f3c7fe8441a09291aca41bf671572431f4eddc59be8fb53abbea76bbbf86
2024-08-27 16:47:54 -04:00
Ava Chow
99b06b7f1d
Merge bitcoin/bitcoin#29369: refactor: Allow CScript construction from any std::input_iterator
fa7b9b99a2 refactor: Require std::input_iterator for all InputIterator in prevector (MarcoFalke)
d444441900 refactor: Allow CScript construction from any std::input_iterator (MarcoFalke)

Pull request description:

  Currently only (pre)vector iterators and raw pointers are accepted. However, this makes it harder to construct from input iterators provided by other classes, such as `std::span`.

  Fix that.

ACKs for top commit:
  delta1:
    reACK fa7b9b9
  achow101:
    ACK fa7b9b99a2
  hodlinator:
    ACK fa7b9b99a2
  ryanofsky:
    Code review ACK fa7b9b99a2

Tree-SHA512: 2760861f8bce42fb27dc0825e61621cb157f1ac3619a0834df38eb8319b6dcf9de43d90397a4c160f43340880c1553df638848e9057a27c792214331243ef4a5
2024-08-27 14:09:56 -04:00
Ava Chow
dc90542a9e
Merge bitcoin/bitcoin#30716: bench: [refactor] iwyu
fab0e834b8 bench: [refactor] iwyu (MarcoFalke)

Pull request description:

  Missing includes are problematic, because:

  * Upcoming releases of a C++ standard library implementation often minimize their internal header dependencies. For example, `_LIBCPP_REMOVE_TRANSITIVE_INCLUDES` (https://libcxx.llvm.org/DesignDocs/HeaderRemovalPolicy.html). This can lead to compile failures, which are easy to fix for developers, but may not be for users. For example, commit 138f867156 had to add missing includes to accommodate GCC 15 (and the commit had to be backported).
  * A Bitcoin Core developer removing a feature from a module and wanting to drop the now unused includes may not be able to do so without touching other unrelated files, because those files rely on the transitive includes.

  Moreover, missing or extraneous includes are problematic, because they may be confusing the code reader as to what the real dependencies are.

  Finally, extraneous includes may slow down the build.

  Fix all issues in `bench`, by applying the rule include-what-you-use (iwyu).

  Follow-up pull requests will handle the other places.

ACKs for top commit:
  hodlinator:
    ACK fab0e834b8
  achow101:
    ACK fab0e834b8
  TheCharlatan:
    ACK fab0e834b8
  hebasto:
    ACK fab0e834b8.
  brunoerg:
    crACK fab0e834b8
  stickies-v:
    ACK fab0e834b8

Tree-SHA512: f079c05d3ddebafabbd5a6c76d43d17337d1a962b97ba0ee27612f91c58491e7ce4e4229be54bd6e75a15512798c6f59925d0a076a37c050f8b9ef455ae5c9a2
2024-08-27 13:57:26 -04:00
Ava Chow
1248d0da22
Merge bitcoin/bitcoin#30719: Pre-28.x branch off version bump and doc updates
1bf9b70658 docs: Add 379 and 387 to bips.md (Ava Chow)
35ef34eab7 docs: Remove release 28.0 release notes fragments (Ava Chow)
f5cf43bb91 build: Bump to 28.99 (Ava Chow)

Pull request description:

  * Bump to 28.99 in preparation for the 28.x branching
  * Remove current release note fragments. They've been moved to [draft release notes]((https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft).) in the wiki.
  * Updated bips.md with missing BIPs that were implemented a while ago.

ACKs for top commit:
  maflcko:
    lgtm ACK 1bf9b70658
  tdb3:
    re ACK 1bf9b70658

Tree-SHA512: 9d5b05330268bb233369d78f06011dbdcdcb87770c499ae743ac4833735ba657b6611ff1e332c9d4added854b694c5a3a961d911247303f7ddaaaa72149d5574
2024-08-27 13:12:01 -04:00
Ava Chow
0022c84716
Merge bitcoin/bitcoin#30695: seeds: Add additional seed source and bump uptime requirements for Onion and I2P nodes
b061b35105 seeds: Regenerate mainnet seeds (virtu)
02dc45c506 seeds: Pull nodes from Luke's seeder (virtu)
7a2068a0ff seeds: Pull nodes from virtu's crawler (virtu)

Pull request description:

  This builds on #30008 and adds data [exported](https://github.com/virtu/seed-exporter) by [my crawler](https://github.com/virtu/p2p-crawler) an additional source for seed nodes. Data covers all supported network types.

  [edit: Added Luke's seeder as input as well.]

  ### Motivation
  - Further decentralizes the seed node selection process (in the long term potentially enabling an _n_-source threshold for nodes to prevent a single source from entering malicious nodes)
  - No longer need to manually curate seed node list for any network type: See last paragraph of OP in #30008. My crawler has been [discovering the handful of available cjdns nodes](https://21.ninja/reachable-nodes/nodes-by-net-type/) for around two months, all but one of which meet the reliability criteria.
  - Alignment of uptime requirements for Onion and I2P nodes with those of clearnet nodes to 50%: If I'm reading the code correctly, seeders appear to optimize for up-to-dateness by using [lower connection timeouts](3c1a63c672/src/crawl.rs (L349)) than [Bitcoin Core](bc87ad9854/src/netbase.cpp (L40C27-L40C48)) to maximize throughput. Since my crawler does not have the same timeliness requirements, it opts for accuracy by using generous timeouts. As a result, its data contains additional eligible Onion (and other darknet nodes), as is shown in the histogram below. Around 4500 Onion nodes are discovered so far (blue); my data adds ~6400 more (orange); ~ 1500 nodes take longer than the default 20-second Bitcoin Core timeout and won't qualify as "good".

  ![Connection time histogram for Onion nodes](https://github.com/user-attachments/assets/c3513604-aa48-4c75-b51d-13421eaed9eb)

  Here's the current results with 512 nodes for all networks except cjdns:
  <details>
  <summary>Using the extra data</summary>

  ```
  IPv4   IPv6  Onion  I2P    CJDNS Pass
  10335   2531  11545   1589     10 Initial
  10335   2531  11545   1589     10 Skip entries with invalid address
  5639   1431  11163   1589      8 After removing duplicates
  5606   1417  11163   1589      8 Enforce minimal number of blocks
  5606   1417  11163   1589      8 Require service bit 1
  4873   1228  11163   1589      8 Require minimum uptime
  4846   1225  11161   1588      8 Require a known and recent user agent
  4846   1225  11161   1588      8 Filter out hosts with multiple bitcoin ports
  512    512    512    512      8 Look up ASNs and limit results per ASN and per net
  ```
  </details>
  <details>
  <summary>Before</summary>

  ```
  IPv4   IPv6  Onion  I2P    CJDNS Pass
  5772   1323    443      0      2 Initial
  5772   1323    443      0      2 Skip entries with invalid address
  4758   1110    443      0      2 After removing duplicates
  4723   1094    443      0      2 Enforce minimal number of blocks
  4723   1094    443      0      2 Require service bit 1
  3732    867    443      0      2 Require minimum uptime
  3718    864    443      0      2 Require a known and recent user agent
  3718    864    443      0      2 Filter out hosts with multiple bitcoin ports
   512    409    443      0      2 Look up ASNs and limit results per ASN and per net
  ```
  </details>

  ### To dos
  - [x] Remove manual nodes and update README
  - [x] Mark nodes with connection times exceeding Bitcoin Core's default as bad in [exporter](https://github.com/virtu/seed-exporter): [done](https://github.com/virtu/seed-exporter/pull/12)
  - [x] Regenerate mainnet seeds
  - [x] Rebase, then remove WIP label once #30008 gets merged

ACKs for top commit:
  achow101:
    ACK b061b35105
  fjahr:
    utACK b061b35105

Tree-SHA512: 63e86220787251c7e8d2d5957bad69352e19ae17d7b9b2d27d8acddfec5bdafe588edb68d77d19c57f25f149de723e2eeadded0c8cf13eaca22dc33bd8cf92a0
2024-08-27 12:52:56 -04:00
Ava Chow
78567b052d
Merge bitcoin/bitcoin#30697: Bugfix: Ensure Atomicity in Wallet Settings Updates from Chain Interface
1b41d45d46 wallet: bugfix: ensure atomicity in settings updates (ismaelsadeeq)

Pull request description:

  This PR fixes #30620.

  As outlined in the issue, creating two wallets with `load_on_startup=true` simultaneously results in only one wallet being added to the startup file.

  The current issue arises because the wallet settings update process involves:
  1. Obtaining the settings value while acquiring the settings lock.
  2. Modifying the settings value.
  3. Overwriting the settings value while acquiring the settings lock again.

  This sequence is not thread-safe. Different threads could modify the same base value simultaneously, overwriting data from other workers without realizing it.

  The PR attempts to  fix this by modifying the chain interface's `updateRwSetting` method to accept a function that will be called with the settings reference. This function will either update or delete the setting and return an enum indicating whether the settings need to be overwritten in this or not.

  Additionally, this PR introduces two new methods to the chain interface:
  - `overwriteRwSetting`: This method replaces the setting with a new value.
  Used in `VerifyWallets`
  - `deleteRwSettings`: This method completely erases a specified setting.
  This method is currently used only in `overwriteRwSetting`.

  These changes ensure that updates are race-free across all clients.

ACKs for top commit:
  achow101:
    ACK 1b41d45d46
  furszy:
    self-code-ACK 1b41d45d46

Tree-SHA512: 50cda612b782aeb5e03e2cf63cc44779a013de1c535b883b57af4de22f24b0de80b4edecbcda235413baec0a12bdf0e5750fb6731c9e67d32e742d8c63f08c13
2024-08-27 12:29:20 -04:00
Ava Chow
1bf9b70658 docs: Add 379 and 387 to bips.md 2024-08-27 11:20:54 -04:00
Ava Chow
35ef34eab7 docs: Remove release 28.0 release notes fragments 2024-08-27 11:14:25 -04:00
brunoerg
8dec4e1294 fuzz: fix timeout in crypto_fschacha20poly1305 2024-08-27 11:25:47 -03:00
merge-script
c6d2d1cb66
Merge bitcoin/bitcoin#30720: chainparams: Remove seed.bitcoinstats.com
c88a7dc53e chainparams: Remove seed.bitcoinstats.com (Ava Chow)

Pull request description:

  This seeder no longer appears to be serving sufficient addresses.

  Fixes #29911

ACKs for top commit:
  1440000bytes:
    ACK c88a7dc53e
  virtu:
    ACK c88a7dc53e
  mzumsande:
    ACK c88a7dc53e
  BrandonOdiwuor:
    ACK c88a7dc53e

Tree-SHA512: 23db3a217bbc3cd96785f17bd2b1db886392cc864dfc00498fa30e69df414ad02cb35f34cd6b7e8adab7c024a7efa8fd4a39b8b8ef274d95974cb16eb1c39a5b
2024-08-27 11:59:01 +01:00
merge-script
e53b1c1a4b
Merge bitcoin/bitcoin#30705: test: Avoid intermittent block download timeout in p2p_ibd_stalling
fa5b58ea01 test: Avoid intermittent block download timeout in p2p_ibd_stalling (MarcoFalke)

Pull request description:

  Fixes #30704

  The goal of the test is to check the stalling timeout, not the block download timeout.

  On extremely slow hardware (for example qemu virtual hardware), downloading the 1023 blocks may take longer than the block download timeout.

  Fix it by pinning the time using mocktime, and only advance it when testing the stalling timeout.

ACKs for top commit:
  tdb3:
    CR ACK fa5b58ea01
  brunoerg:
    utACK fa5b58ea01

Tree-SHA512: 9a9221f264bea52be5e9fe81fd319f5a6970cd315cc5e9f5e2e049c5d84619b19b9f6f075cda8d34565c2d6c17a88fb57e195c66c271e40f73119a77caecb6d7
2024-08-27 11:20:42 +01:00
Hodlinator
49f9b645ea
refactor: Testnet4 - Replace uint256S("str") -> uint256{"str"}
Ran scripted-diff from 2d9d752e4f.

Follow-up to #29775 which overlapped with work on #30560 (the latter includes the scripted-diff commit).
2024-08-27 11:14:53 +02:00
MarcoFalke
fab0e834b8
bench: [refactor] iwyu 2024-08-27 07:33:59 +02:00
virtu
b061b35105 seeds: Regenerate mainnet seeds
Regenerate mainnet seeds from new sources without the need for hardcoded
data. Result has 512 nodes from each network type except cjdns, for
which only eight nodes were found that match the seed node criteria.
2024-08-27 07:00:27 +02:00
virtu
02dc45c506 seeds: Pull nodes from Luke's seeder
Pull additional nodes from Luke's seeder to further decentralize the
generation of seed nodes.
2024-08-27 07:00:27 +02:00
virtu
7a2068a0ff seeds: Pull nodes from virtu's crawler
Pull additional nodes from virtu's crawler. Data includes sufficient
Onion and I2P nodes to align the uptime requirements for these networks
to that of clearnet nodes (i.e., 50%). Data also includes more than
three times the number of CJDNS nodes currently hardcoded into
nodes_main_manual.txt, so hardcoded nodes becomes obsolete.
2024-08-27 07:00:09 +02:00
Ava Chow
37cdb5f248
Merge bitcoin/bitcoin#30008: seeds: Pull additional nodes from my seeder and update fixed seeds
41ad84a00c seeds: Use fjahr's more up to date asmap (Ava Chow)
d8fd1e0faf seeds: Fixed seeds update (Ava Chow)
f1f24d7214 seeds: Add testnet4 fixed seeds file (Ava Chow)
8ace71c737 seeds: Remove manual onion and i2p seeds (Ava Chow)
ed5b86cbe4 seeds: Add testnet instructions (Ava Chow)
0676515397 seeds: Also pull from achow101 seeder (Ava Chow)
5bab3175a6 makeseeds: Configurable minimum blocks for testnet4's smaller chain (Ava Chow)
d2465dfac6 makeseeds: Shuffle ips after parsing (Ava Chow)
af550b3a0f makeseeds: Support CJDNS (Ava Chow)
d5a8c4c4bd makeseeds: Update user agent regex (Ava Chow)

Pull request description:

  The [DNS seeder](https://github.com/achow101/dnsseedrs) that I wrote collects statistics on node reliability in the same way that sipa's seeder does, and also outputs this information in the same file format. Thus it can also be used in our fixed seeds update scripts. My seeder additionally crawls onion v3, i2p, and cjdns, so will now be able to set those fixed seeds automatically rather than curating manual lists.

  In doing this update, I've found that `makeseeds.py` is missing newer versions from the regex as well as cjdns support; both of these have been updated.

  I also noticed that the testnet fixed seeds are all manually curated and sipa's seeder does not appear to publish any testnet data. Since I am also running the seeder for testnet, I've added the commands to generate testnet fixed seeds from my seeder's data too.

  Lastly, I've updated all of the fixed seeds. However, since my seeder has not found any cjdns nodes that met the reliability criteria (possibly due to connectivity issues present in those networks), I've left the previous manual seeds for that network.

ACKs for top commit:
  fjahr:
    re-ACK 41ad84a00c
  virtu:
    ACK [41ad84a](41ad84a00c)

Tree-SHA512: 6ba0141f053d9d6ae7d8c9574f061be38f3e65b28de1d6660c1885ab942623b5a0ec70754b4fcfc5d98fe970f5f179a940d5880b5061ed698f7932500e01d3ee
2024-08-26 15:49:42 -04:00
Ava Chow
c88a7dc53e chainparams: Remove seed.bitcoinstats.com
This seeder no longer appears to be serving sufficient addresses.
2024-08-26 15:44:22 -04:00
Ava Chow
f5cf43bb91 build: Bump to 28.99 2024-08-26 15:12:58 -04:00
Ava Chow
a05987defd
Merge bitcoin/bitcoin#30715: qt: 28.0 translations update
a0cdf43c4d qt: 28.0 translations update (Hennadii Stepanov)

Pull request description:

  The 28.x branching off is [scheduled](https://github.com/bitcoin/bitcoin/issues/29891) for today, so it's [time](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off) to fetch the recent translations from [Transifex.com](https://www.transifex.com/bitcoin/bitcoin) using the [`bitcoin-maintainer-tools/update-translations.py`](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/main/update-translations.py) tool.

  A similar PR from the previous release cycle: https://github.com/bitcoin/bitcoin/pull/29546.

ACKs for top commit:
  achow101:
    ACK a0cdf43c4d
  pablomartin4btc:
    ACK a0cdf43c4d

Tree-SHA512: 865d2dc21415a157fafacfab5e018594fdbfa77ce32600877247bfb755407bd93e894eb3e1c8b6a286013e85b63e3d14baf6853d3308b4dfc0705b7a090fd4aa
2024-08-26 15:04:24 -04:00
Ava Chow
a1f2b5bbb5
Merge bitcoin/bitcoin#30690: devtools, utxo-snapshot: Fix block height out of range in script
5b4f34006d devtools, utxo-snapshot: Fix block height out of range (pablomartin4btc)

Pull request description:

  <details>
  <summary>Fixing a <a href="https://github.com/bitcoin/bitcoin/pull/28553#pullrequestreview-2251032570">bug</a> in <code>utxo_snapshot.sh</code>.</summary>

  ```
  /contrib/devtools/utxo_snapshot.sh 840000 snapshot2.dat ./src/bitcoin-cli -datadir=${AU_DATADIR}
  Do you want to disable network activity (setnetworkactive false) before running invalidateblock? (Y/n):
  Disabling network activity
  false
  error code: -8
  error message:
  Block height out of range
  ```

  And the user will see the following in the node and it would stay there if not reset:

  ```
  2024-08-21T14:44:13Z UpdateTip: new best=00000000000000afa0cd000a16e244f56032735d41acd32ac00337aceb2a5240 height=235382 version=0x00000002 log2_work=69.987697 tx=17492185 date='2013-05-09T23:54:32Z' progress=0.016219 cache=71.0MiB(571085txo)
  2024-08-21T14:44:13Z UpdateTip: new best=0000000000000087c5e0b820afff496b95ba44ad64640c73b234d3261d3f99d2 height=235383 version=0x00000002 log2_work=69.987750 tx=17492341 date='2013-05-09T23:54:47Z' progress=0.016219 cache=71.0MiB(571291txo)
  2024-08-21T14:44:13Z UpdateTip: new best=000000000000014a4b5fddf3c8abb6209247255ca9e8df786b271dd1b2ac82a6 height=235384 version=0x00000002 log2_work=69.987804 tx=17492344 date='2013-05-10T00:20:18Z' progress=0.016219 cache=71.0MiB(571297txo)
  2024-08-21T14:44:13Z SetNetworkActive: false

  ```

  </details>

  This is a "temporary" fix until #29553 gets merged, which will remove the script entirely.

  Handle the "Block height out of range" error gracefully by checking if the node has synchronized to or beyond the required block height, otherwise without this validation the node would keep the network disabled if the user selected that option.

  <details>
  <summary>Provide a user-friendly message if the block height is out of range and exit the script cleanly.</summary>

  ```
  /contrib/devtools/utxo_snapshot.sh 840000 snapshot2.dat ./src/bitcoin-cli -datadir=${AU_DATADIR}
  Error: The node has not yet synchronized to block height 840001.
  Please wait until the node has synchronized past this block height and try again.
  ```

  </details>

ACKs for top commit:
  achow101:
    ACK 5b4f34006d
  fjahr:
    tACK 5b4f34006d

Tree-SHA512: 2b71286b627872d7cfdb367e29361afa3806a7ef9d65075b93892b735ff2ab729069e2f7259d30262909e73cef17fb7dca231615cc1863968cd042f4a2a4f901
2024-08-26 14:55:10 -04:00
Ava Chow
5116dd4b83
Merge bitcoin/bitcoin#30698: test: Add time-timewarp-attack boundary cases
31378d44f4 test: Add time-timewarp-attack boundary cases (Greg Sanders)

Pull request description:

  Basic addition to test case added in https://github.com/bitcoin/bitcoin/pull/30681

ACKs for top commit:
  fjahr:
    Code review ACK 31378d44f4
  achow101:
    ACK 31378d44f4
  tdb3:
    ACK 31378d44f4

Tree-SHA512: 7d18af9e7fac0ecb0fb5e4c009d6ce3f9af849b4abc54ae8cf681dc8b882aaa9b4cffd7f798b7193c76d89f96ec2cb6dbd13e341882d59b0580d3573ec675ffd
2024-08-26 14:44:33 -04:00
Ava Chow
d50f0ce248
Merge bitcoin/bitcoin#30669: test: XORed blocks test follow up
e1d5dd732d test: check xor.dat recreated when missing (tdb3)
d1610962bf test: add null block xor key (tdb3)
1ad999b9da refactor: lift NUM_XOR_BYTES (tdb3)
d8399584dd refactor: move read_xor_key() to TestNode (tdb3)
d43948c3ef refactor: use unlink rather than os.remove (tdb3)
c8176f758b test: add blocks_key_path (tdb3)

Pull request description:

  Builds on PR #30657.

  Refactors `read_xor_key()` from `util.py` to `test_node.py` (comment https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1723358327)

  Adds a check that `xor.dat` is created when missing (comment https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1717724161)

  Help states:
  ```
  -blocksxor
         Whether an XOR-key applies to blocksdir *.dat files. The created XOR-key
         will be zeros for an existing blocksdir or when `-blocksxor=0` is
         set, and random for a freshly initialized blocksdir. (default: 1)
  ```

ACKs for top commit:
  maflcko:
    ACK e1d5dd732d
  achow101:
    ACK e1d5dd732d
  theStack:
    re-ACK e1d5dd732d
  brunoerg:
    reACK e1d5dd732d

Tree-SHA512: 325912ef646ec88e0a58e1ece263a2b04cbc06497e8fe5fcd603e509e80c6bcf84b09dd52dfac60e23013f07fc2b2f6db851ed0598649c3593f452c4a1424bd9
2024-08-26 14:30:07 -04:00
ismaelsadeeq
1b41d45d46
wallet: bugfix: ensure atomicity in settings updates
- Settings updates were not thread-safe, as they were executed in
  three separate steps:

  1) Obtain settings value while acquiring the settings lock.
  2) Modify settings value.
  3) Overwrite settings value while acquiring the settings lock.

  This approach allowed concurrent threads to modify the same base value
  simultaneously, leading to data loss. When this occurred, the final
  settings state would only reflect the changes from the last thread
  that completed the operation, overwriting updates from other threads.

  Fix this by making the settings update operation atomic.

- Add test coverage for this behavior.

Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
2024-08-26 13:41:56 +01:00
Hennadii Stepanov
a0cdf43c4d
qt: 28.0 translations update 2024-08-26 08:38:58 +01:00
tdb3
e1d5dd732d
test: check xor.dat recreated when missing 2024-08-25 08:43:45 -04:00
tdb3
d1610962bf
test: add null block xor key 2024-08-25 08:43:37 -04:00
tdb3
1ad999b9da
refactor: lift NUM_XOR_BYTES 2024-08-25 08:43:33 -04:00
tdb3
d8399584dd
refactor: move read_xor_key() to TestNode 2024-08-25 08:42:38 -04:00
tdb3
d43948c3ef
refactor: use unlink rather than os.remove 2024-08-25 08:41:21 -04:00
merge-script
6d546336e8
Merge bitcoin/bitcoin#30651: fuzz: remove repeated word in note
3f05a1068d remove repeated word in note (sunerok)

Pull request description:

  Fix typo.

ACKs for top commit:
  maflcko:
    ACK 3f05a1068d
  danielabrozzoni:
    ACK 3f05a1068d

Tree-SHA512: 709d96ed18608c0ea788b4f0696abad79ab1b81c4f266487d16bbe4cfca5b99b8f7f9a58f830866db9695aa3aebcc6442098b1533d85507729af99709a53d26a
2024-08-24 18:56:24 +01:00
merge-script
6441c77e97
Merge bitcoin/bitcoin#30687: test: replace deprecated secp256k1 context flags usage
60055f1abc test: replace deprecated secp256k1 context flags usage (Sebastian Falbesoner)

Pull request description:

  The flags `SECP256K1_CONTEXT_{SIGN,VERIFY}` have been marked as deprecated since libsecp256k1 version 0.2 (released in December 2022), with the recommendation to use SECP256K1_CONTEXT_NONE instead, see https://github.com/bitcoin-core/secp256k1/pull/1126 and 1988855079/CHANGELOG.md (L132). Note that in contrast to other deprecated functions/variables, these defines don't have a deprecated attribute and hence don't lead to a compiler warning (see https://github.com/bitcoin-core/secp256k1/pull/1126#discussion_r922105271), so they are not easily detected.

ACKs for top commit:
  TheCharlatan:
    ACK 60055f1abc
  ismaelsadeeq:
    utACK 60055f1abc
  tdb3:
    light CR and test ACK 60055f1abc

Tree-SHA512: d93cf49e018a58469620c0d2f50242141f22dabc70afb2a7cd64e416f4f55588714510ae5a877376dd1e6b6f7494261969489af4b18a1c9dff0d0dfdf93f1fa8
2024-08-24 18:53:41 +01:00
merge-script
d54fbc85c4
Merge bitcoin/bitcoin#30703: test: Avoid duplicate curl call in get_previous_releases.py
fa5aeab3cb test: Avoid duplicate curl call in get_previous_releases.py (MarcoFalke)

Pull request description:

  Seems odd having to translate `404` to "Binary tag was not found". Also, it seems odd to write a for-loop over a list with one item.

  Fix both issues by just using a single call to `curl --fail ...`.

  Can be tested with: `test/get_previous_releases.py -b v99.99.99`

  Before:

  ```
  Releases directory: releases
  Fetching: https://bitcoincore.org/bin/bitcoin-core-99.99.99/bitcoin-99.99.99-x86_64-linux-gnu.tar.gz
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
    0  286k    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
  Binary tag was not found
  ```

  After:

  ```
  Releases directory: releases
  Fetching: https://bitcoincore.org/bin/bitcoin-core-99.99.99/bitcoin-99.99.99-x86_64-linux-gnu.tar.gz
    % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                   Dload  Upload   Total   Spent    Left  Speed
    0  286k    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
  curl: (22) The requested URL returned error: 404

ACKs for top commit:
  fanquake:
    ACK fa5aeab3cb
  brunoerg:
    utACK fa5aeab3cb
  tdb3:
    tested ACK fa5aeab3cb

Tree-SHA512: d5d31e0bccdd9de9b4a8ecf2e69348f4e8cee773050c8259b61db1ce5de73f6fbfffbe8c4d2571f7bef2de29cb42fd244573deebfbec614e487e76ef41681b9c
2024-08-24 16:24:23 +01:00
tdb3
c8176f758b
test: add blocks_key_path
Adds a convenience function to TestNode
to provide the path to the blocks xor key.
Updates util and feature_blocksxor to use it.
2024-08-23 12:49:56 -04:00
MarcoFalke
fa5b58ea01
test: Avoid intermittent block download timeout in p2p_ibd_stalling 2024-08-23 17:49:13 +02:00
stickies-v
18d65d2772
test: use uint256::FromUserHex for RANDOM_CTX_SEED
Removes dependency on unsafe and deprecated uint256S.

This makes parsing more strict, by requiring RANDOM_CTX_SEED
to be a string of up to 64 hex digits (optionally prefixed with
"0x"), whereas previously any string would be accepted, with
non-hex characters silently ignored and input longer than
64 characters (ignoring "0x" prefix) silently trimmed.

Can be tested with:

$ RANDOM_CTX_SEED=z ./src/test/test_bitcoin --log_level=all --run_test=timeoffsets_tests/timeoffsets_warning -- -printtoconsole=1 | grep RANDOM_CTX_SEED
RANDOM_CTX_SEED must consist of up to 64 hex digits ("0x" prefix allowed), it was set to: 'z'.

Co-Authored-By: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2024-08-23 13:53:40 +01:00
stickies-v
6819e5a329
node: use uint256::FromUserHex for -assumevalid parsing
Removes dependency on unsafe and deprecated uint256S.

This makes parsing more strict, by returning an error
when the input contains non-hex characters, or when it
contains more than 64 hex digits.

Also make feature_assumevalid.py more robust by using CBlock.hash
which is guaranteed to be 64 characters long, as opposed to the
variable-length hex(CBlock.sha256)
2024-08-23 13:53:40 +01:00
stickies-v
2e58fdb544
util: remove unused IsHexNumber
The relevant unit tests have been incorporated in
uint256_tests/from_user_hex in a previous commit.
2024-08-23 13:53:40 +01:00
stickies-v
8a44d7d3c1
node: use uint256::FromUserHex for -minimumchainwork parsing
Removes dependency on unsafe and deprecated uint256S.

This makes parsing more strict, by returning an error
when the input contains more than 64 hex digits.
2024-08-23 13:53:40 +01:00
stickies-v
70e2c87737
refactor: add uint256::FromUserHex helper
FromUserHex will be used in future commits to construct
uint256 instances from user hex input without being
unnecessarily restrictive on formatting by allowing
0x-prefixed input that is shorter than 64 characters.
2024-08-23 13:53:39 +01:00
merge-script
c81c6bf65b
Merge bitcoin/bitcoin#30665: test: Enable detect_leaks=1 in ASAN_OPTIONS explicitly
cccc5bfd35 test: Enable detect_leaks=1 in ASAN_OPTIONS explicitly (MarcoFalke)

Pull request description:

  It should be enabled by default, but being explicit can't hurt.

ACKs for top commit:
  fanquake:
    ACK cccc5bfd35

Tree-SHA512: ed284abd05c7a99c30b509844aa75785a5ccb506d8296a71347b4c328750a6a4ed1f87e7a3ec36ab17f27b467c033cc8ca5eb5e2b951f2ae7473327c5eb1ddae
2024-08-23 13:12:53 +01:00
MarcoFalke
fa5aeab3cb
test: Avoid duplicate curl call in get_previous_releases.py 2024-08-23 14:07:31 +02:00
Greg Sanders
31378d44f4 test: Add time-timewarp-attack boundary cases 2024-08-22 12:43:13 -04:00
glozow
55d663cb15
Merge bitcoin/bitcoin#30658: kernel: pre-28.x chainparams and headerssync update
221809b81c headerssync: Update headerssync configuration (Ava Chow)
c2707446f7 params: Update assumevalid and minimum chainwork (Ava Chow)
255d4514d3 params: Update chainTxData (Ava Chow)
6a5bdae322 params: Update assumed blockchain and chainstate sizes (Ava Chow)

Pull request description:

  Update chainparams and headerssync parameters for the pre-28.x branching, per https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off

ACKs for top commit:
  fjahr:
    re-ACK 221809b81c
  Sjors:
    re-ACK 221809b81c
  glozow:
    ACK 221809b81c
  marcofleon:
    ACK 221809b81c

Tree-SHA512: 5106d59f46dbe167fffa339519e52975ae5bfd7e52202d76ec058da0d4e8bf87355e90678f7ace7c8c402a2f7264050a0355680b9f727c7962ff60e8fcdb3a90
2024-08-22 17:19:50 +01:00
Ava Chow
338b9d82dc
Merge bitcoin/bitcoin#30681: Have miner account for timewarp mitigation, activate on regtest, lower nPowTargetTimespan to 144 and add test
59ff17e5af miner: adjust clock to timewarp rule (Sjors Provoost)
e929054e12 Add timewarp attack mitigation test (Sjors Provoost)
e85f386c4b consensus: enable BIP94 on regtest (Sjors Provoost)
dd154b0568 consensus: lower regtest nPowTargetTimespan to 144 (Sjors Provoost)

Pull request description:

  Because #30647 reduced the timewarp attack threshold from 7200s to 600s, our miner code will fail to propose a block template (on testnet4) if the last block of the previous period has a timestamp two hours in the future. This PR fixes that and also adds a test.

  The non-test changes in the last commit should be in v28, otherwise miners have to patch it themselves. If necessary I can split that out into a separate PR, but I prefer to get the tests in as well.

  In order to add the test, we activate BIP94 on regtest.

  In order for the test to run faster, we reduce its difficulty retarget period to 144, the same number that's already used for softfork activation logic. Regtest does not actually adjust its difficulty, so this change has no effect (except for `getnetworkhashps`, see commit).

  An alternative approach would be to run this test on testnet4, by hardcoding its first 2015 in the test suite. But since the timewarp mitigation is a serious candidate for a future mainnet softfork, it seems better to just deploy it on regtest.

  The next commits add a test and fix the miner code.

  The `MAX_TIMEWARP` constant is moved to `consensus.h` so both validation and miner code have access to it.

ACKs for top commit:
  achow101:
    ACK 59ff17e5af
  fjahr:
    ACK 59ff17e5af
  glozow:
    ACK 59ff17e5af

Tree-SHA512: 50af9fdcba9b0d5c57e1efd5feffd870bd11b5318f1f8b0aabf684657f2d33ab108d5f00b1475fe0d38e8e0badc97249ef8dda20c7f47fcc1698bc1008798830
2024-08-22 12:15:19 -04:00
glozow
5ce2285b87
Merge bitcoin/bitcoin#30691: Fix maybe-uninitialized warning in IsSpentKey
17707db939 Fix maybe-uninitialized warning in IsSpentKey (Ava Chow)

Pull request description:

  After 6ed424f2db, I started seeing a maybe-unitialized warning in `CWallet::IsSpentKey`:

  ```
  In destructor ‘constexpr std::_Vector_base<_Tp, _Alloc>::~_Vector_base() [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’,
      inlined from ‘constexpr std::vector<_Tp, _Alloc>::~vector() [with _Tp = unsigned char; _Alloc = std::allocator<unsigned char>]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/bits/stl_vector.h:738:7,
      inlined from ‘constexpr WitnessUnknown::~WitnessUnknown()’ at ./addresstype.h:95:8,
      inlined from ‘constexpr void std::destroy_at(_Tp*) [with _Tp = WitnessUnknown]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/bits/stl_construct.h:88:18,
      inlined from ‘constexpr void std::_Destroy(_Tp*) [with _Tp = WitnessUnknown]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/bits/stl_construct.h:149:22,
      inlined from ‘std::__detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::_M_reset()::<lambda(auto:20&&)> mutable [with auto:20 = WitnessUnknown&]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:498:19,
      inlined from ‘constexpr _Res std::__invoke_impl(__invoke_other, _Fn&&, _Args&& ...) [with _Res = void; _Fn = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::_M_reset()::<lambda(auto:20&&)>; _Args = {WitnessUnknown&}]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/bits/invoke.h:61:36,
      inlined from ‘constexpr std::enable_if_t<is_invocable_r_v<_Res, _Callable, _Args ...>, _Res> std::__invoke_r(_Callable&&, _Args&& ...) [with _Res = void; _Callable = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::_M_reset()::<lambda(auto:20&&)>; _Args = {WitnessUnknown&}]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/bits/invoke.h:111:28,
      inlined from ‘static constexpr decltype(auto) std::__detail::__variant::__gen_vtable_impl<std::__detail::__variant::_Multi_array<_Result_type (*)(_Visitor, _Variants ...)>, std::integer_sequence<long unsigned int, __indices ...> >::__visit_invoke(_Visitor&&, _Variants ...) [with _Result_type = void; _Visitor = std::__detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::_M_reset()::<lambda(auto:20&&)>&&; _Variants = {std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>&}; long unsigned int ...__indices = {8}]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:1064:40,
      inlined from ‘constexpr decltype(auto) std::__do_visit(_Visitor&&, _Variants&& ...) [with _Result_type = void; _Visitor = __detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::_M_reset()::<lambda(auto:20&&)>; _Variants = {variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>&}]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:1823:5,
      inlined from ‘constexpr void std::__detail::__variant::_Variant_storage<false, _Types ...>::_M_reset() [with _Types = {CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown}]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:496:23,
      inlined from ‘constexpr std::__detail::__variant::_Variant_storage<false, _Types ...>::~_Variant_storage() [with _Types = {CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown}]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:506:17,
      inlined from ‘constexpr std::__detail::__variant::_Copy_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::~_Copy_ctor_base()’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:581:12,
      inlined from ‘constexpr std::__detail::__variant::_Move_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::~_Move_ctor_base()’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:618:12,
      inlined from ‘constexpr std::__detail::__variant::_Copy_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::~_Copy_assign_base()’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:656:12,
      inlined from ‘constexpr std::__detail::__variant::_Move_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::~_Move_assign_base()’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:708:12,
      inlined from ‘constexpr std::__detail::__variant::_Variant_base<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::~_Variant_base()’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:762:12,
      inlined from ‘constexpr std::variant<_Types>::~variant() [with _Types = {CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown}]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/variant:1432:28,
      inlined from ‘bool wallet::CWallet::IsSpentKey(const CScript&) const’ at wallet/wallet.cpp:1055:37:
  /usr/lib/gcc/x86_64-pc-linux-gnu/13.3.0/include/c++/bits/stl_vector.h:370:31: error: ‘*(std::_Vector_base<unsigned char, std::allocator<unsigned char> >*)((char*)&<unnamed> + offsetof(const std::CTxDestination, std::variant<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Variant_base<CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Move_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Copy_assign_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Move_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Copy_ctor_base<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::<unnamed>.std::__detail::__variant::_Variant_storage<false, CNoDestination, PubKeyDestination, PKHash, ScriptHash, WitnessV0ScriptHash, WitnessV0KeyHash, WitnessV1Taproot, PayToAnchor, WitnessUnknown>::_M_u) + 8).std::_Vector_base<unsigned char, std::allocator<unsigned char> >::_M_impl.std::_Vector_base<unsigned char, std::allocator<unsigned char> >::_Vector_impl::<anonymous>.std::_Vector_base<unsigned char, std::allocator<unsigned char> >::_Vector_impl_data::_M_end_of_storage’ may be used uninitialized [-Werror=maybe-uninitialized]
    370 |                       _M_impl._M_end_of_storage - _M_impl._M_start);
        |                       ~~~~~~~~^~~~~~~~~~~~~~~~~
  wallet/wallet.cpp: In member function ‘bool wallet::CWallet::IsSpentKey(const CScript&) const’:
  wallet/wallet.cpp:1055:37: note: ‘<anonymous>’ declared here
   1055 |         if (IsAddressPreviouslySpent(pkh_dest)) {
        |             ~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~
  ```

  Refactoring the function so that the `WitnessV0KeyHash`, `ScriptHash`, and `PKHash` are constructed within an `if` statement seems to make it go away without changing behavior.

ACKs for top commit:
  maflcko:
    lgtm ACK 17707db939
  l0rinc:
    utACK 17707db939
  hebasto:
    ACK 17707db939, tested on Ubuntu 24.04. The warning has been silenced for both GCC 13.2.0 and 14.0.1.
  tdb3:
    ACK 17707db939
  BrandonOdiwuor:
    Code Review ACK 17707db939

Tree-SHA512: df10a494adfaecb29060f3936d2910aa4291e8caa2a64cbd48e57ed67208f16a02441a14a599a092e0e8de3b338866f666f042b9c6109d0003c0d3e7af4daf48
2024-08-22 15:08:02 +01:00