Bitcoin Core

mv btcsignals.h to src/util

This is a low-level utility header

refactor: Make scoped_connection ctor explicit

This ensures that all constructions of scoped connections are explict,
making the intent explicit and review easier. This also follows the
default recommendation from
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#c46-by-default-declare-single-argument-constructors-explicit

test: Check btcsignals determinism in thread_safety test case

The test only checked that the single atomic value is greater than 3000.
However, by splitting the atomic into two, one can do one exact check,
and also increase the lower bound on the inexact check.

Also, test disconnect races for every second step, instead of only once
at the end (likely when only one thread is running anyway).

Both changes make the test stricter and may catch non-determinism issues
that are not detected by sanitizers alone.

The test added in this commit should also pass when applied on top of
commit 63c68e2a3f98d2466a7e766d861ba3a94e92cd20, which is still using
the boost implementation.

refactor: Properly return from ThreadSafeQuestion signal

Previously, the signal was using btcsignals::optional_last_value<bool>.
However, this only worked by accident:

The return value was influenced by the order in which the connections
were done. The noui callbacks would always overwrite the return value
with false. This makes the code overall brittle, and confusing.

For example, the following patch that changes the order of connections
would break the only and single place where the return value actually
matters:

```diff
diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp
index 0b89c605b9..976549470e 100644
--- a/src/qt/bitcoin.cpp
+++ b/src/qt/bitcoin.cpp
@@ -488,3 +488,2 @@ int GuiMain(int argc, char* argv[])
btcsignals::scoped_connection handler_message_box = ::uiInterface.ThreadSafeMessageBox_connect(noui_ThreadSafeMessageBox);
- btcsignals::scoped_connection handler_question = ::uiInterface.ThreadSafeQuestion_connect(noui_ThreadSafeQuestion);
btcsignals::scoped_connection handler_init_message = ::uiInterface.InitMessage_connect(noui_InitMessage);
@@ -663,2 +662,3 @@ int GuiMain(int argc, char* argv[])
app.createWindow(networkStyle.data());
+ btcsignals::scoped_connection handler_question = ::uiInterface.ThreadSafeQuestion_connect(noui_ThreadSafeQuestion);
// Perform base initialization before spinning up initialization/shutdown thread
```

This can be tested by applying the patch and then calling:

(May have to be started twice to trigger the question)

```
bitcoin-qt -regtest -datadir=/tmp -mocktime=123456789
```

Before the changes in this commit (on current master), pressing `OK`
would not have any effect and would abort the program.

After the changes in this commit, pressing `OK` will correctly trigger a
-reindex and leave the program running.

refactor: Mark btcsignals operator [[nodiscard]]

This is only relevant in case the result_type is not void, in which case
the caller should either use the return value or mark it unused.

doc: add taproot descriptor to getdescriptors example

Wallets import descriptors for all supported address types in a
BIP44/49/84/86 compatible manner, so show the BIP86 tr() descriptor in
the example response as well.

doc: clarify which commands receive --chain, --fingerprint and --stdin

Bitcoin Core passes --chain and --fingerprint to every signer command
except enumerate, so mark them "(required except for enumerate)" and
name the current commands only as examples. Keep --stdin documented as
required for all commands so the interface stays flexible, noting that
Bitcoin Core currently only uses it for signtx.

Add the missing flags to the getdescriptors and displayaddress usage
examples and the corresponding doxygen comments, matching the order and
form of the actual invocations, add testnet4 to the chain name lists,
and drop the getaddressinfo implementation detail from the
walletdisplayaddress description.

doc, rpc: document enumerate model field and fingerprint deduplication

The external signer "enumerate" response uses the optional "model"
field, not "name". Document that Bitcoin Core maps it to the "name"
field of the enumeratesigners RPC result, and that signers with
duplicate master key fingerprints are skipped.

Also document that wallet operations require exactly one connected
signer.

Co-authored-by: optout <13562139+optout21@users.noreply.github.com>
Co-authored-by: naiyoma <lankas.aurelia@gmail.com>

test: add abortscan unit test

Co-authored-by: w0xlt <woltx@protonmail.com>

wallet: move fAbortRescan reset into WalletRescanReserver reserve()

Reserving the wallet rescan is the first thing done when starting a rescan.
As part of the reservation, clear any leftover state from previous
rescans (reset fAbortRescan). This prevents a race condition
where an abort request arrives before the rescan loop starts; without
this reset, the abort could be ignored and the rescan would proceed.

Co-authored-by: w0xlt <woltx@protonmail.com>

btcsignals: delete broken scoped_connection move assignment

The defaulted move assignment overwrites m_conn without disconnecting
it first, so the previous callback stays registered with the signal and
keeps firing, violating the RAII contract:

