2ef6679c2c test: Check that MuSig2 signing does not reuse nonces (Ava Chow)
bb05986c0a musig: Include pubnonce in session id (Ava Chow)
Pull request description:
It is safe to have multiple musig signing sessions over the same message so long as the nonces used are different. Including the pubnonce in the session id allows for multiple simultaneous signing sessions over the same message, rather than asserting when the user tries to do this.
The second commit tests this behavior, both ensuring that there is no crash, and verifying that both sessions produce unique nonces and signatures to verify that no reuse is occurring.
Lastly, the assertion in `SetMuSig2SecNonce` is retained as hitting it now would indicate that a nonce has been reused. We prefer to assert and crash rather than do something that is highly likely to leak a private key.
Fixes#35250
ACKs for top commit:
rkrux:
lgtm ACK 2ef6679c2c
junbyjun1238:
utACK 2ef6679c2c
theStack:
ACK 2ef6679c2c
Tree-SHA512: 9fb60b68ebe0ea9656408afb65b9ec9f280632e1bb84a4821b074c8d8569847845f7c29da800c757b9ddf3aa31aa890dd9e3646cf119917a714e7daf20be2198
6c525c2ec1 wallet: unfriend LegacyDataSPKM and DescriptorScriptPubKeyMan classes (rkrux)
Pull request description:
After #28333, `LegacyDataSPKM` doesn't need to use the private or protected members
of `DescriptorScriptPubKeyMan` class such as `AddDescriptorKeyWithDB` and
`TopUpWithDB`. Moreover, these two SPKMs are siblings that inherit from the common
`ScriptPubKeyMan` that have non intersecting use cases semantically. It seems reasonable
to me that they are unfriended so that private members of one are not exposed to
another unnecessarily.
ACKs for top commit:
polespinasa:
ACK 6c525c2ec1
sedited:
ACK 6c525c2ec1
Tree-SHA512: ebcd9b4e202b152b53c390f44cac46748b7fdebd4f854e84d322468a1f117e729d75210db5688dc7f38124280378f62fa469d2e5ee9adb4cb77acb2c73cc8480
c8b8c275fa test: Improve loopback address check in `rcp_bind.py` (xyzconstant)
Pull request description:
A [loopback address](https://www.geeksforgeeks.org/computer-networks/what-is-a-loopback-address/) can range from `127.0.0.0` to `127.255.255.255`. This commit relaxes the loopback check in `rpc_bind.py` by checking whether an IP address (from `all_interfaces()`) starts with `'127.'` instead of strictly matching `'127.0.0.1'`.
Programs like VPNs might add an extra loopback address (e.g., 127.1.130.83), which failed under the previous state. These addresses will now pass with this update.
---
**For context:** I found this while running tests with the Mullvad daemon active. Mullvad adds a custom lo0 interface like `inet 127.141.11.239 netmask 0xff000000` that failed with `--nonloopback`, which should not be the case since the address is a valid loopback IP.
ACKs for top commit:
maflcko:
lgtm ACK c8b8c275fa
willcl-ark:
ACK c8b8c275fa
Tree-SHA512: 3b82002d6bc90cfc4023dd0274a40970abb2dc6a9ced77dd97e275b31340bb657d5222bb55a768ebf71047ac1521dd4ba77fb427398f7cc9857738bcd16c5818
2ce4ae7d8f ci: Add dynamic cache switching to warp cache (will)
Pull request description:
The GHA cache is very slow, taking on the order of minutes to save and
restore from.
Use WarpBuild's cache instead as this is in the same region and should
be much faster.
WarpBuild cache action does not auto-fallback to GHA if not being run on
Warp. To allow fork runs to fallback to GHA caching, whilst minimising
duplication in the action files, create new "interal" actions which
perform the switching logic, and use these in the (renamed) cache|save
actions.
Without this we would need the `if` logic in our prvious actions, 4
times in each of save and restore.
ACKs for top commit:
m3dwards:
ACK 2ce4ae7d8f
Tree-SHA512: 966ed85558e8ebeed20a6fdf065f0838c9bee95f7d0a2fd1c8a46de20b956f5eedb6bf5098d742da20ea80c56069b86ff46f3151936dc28d5e4afa70acc5d988
9c1fcaca5c wallet, test: fix sendall anti-fee-sniping when locktime is not specified (rkrux)
8877eec726 wallet: allow anti-fee-sniping in sendall RPC while not relying on RBF default (rkrux)
Pull request description:
Partially fixes https://github.com/bitcoin/bitcoin/issues/32661.
Prerequisite to #35405.
The test change in the second commit would fail without the presence
of the first commit.
ACKs for top commit:
maflcko:
review ACK 9c1fcaca5c🍅
xyzconstant:
Tested ACK 9c1fcaca5c
polespinasa:
code review ACK 9c1fcaca5c
sedited:
ACK 9c1fcaca5c
Tree-SHA512: 048c1a6c64c104f8c27053e4dea139fb8740f30096f4d85daf23aa53a9ad21f0120151a7d1d6ea19b12d174976999335b3f9ac863800629774d8030b7f34918f
A loopback address can range from `127.0.0.0` to `127.255.255.255`.
This commit relaxes the loopback check in `rpc_bind.py` by checking whether
an IP address (from `all_interfaces()`) starts with `'127.'` instead of
strictly matching `'127.0.0.1'`.
Programs like VPNs might add an extra loopback address (e.g., 127.1.130.83),
which failed under the previous state. These addresses will now pass with this update.
131fa570b9 test: Add test for BuildSkip() and skip heights (optout)
Pull request description:
The skip height values computed by the (internal) function `GetSkipHeight()`, and the `CBlockIndex::BuildSkip()` method are not tested directly, and the skip logic is not well documented. To improve test coverage, a new test is added, to verify `CBlockIndex::BuildSkip()` and the skip height bit-manipulation logic.
Note: the original version contained a test for the complexity of the `GetAncestor()` algorithm, whose performance is greatly determined by the skip height logic. This test was out-scoped. (see: https://github.com/bitcoin/bitcoin/pull/33661#issuecomment-4486353485)
The motivation is to document the skip value computation through a test. (The issued was noticed while reviewing #33515.)
ACKs for top commit:
l0rinc:
ACK 131fa570b9
sipa:
ACK 131fa570b9
Tree-SHA512: 468070946419d9d2891e43ed014b040348fc9d3b35dc21522487ac28e06b6d5d556ac02ebc4fa7761e202fc80f9e9ab7a7ec47c6e05ae55e33950ff5f8f3596f
570a627640 kernel: assert invalid buffer preconditions in `btck_*_create` functions (stringintech)
Pull request description:
The kernel API appears to use `nullptr` returns to report failures that callers may reasonably want to recover from: malformed serialized input, object construction failures, chainstate/load failures, and similar runtime conditions.
The raw create-function buffer checks seem to be a different case. A failure of `ptr == nullptr && len > 0` does not indicate malformed input data or a failure encountered while deserializing or constructing the requested object. Returning `nullptr` for these checks widens the recoverable error surface with cases that are better treated as programmer errors, similar to other asserted preconditions in this API such as invalid indices and impossible enum/flag states.
This change switches those buffer argument checks from `nullptr` returns to assertions in `btck_transaction_create`, `btck_script_pubkey_create`, `btck_block_create`, `btck_block_header_create`, and `btck_chainstate_manager_options_create`. `btck_block_header_create` additionally asserts the pre-existing documented length contract (must be 80 bytes). These functions still return `nullptr` when the provided bytes cannot be parsed or when object creation fails during processing.
I ended up looking at this while working on the `kernel-bindings-tests` spec/schema for `btck_script_pubkey_create`, where treating this path as a regular error did not seem like the right contract: https://github.com/stringintech/kernel-bindings-tests/pull/14#discussion_r3240859568.
ACKs for top commit:
stickies-v:
ACK 570a627640
janb84:
ACK 570a627640
w0xlt:
ACK 570a627640
sedited:
ACK 570a627640
Tree-SHA512: 064d834abe0c27245a144e5290bbeeb510daf9e4d50bb3a8e50bd8a0bf897b3dcf6ad5acfcabf1d8110da120e5e014ee3aea0241c0f181a21c6f3c14dc452ade
The GHA cache is very slow, taking on the order of minutes to save and
restore from.
Use WarpBuild's cache instead as this is in the same region and much
faster.
WarpBuild cache action does not auto-fallback to GHA if not being run on
Warp. To allow fork runs to fallback to GHA caching, whilst minimising
duplication in the action files, create new "interal" actions which
perform the switching logic, and use these in the (renamed) cache|save
actions.
Without this we would need the `if` logic in our prvious actions, 4
times in each of save and restore.
Plumb the provider through into the action, as a composite action can't
read `env` (`GITHUB_OUTPUT`) from previous steps.
53388773af guix: Remove redundant ShellCheck `source` directives (Hennadii Stepanov)
62cf7bc53f guix, refactor: Add `BASE` argument to `*_for_host` functions (Hennadii Stepanov)
5d46429e32 guix, refactor: Move `distsrc_for_host()` to `prelude.bash` (Hennadii Stepanov)
cab65ea9c6 guix, refactor: Move duplicated `profiledir_for_host()` to `prelude.bash` (Hennadii Stepanov)
faa9d4345f guix, refactor: Move duplicated `outdir_for_host()` to `prelude.bash` (Hennadii Stepanov)
6b59fd6b8c guix, refactor: Remove `contains()` function (Hennadii Stepanov)
d4c69a7224 guix, refactor: Remove unused `out_name()` function (Hennadii Stepanov)
Pull request description:
While working on https://github.com/bitcoin/bitcoin/pull/35098, I reviewed my notes regarding a few minor Guix script flaws and decided to address them here.
This PR:
1. Removes unused code.
2. Reduces code duplication.
3. Improves consistency across function usage.
4. Minimizes ShellCheck directive usage.
ACKs for top commit:
fanquake:
ACK 53388773af - the move-only deduplication, and dead code removal seem fine. The other (refactoring) changes seem less well motived
Tree-SHA512: d565e31ba49300f3001b04d3143721aced305f41bca6d04c33dc479d568efb2c06d91758619378199f174a40f1306c9418ae80d5478013a65ad96341bea122b1
fa787043f5 doc: Compress doc/build-unix.md dependency package names into table (MarcoFalke)
Pull request description:
Currently, `doc/build-unix.md` is tediously verbose, because for several Linux distros, it has exact duplicate sections with only the package names adjusted.
This is hard to maintain, and review. Also, it is hard to read, and hard to use, because a one-line copy-paste does not work to fetch the list of packages.
Fix all issues by compressing the 150+ lines into a small table and a short description.
ACKs for top commit:
achow101:
ACK fa787043f5
darosior:
ACK fa787043f5
sedited:
ACK fa787043f5
hebasto:
re-ACK fa787043f5.
Tree-SHA512: a6d7f18392ab5a0d468387ffe4335f71ae9656a100ede3de4118e3ef28814e5a70202ffa55eb0218a07effcde220b680499ea9068fd54b91cac42fca8699febf
fad585b6e5 test: Wait for node exit after crash in verify_utxo_hash (MarcoFalke)
faf1475514 ci: Exclude feature_dbcrash.py under --v2transport --usecli (MarcoFalke)
fac27d702f test: Fix feature_dbcrash.py --usecli intermittent error (MarcoFalke)
fa09de8b68 test: [refactor] Simplify submit_block_catch_error (MarcoFalke)
Pull request description:
This fixes a small intermittent issue that snuck in via commit fa8d4d5c35.
Generally, it seems tedious and brittle trying to enumerate all possible exception types on all platforms and all test configs, all possible errno values, etc.
So just treat any `Exception` as crash, and confirm it in the test. The test would still fail, if a crash did not happen after an Exception, but the failure may be minimally more tedious to debug. I think this is fine, because failures should be rare and having simpler and more flexible test code is preferable.
----
Also, disable the test for now in CI, because it is quite slow.
ACKs for top commit:
willcl-ark:
ACK fad585b6e5
Tree-SHA512: 6ff69a53908b22904780f29def473f8e26c5b608d89420ac76768d06ea3e3394b5d3f4d488100f9d47f943ff5778a555302ccdaebc11fda7c009205b6a6ca05c
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 particular test case only needs to ensure that locktime is not
specified in the RPC request, it doesn't need to rely on the wallet optin
RBF default that causes the test to pass coincidentally.
Co-authored-by: maflcko <6399679+maflcko@users.noreply.github.com>
Switch buffer `ptr == nullptr && len > 0` checks from `nullptr` returns to assertions. These checks represent invalid caller preconditions, not failures encountered while deserializing or constructing the requested object. `btck_block_header_create` additionally asserts the pre-existing documented length contract (must be 80 bytes).
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>
After PR 28333, `LegacyDataSPKM` doesn't need to use the private or
protected members of `DescriptorScriptPubKeyMan` class such as
`AddDescriptorKeyWithDB` and `TopUpWithDB`. Moreover, these two SPKMs
are siblings that inherit from the common `ScriptPubKeyMan`.
It seems reasonable to me that they are unfriended so that private
members of one are not exposed to another unnecessarily.
In case locktime (and replaceable) not being specified in this RPC request,
the wallet sets the transaction replaceable due to the default value of opt-in
RBF set in the wallet.
This allowed the anti-fee-sniping flow to be executed but in case of
replaceable being set false in the request, anti-fee-sniping flow would be
missed.
This patch fixes it.
fedeff7f20 crypto: disable ASan instrumentation of SSE4 SHA256 for GCC (deadmanoz)
Pull request description:
Fix the runtime crash described in #34881.
Upstream already disables ASan instrumentation for `sha256_sse4::Transform()` under Clang. This extends the same workaround to GCC by adding an `#elif` branch for `__GNUC__` / `__SANITIZE_ADDRESS__` that applies the same `no_sanitize("address")` attribute.
Testing:
- reproduced the crash before the fix with GCC 13, 14, and 15 on Haswell-class machines / guests without SHA-NI (including by forcing the SSE4 implementation on GitHub CI)
- SEGV in debug builds regardless of optimization level (tested `-O0`, `-O1`, `-O2`, `-O3`)
- verified that the GCC + ASan debug configurations that previously crashed pass with this change
Issue #34881 has more details about the issue.
Note: the original Clang code placed the `__attribute__` between the function declarator and the opening brace. GCC's [Attribute Syntax](https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html) documentation notes that this position in a function definition "may, in future, be permitted," so it is not currently supported. Placing the attribute at the start of the function definition is valid form for both GCC and Clang.
ACKs for top commit:
maflcko:
lgtm ACK fedeff7f20🏒
sedited:
tACK fedeff7f20
Tree-SHA512: d8adda0df140b6c93d18f5ecd096b12012332bb640e678075e668122e596baddcb2182cbaeafe7908ada90b4b5cc776a59dcdfb488229ab6640058ccbbd7ea93
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
451fdd26a4 test: wallet: Constructing a DSPKM that can't TopUp() throws. (David Gumberg)
32946e0291 wallet: Setup new autogenerated descriptors on construction (Ava Chow)
e20aaff70f wallet: Construct ExternalSignerSPKM with the new descriptor (Ava Chow)
aa4f7823aa wallet: include keys when constructing DescriptorSPKM during import (Ava Chow)
6538f69135 fuzz: Skip adding descriptor to wallet if it cannot be expanded (Ava Chow)
8be5ee554b test: wallet: Check that loading wallet with both unencrypted and encrypted keys fails. (David Gumberg)
80b0c25992 wallet: Load everything into DescSPKM on construction (Ava Chow)
f713fd1725 refactor: wallet: Don't reuse WALLET_BLANK flag for born-encrypted wallets. (David Gumberg)
cd912c4e10 wallet: Consolidate generation setup callers into one function (Ava Chow)
0301c758ea wallet migration, fuzz: Migrate hd seed once (Ava Chow)
Pull request description:
Instead of constructing ScriptPubKeyMans with no data, and then loading data as we find it, we should gather everything first and then load it all on construction. If there actually is no data and we want to setup generation, then that should also occur in a constructor rather than afterwards.
This change is only applied to DescriptorScriptPubKeyMan and ExternalSignerScriptPubKeyMan, and should be done for any ScriptPubKeyMans added in the future. I don't think it's really worth it to do this for LegacyScriptPubKeyMan since it would make loading performance worse (or cause layer violations) and it's (supposed to be) going away soon.
ACKs for top commit:
polespinasa:
ACK 451fdd26a4
davidgumberg:
re crACK 451fdd26a4
w0xlt:
ACK 451fdd26a4
Tree-SHA512: 58a889bf7c77d5da78041907a76a1958207f95a19bec8dc4d86d4e4108d256a729e0949c0973f7d447178f78a7fd4268cda71d358cae4dec5a76dc453b5283af
b86c1c443d test: add coverage for migrating ancient wallets (furszy)
fd44d48b24 wallet: fix ancient wallets migration (furszy)
Pull request description:
We currently fail migration if the wallet does not contain the best block locator.
This is a problem for wallets created before https://github.com/bitcoin/bitcoin/pull/152, which are not storing such record.
Missing this record is not an error. it simply means the wallet will scan the chain prior
to finish migration.
ACKs for top commit:
achow101:
ACK b86c1c443d
w0xlt:
reACK b86c1c443d
sedited:
Re-ACK b86c1c443d
Tree-SHA512: 5226934e16d32f3337c432a84e1adce9985518e52c62abfa4a8d6b3d857d4b5c6aa99ac90e84ae6772983ceaf7a67e128ff7e0e174843fcb892728b9be4653cf
4bdd46ace3 ci: switch runners from cirrus to warpbuild (will)
Pull request description:
As cirrus is closing down, switch to warpbuild runners.
Switch runner and provider names over. We now use GHA cache, so we don't need to switch that over here.
ACKs for top commit:
m3dwards:
ACK 4bdd46ace3
maflcko:
review ACK 4bdd46ace3🤾
hebasto:
ACK 4bdd46ace3.
Tree-SHA512: 47ed28a6cb7ab10a973af6aa24f4f7a632f59ed17e189ae4f658de37069d763c92cc0e32769693568db6d0e5d2543abcb77bb0977f0b3f296d80a254d6bb3833
fa51f37f18 doc: Reword the Getting-Started section (MarcoFalke)
fab5733f5d doc: Remove good_first_issue.yml (MarcoFalke)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/issues/35399
IIUC, the good-first-issue label and template were meant to make it easier for completely new contributors to get started with something simple. However, I don't think the label and issue template are applicable anymore:
* There are currently no issues with this label, and directing people toward an empty list seems pointless.
* Historically the issue and label has been used rarely. 2026: once, 2025 twice, 2024 thrice. Source: https://github.com/bitcoin/bitcoin/issues?q=state%3Aclosed%20is%3Aissue%20label%3A%22good%20first%20issue%22
* The template has been mis-used, according to https://github.com/bitcoin/bitcoin/issues/35399
Fix all issues by removing it, since it is clear that it is no longer actively used, nor applicable and possibly a net-negative overall.
Of course, regular devs are still free to open issues of this kind as a normal issue, if they wish. However, having the template for this in this repo and tracking it via a label doesn't seem useful.
Since removing the template and label requires rewriting the "Getting Started" section in `CONTRIBUTING.md`, I went ahead and also removed the mention of `Up for grabs` as good things for newcomers to work on. I don't recall the last new contributor that picked something up successfully. Also, `Up for grabs` is usually stuff that people lost interest in, or is no longer relevant.
Instead I've added a sentence to encourage new contributors to help with critical and broad review, which will naturally guide them to good first follow-up issues to work on.
Meta: I know this topic can be subjective and offer bike-shed potential, but I am happy to iterate a bit on this for a few days.
ACKs for top commit:
stickies-v:
ACK fa51f37f18
danielabrozzoni:
ACK fa51f37f18
sedited:
ACK fa51f37f18
darosior:
ACK fa51f37f18
furszy:
ACK fa51f37f18
winterrdog:
ACK fa51f37f18
Tree-SHA512: 9e6d7fe86262bee2df1e0af33ecfb5f77036da2d5d1832bb6afb08f4107d9313eec06d8b769966bf8ecaf8a4c574da5ff99509cc4b7fd9c53ea86788da29721c
3962138cc0 test: add IPC submitBlock functional test (woltx)
5b60f69e40 mining: add submitBlock IPC method to Mining interface (woltx)
813b4a80d7 refactor: introduce SubmitBlock helper (w0xlt)
Pull request description:
This PR adds a `submitBlock` method to the IPC Mining interface, equivalent to the `submitblock` RPC. It accepts a serialized block over IPC, validates/processes it via the normal block-processing path.
The method uses the same result shape as `checkBlock`: `bool` + `reason/debug out-params`. It reports duplicate, inconclusive, and invalid-block rejection details, and initializes reason/debug on every call.
Closes#34626
ACKs for top commit:
Sjors:
ACK 3962138cc0
optout21:
reACK 3962138cc0
ryanofsky:
Code review ACK 3962138cc0. Just rebased since and made suggested changes since last review.
Tree-SHA512: 705cbb89972a80b6ff0ab75a78f686983d6077c97f1758795efe5b8968f01065ebef664ac850eae2bc86af8964efa2a68e8dfc677209c312856650f9387ed006
Explain how to find good first issues to work on.
Also, remove the mention of up-for-grabs for new contributors, because
up-for-grabs is usually stuff that people lost interest in and is often
no longer relevant.
The test should now be passing, but it is slow.
For example, the commit that enabled the test, took ~47 minutes on a
fast CPU, but using a heavy debug build:
https://github.com/bitcoin/bitcoin/actions/runs/26434786214/job/77815064664?pr=35363#step:11:4101
...
Model name: AMD Ryzen 9 7950X3D 16-Core Processor
...
C++ compiler .......................... GNU 15.2.0, /usr/bin/g++
CMAKE_BUILD_TYPE ...................... Debug
Preprocessor defined macros ........... DEBUG DEBUG_LOCKORDER DEBUG_LOCKCONTENTION RPC_DOC_CHECK ABORT_ON_FAILED_ASSUME _GLIBCXX_DEBUG _GLIBCXX_DEBUG_PEDANTIC _LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_DEBUG
C++ compiler flags .................... -m64 -O0 -ftrapv -O1 -g3 -g3 -std=c++20 ...
...
feature_dbcrash.py | ✓ Passed | 2806 s
Catch any Exception in verify_utxo_hash and let restart_node verify the
crash via wait_for_node_exit.
(Also, use named args in restart_node, while touching this test)
Catching any Exception covers possible subprocess.CalledProcessError
that may happen in a --usecli run. E.g.
TestFramework (INFO): Verifying utxo hash matches for all nodes
TestFramework.bitcoincli (DEBUG): Running bitcoin-cli ['-datadir=/tmp/bitcoin_func_test_gzufs0ht/node0', '-rpcclienttimeout=240', '-rpcconnect=127.0.0.1', '-rpcport=20963', 'gettxoutsetinfo']
TestFramework.bitcoincli (DEBUG): Running bitcoin-cli ['-datadir=/tmp/bitcoin_func_test_gzufs0ht/node1', '-rpcclienttimeout=240', '-rpcconnect=127.0.0.1', '-rpcport=20964', 'gettxoutsetinfo']
TestFramework (ERROR): Called Process failed with stdout='error: timeout on transient error: Could not connect to the server 127.0.0.1:20964 (error code 1 - "EOF reached")
Make sure the bitcoind server is running and that you are connecting to the correct RPC port.
Use "bitcoin-cli -help" for more info.
'; stderr='None';
Traceback (most recent call last):
File "./test/functional/test_framework/test_framework.py", line 143, in main
self.run_test()
~~~~~~~~~~~~~^^
File "./test/functional/feature_dbcrash.py", line 273, in run_test
self.verify_utxo_hash()
~~~~~~~~~~~~~~~~~~~~~^^
File "./test/functional/feature_dbcrash.py", line 182, in verify_utxo_hash
nodei_utxo_hash = self.nodes[i].gettxoutsetinfo()['hash_serialized_3']
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "./test/functional/test_framework/test_node.py", line 963, in __call__
return self.cli.send_cli(self.command, *args, **kwargs)
~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "./test/functional/test_framework/test_node.py", line 1043, in send_cli
raise subprocess.CalledProcessError(returncode, p_args, output=cli_stderr)
Make it catch any Exception and let the caller verify it.
This refactor does not change any behavior, but the code is simpler,
more flexible and still correct, because wait_for_node_exit enforces the
crash to happen.
