e747ed989ebb7b9650ba1478d583f7f507e1083f contrib: fix read metadata related comment (rkrux)
d3095ac35a8891afc891458211130170d5a8a12e contrib: update `dumptxoutset` command in utxo_to_sqlite doc (rkrux)
Pull request description:
I noticed couple discrepancies in the documentation and comments of `utxo_to_sqlite` tool while using it, this PR fixes them. More details in the commit messages.
ACKs for top commit:
theStack:
lgtm ACK e747ed989ebb7b9650ba1478d583f7f507e1083f
Tree-SHA512: c5dab25d97c50a203ba008acfc5c93baf20cf24261a5a5127bdacc41eeeeae1c9d7b129710c2997e9fd1536ec000186e7f6395efc0188e1111b47868b7d9745a
d82dc1041524e8402d201d32c9143233b5ec1baf ci: Fix filtering out Qt generated files from `compile_commands.json` (Hennadii Stepanov)
Pull request description:
This PR:
1. Adjusts the regex for Qt-generated files to match the CMake build directory structure.
2. Moves the filtering command to run before `clang-tidy`, ensuring that Qt-generated files are not needlessly processed.
Fixes https://github.com/bitcoin/bitcoin/issues/31801.
ACKs for top commit:
maflcko:
ACK d82dc1041524e8402d201d32c9143233b5ec1baf 🚂
Tree-SHA512: 12ff40ffb134c56d848976e73aef7a8f895b856992b37b09ce27a1dcf6adef2cf148ba7e7d32b52ce1866c9bbeb165f9b6e6106c6c23b774d00d61bf379e13c9
c73b59d47f1ec6fff1ad9155181c2285a5ef5cf4 fuzz: implement targets for PCP and NAT-PMP port mapping requests (Antoine Poinsot)
1695c8ab5bd3ea2dd0a065bcb8162a973dede7fe fuzz: in FuzzedSock::GetSockName(), return a random-length name (Antoine Poinsot)
0d472c19533a0c13ea8b79e84bcff49230179519 fuzz: never return an uninitialized sockaddr in FuzzedSock::GetSockName (Antoine Poinsot)
39b7e2b5905255645264bc332b934b62441e55b9 fuzz: add steady clock mocking to FuzzedSock (Antoine Poinsot)
6fe1c35c05b353f5cc3f3811fdf46e3b220096e4 pcp: make NAT-PMP error codes uint16_t (Antoine Poinsot)
01906ce912e945c967316f829c1356f8ff38745f pcp: make the ToString method const (Antoine Poinsot)
Pull request description:
Based on https://github.com/bitcoin/bitcoin/pull/31022, this introduces a fuzz target for `PCPRequestPortMap` and `NATPMPRequestPortMap`.
Like in #31022 we set `CreateSock` to return a `Sock` which mocks the responses from the server and uses a mocked steady clock for the `Wait`s. Except here we simply respond with fuzzer-provided data until the client stop sending requests. We also sometimes inject errors and connection failures based on fuzzer-provided data.
We reuse the existing `FuzzedSock`, so a preparatory commit is included that adds steady clock mocking to it. This may be useful for other harnesses as well.
ACKs for top commit:
laanwj:
re-ACK c73b59d47f1ec6fff1ad9155181c2285a5ef5cf4
marcofleon:
ACK c73b59d47f1ec6fff1ad9155181c2285a5ef5cf4
dergoegge:
utACK c73b59d47f1ec6fff1ad9155181c2285a5ef5cf4
Tree-SHA512: 24cd4d958a0999946a0c3d164a242fc3f0a0b66770630252b881423ad0065d29fdaab765014d193b705d3eff397f201d51a88a3ca80c63fd3867745e6f21bb2b
The utxo snapshot metadata doesn't seem to contain any block height as per the
CPP code and no such value is read few lines down by the tool code as well.
Related CPP code: bitcoin/bitcoin/blob/28.x/src/node/utxo_snapshot.h#L60-L66
Running the `dumptxoutset` command without a `type` parameter leads
to the following error. Update the tool documentation to make it
easier to follow.
`Invalid snapshot type "" specified. Please specify "rollback" or "latest"`
7267ed051820b9227856143bdf767ae94a5be1d8 qt: Update `src/qt/locale/bitcoin_en.xlf` after string freeze (Hennadii Stepanov)
Pull request description:
This PR follows our [Release Process](864386a744/doc/release-process.md) and implements the ["Translation string freeze"](https://github.com/bitcoin/bitcoin/issues/31029) step.
Steps to reproduce the diff on Ubuntu:
```
$ cmake --preset dev-mode -DWITH_USDT=OFF -DWITH_MULTIPROCESS=OFF
$ cmake --build build_dev_mode --target translate
```
At the moment, the multiprocess-specific code does not introduce any new translatable strings. Therefore, there is no need to build depends with `MULTIPROCESS=1` to review this PR.
ACKs for top commit:
stickies-v:
ACK 7267ed051820b9227856143bdf767ae94a5be1d8 - I get the same results when building the `translate` target.
pablomartin4btc:
tACK 7267ed051820b9227856143bdf767ae94a5be1d8
Tree-SHA512: dc3641d3288c00cb7802714680508de517e56c615716e52181555634ad489fbed676229063995170edb8efeaa4e900ef2d3d5a0f1b8ce7cec143ef364c96e1c0
d871d778251c35fd55d420ecf5c278f3edfea227 test: Remove non-portable IPv6 test (Hennadii Stepanov)
Pull request description:
On Illumos-based systems, such as OpenIndiana and OmniOS, the assumption that "the default zone ID of 0 can be omitted for the default scope" is incorrect. As a result, `getaddrinfo("fe80::1%0", ...)` returns the `EAI_NONAME` error instead of resolving to "fe80::1".
See: https://www.illumos.org/man/3SOCKET/getaddrinfo.
This PR removes the problematic code introduced in https://github.com/bitcoin/bitcoin/pull/19951.
ACKs for top commit:
fanquake:
ACK d871d778251c35fd55d420ecf5c278f3edfea227
Tree-SHA512: 2ef5c22f826d16661deb1d6d005cbda64179e8b83be43d3d6ac51caff02187cf224355c9da144ff110a6ae2cb68f0338ea9b62af8e0f9f1014a518cf9dad6ab5
2c4b229c906de6250500d3af2b44808e90b9ce0b cmake: Introduce `FUZZ_LIBS` (Hennadii Stepanov)
ea929c0848e2d95a71439e1b3aa0cf350e12bc73 scripted-diff: Rename CMake helper module (Hennadii Stepanov)
8d238c1dfde28bbd38bfba84136900724c0d7d95 cmake: Delete `check_cxx_source_links*` macros (Hennadii Stepanov)
71bf8294a985d818a9d855390bd8503a9ae8074a cmake: Convert `check_cxx_source_compiles_with_flags` to a function (Hennadii Stepanov)
88ee6800c9686931a1550b41fa634b0c6c3988a7 cmake: Delete `check_cxx_source_links_with_flags` macro (Hennadii Stepanov)
09e8fd25b1a5411472564e599ad15059bbf9e8d6 build: Don't override CMake's default try_compile target (Hennadii Stepanov)
Pull request description:
This was requested in https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2515287092.
From https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2511246212:
> (Almost?) every CMake check internally uses the [`try_compile()`](https://cmake.org/cmake/help/latest/command/try_compile.html) command, whose behaviour, in turn, depends on the [`CMAKE_TRY_COMPILE_TARGET_TYPE`](https://cmake.org/cmake/help/latest/variable/CMAKE_TRY_COMPILE_TARGET_TYPE.html) variable:
>
> 1. The default value, `EXECUTABLE`, enables both compiler and linker checks.
>
> 2. The `STATIC_LIBRARY` value enables only compiler checks.
>
>
> To mimic Autotools' behaviour, we [disabled](d3f42fa08f/cmake/module/CheckSourceCompilesAndLinks.cmake (L9-L10)) linker checks by setting `CMAKE_TRY_COMPILE_TARGET_TYPE` to `STATIC_LIBRARY` globally (perhaps not the best design). This effectively separates the entire CMake script into regions where `CMAKE_TRY_COMPILE_TARGET_TYPE` is:
>
> * unset
>
> * set to `STATIC_LIBRARY`
>
> * set to `EXECUTABLE`
From https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2515287092:
> > This seems very fragile and unintuitive, and the fact that this could silently break at any point is not documented in any way. I don't think other bad design decisions should lead to us having to write even more boilerplate code to fix things that should "just work" (minus the upstream bugs).
>
> Agreed. I forgot that we set `CMAKE_TRY_COMPILE_TARGET_TYPE` globally. And even worse, it's buried in a module. If that upsets CMake internal tests, I think we should undo that.
This PR ensures that `CMAKE_TRY_COMPILE_TARGET_TYPE` is modified only within local scopes.
Additionally, the `FUZZ_LIBS` variable has been introduced to handle additional libraries required for linking, rather than link options, in certain build environment, such as OSS-Fuzz.
ACKs for top commit:
TheCharlatan:
Re-ACK 2c4b229c906de6250500d3af2b44808e90b9ce0b
theuni:
utACK 2c4b229c906de6250500d3af2b44808e90b9ce0b
Tree-SHA512: f72ffa8f50f216fc1a2f8027ba8ddfd4acd42b94ff6c1cb2138f2da51eb8f945660e97d3c247d7f3f7ec8dfebbccec3ab84347d6ae2e3f8a40f3d7aa8b14cde9
63a8791e15c3ffb44b84ab3e85db62d7997d25fd contrib: fix BUILDDIR in gen-bitcoin-conf script and gen-manpages.py (jurraca)
Pull request description:
The `gen-bitcoin-conf.sh` and `gen-manpages.py` scripts assume a top level `src/` build dir, but in-tree builds are no longer allowed, nor recommended in the build steps. If a user builds `bitcoind` as recommended, these scripts fail. To fix it, we update the `BUILDDIR` env var and update the README accordingly.
Follows up on initial work and discussion in #31332 .
ACKs for top commit:
fjahr:
Code review ACK 63a8791e15c3ffb44b84ab3e85db62d7997d25fd
achow101:
ACK 63a8791e15c3ffb44b84ab3e85db62d7997d25fd
Tree-SHA512: cf4d5b0d2e8b1f5db759bec01e131d8a0c511a2fd183389d2a0488d5fe4a906db2579d944f408b5c966f619edc6b2534023c3521f1fa5f8edd0216d29f3e48db
ca6aa0b9bee3fdf355b7154a9a686a80977f2a02 doc: loadwallet loads from relative walletdir (am-sq)
Pull request description:
## Why this change?
https://github.com/bitcoin/bitcoin/issues/30269 describes a need for documentation improvement with the `loadwallet` RPC. Namely, [some users have found](https://bitcoin.stackexchange.com/questions/123331/how-do-you-load-a-regtest-wallet) the usage description confusing when it comes to loading wallets that are not in the normal case of being in the default wallet directory.
The default wallet directory, depending on the machine OS, has the base directory defined here: 9c5cdf07f3/src/common/args.cpp (L699) which is then appended with `/wallets`. So for example, for MacOS, it would be `~/Library/Application Support/Bitcoin/wallets`.
## The changes implemented
1. Change the help text to indicate that the filename (or directory) passed in to `loadwallet` is relative to the base wallet directory
2. Adds additional examples to the help page showing how to fetch a wallet within a subdirectory of the base data directory for wallets, or from an absolute path
ACKs for top commit:
achow101:
ACK ca6aa0b9bee3fdf355b7154a9a686a80977f2a02
maflcko:
lgtm ACK ca6aa0b9bee3fdf355b7154a9a686a80977f2a02
rkrux:
ACK ca6aa0b9bee3fdf355b7154a9a686a80977f2a02
jonatack:
ACK ca6aa0b9bee3fdf355b7154a9a686a80977f2a02
Tree-SHA512: 123ae118c79ee1843ed65861e7a008658a53e47d4d14f2b7612561bba1b1dbdb6744f9aaac1587aac231c62d0c1804de848a6d732f1382788b437d9fe6474012
fa8de4706a01322fd8ab8e491d297b97f001ecff ci: Switch to gcr.io mirror to avoid rate limits (MarcoFalke)
Pull request description:
dockerhub seems to have recently started to increase their rate limits further, beyond what is documented, even to the extent where pulling the same image twice at the same time results in a ban. See https://github.com/bitcoin/bitcoin/issues/31797#issuecomment-2656374222
Fix all issues by just using another mirror, as documented in https://cloud.google.com/artifact-registry/docs/pull-cached-dockerhub-images
Fixes https://github.com/bitcoin/bitcoin/issues/31797
ACKs for top commit:
0xB10C:
ACK fa8de4706a01322fd8ab8e491d297b97f001ecff
Tree-SHA512: 83d021059772adf473bd6c01443b53f787259e873016238869d716992a2451e18ab98f034e1ac9cd05f2ddbe45eba86cb9ea690be8e16b69c87dd8649d7ac7d4
9e4a4b4832219d9d11da441779ab8a3b1304bd8b cmake: Check `-Wno-*` compiler options for `leveldb` target (Hennadii Stepanov)
Pull request description:
Otherwise, https://cirrus-ci.com/task/4830737755537408:
```
At global scope:
cc1plus: note: unrecognized command-line option ‘-Wno-conditional-uninitialized’ may have been intended to silence earlier diagnostics
```
ACKs for top commit:
TheCharlatan:
ACK 9e4a4b4832219d9d11da441779ab8a3b1304bd8b
Tree-SHA512: 05553c80399180e01d45c3f02074ca0ce620011b29b03bef5433b87c9d88fd281fb6bf0203fc6fff590f3780c182a3fab8307002536b6350e03748420c346602
c4c5cf174883cb53256e869f0d1673e29576a97c cmake: Fix passing `APPEND_*FLAGS` to `secp256k1` subtree (Hennadii Stepanov)
eb540a262953baf9ed920f98efc4044a353b278a cmake: Remove `core_sanitizer_{cxx,linker}_flags` helper variables (Hennadii Stepanov)
Pull request description:
On the master branch @ 70e20ea024ce4f39abc4022e1ba19d5a6db2a207, the `APPEND_CPPFLAGS`, `APPEND_CFLAGS` and `APPEND_LDFLAGS` are not correctly applied when building C code in the `secp256k1` subtree, as intended.
This behaviour occurs due to two issues:
1. The command here: 70e20ea024/src/CMakeLists.txt (L77)
does not affect the code in `add_subdirectory(secp256k1)` above it.
2. `APPEND_LDFLAGS` is not passed to the subtree's build system at all.
This PR fixes both issues.
Additionally, the helper variables `core_sanitizer_cxx_flags` and `core_sanitizer_linker_flags` have been removed.
ACKs for top commit:
theuni:
utACK c4c5cf174883cb53256e869f0d1673e29576a97c.
TheCharlatan:
ACK c4c5cf174883cb53256e869f0d1673e29576a97c
Tree-SHA512: 707acfa623f0436e34e9e6ba8ce2979e0fde5e196e2242fd1cde4c50f433938549781193d8a06419a0866bbe6d69d76f8383d973afbd87d944407963b318c5c9
CMake distinguishes recommended methods for handling (1) linker options
and (2) libraries used during linking. Therefore, it is both reasonable
and consistent to introduce a dedicated variable for the latter,
particularly when a build environment, such as OSS-Fuzz, requires
linking against additional libraries.
CMake assumes the default value internally, so overriding this causes
problems. The minimal speedup of skipping the linker isn't worth the
complexity of setting it to static.
ff4ddd3d2e3b08156fd0a0aeb954fde0a5f4cb03 Revert "cmake: Ensure generated sources are up to date for `translate` target" (Hennadii Stepanov)
03b3166aac5acbf3eec95681c0ab98c4f94ab177 cmake: Exclude generated sources from translation (Hennadii Stepanov)
Pull request description:
This PR fixes an error encountered when building the `translate` target:
```
$ gmake -j $(nproc) -C depends MULTIPROCESS=1
$ cmake -G "Unix Makefiles" --preset dev-mode --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake -DWITH_USDT=OFF
$ cmake --build build_dev_mode -t translate
gmake[3]: *** No rule to make target 'src/test/ipc_test.capnp.c++', needed by 'src/qt/CMakeFiles/translate'. Stop.
gmake[2]: *** [CMakeFiles/Makefile2:1646: src/qt/CMakeFiles/translate.dir/all] Error 2
gmake[1]: *** [CMakeFiles/Makefile2:1653: src/qt/CMakeFiles/translate.dir/rule] Error 2
gmake: *** [Makefile:699: translate] Error 2
```
The previous [attempt](864386a744) to address this issue worked only with Ninja generators and has been reverted.
Essentially, this PR modifies the `translate` target so that it ignores generated sources rather than attempting to update them.
At present, multiprocess-specific sources do not contain any translatable strings. Nonetheless, it is prudent to maintain a general approach.
ACKs for top commit:
TheCharlatan:
ACK ff4ddd3d2e3b08156fd0a0aeb954fde0a5f4cb03
pablomartin4btc:
tACK ff4ddd3d2e3b08156fd0a0aeb954fde0a5f4cb03
Tree-SHA512: 6471498a33b145e073f76bd007591b0449e5d520f141c3e3afeca02a09c160fd0f572ec7172dd84703cdc2a1175ad8f3c91e8b0bf705d671338d760786765f56
3e9b12b3e0f039a8760410afed74c7e4d15afbe6 Revert "Merge bitcoin/bitcoin#31826: random: Check `GetRNDRRS` is supported in `InitHardwareRand` to avoid infinite loop" (Antoine Poinsot)
Pull request description:
PR #31826 was merged [despite the code not compiling](https://github.com/bitcoin/bitcoin/pull/31826#discussion_r1961315638).
#31902 was opened to fix the code but since this code is only targeting a not officially supported platform, we don't have a CI in place to compile and run tests on this platform, neither apparently reviewers do (nor does the author?), don't take more risk right before 29 and revert the original broken PR.
ACKs for top commit:
sipa:
ACK 3e9b12b3e0f039a8760410afed74c7e4d15afbe6
achow101:
ACK 3e9b12b3e0f039a8760410afed74c7e4d15afbe6
TheCharlatan:
ACK 3e9b12b3e0f039a8760410afed74c7e4d15afbe6
eval-exec:
ACK 3e9b12b3e0
laanwj:
ACK 3e9b12b3e0f039a8760410afed74c7e4d15afbe6
Tree-SHA512: e90f8ffb2eebe77e5b6f1c273fbeb29dd5bd6a76698d9a6048c33f3349033c56ea984dd9b64704698da01ecad4c47f98acac1a30312bf2499dbdd1931596953f
0c1b29a05777256c5ee686fff60f281dfeae289c ci: use GCC 13 for some jobs (fanquake)
cbc65b3ad5ad573844f9841199e1b0817f6c648a guix: use GCC 13.3.0 for base toolchain. (fanquake)
Pull request description:
Switch release builds to using GCC 13.3.0: https://gcc.gnu.org/gcc-13/, which landed in Guix in: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=750148ce1ea6c65a7c14424546db0078161f7e17.
Does not solve the cross-arch non-determinism for `powerpc64le-linux-gnu` builds.
ACKs for top commit:
achow101:
ACK 0c1b29a05777256c5ee686fff60f281dfeae289c
hebasto:
ACK 0c1b29a05777256c5ee686fff60f281dfeae289c.
TheCharlatan:
Re-ACK 0c1b29a05777
Tree-SHA512: eb3f091278d371166eb1df4718b6d0d68b09db65291d563dddd581964f2b488f901e4ba43831a699e2d0fd053d6e9038a307cbea78d5597da77699c34b440ea6
09b150bb8adae00854f02ece69fc6ef222fb07d9 In `InitHardwareRand`, do trail test for `RNDRRS` by `VerifyRNDRRS` (Eval EXEC)
Pull request description:
This PR want to fix#31817 by added a maximum retry limit (`max_retries`) to the `GetRNDRRS` function to prevent it from entering an infinite loop if the hardware random number generator fails to return a valid random number. This change improves stability and ensures that the function terminates after a predefined number of retries.
ACKs for top commit:
achow101:
ACK 09b150bb8adae00854f02ece69fc6ef222fb07d9
sipa:
utACK 09b150bb8adae00854f02ece69fc6ef222fb07d9
Tree-SHA512: 5626b6b182a55d344a3aba11b782259ecc6bbec513771d50077874c5f70934750e68add8f63aa0bf69c6b7b313112940a85508af5447622c703cc5e92439ab4a
e4dd5a351bde88a94326824945f4c8b1e4c15df2 test: wallet, abandon coinbase txs and their descendants during startup (furszy)
474139aa9bf7109df78e46936e5a649c70703386 wallet: abandon inactive coinbase tx and their descendants during startup (furszy)
Pull request description:
Since #26499, we mark coinbase transactions and their descendants as abandoned when a reorg arises through the "block disconnection" signal handler. However, this does not cover all scenarios; external wallets could contain coinbase transactions from blocks the node has not seen yet, or the user could have replaced the chain with an earlier or different version (one without the coinbase chain).
This affects balance calculation as well as mempool rebroadcast (descendants shouldn't be relayed).
Fix this by marking orphaned coinbase transactions and their descendants as abandoned during wallet startup.
ACKs for top commit:
achow101:
ACK e4dd5a351bde88a94326824945f4c8b1e4c15df2
rkrux:
tACK e4dd5a351bde88a94326824945f4c8b1e4c15df2
mzumsande:
Code Review ACK e4dd5a351bde88a94326824945f4c8b1e4c15df2
Tree-SHA512: 461a43de7a6f5a580f2e6e3b56ec9bc92239cd45e850a2ff594ab5488dcd4a507f68fbbf550a33d7173b2add0de80de1e1b3841e1dfab0c95b284212d8ced08a
bb633c9407c46eeadf6fe85e859cea1fed24473f tests: add functional test for miniscript decaying multisig (Michael Dietz)
Pull request description:
This is very closely based on [test/functional/wallet_multisig_descriptor_psbt.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_multisig_descriptor_psbt.py) both in code and concept. It should serve as some integration testing for Miniscript descriptors, and also documents a simple multisig that starts as 4-of-4 and decays to 3-of-4, 2-of-4, and finally 1-of-4 at block heights (I think in the real world aligning this to halvenings would be nice).
ACKs for top commit:
achow101:
ACK bb633c9407c46eeadf6fe85e859cea1fed24473f
rkrux:
reACK bb633c9407c46eeadf6fe85e859cea1fed24473f
hodlinator:
ACK bb633c9407c46eeadf6fe85e859cea1fed24473f
Tree-SHA512: 1f8e8e50258d45d8f2b882b5f86dcd390d86c543ff4801f397733017102e0854ac387960b6e296bb164603545615d224a4b400247cbbc07bf21b2f4b718ab2ff
Improves the documentation of help output for loadwallet
to clarify that filename is relative to the default
wallet directory. Adds examples that get a wallet from
sub-directories.
113a7a363faf1ace6b08a28cf4075dbce05f8f04 build: remove ENABLE_HARDENING cond from check-security (fanquake)
Pull request description:
This check is only used in release builds, where hardening should always be enabled. I can't think of a reason we'd want to silently skip these checks if hardening was inadvertently disabled.
ACKs for top commit:
maflcko:
lgtm ACK 113a7a363faf1ace6b08a28cf4075dbce05f8f04
TheCharlatan:
ACK 113a7a363faf1ace6b08a28cf4075dbce05f8f04
hebasto:
ACK 113a7a363faf1ace6b08a28cf4075dbce05f8f04.
Tree-SHA512: 46217e8ab238e23374d758b12e5b6bdc22353d8bf272aa0d2260cdea023b5b80aba972dccaa0a4fb8da21c8c665991848f7fd79966d20ac2489d499c68d95639
`OBJECT` libraries have historically exhibited poor support for various
features, both in the past and now. For example, see one of the latest
issues:
- https://gitlab.kitware.com/cmake/cmake/-/issues/24058
Furthermore, CMake maintainers have acknowledged:
> In general, however, where there is a choice, static libraries will
> typically be the more convenient choice in CMake projects.
This change:
1. Converts the `bitcoin_clientversion` library from an `OBJECT` library
to a `STATIC` library.
2. Removes an obsolete workaround.
9cf746d6631739df9c9f80accd5812b319efcfec cmake: add optional source files to crc32c directly (Daniel Pfeifer)
9c7823c5b531ac1bbe5bdb9f2731bfae06cf695a cmake: add optional source files to bitcoin_crypto directly (Daniel Pfeifer)
Pull request description:
Avoid having many static libraries by adding the optional sources to the target `bitcoin_crypto` directly.
Set the necessary compile options at the source file level, rather than the target level.
fixes: #31697
ACKs for top commit:
s373nZ:
ACK 9cf746d6631739df9c9f80accd5812b319efcfec
hebasto:
re-ACK 9cf746d6631739df9c9f80accd5812b319efcfec.
TheCharlatan:
ACK 9cf746d6631739df9c9f80accd5812b319efcfec
Tree-SHA512: 04b468ccbd284d63fc83b382177bb8183b325369835c3b92e555e159955c73d71712a63a2e556f8da68a1232ac07d3845e11f1057c50666843db91db98fca979
fa3e409c9a084112fc2644a2bba9aa196bdb229d contrib: Add deterministic-fuzz-coverage (MarcoFalke)
Pull request description:
The goal of this script is to detect and debug the remaining fuzz determinism and stability issues (https://github.com/bitcoin/bitcoin/issues/29018).
ACKs for top commit:
marcofleon:
Tested ACK fa3e409c9a084112fc2644a2bba9aa196bdb229d
brunoerg:
tested ACK fa3e409c9a084112fc2644a2bba9aa196bdb229d
Tree-SHA512: f336537d64188d6bc3c53880f4552a09cc498841c539cb7b4f14e622c9542531b970c1a6910981f7506e7bf659d2ce83471d58f5f51b0a411868f4c11eaf6b2a
This check is only used in release builds, where hardening should always
be enabled. I can't think of a reason we'd want to silently skip these
checks if hardening was inadvertently disabled.
4080b66cbec2b6fc2fcfd7356941236f65d508e3 test: add test for utxo-to-sqlite conversion script (Sebastian Falbesoner)
ec99ed738083fc1ad4c9d85095e26e7e58372217 contrib: add tool to convert compact-serialized UTXO set to SQLite database (Sebastian Falbesoner)
Pull request description:
## Problem description
There is demand from users to get the UTXO set in form of a SQLite database (#24628). Bitcoin Core currently only supports dumping the UTXO set in a binary _compact-serialized_ format, which was crafted specifically for AssumeUTXO snapshots (see PR #16899), with the primary goal of being as compact as possible. Previous PRs tried to extend the `dumptxoutset` RPC with new formats, either in human-readable form (e.g. #18689, #24202), or most recently, directly as SQLite database (#24952). Both are not optimal: due to the huge size of the ever-growing UTXO set with already more than 80 million entries on mainnet, human-readable formats are practically useless, and very likely one of the first steps would be to put them in some form of database anyway. Directly adding SQLite3 dumping support on the other hand introduces an additional dependency to the non-wallet part of bitcoind and the risk of increased maintenance burden (see e.g. https://github.com/bitcoin/bitcoin/pull/24952#issuecomment-1163551060, https://github.com/bitcoin/bitcoin/issues/24628#issuecomment-1108469715).
## Proposed solution
This PR follows the "external tooling" route by adding a simple Python script for achieving the same goal in a two-step process (first create compact-serialized UTXO set via `dumptxoutset`, then convert it to SQLite via the new script). Executive summary:
- single file, no extra dependencies (sqlite3 is included in Python's standard library [1])
- ~150 LOC, mostly deserialization/decompression routines ported from the Core codebase and (probably the most difficult part) a little elliptic curve / finite field math to decompress pubkeys (essentialy solving the secp256k1 curve equation y^2 = x^3 + 7 for y given x, respecting the proper polarity as indicated by the compression tag)
- creates a database with only one table `utxos` with the following schema:
```(txid TEXT, vout INT, value INT, coinbase INT, height INT, scriptpubkey TEXT)```
- the resulting file has roughly 2x the size of the compact-serialized UTXO set (this is mostly due to encoding txids and scriptpubkeys as hex-strings rather than bytes)
[1] note that there are some rare cases of operating systems like FreeBSD though, where the sqlite3 module has to installed explicitly (see #26819)
A functional test is also added that creates UTXO set entries with various output script types (standard and also non-standard, for e.g. large scripts) and verifies that the UTXO sets of both formats match by comparing corresponding MuHashes. One MuHash is supplied by the bitcoind instance via `gettxoutsetinfo muhash`, the other is calculated in the test by reading back the created SQLite database entries and hashing them with the test framework's `MuHash3072` module.
## Manual test instructions
I'd suggest to do manual tests also by comparing MuHashes. For that, I've written a go tool some time ago which would calculate the MuHash of a sqlite database in the created format (I've tried to do a similar tool in Python, but it's painfully slow).
```
$ [run bitcoind instance with -coinstatsindex]
$ ./src/bitcoin-cli dumptxoutset ~/utxos.dat
$ ./src/bitcoin-cli gettxoutsetinfo muhash <block height returned in previous call>
(outputs MuHash calculated from node)
$ ./contrib/utxo-tools/utxo_to_sqlite.py ~/utxos.dat ~/utxos.sqlite
$ git clone https://github.com/theStack/utxo_dump_tools
$ cd utxo_dump_tools/calc_utxo_hash
$ go run calc_utxo_hash.go ~/utxos.sqlite
(outputs MuHash calculated from the SQLite UTXO set)
=> verify that both MuHashes are equal
```
For a demonstration what can be done with the resulting database, see https://github.com/bitcoin/bitcoin/pull/24952#pullrequestreview-956290477 for some example queries. Thanks go to LarryRuane who gave me to the idea of rewriting this script in Python and adding it to `contrib`.
ACKs for top commit:
ajtowns:
ACK 4080b66cbec2b6fc2fcfd7356941236f65d508e3 - light review
achow101:
ACK 4080b66cbec2b6fc2fcfd7356941236f65d508e3
romanz:
tACK 4080b66cbe on signet (using [calc_utxo_hash](8981aa3e85/calc_utxo_hash/calc_utxo_hash.go)):
tdb3:
ACK 4080b66cbec2b6fc2fcfd7356941236f65d508e3
Tree-SHA512: be8aa0369a28c8421a3ccdf1402e106563dd07c082269707311ca584d1c4c8c7b97d48c4fcd344696a36e7ab8cdb64a1d0ef9a192a15cff6d470baf21e46ee7b
a85e8c0e6158fad2408bda5cb1e36da707eb081b doc: Add some general documentation about negated options (Ryan Ofsky)
490c8fa17829c3f8ae4da739f526531c91f3ed87 doc: Add release notes summarizing negated option behavior changes. (Ryan Ofsky)
458ef0a11b57cb5af0e8903b50927723fbb3fcd6 refactor: Avoid using IsArgSet() on -connect list option (Ryan Ofsky)
752ab9c3c65e47fc05545d9b9c919be945851d51 test: Add test to make sure -noconnect disables -dnsseed and -listen by default (Ryan Ofsky)
3c2920ec98fc7d9f77abfd08fea17211b9ca7099 refactor: Avoid using IsArgSet() on -signetseednode and -signetchallenge list options (Ryan Ofsky)
d05668922a28e4e2c78dab2d4737433cd52d6302 refactor: Avoid using IsArgSet() on -debug, -loglevel, and -vbparams list options (Ryan Ofsky)
3d1e8ca53a05e7d4735a2207d1b200e1dcddc534 Normalize inconsistent -noexternalip behavior (Ryan Ofsky)
ecd590d4c1e7f310c6ba3b58373bc30679b491df Normalize inconsistent -noonlynet behavior (Ryan Ofsky)
5544a19f863737518944950fc73f97d9c1399a46 Fix nonsensical bitcoin-cli -norpcwallet behavior (Ryan Ofsky)
6e8e7f433fc3f753a20833aebe54692cdfe5ed75 Fix nonsensical -noasmap behavior (Ryan Ofsky)
b6ab3508064cd3135e1a356c884ae1269cda5250 Fix nonsensical -notest behavior (Ryan Ofsky)
6768389917a8d744f1b1ada4556d3d4fe63c310e Fix nonsensical -norpcwhitelist behavior (Ryan Ofsky)
e03409c70f7472d39e45d189df6c0cf6b676b761 Fix nonsensical -norpcbind and -norpcallowip behavior (Ryan Ofsky)
40c4899bc209921fb4bde02840359c3253663766 Fix nonsensical -nobind and -nowhitebind behavior (Ryan Ofsky)
5453e66fd91c303d04004d861ecad183ff177823 Fix nonsensical -noseednode behavior (Ryan Ofsky)
Pull request description:
The PR changes behavior of negated `-noseednode`, `-nobind`, `-nowhitebind`, `-norpcbind`, `-norpcallowip`, `-norpcwhitelist`, `-notest`, `-noasmap`, `-norpcwallet`, `-noonlynet`, and `-noexternalip` options, so negating these options just clears previously specified values doesn't have other side effects.
Negating options on the command line can be a useful way of resetting options that may have been set earlier in the command line or config file. But before this change, negating these options wouldn't fully reset them, and would have confusing and undocumented side effects (see commit descriptions for details). Now, negating these options just resets them and behaves the same as not specifying them.
Motivation for this PR is to fix confusing behaviors and also to remove incorrect usages of the `IsArgSet()` function. Using `IsArgSet()` tends to lead to negated option bugs in general, but it especially causes bugs when used with list settings returned by `GetArgs()`, because when these settings are negated, `IsArgSet()` will return true but `GetArgs()` will return an empty list. This PR eliminates all uses of `IsArgSet()` and `GetArgs()` together, and followup PR #17783 makes it an error to use `IsArgSet()` on list settings, since calling `IsArgSet()` is never actually necessary. Most of the changes here were originally made in #17783 and then moved here to be easier to review and avoid a dependency on #16545.
ACKs for top commit:
achow101:
ACK a85e8c0e6158fad2408bda5cb1e36da707eb081b
danielabrozzoni:
re-ACK a85e8c0e6158fad2408bda5cb1e36da707eb081b
hodlinator:
re-ACK a85e8c0e6158fad2408bda5cb1e36da707eb081b
Tree-SHA512: dd4b19faac923aeaa647b1c241d929609ce8242b43e3b7bc32523cc48ec92a83ac0dc5aee79f1eba8794535e0314b96cb151fd04ac973671a1ebb9b52dd16697