btcsignals::scoped_connection sc0 = sig.connect(IncrementCallback);
btcsignals::scoped_connection sc1 = sig.connect(SquareCallback);
sc0 = std::move(sc1);
val = 3; sig(val); // both callbacks fire: 16 instead of 9

Move assignment is unused in the codebase, so delete it rather than
fixing it. It can be implemented properly if a use case arises.

test: add coverage for importdescriptors when manually interrupting a wallet rescan

Co-authored-by: w0xlt <woltx@protonmail.com>

test: add coverage for importdescriptors errors when using assumeutxo

Co-authored-by: w0xlt <woltx@protonmail.com>

test: add coverage for importdescriptors while wallet is rescanning

Co-authored-by: w0xlt <woltx@protonmail.com>

test: updated different_key to be different_field and also used a single assert_equal with 3 args instead of multiple assert_equals

net: move cs_main up in FetchBlock to fix rpc assert crash

1. FetchBlock runs in a http worker thread. It acquires a PeerRef,
locks cs_main, then may later call BlockRequested which asserts
that CNodeState exists for the peer.

2. FinalizeNode may run in either the bitcoind or b-net threads. It
locks cs_main, fetches a PeerRef from RemovePeer, fetches a CNodeState,
and later removes it from m_node_states.

Because of the lock placement in FetchBlock, the http worker thread in 1)
can acquire a valid PeerRef and block while the b-net thread in 2) is
cleaning up the peer in FinalizeNode. When the worker thread later acquires
cs_main, it may crash in BlockRequested since no CNodeState exists. Fix
this by acquiring the lock earlier in FetchBlock.

The lock can be replaced with a net-specific lock when the remaining
CNodeState fields are moved to Peer.

test: Use FakeNodeClock in more places

The context is easier to reason about: E.g.,

* in TestBasicMining it allows to drop manual SetMockTime(0) calls,
* in connections_desirable_service_flags it allows to drop manual calls
to SetMockTime(GetTime<std::chrono::seconds>() + _n_) and replace them
by operator+=(_n_)
* in wallet_tests it clarifies that the mocktime does not persist
outside the AddTx function

scripted-diff: Rename UNIQUE_NAME to BITCOIN_UNIQUE_NAME

The `nb30.h` Windows header defines `UNIQUE_NAME` as a macro.

This introduces a fragile dependency on header inclusion order: if
Windows headers happen to be included before `UNIQUE_NAME` is used, the
preprocessor expands it into a numeric literal, causing syntax errors.

Rename the macro to `BITCOIN_UNIQUE_NAME` to remove this fragility and
avoid the collision entirely.

-BEGIN VERIFY SCRIPT-
sed -i 's/\<UNIQUE_NAME\>/BITCOIN_UNIQUE_NAME/g' $(git grep -l 'UNIQUE_NAME' ./src/)
-END VERIFY SCRIPT-

test: add coverage for importdescriptor with an encrypted wallet

scripted-diff: Rename NodeClockContext to FakeNodeClock

The previous name did not indicate the type was intended for
testing. Renaming to FakeNodeClock makes this explicit and
allows call sites to drop the ctx suffix on the variable name.

Suggested in #34858 review feedback.

-BEGIN VERIFY SCRIPT-
s() { git grep -l "$1" -- src | xargs sed -i "s/$1/$2/g"; }

s '\<NodeClockContext\>' 'FakeNodeClock'
s '\<clock_ctx\>' 'clock'
-END VERIFY SCRIPT-

test: Use NodeClockContext in more call sites

This refactor is a follow-up to commit
faad08e59c4419e09eb75054bf468ca98a837ca8 and does not
change any behavior.

These call sites are clean mechanical swaps. The remaining ones
require non-trivial test refactoring and are left for future
follow-ups.

add release notes

fuzz: fix dead HD keypaths (de)serialization round-trip

`DeserializeHDKeypaths()` was writing into the original `hd_keypaths`
map instead of `deserialized_hd_keypaths`. As a result the latter was
always empty and the round-trip assertion following was trivially true,
so the serialize/deserialize round-trip wasn't actually being exercised.

That bug was introduced with the commit introducing the fuzz target
(commit f898ef65c947776750e49d050633f830546bbdc6, #18994).

rpc: introduce HelpElision variant and ElideGroup helper

Switch RPCResultOptions::print_elision to a variant
<HelpElisionNone, HelpElisionSkip, std::string> and add a constructor
that copies a result with replacement options.

Use the new ElideGroup() in TxDoc() to apply elision to field groups
instead of setting print_elision per field. Type::ELISION stays as a
deprecated alias.

test_framework: BIP 434 support