08c3c37d12 bitcoin-cli: note -rpcclienttimeout is not implemented for IPC connections (Ryan Ofsky)
Pull request description:
The `-rpcclienttimeout` option applies only to HTTP connections. When `bitcoin-cli` connects via IPC, the timeout is silently ignored. Clarify this in the help string.
Noted by pinheadmz in https://github.com/bitcoin/bitcoin/pull/32297#pullrequestreview-3922849038
ACKs for top commit:
sedited:
ACK 08c3c37d12
Tree-SHA512: 82c99bbcb6a776636197407e4371047d6f1386cbe78823358fb5cceb080a976a4f0ac2c195b7ca734acc81d7e5f32e77887aa11a61860596e3bb8dfd2f523531
a154c05d49 cmake: Check dependencies after build option interaction (Hennadii Stepanov)
Pull request description:
At present, `CMakeLists.txt` interleaves configuration-option handling with dependency discovery. As a result, unnecessary checks may be performed. For example:
```
$ cmake -B build --preset dev-mode -DBUILD_FOR_FUZZING=ON
<snip>
-- Found PkgConfig: /usr/bin/pkg-config (found version "2.3.0")
-- Found ZeroMQ: /usr/lib64 (found suitable version "4.3.5", minimum required is "4.0.0")
-- Performing Test HAVE_USDT_H
-- Performing Test HAVE_USDT_H - Success
-- Found USDT: /usr/include
-- Found QRencode: /usr/lib64/libqrencode.so (found version "4.1.1")
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Performing Test HAVE_STDATOMIC
-- Performing Test HAVE_STDATOMIC - Success
-- Found WrapAtomic: TRUE
-- Found OpenGL: /usr/lib64/libOpenGL.so
-- Found WrapOpenGL: TRUE
-- Found WrapVulkanHeaders: /usr/include
-- Found Qt: /usr/lib64/cmake/Qt6 (found suitable version "6.10.1", minimum required is "6.2")
CMake Warning at CMakeLists.txt:206 (message):
BUILD_FOR_FUZZING=ON will disable all other targets and force
BUILD_FUZZ_BINARY=ON.
<snip>
```
This PR restructures the code to ensure that all dependencies are checked only after the final build option interaction.
ACKs for top commit:
fanquake:
ACK a154c05d49
sedited:
Re-ACK a154c05d49
Tree-SHA512: f863316770a1e27e6c08736932f73f297add326943e9c32c721f7e771f38b0fdc651abfed61add5ee4924cfbf2afff91fde005d41d1fd239d401184b6caf5453
fa24693819 test: Allow --usecli in tests that already support it (MarcoFalke)
fa8d4d5c35 test: Catch CalledProcessError to support --usecli in feature_dbcrash.py (MarcoFalke)
faf0f848ef test: use echojson to allow rpc_named_arguments.py --usecli (MarcoFalke)
faf993ee44 test: Stop node before modifying config to support rpc_users.py --usecli (MarcoFalke)
fa4fc8c1d7 test: Set TestNode url field early, so that feature_loadblock.py --usecli works (MarcoFalke)
Pull request description:
Some tests disallow to be run under `--usecli`. This reduces the coverage and risks that bugs in the bitcoin-cli go unnoticed.
The commits should be self-explanatory and can be reviewed and tested one-by-one.
ACKs for top commit:
willcl-ark:
ACK fa24693819
Tree-SHA512: e34077be98f88ad1e8649600a5f43fc8c77e4ebb03bbccd88c33f2d67882ccdd52b5d18bcfbfc611dff3ebf7455f8e624a88d062aa1863c5eb813bbf4f48e58b
7be0d6fa18 test: remove the lazy import of util in authproxy (rkrux)
779f444680 test: move out JSONRPCException from authproxy to util (rkrux)
Pull request description:
I noticed this issue while reviewing #34773 where a lazy import
is added in the __call__ method of the AuthServiceProxy class in
authproxy.py
There's a circular dependency between authproxy.py and util.py
due to which the former can't use the common utility functions
and thus lazy imports are used as a workaround.
This patch set breaks the dependency so that authproxy.py can use
the utility functions from util.py in a standard fashion. Few tests that
explicitly use get_rpc_proxy and JSONRPCException needed to have
their imports updated.
ACKs for top commit:
maflcko:
review ACK 7be0d6fa18 🏽
Tree-SHA512: 56775cb13d989342ba9482edb255170d695ce5c2d5efbbd64586e0d5463af16467dbf9efe8a0411bde4dfb9bb531839284b2d6f5d828080171d847b70570977d
5faf2ad880 doc: add release notes for deprecation of wallet rbf & bip125 fields (rkrux)
aba24a9b62 wallet: remove "RPC Only" from -walletrbf option help description (rkrux)
97f7cc0233 wallet: mark -walletrbf startup option as deprecated (rkrux)
c4a7613e6a wallet: mark `bip125-replaceable` key as deprecated in transaction RPCs (rkrux)
Pull request description:
Partially fixes#32661.
This patch set is in line with the deprecation of outdated
BIP 125 opt-in RBF signalling and fullrbf in wallet transaction
RPCs and startup options.
ACKs for top commit:
achow101:
ACK 5faf2ad880
w0xlt:
ACK 5faf2ad880
polespinasa:
code reviewed ACK 5faf2ad880
Tree-SHA512: fe6e57f49bef7245b2f564ba705647fb49f0bd370da2e9cfdce45c64a2d8b33ea10a8a802c6619c6382a9bbd2b0e2e4792b08077bc4cfa9b03f7916e2185652a
f05b1a3532 rpc: Fix for duplicate external signers case (optout)
Pull request description:
A straightforward fix for a minor bug in a low-probability case; when there are multiple signers with the same fingerprint, the signers following the duplicate are also thrown away (due to a `break` instead of `continue`).
Background. Bitcoin core can work with _external signers_. They can be configured using the `-signer=<cmd>` argument. The `enumeratesigners` RPC can be used to retrieve the available signers.
Precondition A minor bug was found in a special case:
- multiple external signers are detected, and
- at least two of them are duplicate, i.e., have identical fingerprint, and
- the duplicates are followed by at least one additional signer (that is, the last one is not a duplicate),
Current behavior: Only one of the duplicates is kept, however all subsequent signers are also ignored. This last part is probably unintended, thus it's a minor bug.
To put it in another way, the set of fingerprints returned depends on the actual order in which the devices are returned by the external tool, which may be arbitrary.
Expected behavior: De-duplicate the found signers by fingerprint:
- keep one signer for each fingerprint
- keep all non-duplicates.
Fix: The early `break` of the loop of signers upon duplication has been changed to `continue`.
Test added/extended: `RPCSignerTest` in `test/functional/rpc_signer.py` has been extended with a case of multiple duplicates.
ACKs for top commit:
l0rinc:
ACK f05b1a3532
achow101:
ACK f05b1a3532
polespinasa:
code review ACK f05b1a3532
naiyoma:
ACK f05b1a3532
Tree-SHA512: ac685e233192ecaf5bd535dc66badeb84d78212b3242df71f11c682828955a40167438e16fccc667cd8beef574bc4137a586f44b0f119d7574422c70bb078293
So that util is not dependent on authproxy at all and going
forward authproxy can use util methods.
Reviewing with --color-moved=dimmed-zebra option can be helpful.
dfe5d6a81d fuzz: apply node context reset pattern to p2p_handshake (frankomosh)
Pull request description:
Follow-up to #34302. Applies the node context reset pattern from fabf8d1 to `p2p_handshake`.
Previous code pattern created local `AddrMan` and `node::Warnings` objects, and passed them to `PeerManager::make`. `connman` was left holding a dangling `reference_wrapper<AddrMan>` across iterations, since the local objects destruct at iteration end while `connman` is global.
Like in fabf8d1 , reset and reinstall `node.addrman` and `node.peerman` on each iteration. This PR also removes `includes` made unused by this or prior refactors.
ACKs for top commit:
nervana21:
tACK dfe5d6a81d
maflcko:
review ACK dfe5d6a81d🦏
sedited:
ACK dfe5d6a81d
Tree-SHA512: 141ddec03c6d37f76a3b2d94701d18c851e85ea74e57716abb69ecc955d30371e342c6e267d2669ad853fe2d95fb77dd2fb506e4233ae3a88501d59ee1bbae30
faf02674b3 refactor: Set TestNode.cli only after RPC is connected (MarcoFalke)
fae376cafb refactor: Inline get_rpc_proxy (MarcoFalke)
fa00c7c7a4 test: Allow rpc_bind.py --usecli (MarcoFalke)
fa8f25118c refactor: Use create_new_rpc_connection in wallet_multiwallet.py (MarcoFalke)
fa3ae6c7d3 test: Allow feature_shutdown.py --usecli (MarcoFalke)
fa2a3683d5 test: Allow mining_getblocktemplate_longpoll.py --usecli (MarcoFalke)
fa37c6a529 test: [move-only] Extract create_new_rpc_connection (MarcoFalke)
Pull request description:
Some tests disallow to be run under `--usecli`. This reduces the coverage and risks that bugs in the bitcoin-cli go unnoticed.
The commits should be self-explanatory and can be reviewed and tested one-by-one.
ACKs for top commit:
willcl-ark:
reACK faf02674b3
Tree-SHA512: 83cd6a696e6dd6efd4f2d295e65bfac51fe26404c37f25936808005cc6136e469a30aebaac547af1c722ed5ac827eaf009a150d82420b6b4e242e89305475abe
7777a92a92 ci: Use path with spaces on windows as well (MarcoFalke)
fac6c4270d ci: Put space and non-ASCII char in scratch dir (MarcoFalke)
fa38759823 ci: Require $FILE_ENV (MarcoFalke)
Pull request description:
It seems unlikely that many users have a space in their paths, but it seems a use-case worth enough to be tested by CI, so that it does not have to be done manually. Ref https://github.com/bitcoin/bitcoin/pull/33929#discussion_r2590523065 / https://mirror.b10c.me/bitcoin-bitcoin/33929/#discussion_r2590523065
So do that here, and also add a non-ASCII char while touching.
Also, fix all tests that are broken and assume no space exists in paths.
ACKs for top commit:
hebasto:
ACK 7777a92a92.
sedited:
ACK 7777a92a92
Tree-SHA512: eceb1f6c932c6966cdca8ca8df750081ec5134db5e5f558f7d955716409117bec7c8585d75865e2c98bc1ae7394f3ce64dff87bcebe1e68591afaeef1831d6dd
c03107acf5 ci: switch to GitHub cache for all runners (willcl-ark)
Pull request description:
Cirrus is winding down, and github now offers more than 10GB cache.
Switch to GH cache for all runner-types. Simplify configure-docker action.
ACKs for top commit:
maflcko:
review ACK c03107acf5🚴
Tree-SHA512: b6111c7559a86eed8488a4b0775df812a303a99eed5c80297593936e61d1d5e2ce72fe2fa615625816672414e6947ac4d93b9fd2925522fba06417ea4711ce79
fa99a3ccac ci: Enable pipefail in 03_test_script.sh (MarcoFalke)
Pull request description:
The CI script is problematic, because it is written in Bash, without pipefail enabled. Thus, some failures are silently ignored.
Enabling pipefail is a bit tedious, because:
* The IWYU task has no (`--verbose`) ccache output, so the pipe fails after `grep` [1]. Also, right now on master, the if silently skips: `ci/test/03_test_script.sh: line 122: [: : integer expression expected`.
* The Alpine task has `Hits:` twice in the output, so the pipe fails after `head -1` [2]
Not sure what the easiest way to fix this would be. Some options are:
* Just use `tail -1` and `0` as fallback: `hit_rate=$(ccache --show-stats | grep "Hits:" | tail -1 | sed 's/.*(\(.*\)%).*/\1/' || echo "0")`
* Properly parse, using Python and `--print-stats` (this pull)
[1]
```
+ ccache --version
+ head -n 1
ccache version 4.11.2
+ ccache --show-stats --verbose
Cache directory: /home/admin/actions-runner/_work/_temp/ccache_dir
Config file: /home/admin/actions-runner/_work/_temp/ccache_dir/ccache.conf
System config file: /etc/ccache.conf
Stats updated: Tue Feb 17 08:40:20 2026
Local storage:
Cache size (GB): 0.0 / 2.0 ( 0.00%)
Files: 0
Hits: 0
Misses: 0
Reads: 0
Writes: 0
++ ccache --show-stats
++ grep Hits:
++ head -1
++ sed 's/.*(\(.*\)%).*/\1/'
+ hit_rate=
Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', 'f5e8f319c22101ada5be9d4c5fd7d883ce37b830e86ec64627cb7d2b96749053', '/home/admin/actions-runner/_work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 1.
Error: Process completed with exit code 1.
```
[2]
```
+ ccache --version
+ head -n 1
ccache version 4.12.1
+ ccache --show-stats --verbose
Cache directory: /home/admin/actions-runner/_work/_temp/ccache_dir
Config file: /home/admin/actions-runner/_work/_temp/ccache_dir/ccache.conf
System config file: /etc/ccache.conf
Stats updated: Tue Feb 17 08:40:35 2026
Cacheable calls: 873 / 873 (100.0%)
Hits: 846 / 873 (96.91%)
Direct: 822 / 846 (97.16%)
Preprocessed: 24 / 846 ( 2.84%)
Misses: 27 / 873 ( 3.09%)
Successful lookups:
Direct: 822 / 873 (94.16%)
Preprocessed: 24 / 51 (47.06%)
Local storage:
Cache size (GB): 2.0 / 2.0 (99.95%)
Files: 2580
Cleanups: 13
Hits: 846 / 873 (96.91%)
Misses: 27 / 873 ( 3.09%)
Reads: 1772
Writes: 52
++ ccache --show-stats
++ grep Hits:
++ head -1
++ sed 's/.*(\(.*\)%).*/\1/'
+ hit_rate=96.91
Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', '272a66a48206f1f6096612e196127ce46ea4dbff5dc14be3a4a20c4ee523956f', '/home/admin/actions-runner/_work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 141.
Error: Process completed with exit code 1.
```
ACKs for top commit:
willcl-ark:
ACK fa99a3ccac
Tree-SHA512: e5b0ddad8f279bd48102543b0496fa2ecdfc6938d208078595a60b96680467a80504b21acdecd86204b82ce2770eede23e498f04c6a9cee59634f83d44cfe094
fa89098888 test: Document rare failure in test_inv_block better (MarcoFalke)
Pull request description:
This test may fail intermittently, see the tracking issue https://github.com/bitcoin/bitcoin/issues/19732 and https://github.com/bitcoin-core/gui/actions/runs/25819423445/job/75856716144?pr=936#step:10:5303 specifically:
```
2026-05-13T19:37:19.624321Z TestFramework (INFO): Tx should be received at node 1 after 64 seconds
2026-05-13T20:17:19.627627Z TestFramework (ERROR): Unexpected exception:
Traceback (most recent call last):
File "/home/runner/work/_temp/test/functional/test_framework/test_framework.py", line 143, in main
self.run_test()
File "/home/runner/work/_temp/build/test/functional/p2p_tx_download.py", line 413, in run_test
test()
File "/home/runner/work/_temp/build/test/functional/p2p_tx_download.py", line 130, in test_inv_block
self.sync_mempools()
File "/home/runner/work/_temp/test/functional/test_framework/test_framework.py", line 721, in sync_mempools
raise AssertionError("Mempool sync timed out after {}s:{}".format(
AssertionError: Mempool sync timed out after 2400s:
{'2db3a22f9370eb32f110597882acebef1c76cf319837bac69b829917730d2349'}
set()
```
The test assumes that node1 gets the tx from its honest outbound peer (node0). However, the same connection is inbound from the view of node0, so node0 will use an exponentially distributed delay, which may be more than the expected timeout.
Document this clearer, so that the this specific failure can simply be re-run without having to spend a long time thinking and debugging it every time it happens.
ACKs for top commit:
willcl-ark:
ACK fa89098888
Tree-SHA512: b7fa1a584bc61595d7e1b73ed1fbe446604515c24786565e76b385c7be3278cdb8909d8aeabe6d3f05685281851836f2267b97d7f022d634d833d2124ba53c00
d61053d97b build: Drop libevent from bitcoin-cli link libraries (Fabian Jahr)
798d051c80 cli: Remove libevent usage (Fabian Jahr)
376e7ef07c util: Expose IOErrorIsPermanent in sock header (Fabian Jahr)
5d562430de netbase: Add timeout parameter to ConnectDirectly (Fabian Jahr)
a988ac592f cli: Add HTTPResponseHeaders class for parsing response headers (Fabian Jahr)
c471c5085b common: Add unused UrlEncode function (Fabian Jahr)
9687ef1bd9 ci: Tolerate unused free functions in intermediate commits (Fabian Jahr)
Pull request description:
Part of the effort to remove the libevent dependency altogether, see #31194
This takes the parsing logic from the [`HTTPHeaders` class](d549f01caa) from #32061 and puts it into `bitcoin-cli` as a small `HTTPResponseHeaders` class with a comment to revisit potentially sharing this code somehow. This decoupled the two pulls which seems like the most sensible way to deal with this since the actual overlap is very small compared to the impact of each of the pulls which should ideally not block each other.
Otherwise the change itself replaces the libevent-based HTTP client with a simple synchronous implementation which uses the `Sock` class directly.
ACKs for top commit:
hodlinator:
re-ACK d61053d97b
theStack:
re-ACK d61053d97b
w0xlt:
ACK d61053d97b
Tree-SHA512: a3580a45faf540ee844aac8cb1dc056a89e8e11b45781d2807baa4736d5c0934284c6066206101b6984111a48a186d67845545d07639b623cb35ccc2d85d3ab2
Cirrus is winding down and github now offers more than 10GB cache.
Switch to GH cache for all runner-types. Simplify docker build arg
construction, and reduce the number of needed action permissions.
18c1cc65e9 kernel: improve BITCOINKERNEL_WARN_UNUSED_RESULT usage (stickies-v)
Pull request description:
Similar to `[[nodiscard]]`, `BITCOINKERNEL_WARN_UNUSED_RESULT` is used to indicate that ignoring a function's return value is almost certainly a bug.
It is used in cases such as a resource leak (e.g. an owning handle returned by a `*_create` or `*_copy` function), or when the returned value is itself an error/status code. It is not used merely because discarding the result is wasteful, e.g. on getters or predicates.
Fix the incorrect usage, and properly document the attribute. Having this clearly documented helps avoid bikeshedding on future PRs.
ACKs for top commit:
w0xlt:
ACK 18c1cc65e9
stringintech:
ACK 18c1cc65
sedited:
ACK 18c1cc65e9
Tree-SHA512: ba5aa19f0b90ef12ed3c200449734d9aa67af166f2a2b29e321d3ec1e9b49483215c5078ae1dd945ea65650a2017f3101bf75abcc9d3388eb1a3d4c8070c22dd
The cli can only be called after the RPC is connected, so the cli field
should only be set after that. This is similar to the _rpc field.
This change has also other benefits:
* Manually overwriting the cli with a new rpchost is no longer needed,
so one cli-specific line can be removed from the rpc_bind.py test.
* The datadir and rpc_timeout fields are removed from the TestNodeCLI
struct. They were redundant with the -datadir and -rpcclienttimeout
command line options. Any command line option can be overwritten by
appending it with a new value, if needed.
This allows to run the test under bitcoin-cli, except for one small
part, which is skipped for now.
This also inlines rpc_url directly into create_new_rpc_connection,
because this is the only place where it is needed.
02b2c41103 logging: use util/log.h where possible (Anthony Towns)
57d7495fe5 IWYU fixes (Anthony Towns)
611878b46f scripted-diff: logging: Drop LogAcceptCategory (Anthony Towns)
34332dba2f util/log, logging: Provide ShouldDebugLog and ShouldTraceLog instead of a generic ShouldLog (Anthony Towns)
abea304dd6 logging: Move GetLogCategory into Logger class (Anthony Towns)
58113e5833 util/log: Rename LogPrintLevel_ into detail_ namespace (Anthony Towns)
f69d1ae56d util/log: Provide util::log::NO_RATE_LIMIT to avoid rate limits (Anthony Towns)
72e92d67df logging: Protect ShrinkDebugFile by m_cs (Anthony Towns)
904c0d07bb util/stdmutex: Drop StdLockGuard (Anthony Towns)
Pull request description:
`ShrinkDebugFile` now takes the logging mutex for its entire run; though it's only called in init so shouldn't have any races in the first place.
Adds a `NO_RATE_LIMIT` tag that can be used with info/warning/error logs to avoid rate-limiting. This allows `LogPrintLevel_` to be restricted to being an internal API.
The `GetLogCategory` function is moved out of the global namespace.
`ShouldLog` is split into separate `ShouldDebugLog` and `ShouldTraceLog` so that filtering checks are somewhat more enforced via function signature checks.
Redundant `LogAcceptCategory` function is removed.
More files are pointed at util/log.h instead of logging.h.
ACKs for top commit:
maflcko:
review ACK 02b2c41103📅
sedited:
Re-ACK 02b2c41103
l0rinc:
untested ACK 02b2c41103
ryanofsky:
Code review ACK 02b2c41103435d8dbaa77a526e484066471b2b8c! Overall a lot of nice improvements here.
Tree-SHA512: 3bffdca91afbe5c45a522815fe82e6f4cfa96529a4a243b29aad21234650502d6cac780126b584ee3e7ec129d8fdd50670d8a05036cc5c36e586b8c4c3563970
ac9aa71b7f mempool: remove all subsequent tx in pkg on failure (Greg Sanders)
Pull request description:
This belt-and-suspenders check, if ever hit in production, could result in an inconsistent mempool if somehow the parent failed in the ConsensusScriptChecks but the child did not. Rather than allow the mempool to get in an inconsistent state, remove the following txs in the package.
ACKs for top commit:
ismaelsadeeq:
Code review ACK ac9aa71b7f
marcofleon:
ACK ac9aa71b7f
sedited:
ACK ac9aa71b7f
Tree-SHA512: c25310045fa4dfd40bd38c9d54fff3f9fdb817e617154444d4691576393eee5f9cc86e26c59ddcdeef20eb6dddab0a168e91aec85c8291e6d68abbcb333d24f8
ca5483a662 qa: use NORMAL_GBT_REQUEST_PARAMS consistently (Antoine Poinsot)
Pull request description:
Functional tests have a constant that defines normal parameters to `getblocktemplate` but some tests were still hardcoding its value. This PR updates those tests to use the constant instead, so that if normal parameters to `getblocktemplate` need to be changed, it is only necessary to change them in a single place.
This is preparatory work for the implementation of BIP 54, which introduces a new GBT "forced" rule.
ACKs for top commit:
fanquake:
ACK ca5483a662
sedited:
ACK ca5483a662
Tree-SHA512: 07374e501ea66ba7b62f610797758fd3528950215b987a50cb18aa844bbf5fb5f9f49aafebc1472bf336de99ff96d1e74f550c09866fdf2882c73b87ac2a715e
7209eb7790 test: suppress ECONNABORTED in wait_for_rpc_connection on Windows (Ryan Ofsky)
Pull request description:
Since bitcoin/bitcoin#33362, `feature_bind_port_externalip.py` auto-detects whether 1.1.1.5 is available by starting a node with `-bind=1.1.1.5` and converting a bind failure into a skip. On Windows CI the address is not configured, so bitcoind exits as expected — but intermittently an RPC probe raises `ConnectionAbortedError` (WSAECONNABORTED/WinError 10053) while the process is shutting down, causing the test to fail rather than skip.
Fix by treating ECONNABORTED the same as the other transient connection errors already suppressed during startup (ECONNRESET, ETIMEDOUT, ECONNREFUSED).
Fixes#35343
ACKs for top commit:
maflcko:
lgtm ACK 7209eb7790
willcl-ark:
utACK 7209eb7790
hodlinator:
crACK 7209eb7790
Tree-SHA512: 04cf70a2d9247b32e9302827911b0d00a41aa4ac3cf0ba7a9c2aa86009019b14b3f914a3049e08b66b45ccd00530a77129aecf14a953007d28d3322f5ca16a70
Similar to [[nodiscard]], BITCOINKERNEL_WARN_UNUSED_RESULT is used
to indicate that ignoring a function's return value is almost
certainly a bug.
It is used in cases such as a resource leak (e.g. an owning handle returned
by a *_create or *_copy function), or when the returned value is itself an
error/status code. It is not used merely because discarding the result is
wasteful, e.g. on getters or predicates.
Fix the incorrect usage, and properly document the attribute.
Since bitcoin/bitcoin#33362, feature_bind_port_externalip.py runs in CI and
auto-detects whether 1.1.1.5 is assigned by starting nodes with -bind=1.1.1.5,
then converting a FailedToStartError containing "Unable to bind to" into a
SkipTest. On Windows CI the address is not configured, so bitcoind fails as
expected — but intermittently, while the process is shutting down, an RPC probe
raises ConnectionAbortedError (WSAECONNABORTED/WinError 10053) before
wait_for_rpc_connection() notices that the process has exited. That error was
not in the suppressed set, so it escaped and the test failed instead of being
skipped.
The intermittency is a race on TCP connection timing. If the probe connects
before the RPC port is listening, connect can fail with ECONNREFUSED. If the
TCP connection is established but is then closed abortively during shutdown,
the probe can see a connection-reset/aborted error instead. On POSIX systems
this commonly shows up as ECONNRESET ("connection reset by peer"). Winsock
also has WSAECONNABORTED for cases where an established connection is aborted
locally or otherwise terminated due to a timeout/protocol failure, in addition
to WSAECONNRESET for a reset by the remote side. This is why the
Windows-specific error may need to be suppressed separately.
Fix by treating ECONNABORTED identically to ECONNRESET, ETIMEDOUT, and
ECONNREFUSED: retry the probe rather than raising.
Fix was suggested by willcl-ark in
https://github.com/bitcoin/bitcoin/issues/35343#issuecomment-4507329622Fixes#35343
8ce84321ce musig: Reject empty pubkey list in GetMuSig2KeyAggCache (nervana21)
Pull request description:
Per [BIP327](https://github.com/bitcoin/bips/blob/master/bip-0327.mediawiki?plain=1#L300), MuSig2 key aggregation is defined for `u` public keys where `0 < u < 2^32`.
Previously, the code did not handle `u == 0`.
This patch updates the code to reject an empty pubkey list and adds a regression test.
ACKs for top commit:
achow101:
ACK 8ce84321ce
rkrux:
lgtm ACK 8ce84321ce
Tree-SHA512: aa662eee92b6c637683b8535fd9e62431538ba58f38d32f4538259f0b534793625b74a3a10c0c5bb23a7ec7ec83925170b23e47574ef4ebde283a18a0b143d86
b63ef20d54 test: add fuzz harness for CDBWrapper (Andrew Toth)
32169c3855 dbwrapper: accept optional testing leveldb::Env in DBParams (Andrew Toth)
8d390c93fc dbwrapper: make max_file_size a configurable DBParams field (Andrew Toth)
Pull request description:
Inspired by https://github.com/bitcoin/bitcoin/pull/34866#issuecomment-4090291488.
We currently don't have a dedicated harness targeting `CDBWrapper`. OSS-Fuzz has a [rudimentary harness](https://github.com/google/oss-fuzz/blob/master/projects/leveldb/fuzz_db.cc) for levelDB [which fails](https://issues.oss-fuzz.com/issues/447252244), so doesn't appear maintained.
This PR adds a harness targeting `CDBWrapper` against an in-memory oracle to verify correctness.
A `DeterministicEnv` wraps levelDB's `memenv` to eliminate non-determinism by capturing background compaction and running it at fuzzer-chosen points.
The fuzzer also controls the cache_bytes and max_file_size sizes so that small values trigger memtable flushes and compaction.
ACKs for top commit:
l0rinc:
code review ACK b63ef20d54
marcofleon:
ACK b63ef20d54
dergoegge:
utACK b63ef20d54
sedited:
ACK b63ef20d54
Tree-SHA512: da1f738ec90c49830a05b8990bdaa474299b573e966e60f4febef1292d9682f2e50f0016831f26bf4677e5afdaa142dc8766d871c6bce90d35f1695d480ac8c1
eff9e798b9 coinselection: Tiebreak SRD eviction by weight (Murch)
Pull request description:
yancyribbens [pointed out](https://github.com/p2pderivatives/rust-bitcoin-coin-selection/pull/108#issuecomment-3202107069) that SRD would fail to find a possible solution if there are multiple UTXOs of the same effective value with diverse weight.
This adds a tiebreaker that will make SRD succeed in such a scenario.
ACKs for top commit:
yancyribbens:
utACK eff9e798b9
Tree-SHA512: 6a26f3c06f3346cef7e06926d9f747ecea91f057435f22fa8c3333f129227cf5a361f45047003e8a762a99225e3df27e3980a8b4397b36dc9b59c44a9626a2ec
90eda67bb8 Remove opt-in RBF (Pol Espinasa)
Pull request description:
With the whole network mostly running with Full Replace By Fee, it does not make sense to let the user signal or not to signal for RBF on their transactions.
With this PR the transactions created using the GUI will fallback to the wallet configuration in order to signal or not to signal BIP 125. (True by default).
ACKs for top commit:
achow101:
ACK 90eda67bb8
sedited:
ACK 90eda67bb8
Tree-SHA512: 6253501b89ae509eb5bf3c6c81aaf117f03ef2ba411aa3b0a6f05bca07cbf7a1030c519f8825eb2d7269cc286a42ed17c0becccc04ad252bcc9c868086ff4cdc
