9a8ef9b0a3 test: SOCKS5 proxy: expect that connection may be reset during handshake (Vasil Dimov)
eb3208364a test: SOCKS5 proxy: expect that connection may be reset when forwarding (Vasil Dimov)
Pull request description:
The `forward_sockets()` function used by the SOCKS5 proxy forwards data between two connected sockets. It might happen that one of those sockets gets closed/reset abruptly, without sending EOF first. This is to be expected if e.g. `bitcoind` is shutdown and shouldn't result in noisy harmless messages like:
```
2026-06-03T13:23:56.966859Z TestFramework.socks5 (ERROR): socks5 request handling failed (running True)
Traceback (most recent call last):
File ".../socks5.py", line 199, in handle
forward_sockets(self.conn, conn_to, self.wakeup_socket_pair[1], self.serv)
~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File ".../socks5.py", line 76, in forward_sockets
data = s.recv(4096)
ConnectionResetError: [Errno 104] Connection reset by peer
```
Instead turn this into a debug log message with a nice prefix containing enough information to identify the two forwarded sockets.
---
Also expect that the connection might be closed during the SOCKS5 handshake and only log a debug message if that happens.
ACKs for top commit:
optout21:
crACK 9a8ef9b0a3
danielabrozzoni:
reACK 9a8ef9b0a3
sedited:
ACK 9a8ef9b0a3
Tree-SHA512: 24e25a30529eda3536ebf472f63a93fd80fff46273054a7075490c88737f8870c0141b2bc99d9ef39e6b4f592af2801350fdfbc71927f573738b4a14f5fd7ce0
Introduce a new simple `NodeSigner` wallet replacement that delegates
signing to a specified node via the `signrawtransactionwithkey` RPC,
and plug in that new class to the functional test feature_taproot.py,
so that it can be ran without the Bitcoin Core wallet compiled.
This can be useful for spending funds that are received at the created
address. Preparatory for the next commit, which modifies the taproot
functional test to work without the Bitcoin Core wallet compiled in.
8791c4764c test: use ExtendedPrivateKey in wallet_taproot.py (rkrux)
89ceafafb9 test: use ExtendedPrivateKey in wallet_listdescriptors.py (rkrux)
bbfffcab58 test: use ExtendedPrivateKey in wallet_send.py (rkrux)
2ab6e590f7 test: use ExtendedPrivateKey in wallet_keypool.py (rkrux)
9e20118720 test: use ExtendedPrivateKey in wallet_fundrawtransaction.py (rkrux)
06af0cddbb test: use ExtendedPrivateKey in wallet_descriptor.py (rkrux)
4100fac20e test: use ExtendedPrivateKey in wallet_createwallet.py (rkrux)
ff3f6def9a test: use ExtendedPrivateKey in wallet_bumpfee.py (rkrux)
003f2a01f6 test: use ExtendedPrivateKey in feature_notifications.py (rkrux)
f988e6d6e6 test: use ExtendedPrivateKey in wallet_importdescriptors.py (rkrux)
d2a03d50ac test: add extendedkey.py unit tests by using BIP32 test vectors (rkrux)
afdb378082 test: introduce ExtendedPrivateKey and ExtendedPublicKey classes (rkrux)
4dbaa7cc65 test: generalise byte_to_base58 utility function to allow more version types (rkrux)
Pull request description:
Many a times there has been a need to come up with dynamic xprvs and xpubs
in the functional tests, but the lack of code that creates them dynamically has
led to the presence of several hardcoded keys in the testing framework. This
is not developer friendly and not self-documenting, clutters the testing code,
and makes it difficult to update the tests in the future.
This PR introduces two utility classes ExtendedPrivateKey and
ExtendedPublicKey that allows the developer to create them on the fly
to be used in the tests. I have intentionally not introduced any library for this
purpose and have reused the existing libraries and functions in the framework.
The implementation is supposed to provide basic functionality for creating
xprv randomly or from a fixed seed, creating corresponding xpub, and
deriving child xprvs and xpubs at custom derivation paths.
I've updated many tests to show how these can be used, there are more
tests as well that can be updated in the future to completely remove such
non-deterministic hardcoded keys.
ACKs for top commit:
achow101:
ACK 8791c4764c
w0xlt:
ACK 8791c4764c
Tree-SHA512: f8ec4e09eaa6cc44b0f1c9a91337e570b12fb882c258be89b470de1a8cecf9d2fd40d9f02ee739dcbf639462ea7710aa145a3726f0f537f5a1f1e7772e5b019d
Creates logic to deprecate logging categories but still
"support" them so the software doesn't quit with unknown category
on startup. Deprecated categories are always false and attempts
to switch them are logged as warnings.
735b25519a support: clamp RLIMIT_MEMLOCK to size_t (Sjors Provoost)
8ab4b9fc85 init: clamp fd limits to int (Sjors Provoost)
4afbabdcef Fix startup failure with RLIM_INFINITY fd limits (Sjors Provoost)
Pull request description:
When setting the fd limit to unlimited, the node fails to start:
```sh
ulimit -n unlimited
build/bin/bitcoind
Error: Not enough file descriptors available. -1 available, 160 required.
```
This was caused by `RaiseFileDescriptorLimit()` (introduced in #2568) casting `limitFD.rlim_cur` to `int`, which for `RLIM_INFINITY` overflows to `-1`. Fix it by returning `std::numeric_limits<int>::max()` instead.
Some platforms implement `RLIM_INFINITY` as the maximum uint64, others as int64 (-1). So simply changing the return type to `uint64_t` wouldn't work.
Similarly, though unlikely to actually happen:
```sh
ulimit -n 214748364
build/bin/bitcoind
Error: Not enough file descriptors available. -2147483648 available, 160 required.
```
The second commit expands the fix by clamping all values above `std::numeric_limits<int>::max()` instead of letting them overflow.
This PR also expands `test/functional/feature_init.py` to cover these, using `resource.setrlimit`. The check is skipped on environments with a hard limit below infinity (or that don't have the Python [Resource module](https://docs.python.org/3/library/resource.html)).
macOS by default has a hard limit of infinity, but on e.g. Ubuntu the default hard limit is 524288.
The third commit applies a similar fix to `PosixLockedPageAllocator::GetLimit()` for 32-bit systems, but without a test.
ACKs for top commit:
winterrdog:
Re-ACK 735b25519a
achow101:
ACK 735b25519a
sedited:
Re-ACK 735b25519a
pinheadmz:
ACK 735b25519a
Tree-SHA512: 0ce0292ecd61456bdec6943b06cbb9ecfc5180ee6dce850f8496ef54af22c1fae6ea473085202f5ba6f72e4dc51a29247620c9a0eae31e96658adc77b293129f
Using these classes allows the developers to dynamically create
xprvs and xpubs so that they don't need to hardcode such long keys
in the tests that most of the times clutter the tests and make them
difficult to update.
Co-authored-by: w0xlt <94266259+w0xlt@users.noreply.github.com>
The client (e.g. `bitcoind`) may open a connection to the proxy and
close it in the middle of the SOCKS5 handshake if it is restarted for
example. Log these as debug messages instead of full blown Python
exception error messages with backtraces.
The `forward_sockets()` function used by the SOCKS5 proxy forwards data
between two connected sockets. It might happen that one of those sockets
gets closed/reset abruptly, without sending EOF first. This is to be
expected if e.g. `bitcoind` is shutdown and shouldn't result in noisy
harmless messages like:
```
2026-06-03T13:23:56.966859Z TestFramework.socks5 (ERROR): socks5 request handling failed (running True)
Traceback (most recent call last):
File ".../socks5.py", line 199, in handle
forward_sockets(self.conn, conn_to, self.wakeup_socket_pair[1], self.serv)
~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File ".../socks5.py", line 76, in forward_sockets
data = s.recv(4096)
ConnectionResetError: [Errno 104] Connection reset by peer
```
Instead turn this into a debug log message with a nice prefix containing
enough information to identify the two forwarded sockets.
This feature was broken for more than a year and no developer apparently
even noticed, so one can conclude that it is largely unused; it seems
thus reasonable to remove it to reduce maintenance burden.
472b950b7f qa: Use custom assert_greater_than() over naked assert (Hodlinator)
f42226d526 qa: Silence socket.timeout exception when substituting it for a JSONRPCException (Hodlinator)
659671ac3d qa: Avoid cleanup when exception is raised (Hodlinator)
Pull request description:
Clean up some cases in which we would trigger multiple tracebacks, which makes it unclear what issue is occuring (https://github.com/bitcoin/bitcoin/issues/31894#issuecomment-4616130031).
CI log of this occurring: https://github.com/bitcoin/bitcoin/actions/runs/26907239657/job/79375388058?pr=35179
<details><summary>Relevant log excerpt</summary>
```
test 2026-06-03T19:28:28.119804Z TestFramework.node0 (DEBUG): TestNode.generate() dispatches `generate` call to `generatetoaddress`
test 2026-06-03T19:28:28.120483Z TestFramework (ERROR): Unexpected exception:
Traceback (most recent call last):
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/authproxy.py", line 165, in _get_response
http_response = self.__conn.getresponse()
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/http/client.py", line 1448, in getresponse
response.begin()
File "/usr/lib/python3.12/http/client.py", line 336, in begin
version, status, reason = self._read_status()
^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/http/client.py", line 297, in _read_status
line = str(self.fp.readline(_MAXLINE + 1), "iso-8859-1")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/lib/python3.12/socket.py", line 707, in readinto
return self._sock.recv_into(b)
^^^^^^^^^^^^^^^^^^^^^^^
TimeoutError: timed out
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/runner/work/bitcoin/bitcoin/ci_build/test/functional/p2p_orphan_handling.py", line 54, in wrapper
func(self)
File "/home/runner/work/bitcoin/bitcoin/ci_build/test/functional/p2p_orphan_handling.py", line 633, in test_maximal_package_protected
testres = node.testmempoolaccept([large_orphan.serialize().hex()])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/authproxy.py", line 128, in __call__
response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/authproxy.py", line 102, in _request
return self._get_response()
^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/authproxy.py", line 167, in _get_response
raise JSONRPCException({
test_framework.util.JSONRPCException: 'testmempoolaccept' RPC took longer than 30.000000 seconds. Consider using larger timeout for calls that take longer to return. (-344) [http_status=None]
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 143, in main
self.run_test()
File "/home/runner/work/bitcoin/bitcoin/ci_build/test/functional/p2p_orphan_handling.py", line 837, in run_test
self.test_maximal_package_protected()
File "/home/runner/work/bitcoin/bitcoin/ci_build/test/functional/p2p_orphan_handling.py", line 57, in wrapper
self.generate(self.nodes[0], 1)
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_framework.py", line 666, in generate
blocks = generator.generate(*args, called_by_framework=True, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_node.py", line 445, in generate
return self.generatetoaddress(nblocks=nblocks, address=self.get_deterministic_priv_key().address, maxtries=maxtries, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/test_node.py", line 453, in generatetoaddress
return self.__getattr__('generatetoaddress')(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/coverage.py", line 50, in __call__
return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/authproxy.py", line 128, in __call__
response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/runner/work/bitcoin/bitcoin/test/functional/test_framework/authproxy.py", line 101, in _request
self.__conn.request(method, path, postdata, headers)
File "/usr/lib/python3.12/http/client.py", line 1356, in request
self._send_request(method, url, body, headers, encode_chunked)
File "/usr/lib/python3.12/http/client.py", line 1367, in _send_request
self.putrequest(method, url, **skips)
File "/usr/lib/python3.12/http/client.py", line 1193, in putrequest
raise CannotSendRequest(self.__state)
http.client.CannotSendRequest: Request-sent
test 2026-06-03T19:28:28.124908Z TestFramework (DEBUG): Closing down network thread
```
</details>
Fix in authproxy.py can be verified through applying the below diff with/without PR changes and running `./build/test/functional/p2p_orphan_handling.py`:
```diff
--- a/test/functional/test_framework/authproxy.py
+++ b/test/functional/test_framework/authproxy.py
@@ -162,6 +162,8 @@ class AuthServiceProxy():
def _get_response(self):
req_start_time = time.time()
try:
+ if AuthServiceProxy.__id_count > 55:
+ raise socket.timeout()
http_response = self.__conn.getresponse()
except socket.timeout:
raise JSONRPCException({
```
ACKs for top commit:
maflcko:
review ACK 472b950b7f🔼
polespinasa:
reACK 472b950b7f
Tree-SHA512: ec1ede0340da9d1338643980e7f2e4646f0aed2e64f339f8b41a447cbbfb4da8158c1cbcb22f2d0cc383c2dd945075602d02b07a47b33b44cc6215f3292c1d46
BIP434 defines FEATURE messages which are sent between VERSION and VERACK
to indicate support for new P2P protocol features. This commit provides
the infrastructure for easily using BIP434 negotiation when implementing
such new P2P protocol features. Note that advertised protocol version
is bumped to 70017, as per BIP434's specification.
bf0d257c11 net: un-default the OpenNetworkConnection()'s proxy_override argument (Eugene Siegel)
5a3756d150 test: add a regression test for private broadcast v1 retries (Vasil Dimov)
ab35a028ed test: make reusable filling of a node's addrman (Vasil Dimov)
2333be9cbc test: make reusable starting a standalone P2P listener (Vasil Dimov)
2ffa81fac4 test: make reusable SOCKS5 server starting (Vasil Dimov)
32d072a49f doc: add release notes for #35319 (Vasil Dimov)
d01b461f71 net: ensure no direct private broadcast connections (Vasil Dimov)
fd230f942d net: use the proxy if overriden when doing v2->v1 reconnections (Vasil Dimov)
Pull request description:
This PR includes https://github.com/bitcoin/bitcoin/pull/35319 and on top of that adds a regression functional test.
The functional test exercises the relevant code paths without modifying non-test code. To do that it does:
* Add a bunch of IPv4 addresses to the node's addrman (they will be added without P2P_V2 flag).
* Get them to report P2P_V2 in their service flags and connect to each one, so that the flags
in addrman are updated to contain P2P_V2.
* Get one successful connection to a Tor peer (.onion) so that bitcoind assumes the configured
Tor proxy works and is indeed a proxy to the Tor network. This will make it open private
broadcast connections also to IPv4 addresses via that proxy.
* Start some private broadcast connections.
* Remember the destination IPv4 address of the first connection and get it to fail the v2
transport.
* Wait for a subsequent connection also through the Tor proxy to the same IPv4 and expect
it to be v1, i.e. the v2->v1 downgrade retry.
The test fails without the fix - the v1 retry never arrives to the Tor proxy. And passes with the fix. The fix is in the first commit here and in https://github.com/bitcoin/bitcoin/pull/35319, can remove it by `git show fd230f942d | git apply -R`.
ACKs for top commit:
Crypt-iQ:
reACK bf0d257c11
andrewtoth:
ACK bf0d257c11
instagibbs:
ACK bf0d257c11
sedited:
utACK bf0d257c11
Tree-SHA512: 11e89be36577199e0312e5e63efeac04e295faaba1cf1c13a30e683d35f473c8dbb419d1897b0333c2e993c10637adecafcf90fe08c812065c793cbc903744c9
ac09260982 test: restore JSONRPCException error format (rkrux)
Pull request description:
This is a follow-up to PR #34575.
ACKs for top commit:
maflcko:
lgtm ACK ac09260982
Tree-SHA512: 15979f4e2c07993f283640ebfe570e9f8d3842a23a8118042f5b618273e0da8a01bcabe1ec90b6cc49ebf28e9819d1b4f077ac18f62f681a4d4f58ad8e11bdb1
This is a follow-up to PR 34575.
Copy is done so that checking of error["code"] in test_node.py
while handling this exception doesn't fail.
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Extract the part of `p2p_private_broadcast.py` that starts
listening on a `P2PConnection` object (or its children classes)
and put it into `test_framework/p2p.py`.
Extract the part of `p2p_private_broadcast.py` that configures and
starts the SOCKS5 server into a reusable function and put it into
`test_framework/socks5.py`.
Use bind port 0 to let the OS pick an available port instead of
hackishly assuming that `p2p_port(N)` is available where N is more
than the number of the nodes the test uses.
f701cd159a doc: fix typo in release notes of #34917 (rkrux)
7bc39e3d08 wallet, test: add wallet_deprecated_rbf.py for walletrbf deprecated keys & options (rkrux)
2cbbcb5659 wallet, test: remove -deprecatedrpc=bip125 from wallet_send.py (rkrux)
307134bd7e wallet, test: remove -deprecatedrpc=bip125 from wallet_migration.py (rkrux)
3ec550d168 wallet, test: remove -deprecatedrpc=bip125 from wallet_basic.py (rkrux)
a52ea9bff9 wallet, test: remove -walletrbf startup option from wallet_backwards_compatibility.py (rkrux)
42330922dd wallet, test: remove -walletrbf startup option from wallet_backwards_compatibility.py (rkrux)
8cb6e405d8 wallet, test: remove -walletrbf startup option from wallet_listtransactions.py (rkrux)
0ee94b2fef wallet, test: remove -deprecatedrpc=bip125 from wallet_listtransactions.py (rkrux)
5e833e068d wallet, test: -walletrbf startup option from wallet_bumpfee.py (rkrux)
a2a2b1745f wallet, test: remove -walletrbf startup option from rpc_psbt.py (rkrux)
a3fe455a95 wallet: refactor to read -walletrbf only once instead of twice (rkrux)
Pull request description:
Prerequisite to #35404 and #35405.
All these changes address the points raised in the review of PR #34917
here: https://github.com/bitcoin/bitcoin/pull/34917#pullrequestreview-4362148900.
Essentially updating the existing wallet functional tests without using
the -deprecatedrpc=bip125 and -walletrbf startup options. Instead,
these two are added and tested via a singular new
wallet_deprecated_rbf.py test that can be removed easily later when
these startup options are completely removed from the wallet post
deprecation.
ACKs for top commit:
maflcko:
review ACK f701cd159a🌄
achow101:
ACK f701cd159a
Tree-SHA512: 700785062b5de8ee3b6c4f50570b769d56c6c4960f2b6e2a2e71be8085c6b51eaeb34fb158fae76f812fe82791aaa0c0277f964f0472cb0784b86caabe6d4ec9
Test the new Mining.submitBlock IPC method:
- Invalid block (bad version) returns failure with reason
- Valid block (with a real mempool tx) is accepted and propagates
- Duplicate block returns failure with "duplicate" reason
- Witness commitment without coinbase witness nonce is rejected
(bad-witness-nonce-size), confirming no auto-fix behavior
- submitBlock then submitSolution: duplicate is accepted (submitSolution
returns true for already-known blocks)
- submitSolution then submitBlock interaction (duplicate)
Build candidate blocks from BlockTemplate data in the existing coinbase and
submission test, then exercise checkBlock(), submitSolution(), and
submitBlock() against those candidates. submitBlock() uses an isolated IPC
node for cases that would otherwise affect the main submitSolution() and
checkBlock() assertions.
1e5d3b4f0d doc: add release note for mining option validation (Sjors Provoost)
0317f52022 ci: enforce iwyu for touched files (Sjors Provoost)
8c58f63578 refactor: have mining files include what they use (Sjors Provoost)
3bb6498fb0 mining: store block create options in NodeContext (Sjors Provoost)
4637cd157d mining: reject invalid block create options (Sjors Provoost)
8daac1d6eb mining: add block create option helpers (Sjors Provoost)
128da7c3ff miner: add block_max_weight to BlockCreateOptions (Sjors Provoost)
fa81e51eae mining: parse block creation args in mining_args (Sjors Provoost)
020166080c mining: use interface for tests, bench and fuzzers (Sjors Provoost)
44082bea47 interfaces: make Mining use const NodeContext (Sjors Provoost)
d4368e059c move-only: add node/mining_types.h (Sjors Provoost)
6aeb1fbea2 test: cover IPC blockmaxweight policy (Sjors Provoost)
63b23ea1e9 test: regression test for waitNext mining policy (Sjors Provoost)
24750f8b31 test: add createNewBlock failure helper (Sjors Provoost)
63ee9cd15b test: misc interface_ipc_mining.py improvements (Sjors Provoost)
Pull request description:
Although this PR is primarily a refactor, _there are behavior changes_ documented in the release note:
- the IPC mining interface now rejects out-of-range block template options instead of silently clamping them;
- startup now rejects `-blockmaxweight` values lower than `-blockreservedweight`, instead of allowing them to be clamped later.
The interaction between node startup options like `-blockreservedweight` and runtime options, especially those passed via IPC, is confusing.
They're combined in `BlockAssembler::Options`, which this PR gets rid of in favour of `BlockCreateOptions`.
`BlockCreateOptions` is used by interface clients. As before, IPC clients have access to a safe / sane subset, whereas RPC and test code can use all fields. The same type is also used to store mining defaults parsed once during node startup in `NodeContext`.
The maximum block weight setting (`block_max_weight`) is optional. When read from startup options it matches `-blockmaxweight`; when provided by callers it is a runtime override. `Merge()` fills unset fields from startup defaults while preserving caller-provided values.
This all happens in commits `mining: add block create option helpers` and `mining: store block create options in NodeContext`, and requires some preparation to keep things easy to review.
We get rid of `BlockAssembler::Options` but this is used in many tests. Since large churn is inevitable, we might as well switch all tests, bench and fuzzers over to the Mining interface. The `mining: use interface for tests, bench and fuzzers` commit does that, dramatically reducing direct use of `BlockAssembler`. Two exceptions are documented in the commit message. Because `test_block_validity` wasn't available via the interface and the block_assemble benchmark needs it, it's moved from `BlockAssembler::Options` to `BlockCreateOptions` (still not exposed via IPC).
We need access to mining related structs from both the miner and node initialization code. To avoid having to pull in all of `BlockAssembler` for the latter, the `move-only: add node/mining_types.h` commit introduces `node/mining_types.h` and moves `BlockCreateOptions`, `BlockWaitOptions` and `BlockCheckOptions` there from `src/node/types.h`.
I considered also moving `DEFAULT_BLOCK_MAX_WEIGHT`, `DEFAULT_BLOCK_RESERVED_WEIGHT`, `MINIMUM_BLOCK_RESERVED_WEIGHT` and `DEFAULT_BLOCK_MIN_TX_FEE` there from `policy.h`, since they are distinct from relay policy and not needed by the kernel. But this seems more appropriate for a follow-up and requires additional discussion.
---
I kept variable renaming and other formatting changes to a minimum to ease review with `--color-moved=dimmed-zebra`.
## Commit summary
Tests and test cleanup:
- `test: misc interface_ipc_mining.py improvements`
- `test: add assert_create_fails helper`
- `test: regression test for waitNext mining policy`
- `test: cover IPC blockmaxweight policy`
Refactoring test/bench/fuzz callers:
- `interfaces: make Mining use const NodeContext`
- `mining: use interface for tests, bench and fuzzers`
Moving mining interface types:
- `move-only: add node/mining_types.h`
Separating startup defaults from runtime options:
- `mining: parse block creation args in mining_args`: adds `node/mining_args.{h,cpp}` and moves mining option parsing out of `init.cpp`, without storing the parsed values yet.
- `miner: add block_max_weight to BlockCreateOptions`: moves the runtime maximum block weight setting into `BlockCreateOptions` as an optional value, so it can later be defaulted from startup args when unset.
- `mining: add block create option helpers`: centralizes block template option defaulting and merging, removes `BlockAssembler::Options`, and preserves behavior except for dropping the `Specified ` prefix from startup option error messages.
- `mining: reject invalid block create options`: checks typed `BlockCreateOptions` before block template creation, so invalid runtime options are rejected instead of silently clamped. Startup validation also rejects `-blockmaxweight` values lower than `-blockreservedweight`.
- `mining: store block create options in NodeContext`: stores the startup mining options in `NodeContext` as `BlockCreateOptions`, so startup defaults and runtime overrides can be merged with the same option type.
Include hygiene, CI and release note:
- `refactor: have mining files include what they use`
- `ci: enforce iwyu for touched files`
- `doc: add release note for mining option validation`
ACKs for top commit:
w0xlt:
reACK 1e5d3b4f0d
sedited:
ACK 1e5d3b4f0d
ryanofsky:
Code review ACK 1e5d3b4f0d. Looks good, thanks for the updates!
Tree-SHA512: 28c715023cb78f02775caa787b243c994bd0f8ce4559afc8db9301e93400ebbc74963626a4afe65ae15bcc16b9192d051a745839f4c804848d50746ea5a224b4
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
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.
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
Check BlockCreateOptions before block template creation instead of
clamping runtime values. This makes invalid runtime block creation
options, including those passed by IPC mining clients, fail explicitly
instead of silently mining with different values than the caller
requested.
Runtime option validation now uses the same error wording as startup
option validation. Startup validation also rejects -blockmaxweight
values lower than -blockreservedweight instead of allowing them to be
clamped later.
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
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.
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
All transactions are by default replaceable since v28, the wallet need not have
a configuration option to opt into RBF signalling because it seems redundant
now. Emit a warning if this option is used.
Re-using the same rpc connection on multiple threads is obviously
unsafe, so this helper can be used to create one connection per thread.
This refactor does not change any behavior and can be reviewed with the
git options:
--color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
75cf9708a0 ci: add one more routable address to the VMs (docker containers) (Vasil Dimov)
1b93983bf5 test: make feature_bind_port_(discover|externalip).py auto-detect the skip condition (Vasil Dimov)
Pull request description:
`feature_bind_port_discover.py` and `feature_bind_port_externalip.py` require a routable address on the machine to run. Since that was not predictably available on CI, those tests required a manual setting up of IP addresses (e.g. using `ifconfig`) and then running the tests with a command line option telling them that the addresses are set up. The tests were not run in CI and [got rot](https://github.com/bitcoin/bitcoin/issues/31293#issuecomment-2497792487).
Change that to auto-detect, from the tests, whether the needed IP addresses are present and if yes, run the test, otherwise skip it. Also change the CI to configure the needed addresses when running the functional tests. This way the tests will be run regularly on CI.
Fixes: https://github.com/bitcoin/bitcoin/issues/31336
ACKs for top commit:
willcl-ark:
ACK 75cf9708a0
frankomosh:
Tested ACK 75cf9708a0. Built from source.
ryanofsky:
Code review ACK 75cf9708a0. Tested locally with and without the special addresses, and the detection seems to work well.
Tree-SHA512: 252911a37a06764f644a1a83c808f5255ac3bc74919426afa5d082c59e1ea924196354735f229d381cb5aff2340e001c2240bbadc8b5f27e5321fb4cfaef0fdb
1c500b1709 test: avoid non-loopback network traffic from node_init_tests/init_test (Vasil Dimov)
Pull request description:
The test `node_init_tests/init_test` calls:
`AppInitMain()` -> `StartMapPort()` -> `StartThreadMapPort()` -> `ThreadMapPort()` -> `ProcessPCP()` -> `PCPRequestPortMap()` -> `CreateSock()` and on the returned value from `CreateSock()` it calls the `Connect()` method.
Thus, change `BasicTestingSetup::BasicTestingSetup()` to set `-natpmp` to 0. This way `node_init_tests/init_test` or other tests will not do network activity due to `ThreadMapPort()`.
Also add a comment about `natpmp=0` in
`test/functional/test_framework/util.py`.
Also set `-dnsseed=0` in `BasicTestingSetup::BasicTestingSetup()` to
avoid DNS queries.
ACKs for top commit:
fjahr:
re-ACK 1c500b1709
ryanofsky:
Code review ACK 1c500b1709, just disabling -dnsseed since previous review, which makes sense.
Tree-SHA512: 3b275d91361804da6d1dc109dffe741ea4b3dd3be916eb12fa63efa233e13ea4dab5a9f8448bd8bf99bc41817f3b7a768abe91531a3f63eae8b4c912bfcbd13e
The test calls:
`AppInitMain()` -> `StartMapPort()` -> `StartThreadMapPort()` ->
`ThreadMapPort()` -> `ProcessPCP()` -> `PCPRequestPortMap()` ->
`CreateSock()` and on the returned value from `CreateSock()` it calls
the `Connect()` method.
Thus, change `BasicTestingSetup::BasicTestingSetup()` to set `-natpmp`
to 0. This way `node_init_tests/init_test` or other tests will not do
network activity due to `ThreadMapPort()`.
Also add a comment about `natpmp=0` in
`test/functional/test_framework/util.py`.
Also set `-dnsseed=0` in `BasicTestingSetup::BasicTestingSetup()` to
avoid DNS queries.
On new regtest / signet chains (heights <= 16), createBlock() fails
internally with bad-cb-length. Since mining.capnp does not expose
include_dummy_extranonce, IPC clients can't work around this.
Add a functional test to illustrate this issue and use RPC to
work around it. The next commit introduces a fix.
Needed in a later commit to correctly derive the BIP34 prefix
for heights <= 16.
Add a padding parameter to script_BIP34_coinbase_height() that
controls whether the OP_0 dummy extranonce is appended for
heights <= 16.
Use this helper with padding=False in the IPC mining test's
build_coinbase_test().
9fa4076b20 test: Test merging implicit PSBTv0 with explicit PSBTv0 (w0xlt)
1660c18232 doc: Release notes for psbtv2 (Ava Chow)
470e52a5f8 fuzz: Enforce additional version invariants in PSBT fuzzer (Antoine Poinsot)
5bd0579c09 test: Tests for PSBT AddInput and AddOutput (Ava Chow)
b8b6e7f0c2 tests: Add PSBT unit test for ComputeTimeLock (Ava Chow)
0bc1c2e508 tests: Add test vectors from BIP 370 (Ava Chow)
e0e4dbdeb5 psbt: Change default psbt version to 2 (Ava Chow)
bcc1dca77b Add psbt_version to PSBT RPCs and default to v2 (Ava Chow)
ab38c30195 Implement PSBTv2 field merging (Ava Chow)
93e339e29f Implement PSBTv2 AddInput and AddOutput (Ava Chow)
b39c86ae60 Allow specifying PSBT version in constructor (Ava Chow)
dcc9a3c8df Implement PSBTv2 in decodepsbt (Ava Chow)
5770dbd39f Add PSBT::ComputeLockTime() (Ava Chow)
863cf47b33 Update test_framework/psbt.py for PSBTv2 (Ava Chow)
925161eaf0 Implement PSBTv2 fields de/ser (Ava Chow)
d9cf658ee0 Restrict joinpsbts to PSBTv0 only (Ava Chow)
3da0e16012 Replace PSBT.tx with PSBT::GetUnsignedTx and PSBT::GetUniqueID (Ava Chow)
c568624ff2 psbt: Return std::optional from PrecomputePSBTData (Ava Chow)
092de4f1f6 Replace PSBT::GetInputUTXO with PSBTInput::GetUTXO (Ava Chow)
82c9fe3179 psbt: Use PSBTInput and PSBTOutput fields instead of accessing global tx (Ava Chow)
95897507e9 psbt: AddInput and AddOutput should take only PSBTInput and PSBTOutput (Ava Chow)
1b7d323a72 Add PSBTInput::GetOutPoint (Ava Chow)
543d3e1cdc psbt: add PSBTv2 global tx fields (Ava Chow)
c01c7f068c psbt: Remove default constructor (Ava Chow)
9671aa08c2 psbt: add tx input and output fields in PSBTInput and PSBTOutput (Ava Chow)
990b084f11 Have PSBTInput and PSBTOutput know the PSBT's version (Ava Chow)
7eacc21ff6 psbt: make PSBT structs into classes (Ava Chow)
f926c326bb gui: Store PSBT in std::optional in PSBTOperationsDialog (Ava Chow)
1e2d146b47 psbt: Refactor duplicate key lookup and size checks (Ava Chow)
88384180d3 test: PSBTs should roundtrip through RPCs that do nothing (Ava Chow)
001877500d test: construct psbt with unknown field programmatically (David Gumberg)
0cb884e6df psbt: Fill hash preimages and taproot builder from SignatureData (Ava Chow)
Pull request description:
BIP 370 PSBTv2 introduces several new fields and different invariants for PSBT. This PR implements those new fields and restructures the PSBT implementation to match PSBTv2 but still remain compatible with PSBTv0.
ACKs for top commit:
nervana21:
re-ACK 9fa4076b20
theStack:
re-ACK 9fa4076b20
w0xlt:
ACK 9fa4076b20
Tree-SHA512: ab0a5ada4fa5fca27ba9ec9c291a44b30e69d6db11971957572d86c58c71c4caa4557dc25f403e1170ba4fac751306d074cc582defefc6e2fdd37be51c3d9dd0
1950da94fc test: enable `rpc_bind` on macOS and BSD (Lőrinc)
7236a05503 test: enable `feature_bind_extra` on macOS and BSD (Lőrinc)
Pull request description:
### Problem
Some functional tests are shown as skipped when running on macOS & BSD because `test_framework/netutil.py` only implemented the Linux-specific logic for checking which TCP sockets a node is listening on.
### Fix
Add macOS and BSD implementations in `test/functional/test_framework/netutil.py` so tests can query:
* which TCP sockets a node is listening on (`get_bind_addrs()`, via `lsof`)
* a non-loopback interface address (`all_interfaces()`, via `ifconfig`)
Then enable the previously Linux-only tests by switching to a shared POSIX platform guard.
### Commands
<details>
<summary><code>get_bind_addrs()</code> (<code>lsof</code> + regex)</summary>
> Command used
```bash
lsof -nP -a -p <pid> -iTCP -sTCP:LISTEN -Ftn
```
> Flags
- -D: device cache warnings
- -n: no hostname resolution
- -P: no service/port-name resolution
- -a: AND all conditions
- -p <pid>: filter by process ID
- -iTCP: TCP sockets only
- -sTCP:LISTEN: listening sockets only
- -Ftn: machine-readable output (fields: type `t`, name `n`)
> Regex parser
```regex
t(IPv[46])\nn(\*|\[.+?]|[^:]+):(\d+)
```
> Captured groups
- group 1: IPv4 / IPv6 (used to disambiguate `*`)
- group 2: host (`*`, `[::1]`, `127.0.0.1`, ...)
- group 3: port
</details>
<details>
<summary><code>all_interfaces()</code> (<code>ifconfig</code> + regex)</summary>
> Command used
```bash
ifconfig -au
```
> Regex parsing
Interface blocks:
```regex
(?m)^(?P<iface>\S+):(?P<block>[^\n]*(?:\n[ \t]+[^\n]*)*)
```
IPv4 extraction within each block:
```regex
inet (\S+)
```
</details>
### Notes
The only remaining platform skips on macOS are the USDT/BPF tracing tests (`interface_usdt_*.py`).
ACKs for top commit:
Sjors:
ACK 1950da94fc
achow101:
ACK 1950da94fc
willcl-ark:
tACK 1950da94fc
Tree-SHA512: 4cecc88852623f3fe3a7dccceb0e71932824c1ed7f1d4ab89b953ff6b7991afbd0b016c819c17e966bed53082dd623a832752b8847711861009cd5ffc4677367
