fa0e30b93aad29c17c173fab0ac9d339db6306d9 fuzz: Fix test_runner error reporting (MarcoFalke)
Pull request description:
The error reporting is confusing, because right now it prints:
https://cirrus-ci.com/task/4846031060336640?logs=ci#L4931
```
...
Traceback (most recent call last):
File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/fuzz/test_runner.py", line 411, in <module>
main()
File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/fuzz/test_runner.py", line 199, in main
run_once(
File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/fuzz/test_runner.py", line 376, in run_once
assert len(done_stat) == 1
^^^^^^^^^^^^^^^^^^^
AssertionError
```
This is harmless, but confusing.
Fix it by collecting statistics only when the program has not aborted. (Can be reviewed with `--color-moved=dimmed-zebra`)
Also, reword the error message to align it with error messages in other test_runners in this repo.
ACKs for top commit:
dergoegge:
utACK fa0e30b93aad29c17c173fab0ac9d339db6306d9
brunoerg:
code review ACK fa0e30b93aad29c17c173fab0ac9d339db6306d9
marcofleon:
Tested ACK fa0e30b93aad29c17c173fab0ac9d339db6306d9. Prints out the error for the target that crashed. Much clearer than the current error message.
Tree-SHA512: 5e8d3fc0e4837b3264ff0c3cb322fe7fe2ec7af48d35e2a14f82080d03ace793963c3314611b0a170a38e200497d7ba703d9c35c9a7ed3272d93e43f0f0e4c2b
2b9ff4a66d31d06fd97be9729b8f71458b5e982e build: use `-mbig-obj` for mingw-w64 Debug builds (fanquake)
Pull request description:
Windows cross builds using `-O0` (`-DCMAKE_BUILD_TYPE=Debug`) currently fail to compile, as some objects have too many sections. As a convenience, add `-mbig-obj` to our compile flags when using the `Debug` build type, so that if someone tries to build this way, it will work.
This would also be needed if we switched the depends flags to -O0. (maybe in #29796).
`-mbig-obj`
> On PE/COFF target this option forces the use of big object
> file format, which allows more than 32768 sections.
Closes#28109. Seems unlikely that we are going to break up the relevant object files, and the main issue is still the inclusion of Boost.
ACKs for top commit:
theuni:
utACK 2b9ff4a66d31d06fd97be9729b8f71458b5e982e
hebasto:
ACK 2b9ff4a66d31d06fd97be9729b8f71458b5e982e, tested in the following scenarios:
Tree-SHA512: 9ad36de172629a8b7e5371fe3cd75ac2f3c29856040569052cc59e42825eec9121e012dd2178e00b163173c98e78f79dd16b8cee2c93daa2ee0d7e99799325cd
a10bb400e8cb0da0030114ee59f2e7c8494aef42 depends: Fix CXXFLAGS on NetBSD (Hennadii Stepanov)
Pull request description:
This PR corrects an issue where `CXXFLAGS` were mistakenly overridden by `CFLAGS`. This behaviour was introduced in 7e7b3e42fa98b584ae60513a6774037bf677b8ce (from https://github.com/bitcoin/bitcoin/pull/22380).
On the master branch:
```
$ gmake --no-print-directory -C depends print-x86_64_netbsd_CXXFLAGS
x86_64_netbsd_CXXFLAGS=-pipe -std=c11
```
With this PR:
```
$ gmake --no-print-directory -C depends print-x86_64_netbsd_CXXFLAGS
x86_64_netbsd_CXXFLAGS=-pipe -std=c++20
```
ACKs for top commit:
theuni:
utACK a10bb400e8cb0da0030114ee59f2e7c8494aef42
Tree-SHA512: 0c842db2965ebb0a58693394715922810235d9e5f2a7416fe258eb252dbd68ec04f90a0f7948abe938caf94a9194cca7deb53a08335c4404cce3a40c5cb44944
46e207d3296c1423eff7d5b204fad850e15aaec0 cmake: Link `bitcoin_consensus` as a library (Hennadii Stepanov)
Pull request description:
The [`TARGET_OBJECTS`](https://cmake.org/cmake/help/latest/manual/cmake-generator-expressions.7.html#genex:TARGET_OBJECTS) generator expression was introduced in the staging branch when we aimed to build the libbitcoinconsensus shared library. However, `bitcoin_consensus` is a `STATIC` library, not an `OBJECT` library.
This change updates the build system to link `bitcoin_consensus` normally to `test_bitcoin`, resolving [linking issues](https://github.com/bitcoin/bitcoin/issues/31456#issuecomment-2538798107) when building with clang-cl.
ACKs for top commit:
TheCharlatan:
ACK 46e207d3296c1423eff7d5b204fad850e15aaec0
theuni:
utACK 46e207d3296c1423eff7d5b204fad850e15aaec0
Tree-SHA512: b5400be8e8350f80c9fc8b66c4a22032a51578e409eb1817309116fbf0bddeb5fcadd5fda685c98859730ee6cc904adb29d54207387732c8b574a1feb2be906f
3353d4a5e9fc8887b2ebe54d49ec0675147d9d47 depends: Ignore prefix directory on OpenBSD (Hennadii Stepanov)
Pull request description:
On OpenBSD, the prefix directory is named as follows:
```
$ gmake --no-print-directory -C depends print-x86_64_openbsd_prefix
x86_64_openbsd_prefix=/home/hebasto/dev/bitcoin/depends/amd64-unknown-openbsd7.6
```
This name does not match any pattern in `depends/.gitignore`.
This PR resolves this issue.
ACKs for top commit:
tdb3:
ACK 3353d4a5e9fc8887b2ebe54d49ec0675147d9d47
theuni:
utACK 3353d4a5e9fc8887b2ebe54d49ec0675147d9d47
theStack:
Tested ACK 3353d4a5e9fc8887b2ebe54d49ec0675147d9d47 🐟
Tree-SHA512: 82dfff1af974aa43c21e5e5a4483256d5ab4efdf1a15073fb864e635eff52eb8414346cda125f097af59e3342ac031a52683529f4e64df9fc60c8783fcd85e74
To avoid future code changes from reintroducing the ambiguity fixed
by the previous commit, mark m_tip_block private and Assume that
it's not set to uint256::ZERO.
Belt and suspenders for future code changes.
Currently this function is only called from TransactionMerklePath() which sets leaves to the block transactions, so the Assume always holds.
The TARGET_OBJECTS generator expression was introduced in the staging
branch when we aimed to build the libbitcoinconsensus shared library.
However, `bitcoin_consensus` is a STATIC library, not an OBJECT library.
This change updates the build system to link `bitcoin_consensus`
normally to `test_bitcoin`, resolving linking issues when building with
clang-cl.
The variable is conditionally assigned toward the end of the loop and
not used after. It's then set back to its default value at the beginning
of the loop.
1dd3af8fbc350c6f1efa8ae6449e67e1b42ccff4 Add release note for #31223 (Martin Zumsande)
997757dd2b4d7b20b17299fbd21970b2efb8bbc8 test: add functional test for -port behavior (Martin Zumsande)
0e2b12b92a28a2949e75bf50f31563f52e647d6e net, init: derive default onion port if a user specified a -port (Martin Zumsande)
Pull request description:
This resolves#31133 (setups with multiple local nodes each using a different `-port` no longer working with v28.0, see the issue description for more details) by deriving the default onion listening port to be the value specified by `-port` incremented by 1 (idea by vasild / laanwj).
Note that with this fix, the chosen `-port` values of two local nodes cannot be adjacent, otherwise there will be port collisions again.
From the discussion in the linked issue, this was the most popular option, followed by doing nothing and telling affected users to change their setups to use `-bind` instead of `-port`. But more opinions are certainly welcome!
I think that if we decide to do something about the problem described in the issue, we should do so soon (in 28.1.), so I opened this PR.
Fixes#31133
ACKs for top commit:
achow101:
ACK 1dd3af8fbc350c6f1efa8ae6449e67e1b42ccff4
laanwj:
Tested ACK 1dd3af8fbc350c6f1efa8ae6449e67e1b42ccff4
tdb3:
Code review ACK 1dd3af8fbc350c6f1efa8ae6449e67e1b42ccff4
Tree-SHA512: 37fda2b23bbedcab5df3a401cf5afce66ae5318fb78f9660f83e3fd075b528e8156d7a0903f9a12ffe97ab5d83860587116b74af28670a1f4c2f0d1be4999f40
- No empty line separating errors and arrows ("^^^"). Keeping them together signals they are related.
- No empty line separating error message and linter failure line (not completely empty, it contains several spaces left over from Rust multi-line literal).
- Keep the linter description on the same line as the failure line, otherwise it looks like it's a description for the following step.
Windows cross builds using `-O0` currently fail to compile, as some
objects have too many sections. As a convenience, add `-mbig-obj` to
our compile flags when using the `Debug` build type, so that if someone
tries to build this way, it will work.
This would also be needed if we switched the depends flags to -O0.
`-mbig-obj`
> On PE/COFF target this option forces the use of big object
> file format, which allows more than 32768 sections.
Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
37946c0aafeebc1585f1316fb05f252f7fb51e91 Set notifications m_tip_block in LoadChainTip() (Sjors Provoost)
Pull request description:
Ensure KernelNotifications `m_tip_block` is set even if no new block arrives.
Suggested in https://github.com/bitcoin/bitcoin/pull/31297#issuecomment-2486457573
ACKs for top commit:
ryanofsky:
Code review ACK 37946c0aafeebc1585f1316fb05f252f7fb51e91, fixing comment bug caught by @mzumsande in https://github.com/bitcoin/bitcoin/pull/31346#discussion_r1870315593 in another really helpful clarification
mzumsande:
Code Review ACK 37946c0aafeebc1585f1316fb05f252f7fb51e91
TheCharlatan:
ACK 37946c0aafeebc1585f1316fb05f252f7fb51e91
Tree-SHA512: 931bf820440a0cdda276f6dbd63f03fdbcdc90b18e7d5e160a74bdd9d0290acc706c35aab15bbdcd6e5e0b77565b3d07ff49b0dcf6551cb83961bae67be5d1bb
fa47baa03bcfcf44fb2ed05f009a32d32f860c45 ci: Bump centos gcc (MarcoFalke)
Pull request description:
Currently the centos stream9 CI task is using gcc-11. This is fine, because this is also the minimum supported.
However:
* There is already a CI task that is checking the minimum supported version: 62bd61de11/ci/test/00_setup_env_native_previous_releases.sh (L11-L12)
* The CI log is a bit useless, because it is mostly just `#warning _FORTIFY_SOURCE > 2 is treated like 2 on this platform [-Werror=cpp]`. This makes it harder to spot real warnings, such as https://github.com/bitcoin/bitcoin/issues/31476
Fix both issues by using gcc-12.
ACKs for top commit:
hebasto:
ACK fa47baa03bcfcf44fb2ed05f009a32d32f860c45.
Tree-SHA512: 573618efc949437d33365a24f77a26a9b68457f7fb9bd603ee92bc5f17fec73ccba114cafb900eddee3531af47508ce5c246def93268787cdfa2b99e6f45a13d
5cd9e95eea12fddd2a1a668d509f0a5b6e267516 depends: update capnproto to 1.0.2 (fanquake)
Pull request description:
This fixes compilation on FreeBSD:
```bash
-- Build files have been written to: /tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4
Building native_capnp...
gmake[1]: Entering directory '/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4'
[ 1%] Building CXX object src/kj/CMakeFiles/kj.dir/array.c++.o
[ 2%] Building CXX object src/kj/CMakeFiles/kj.dir/cidr.c++.o
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:112:71: error: member access into incomplete type 'const struct sockaddr_in6'
otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:112:51: note: forward declaration of 'sockaddr_in6'
otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:123:63: error: member access into incomplete type 'const struct sockaddr_in'
&reinterpret_cast<const struct sockaddr_in*>(addr)->sin_addr.s_addr);
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:123:44: note: forward declaration of 'sockaddr_in'
&reinterpret_cast<const struct sockaddr_in*>(addr)->sin_addr.s_addr);
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:133:69: error: member access into incomplete type 'const struct sockaddr_in6'
otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
^
/tmp/cirrus-ci-build/bitcoin-core/depends/work/build/x86_64-unknown-freebsd14.0/native_capnp/1.0.1-867405dd2c4/src/kj/cidr.c++:133:49: note: forward declaration of 'sockaddr_in6'
otherBits = reinterpret_cast<const struct sockaddr_in6*>(addr)->sin6_addr.s6_addr;
^
3 errors generated.
```
See: 1c19c362b4.
ACKs for top commit:
kevkevinpal:
Concept ACK [5cd9e95](5cd9e95eea)
theuni:
utACK 5cd9e95eea12fddd2a1a668d509f0a5b6e267516
ryanofsky:
Code review ACK 5cd9e95eea12fddd2a1a668d509f0a5b6e267516. Downloaded the file and checked the hash. Also followed theuni's lead and looked at the source changes which were very minor. It did look like thousands of lines changed in the autotools build, but this should not affect us as we are using the cmake build.
Tree-SHA512: 5d78887a9e950c8532c427b17969128de0c6d466ec5ffba85241457e8e19673c22ddb3493cdfce5086f57ba760eac5e91f703992b2f70f2a7c82ba885255279c
62b2d23edbad76c1d68bb8c0fc8af54e7c0d7dc8 wallet: Migrate non-HD keys to combo() descriptor (Ava Chow)
Pull request description:
Non-HD keys do not have an HD seed ID associated with them, so if this value is the null value (all 0s), then we should not perform any seed ID comparison that would result in excluding the keys from combo() migration.
This changes the migration of non-HD wallets (or blank wallets with imported private keys) to make a single combo() descriptors for the non-HD/imported keys, rather than pk(), pkh(), sh(wpkh()), and wpkh() descriptors for the keys.
Implements https://github.com/bitcoin/bitcoin/pull/31374#discussion_r1876650074
ACKs for top commit:
laanwj:
Concept and code review ACK 62b2d23edbad76c1d68bb8c0fc8af54e7c0d7dc8
brunoerg:
code review ACK 62b2d23edbad76c1d68bb8c0fc8af54e7c0d7dc8
furszy:
Nice catch. ACK 62b2d23edbad76c1d68bb8c0fc8af54e7c0d7dc8
theStack:
ACK 62b2d23edbad76c1d68bb8c0fc8af54e7c0d7dc8
rkrux:
tACK 62b2d23edbad76c1d68bb8c0fc8af54e7c0d7dc8
Tree-SHA512: 86a80b7dcc1598ab18068a2572ff4b4920b233178b760f7b76c5b21a9e6608005ac872f90e082a8f99b51daab0b049e73e4bee5b8e0b537d56ed0d34122a1f49
On failure, this makes the output more consistent with the other linter.
Each failure will be marked with an '⚠️ ' emoji and explanation, making
it easier to spot.
Also, add --line-number to the filesystem linter.
Also, add newlines after each failing check, to visually separate
different failures from each other.
Can be reviewed with:
"--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space"
df27ee9f024f69a8bdac8b0ee3bd7882f6a54294 refactor: Fix "modernize-use-starts-ends-with" clang-tidy warning (Hennadii Stepanov)
Pull request description:
This PR is a follow-up to #31306 and fixes the "modernize-use-starts-ends-with" warning in the multiprocess code (see https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2527008761).
Fixes https://github.com/chaincodelabs/libmultiprocess/issues/124.
ACKs for top commit:
l0rinc:
ACK df27ee9f024f69a8bdac8b0ee3bd7882f6a54294
theuni:
utACK df27ee9f024f69a8bdac8b0ee3bd7882f6a54294
ryanofsky:
Code review ACK df27ee9f024f69a8bdac8b0ee3bd7882f6a54294
Tree-SHA512: b5685818e9a3f161949b79586138e4341c5fbcc77296d5f4b13ff0183b6efaac1baea8a6d9304566f25517018d4f67b6d407105a36971a03f4077697d53f17ff
e2d3372e558d227aa6dd604b67724685d62e547e lint: Disable signature output in git log (Hodlinator)
Pull request description:
Necessary for users that have signature output enabled by default, since the script would stumble on them and error out.
---
### Testing setup
Set local repo config to show signatures in log by default, simulating a user having that setting turned on globally.
```
₿ git config set log.showSignature true
```
### Command under test
```
₿ ( cd ./test/lint/test_runner/ && COMMIT_RANGE='HEAD^..HEAD' cargo run )
```
#### Before
```
...
fatal: invalid object name 'gpg'.
Traceback (most recent call last):
File "/home/hodlinator/bitcoin/test/lint/lint-git-commit-check.py", line 52, in <module>
main()
File "/home/hodlinator/bitcoin/test/lint/lint-git-commit-check.py", line 42, in main
commit_info = check_output(["git", "log", "--format=%B", "-n", "1", hash], text=True, encoding="utf8").splitlines()
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/nix/store/wfbjq35kxs6x83c3ncpfxdyl5gbhdx4h-python3-3.12.6/lib/python3.12/subprocess.py", line 466, in check_output
return run(*popenargs, stdout=PIPE, timeout=timeout, check=True,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/nix/store/wfbjq35kxs6x83c3ncpfxdyl5gbhdx4h-python3-3.12.6/lib/python3.12/subprocess.py", line 571, in run
raise CalledProcessError(retcode, process.args,
subprocess.CalledProcessError: Command '['git', 'log', '--format=%B', '-n', '1', 'gpg: Signature made ons 11 dec 2024 10:46:34 CET']' returned non-zero exit status 128.
^---- ⚠️ Failure generated from lint-git-commit-check.py
...
```
#### After
(No failure generated by *lint-git-commit-check.py*).
ACKs for top commit:
maflcko:
lgtm ACK e2d3372e558d227aa6dd604b67724685d62e547e
willcl-ark:
ACK e2d3372e558d227aa6dd604b67724685d62e547e
Tree-SHA512: 584ccece1e6e0f4691683a2b1816eff33b88f48e9ead9272e2dc73ea9c637b182632108fbeddea1ffc8ed6ba5a5838d7eac7a9f33dfda5bdf325dd7a41e43365
b7ec69c25cf316c21bafa075dd895bf90dc4e59b depends: add -g to *BSD_debug flags (fanquake)
Pull request description:
To match the other HOST_debug_flags. Pulled out of #29796.
ACKs for top commit:
theuni:
utACK b7ec69c25cf316c21bafa075dd895bf90dc4e59b
Tree-SHA512: 654a6dc2c1e295021380f18565379ccde5c5bebcbb5e48ab0364aa79c6f15d301b4acf058d75629a4b217483c6788a0ecb60560e8701882e09490b92c4c346d0
During migration failure, only load wallet back into memory when the
wallet was loaded prior to migration. This fixes the case where BDB
is not supported, which implies that no legacy wallet can be loaded
into memory due to the lack of db writing functionality.
This commit also improves migration backup related comments to better
document the current workflow.
Co-authored-by: Ava Chow <github@achow101.com>
f6496a838828aa6d5a2a40380c2338fda2e0d812 guix: disable gcov in base-linux-gcc (fanquake)
Pull request description:
In a `x86_64-linux-gnu` build, this drops:
```bash
x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov
x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov-dump
x86_64-linux-gnu/bin/x86_64-linux-gnu-gcov-tool
x86_64-linux-gnu/lib/gcc/x86_64-linux-gnu/12.4.0: libgcov.a
```
For mingw-w64-gcc, `--disable-gcov` is currently passed for this target in Guix, due to issues with mingw-w64, see
8bed031e58/gnu/packages/gcc.scm (L99-L102). However we'll add it in any case, in case it's re-enabled in future, when the underlying issues are fixed.
ACKs for top commit:
TheCharlatan:
ACK f6496a838828
Tree-SHA512: ad6de53f63e7bb658cac05fb023fb1f8e76103073c7dffb4267412d3046148e1389df8848010128c1bd3d428f05e1587b656ef2cad8c7d9078ebec83a68bad49
c93bf0e6e2cd0e9dc979ed996e41cc1034bc1ad1 test: Add missing %c character test (Hodlinator)
76cca4aa6fcd363dee821c837b1b627ea137b7a4 test: Document non-parity between tinyformat and ConstevalFormatstring (Hodlinator)
533013cba20664e3581a8e4633cc188d5be3175a test: Prove+document ConstevalFormatString/tinyformat parity (Hodlinator)
b81a4659950a6c4e22316f66b55cae8afc4f4d9a refactor test: Profit from using namespace + using detail function (Hodlinator)
Pull request description:
Clarifies and puts the extent of parity under test.
Broken out from #30546 based on https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1755013263 and https://github.com/bitcoin/bitcoin/pull/30546#discussion_r1756495304.
ACKs for top commit:
maflcko:
re-ACK c93bf0e6e2cd0e9dc979ed996e41cc1034bc1ad1 🗜
l0rinc:
ACK c93bf0e6e2cd0e9dc979ed996e41cc1034bc1ad1
ryanofsky:
Code review ACK c93bf0e6e2cd0e9dc979ed996e41cc1034bc1ad1. Just a few cleanups tweaking function declarations and commit comments and consolidating some test cases since last review.
Tree-SHA512: 5ecc893b26cf2761c0009861be392ec4c4fceb0ef95052a2f6f9df76b2e459cfb3f9e257f61be07c3bb2ecc6e525e72c5ca853be1f63b70b52785323d3db6b42
988721d37a3c65e4ef86945133207fda243f2fba test: avoid internet traffic in rpc_net.py (Sebastian Falbesoner)
Pull request description:
In order to avoid connecting to the internet in the functional test `rpc_net.py`, specify a non-working proxy (parameter `-proxy=127.0.0.1:1`, same approach as in #31142) for the nodes. There is at least one known instance where this is currently happening on master where a connection attempt to a public IP is made (see also the discussion in #31339):
17834bd197/test/functional/rpc_net.py (L253)
Can be tested by running
```
$ sudo tcpdump -i eth0 host 11.22.33.44
```
both on master and the PR branch and verifying that no packets appear in the tcpdump in the latter anymore.
ACKs for top commit:
achow101:
ACK 988721d37a3c65e4ef86945133207fda243f2fba
tdb3:
ACK 988721d37a3c65e4ef86945133207fda243f2fba
vasild:
ACK 988721d37a3c65e4ef86945133207fda243f2fba
Tree-SHA512: 0f51fedccbfac0f80a7e6f9c5ba9193d0c20b5a788553c7cd7e583225df7b1151b86cd848d6ccf61f7b2de848f0ac98d73d7b5db100aa54fe8cbeeb4c0549106
ec777917d6eba0b417dbc90b9b891240a44b7ec4 test: Fix intermittent issue in wallet_backwards_compatibility.py (Randall Naar)
Pull request description:
When creating and replacing a transaction using `bumpfee`, an async update is sent in the form of the `TransactionAddedToMempool` and `TransactionRemovedFromMempool` signals. When `wallet_backwards_compatibility.py` creates `tx3_id` this way and replaces it with `tx4_id`, the `abandontransaction` rpc is called right after. In some cases the `TransactionAddedToMempool` and `TransactionRemovedFromMempool` is handled after the transaction is abandoned in the wallet, and overwrites the transaction's `abandoned` flag. This PR forces the signals to get handled before `abandontransaction` is called by invoking `self.sync_mempools` which calls `syncwithvalidationinterfacequeue` on every node's rpc connection.
This will mitigate the immediate inconsistency observed with the abandontransaction call, but the potential race conditions between the signals and wallet operations may also be useful to note in a separate issue (if it's okay to not address it in this one).
Fixes#29806
ACKs for top commit:
achow101:
ACK ec777917d6eba0b417dbc90b9b891240a44b7ec4
tdb3:
ACK ec777917d6eba0b417dbc90b9b891240a44b7ec4
Tree-SHA512: e75bc2c1f7fefc4f4910bb353654848fed5661c1436416798a5f4e0c5a76bde15617a5af04c2384464005953326317b8f273039e47508d5124677908cf36d31e
bb7e686341e437b2e7aae887827710918c00ae0f fuzz: add cstdlib to FuzzedDataProvider (fanquake)
Pull request description:
Same as https://github.com/llvm/llvm-project/pull/113951.
Avoids compile failures under clang-20 & `D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES`:
```bash
In file included from /bitcoin/src/test/fuzz/addition_overflow.cpp:5:
/bitcoin/src/test/fuzz/FuzzedDataProvider.h:209:5: error: use of undeclared identifier 'abort'
209 | abort();
| ^
/bitcoin/src/test/fuzz/FuzzedDataProvider.h:250:5: error: use of undeclared identifier 'abort'
250 | abort();
```
ACKs for top commit:
dergoegge:
utACK bb7e686341e437b2e7aae887827710918c00ae0f
brunoerg:
ACK bb7e686341e437b2e7aae887827710918c00ae0f
Tree-SHA512: 22efd5505273ec7254e8dccbb275e648fe02107397c45eff6752e4a6ea787d9d2e45eb0f2ee309df431e9b92ffd14cbcba4b0f4b11a127664466e20be43c383e
Non-HD keys in legacy wallets without a HD seed ID were being migrated
to separate pk(), pkh(), sh(wpkh()), and wpkh() descriptors for each key.
These could be more compactly represented as combo() descriptors, so
migration should make combo() for them.
It is possible that existing non-HD wallets that were migrated, or
wallets that started blank and had private keys imported into them have
run into this issue. However, as the 4 descriptors produce the same output
scripts as the single combo(), so any previously migrated wallets are
not missing any output scripts. The only observable difference should be
performance related, and the wallet size on disk.
