Bitcoin Core

Merge bitcoin/bitcoin#36136: rpc: remove stale "canonical form" claim from getdescriptorinfo help

1dad06eff3d564a56b72d35b7984ce868957daa7 remove stale canonical form claim from getdescriptorinfo help (Craig Raw)

Pull request description:

`getdescriptorinfo` describes its `descriptor` result as:

> The descriptor in canonical form, without private keys.

The returned string is a re-serialisation of the parsed descriptor with private keys removed. It is not a canonical form: descriptors that describe the same wallet routinely come back as different strings with different checksums, and this is deliberate.

Three things the RPC does not canonicalise:

- **The hardened derivation marker.** #26076 added `m_apostrophe` so that the marker the caller used is preserved rather than rewritten (`src/script/descriptor.cpp:262`, `:512`); it first shipped in v26.0. Before that, `FormatHDKeypath()` emitted `'` unconditionally (v25.0, `src/util/bip32.cpp:54`), so `h` supplied by the caller was rewritten — the behaviour #15740 objected to under the name "canonicalize". Where a single key expression mixes both markers, the style of its last hardened element is applied to the whole expression, which is still input-dependent.
- **Key order in `multi()` / `sortedmulti()`.** The order is preserved as given. For `sortedmulti()` the written order carries no meaning at all, since BIP 383 sorts the derived keys when the output script is built, so the same wallet has n! equally valid descriptors.
- **The checksum.** The `checksum` field is computed over the *input* string (`src/rpc/output_script.cpp:215`), so it can differ from the checksum embedded in the returned `descriptor` — e.g. when a private key was supplied.

```
$ bitcoin-cli getdescriptorinfo "sortedmulti(1,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)" | jq -r .descriptor
sortedmulti(1,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235)#fne5696l

$ bitcoin-cli getdescriptorinfo "sortedmulti(1,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)" | jq -r .descriptor
sortedmulti(1,04a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd5b8dec5235a0fa8722476c7709c02559e3aa73aa03918ba2d492eea75abea235,03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)#w5gau8hw
```

Both produce the same output script; neither is more canonical than the other. The same applies to the marker:

```
$ bitcoin-cli getdescriptorinfo "wpkh([f6bb4c63/0h/0h/30h]028429a37c3f09c8c5cc1fab58df32d1a7da7616c748a40eeb1aae1d64acb9c5cc)" | jq -r .descriptor
wpkh([f6bb4c63/0h/0h/30h]028429a37c3f09c8c5cc1fab58df32d1a7da7616c748a40eeb1aae1d64acb9c5cc)#vk9vfu0h

$ bitcoin-cli getdescriptorinfo "wpkh([f6bb4c63/0'/0'/30']028429a37c3f09c8c5cc1fab58df32d1a7da7616c748a40eeb1aae1d64acb9c5cc)" | jq -r .descriptor
wpkh([f6bb4c63/0'/0'/30']028429a37c3f09c8c5cc1fab58df32d1a7da7616c748a40eeb1aae1d64acb9c5cc)#5wdxpxcx
```

The wording dates from v0.18.0 (`src/rpc/misc.cpp:153`), where it did describe the behaviour, and has been carried forward unchanged since. Whether the RPC should canonicalise was settled in #15740 in favour of round-tripping what the caller supplied; this only brings the description into line with that outcome.

I have deliberately not replaced the phrase with "normal form" or "normalized". Those terms already denote a different transformation in this codebase — BIP 380's "Normalization of Key Expressions with Hardened Derivation", implemented as `Descriptor::ToNormalizedString()` ("Normalized descriptors have the xpub at the last hardened step", `src/script/descriptor.h:140`) — which `getdescriptorinfo` does not perform. Dropping the qualifier avoids the collision.

Documentation only; no behaviour change. The phrase occurs nowhere else in the repo.

```diff
-{RPCResult::Type::STR, "descriptor", "The descriptor in canonical form, without private keys. For a multipath descriptor, only the first will be returned."},
+{RPCResult::Type::STR, "descriptor", "The descriptor, without private keys. For a multipath descriptor, only the first will be returned."},
```

If reviewers would rather the help positively state what is preserved (hardened marker and key order as supplied, checksum recomputed), I am happy to expand it; I have kept the change minimal.

ACKs for top commit:
l0rinc:
ACK 1dad06eff3d564a56b72d35b7984ce868957daa7
Eunovo:
ACK https://github.com/bitcoin/bitcoin/pull/36136/commits/1dad06eff3d564a56b72d35b7984ce868957daa7:
rkrux:
lgtm ACK 1dad06eff3d564a56b72d35b7984ce868957daa7

Tree-SHA512: 7b03100fdbc71c867d26094eb3975dfe967875a977ef3c7264b8bb3b920db319d6f11dbde1d39bb70fff2b2288eca42aadffc215ab7503dcc7b3bf62fc2692e5

Merge bitcoin/bitcoin#36169: http: Use SO_EXCLUSIVEADDRUSE on Windows

bcb09b3f4aec73b5e17d1266ec21983d7add118c qa: Verify HTTP listen port exclusivity (Hodlinator)
af65069fd15bbfca5c93b6099a9cc1d5b8de30bc windows: Use SO_EXCLUSIVEADDRUSE over SO_REUSEADDR (Hodlinator)

Pull request description:

#### Problem

`HTTPServer::BindAndStartListening()` unconditionally enables `SO_REUSEADDR` before binding the RPC listener. On Windows, a reuse-enabled listener does not reserve the port exclusively: another local process can request `SO_REUSEADDR` and bind to the same port (see https://learn.microsoft.com/en-us/windows/win32/winsock/using-so-reuseaddr-and-so-exclusiveaddruse).

If the competing socket receives a new connection, it can capture the HTTP Basic `Authorization` header (including the cookie credential) and proxy or issue privileged RPC calls as the victim. This crosses a local-user boundary and can expose wallet-controlling RPC credentials.

#### Fix

Have Windows use `SO_EXCLUSIVEADDRUSE` instead which makes the port exclusive to the process which first requests it, while retaining the restart-friendly behavior which `SO_REUSEADDR` enabled. Abort if another process is already bound to the port.

#### Further context & rationale

This issue is new in our homegrown HTTP server implementation, since libevent had a guard against setting `SO_REUSEADDR` on Windows, see `evutil_make_listen_socket_reuseable()` https://github.com/libevent/libevent/blob/d82464a277d0f42703702c4dfd9af6af38595a83/evutil.c#L483. libevent does not reference `SO_EXCLUSIVEADDRUSE`.

Why should we not just avoid `SO_REUSEADDR` on Windows and skip `SO_EXCLUSIVEADDRUSE` like the libevent approach?
Because setting either option makes the process less prone to failing to bind to a port after having been restarted. Not sure why this wasn't an issue before, maybe the node startup was usually slow enough to time out the port before we tried to re-bind it on Windows.

---

Discovered by Project Loupe.

ACKs for top commit:
pinheadmz:
ACK bcb09b3f4aec73b5e17d1266ec21983d7add118c
sedited:
utACK bcb09b3f4aec73b5e17d1266ec21983d7add118c
jeanpablojp:
tACK bcb09b3f4aec73b5e17d1266ec21983d7add118c

Tree-SHA512: 7f2362cc8399e8c4e95b27b39066d3e591b5aebfc2b562aba10786609456f526f562394818a3d1042d64dabf497548ffabf0757322cfb201454a134321108cf5

Merge bitcoin/bitcoin#36123: http: throttle per-connection reads while a request is in flight

3d1004cb9b8f27bd328d95b4c7524e878c296d61 http: throttle per-connection reads while a request is in flight (Matthew Zipkin)

Pull request description:

This patches a memory exhaustion scenario found while auditing the new http server with kimi-k3. A shallow version of this scenario was addressed in #35735 (See https://github.com/bitcoin/bitcoin/pull/35735#discussion_r3720177656 and https://github.com/bitcoin/bitcoin/pull/35735#issuecomment-5217000202) but a OOM vector still remained.

On master when the sever is busy handling a request from a client, it will still read data from that client and "queue up" the next request. In #35735 we handled the scenario where that additional incoming data was an invalid HTTP request by not attempting to parse the data. However, we didn't add a size limit.

A misbehaving client could block its request queue with something like `waitforblock` and then flood the server with nonsense data without any limit.

The solution in this patch is to not even read from the socket at all if we are busy with a request. Similar to the intent of #35735, the kernel will buffer incoming data until backpressure kicks in and the TCP window drops to 0.

If unaddressed, the attack vector is still limited to authenticated clients: unauthenticated REST requests don't block for very long, so the server *should* be able to drain the receive buffer.

ACKs for top commit:
jeanpablojp:
tACK 3d1004cb9b8f27bd328d95b4c7524e878c296d61
frankomosh:
ACK 3d1004cb9b8f27bd328d95b4c7524e878c296d61
hodlinator:
ACK 3d1004cb9b8f27bd328d95b4c7524e878c296d61
winterrdog:
tACK 3d1004cb9b8f27bd328d95b4c7524e878c296d61
sedited:
ACK 3d1004cb9b8f27bd328d95b4c7524e878c296d61

Tree-SHA512: 56f7678a9ab6789aa542c1f252df0b6ccf9137cb426ff915a0a3fe8285200fdb62b7a47c476ed8617c3592e7a7eac18158cd8c0dac309cdcf4e5fd887e016209

Merge bitcoin/bitcoin#36163: test: Add coverage for unsatisfiable locktime combination in PSBT `ComputeTimeLock()`

69a640e05ec072c8bdce8d52dfd982df274e8262 test: Add coverage for unsatisfiable locktime combination in PSBT ComputeTimeLock (nebula-21)

Pull request description:

This PR adds a test case to `psbt2_timelock_test` covering an unsatisfiable locktime combination in `PartiallySignedTransaction::ComputeTimeLock()`.

When different PSBT v2 inputs specify their own timelock requirement, `ComputeTimeLock()` needs to reconcile all of those into a single locktime for the whole transaction. To reconcile this locktime, all the inputs locktimes need to be height or time-based, but not a mix of them.

The existing test already covers this failure when the input #0 is height-based and a later input is time-based, returning `std::nullopt`.
This PR adds the other case when the input #0 is time-based and a later input is height-based, returning `std::nullopt`.
I've basically swapped the PSBT inputs from the already existing case to cover this one.

ACKs for top commit:
sedited:
ACK 69a640e05ec072c8bdce8d52dfd982df274e8262

Tree-SHA512: e7a7556df3bd278686a2d53a11b228f6f8c0e8dda79f050bad83dea89824de2e3766518fa3450b3722be86927fb4e6b9061dcd2cdb45010080b37a4fa2baffe0

Merge bitcoin/bitcoin#35958: net: align v2 message type validation with v1 range

cc577de954671378268bab2a78714869b4f7388d net: align v2 message type validation with v1 range (Bruno Garcia)

Pull request description:

BIP324 specifies the 13-byte long-form message type encoding as "an ASCII message type (as in the v1 P2P protocol)", but V2Transport::GetMessageType() accepted bytes up to 0x7F, while for V1 it only accepts printable ASCII (0x20-0x7E).

This changes V2 to match V1 on it and add test coverage.

ACKs for top commit:
nervana21:
tACK cc577de954671378268bab2a78714869b4f7388d
ajtowns:
utACK cc577de954671378268bab2a78714869b4f7388d
w0xlt:
ACK cc577de954671378268bab2a78714869b4f7388d
sedited:
ACK cc577de954671378268bab2a78714869b4f7388d

Tree-SHA512: 8c97ee20df2311949bbe9655c7e04507c4b47d3b18766aa6ae51691d0870f8a5c25ea54d74c9afb797754572d057b4240533da6bf3c2e0435f3cb32c5fb1c3af

Merge bitcoin/bitcoin#36131: rpc: Improve two field's OpenRPC types

78e691ea10a17092dd65cf0b22c814e9fb3438f0 rpc: Change listunspent's ancestorfees type to NUM (sedited)
73fb9ced563cdf59bfee9462025e32e73f08ecd1 rpc: Fix private key type in signrawtransactionwithkey (sedited)

Pull request description:

This corrects the types for two fields in the OpenRPC dump. Both changes have no effect on the rpc help output. The changes to the schema's format are:

```diff
diff dump.json dump_new.json
11452d11451
< "x-bitcoin-unit": "amount",
13844,13845c13843
< "type": "string",
< "pattern": "^[0-9a-fA-F]+$"
---
> "type": "string"
```

I asked Claude to flag any inconsistencies in the dump and these were the two, out of many others, that I thought were worthwhile to fix.

ACKs for top commit:
maflcko:
lgtm ACK 78e691ea10a17092dd65cf0b22c814e9fb3438f0
stickies-v:
ACK 78e691ea10a17092dd65cf0b22c814e9fb3438f0
musaHaruna:
Tested ACK [78e691e](https://github.com/bitcoin/bitcoin/pull/36131/changes/78e691ea10a17092dd65cf0b22c814e9fb3438f0)

Tree-SHA512: 121d80520a39738c1c7375a50bb552203fe2db403cb3414195e6a79142677ac3c3509ba5f18d4b1982a8e2872c73e47cf6e54b6acd66b1a71ddcbe335ea33f34

Merge bitcoin/bitcoin#35477: test: exercise Schnorr signature cache in txvalidationcache_tests.cpp

3ba1bbfa3f85fae32c0b6614db9bb1a1b2db8ce7 test: exercise Schnorr signature cache in txvalidationcache_tests.cpp (Sebastian Falbesoner)
198b36bc85d54792af82ea9b9e58554608573cb6 test: respect "TAPROOT requires WITNESS" rule in `ValidateCheckInputsForAllFlags` (Sebastian Falbesoner)
e78a2a0d00cf67637eae4caf4db1735f110aff14 test: refactor: simplify tx vin/vout creation in txvalidationcache_tests.cpp (Sebastian Falbesoner)

Pull request description:

The Schnorr verification path of the signature cache is currently never hit in the unit tests, i.e. with the following patch they still pass:
```diff
diff --git a/src/script/sigcache.cpp b/src/script/sigcache.cpp
index c6fcc8f8eb..87688c1049 100644
--- a/src/script/sigcache.cpp
+++ b/src/script/sigcache.cpp
@@ -44,6 +44,7 @@ void SignatureCache::ComputeEntryECDSA(uint256& entry, const uint256& hash, cons

void SignatureCache::ComputeEntrySchnorr(uint256& entry, const uint256& hash, std::span<const unsigned char> sig, const XOnlyPubKey& pubkey) const
{
+ assert(false);
CSHA256 hasher = m_salted_hasher_schnorr;
hasher.Write(hash.begin(), 32).Write(pubkey.data(), pubkey.size()).Write(sig.data(), sig.size()).Finalize(entry.begin());
}
```
This PR adds missing coverage for that by adding a Taproot key-path spend to `checkinputs_test` in `txvalidationcache_tests.cpp`. Same as for the already-existing ECDSA spends, the caching is tested across a large number of flag combinations (using `ValidateCheckInputsForAllFlags`), both with an invalid Schnorr signature (-> should only fail if `SCRIPT_VERIFY_TAPROOT` is set) and a valid one (-> should pass for all flag combinations).

ACKs for top commit:
Bortlesboat:
tACK 3ba1bbfa3f
sedited:
ACK 3ba1bbfa3f85fae32c0b6614db9bb1a1b2db8ce7
instagibbs:
ACK 3ba1bbfa3f85fae32c0b6614db9bb1a1b2db8ce7

Tree-SHA512: e43f7077d9e9ab6f8b5e9e70f0187767d65f686ce24350ce5d61cc4cdf07d5eebdf5e4327ce665c212b7cc02be1e8632a6a9fbcf6be2916f8e058993e5fb2650

Merge bitcoin/bitcoin#36054: test: add script_tests cases covering interpreter mutants

4a12773f269742d2c655beb1b3f5ffe98e9beadb test: cover DERSIG rejects a non-compound signature type (ViniciusCestarii)
86c7fb910d674c757c4829cd006b3732d3a5e78b test: cover OP_16 does not count towards the opcode limit (ViniciusCestarii)
331bf798819ca4f2815450ba50d5920e2d2c2aa6 test: cover OP_WITHIN must pop all 3 elements (ViniciusCestarii)
3bb87bc61b489911cf36e50f74ac8b6ee201b82b test: cover OP_FROMALTSTACK must pop the altstack (ViniciusCestarii)

Pull request description:

Kills some live mutants on interpreter.cpp that affect consensus found by https://bitcoincore.space. They are:

<details>
<summary><a href="https://bitcoincore.space/src/script/interpreter.cpp#3951">interpreter.cpp#3951</a>: <code>OP_FROMALTSTACK</code>: removed <code>popstack(altstack)</code></summary>

```diff
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 98b16eca6b..68265d20b5 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -698,7 +698,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
if (altstack.size() < 1)
return set_error(serror, SCRIPT_ERR_INVALID_ALTSTACK_OPERATION);
stack.push_back(altstacktop(-1));
- popstack(altstack);
+
}
break;
```

</details>

<details>
<summary><a href="https://bitcoincore.space/src/script/interpreter.cpp#4084">interpreter.cpp#4084</a>: <code>OP_WITHIN</code>: removed one <code>popstack(stack)</code></summary>

```diff
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 98b16eca6b..874cf5e1cf 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -1018,7 +1018,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&
CScriptNum bn2(stacktop(-2), fRequireMinimal);
CScriptNum bn3(stacktop(-1), fRequireMinimal);
bool fValue = (bn2 <= bn1 && bn1 < bn3);
- popstack(stack);
+
popstack(stack);
popstack(stack);
stack.push_back(fValue ? vchTrue : vchFalse);
```

</details>

<details>
<summary><a href="https://bitcoincore.space/src/script/interpreter.cpp#3883">interpreter.cpp#3883</a>: opcode limit: <code>opcode > OP_16</code> → <code>opcode >= OP_16</code></summary>

```diff
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 98b16eca6b..e985643606 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -459,7 +459,7 @@ bool EvalScript(std::vector<std::vector<unsigned char> >& stack, const CScript&

if (sigversion == SigVersion::BASE || sigversion == SigVersion::WITNESS_V0) {
// Note how OP_RESERVED does not count towards the opcode limit.
- if (opcode > OP_16 && ++nOpCount > MAX_OPS_PER_SCRIPT) {
+ if (opcode >= OP_16 && ++nOpCount > MAX_OPS_PER_SCRIPT) {
return set_error(serror, SCRIPT_ERR_OP_COUNT);
}
}
```

</details>

<details>
<summary><a href="https://bitcoincore.space/src/script/interpreter.cpp#3808">interpreter.cpp#3808</a>: <code>IsValidSignatureEncoding</code>: compound type check returns <code>true</code></summary>

```diff
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 98b16eca6b..b613a6ac19 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -133,7 +133,7 @@ bool static IsValidSignatureEncoding(const std::vector<unsigned char> &sig) {
if (sig.size() > 73) return false;

// A signature is of type 0x30 (compound).
- if (sig[0] != 0x30) return false;
+ if (sig[0] != 0x30) return true;

// Make sure the length covers the entire signature.
if (sig[1] != sig.size() - 3) return false;
```

</details>

Recommend reviewing per commit.

ACKs for top commit:
instagibbs:
ACK 4a12773f269742d2c655beb1b3f5ffe98e9beadb
brunoerg:
ACK 4a12773f269742d2c655beb1b3f5ffe98e9beadb
jeanpablojp:
tACK 4a12773f269742d2c655beb1b3f5ffe98e9beadb

Tree-SHA512: 5f53c733d11cb5d645f420d90ab626f894ef0bb155d01b9de0cae502109b2eaa46c072797d08df115da7a8738f01f31212a207a4d0e6f782128beb37332cf46e

Merge bitcoin/bitcoin#36111: rpc: bound memory for overlong Bech32 errors

7fcaccd9d0b7b9ec80b8224239aeb3c13b90593d bech32: bound overlength error locations (Lőrinc)

Pull request description:

**Problem:** `validateaddress` reports likely error positions for invalid Bech32 inputs, including multiple useful positions for character and checksum errors.
For an overlength input, `LocateErrors()` returns every position after the 90-character limit, which the RPC converts to a `UniValue` number before serializing the response.
A near-limit authenticated request therefore creates about 33 million `int` values and 33 million `UniValue` objects.

**Fix:** Return position 90 for an overlength input, which identifies where the single length violation begins.
Character and checksum errors continue to return multiple useful positions when they can be determined.
The tests now include an oversized example and pin the bounded result.

**Reproducer:** Peak memory usage for a near-limit authenticated request:

<details>
<summary>Linux reproducer</summary>

```bash
sed -i "/def test_validateaddress(self):/a\\
self.nodes[0].validateaddress('bcrt1' + 'q' * (2**25 - 100))\\
__import__('time').sleep(30)" test/functional/rpc_invalid_address_message.py
cmake -B build && cmake --build build -j2
build/test/functional/rpc_invalid_address_message.py >/dev/null 2>&1 &
sleep 20 && awk '/VmHWM/' /proc/$(pgrep bitcoind)/status
```
</details>

```text
Before ████████████████████████ 5.69 GiB
After █░░░░░░░░░░░░░░░░░░░░░░░ 240 MiB
```

ACKs for top commit:
maflcko:
lgtm ACK 7fcaccd9d0b7b9ec80b8224239aeb3c13b90593d
sedited:
ACK 7fcaccd9d0b7b9ec80b8224239aeb3c13b90593d
janb84:
ACK 7fcaccd9d0b7b9ec80b8224239aeb3c13b90593d

Tree-SHA512: 3d439774d394f081b8107f8131963f7aa23ed048b0d6d349a80f9b3481fefeef7b5ce239fbd33606ad1f4960e6bfd899f968c50febc70d38b2fe731c6049583f

Merge bitcoin/bitcoin#36092: fix: UB sanitizer in mempool estimator logging

576a0ebb536f93469a1ced6d497e11adf7c6a74d fix: UB sanitizer in mempool estimator logging (rustaceanrob)

Pull request description:

The following is failing in CI, when a block has `m_height` of 64 bit max:
```
SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /home/runner/work/_temp/src/policy/fees/mempool_estimator.cpp:210:62
MS: 0 ; base unit: 0000000000000000000000000000000000000000
0x1,0x0,0x0,0x0,0x3,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x1,0x0,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x0,0x0,0x26,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0xd6,0x1,0x0,0x86,0x0,0x0,0x0,0x2a,0x0,0xff,0xff,0xff,
\001\000\000\000\003\377\377\377\377\377\377\377\377\001\000\377\377\377\377\377\377\377\377\000\000&\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z??\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z???\377\377\377\377\377\030\000\000\205?\377\377\377\377\377\030\000\000\205\326\001\000\206\000\000\000*\000\377\377\377
artifact_prefix='./'; Test unit written to ./crash-b4333d1fe3993fe8610b86654e385682c23050b9
Base64: AQAAAAP//////////wEA//////////8AACYAAAAAAAAAAAAAAAAAAAAAAAAvAAMCAgICAgICAAAAAAAAAHo/PwAALwADAgICAgICAgAAAAAAAAB6Pz8///////8YAACFP///////GAAAhdYBAIYAAAAqAP///w==

⚠️ Failure generated from target with exit code 1: ['/home/runner/work/_temp/build_ ₿🧪_/bin/fuzz', '-runs=1', PosixPath('/home/runner/work/_temp/ci/scratch_ ₿🧪_/qa-assets/fuzz_corpora/policy_estimator_io')]
Check if using libFuzzer ... True
Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', '8100bf684275e706787e07f8ab94431926ba5184562c52bce95c932210f6f38f', '/home/runner/work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 1.

```

ACKs for top commit:
maflcko:
lgtm ACK 576a0ebb536f93469a1ced6d497e11adf7c6a74d
marcofleon:
ACK 576a0ebb536f93469a1ced6d497e11adf7c6a74d
jeanpablojp:
tACK 576a0ebb536f93469a1ced6d497e11adf7c6a74d

Tree-SHA512: a7533e68a95b2f0200abdcf08ae72a7f3654db03cffe642ed39b0a5aa48d932a8b07ee4e4477b591a488b5358dd19fe85a03015642b663fd41a771b3090bc8a3

fix: UB sanitizer in mempool estimator logging

The following is failing in CI, when a block has `m_height` of 64 bit
max:
```
SUMMARY: UndefinedBehaviorSanitizer: unsigned-integer-overflow /home/runner/work/_temp/src/policy/fees/mempool_estimator.cpp:210:62
MS: 0 ; base unit: 0000000000000000000000000000000000000000
0x1,0x0,0x0,0x0,0x3,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x1,0x0,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0xff,0x0,0x0,0x26,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x0,0x0,0x2f,0x0,0x3,0x2,0x2,0x2,0x2,0x2,0x2,0x2,0x0,0x0,0x0,0x0,0x0,0x0,0x0,0x7a,0x3f,0x3f,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0x3f,0xff,0xff,0xff,0xff,0xff,0x18,0x0,0x0,0x85,0xd6,0x1,0x0,0x86,0x0,0x0,0x0,0x2a,0x0,0xff,0xff,0xff,
\001\000\000\000\003\377\377\377\377\377\377\377\377\001\000\377\377\377\377\377\377\377\377\000\000&\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z??\000\000/\000\003\002\002\002\002\002\002\002\000\000\000\000\000\000\000z???\377\377\377\377\377\030\000\000\205?\377\377\377\377\377\030\000\000\205\326\001\000\206\000\000\000*\000\377\377\377
artifact_prefix='./'; Test unit written to ./crash-b4333d1fe3993fe8610b86654e385682c23050b9
Base64: AQAAAAP//////////wEA//////////8AACYAAAAAAAAAAAAAAAAAAAAAAAAvAAMCAgICAgICAAAAAAAAAHo/PwAALwADAgICAgICAgAAAAAAAAB6Pz8///////8YAACFP///////GAAAhdYBAIYAAAAqAP///w==

⚠️ Failure generated from target with exit code 1: ['/home/runner/work/_temp/build_ ₿🧪_/bin/fuzz', '-runs=1', PosixPath('/home/runner/work/_temp/ci/scratch_ ₿🧪_/qa-assets/fuzz_corpora/policy_estimator_io')]
Check if using libFuzzer ... True
Command '['docker', 'exec', '--env', 'DANGER_RUN_CI_ON_HOST=1', '8100bf684275e706787e07f8ab94431926ba5184562c52bce95c932210f6f38f', '/home/runner/work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 1.

```

Merge bitcoin/bitcoin#36077: bugfix: give TxDownloadManager its own RNG

80eaa6cabf28b7d68c61fc5aced189b36970f17e bugfix: give TxDownloadManager its own RNG (Greg Sanders)

Pull request description:

TxDownloadManagerImpl retains a reference to PeerManagerImpl::m_rng,
which is non-thread-safe and guarded by g_msgproc_mutex.

BlockConnected runs on the validation background thread while holding
only m_tx_download_mutex. Reconsidering an orphan with multiple
announcers could therefore use m_rng concurrently with message
processing.

Regression introduced in #35986

Added a regression test on second commit, can remove it from the PR if deemed superfluous.

This is a Project Loupe find.

ACKs for top commit:
maflcko:
review ACK 80eaa6cabf28b7d68c61fc5aced189b36970f17e 🐓
hodlinator:
ACK 80eaa6cabf28b7d68c61fc5aced189b36970f17e
sedited:
ACK 80eaa6cabf28b7d68c61fc5aced189b36970f17e

Tree-SHA512: 2dbc4a9298bfa1375dc364ead4b1ec74c2ebe54fb7c311180fa06fc32240406be2979a2dd6ae0e7a23b099ddcd63f5c76c126d84a1405fc7eacd337eb009dd88

Merge bitcoin/bitcoin#36025: psbt: avoid duplicate taproot leaf script keys when merging

1cb416397b7aa4addd714f9ab68ae0a861ff81c3 psbt: avoid duplicate taproot leaf script keys when merging (Shuvam Pandey)

Pull request description:

Follow-up to #35665, which fixed the same combiner defect for `PSBT_GLOBAL_XPUB`. thomasbuilds
and winterrdog asked for this one as its own PR when I reported it there.

`m_tap_scripts` maps a leaf script to a set of control blocks, but is serialized as one record
per control block, keyed by the control block (`SerializeToVector(s, PSBT_IN_TAP_LEAF_SCRIPT,
std::span{control_block})`). `PSBTInput::Merge` unions it by the map key, so two PSBTs that map
the same control block to different leaf scripts merge into an input that serializes the `0x15`
key twice. Duplicate keys make a PSBT invalid, so it is the same `combinepsbt` then
`decodepsbt` failure as the xpub case, at the input level. Present since #22558 (v24.0).

Both decode on their own, and differ only in the leaf script the control block maps to, `OP_1`
against `OP_1 OP_1`:

```
$ A=cHNidP8BADwCAAAAAaqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAIhXAUJKbdMGgSVS3i0tgNel6XgeKWg8o7JbVR7/ums6AOsACUcAAAA==
$ B=cHNidP8BADwCAAAAAaqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqAAAAAAAAAAAAAQAAAAAAAAAAAAAAAAAAIhXAUJKbdMGgSVS3i0tgNel6XgeKWg8o7JbVR7/ums6AOsADUVHAAAA=
$ bitcoin-cli -regtest decodepsbt "$(bitcoin-cli -regtest combinepsbt "[\"$A\",\"$B\"]")"
error code: -22
error message:
TX decode failed Duplicate Key, input key "15c050929b74c1a04954b78b4b6035e97a5e078a5a0f28ec96d547bfee9ace803ac0" already provided: unspecified iostream_category error
```

winterrdog reproduced it on the #35665 thread with another pair.

Merge the records rather than the map entries, keeping the leaf script already there. BIP 174
lets the combiner "pick arbitrarily when conflicts occur", and unknown and proprietary records
already resolve that way. Refusing to combine is the BIP's other option, but that would fail
`combinepsbt` on input it accepts today.

Merging by map key drops records as well. `std::map::insert` leaves existing keys alone, so
when both PSBTs carry the same leaf script with different control blocks, the incoming set was
dropped. Those keys do not conflict, so merging per record keeps them.

The control blocks already present are collected once per merge rather than searched for per
incoming record, which would be quadratic in the size of the two PSBTs `combinepsbt` takes from
the caller.

Since this is the second field with this shape I checked the rest. `m_xpubs` (#35665) and
`m_tap_scripts` are the only two whose record key comes from the value, so two map entries can
serialize the same key. `partial_sigs` is keyed by `CKeyID` and serialized under the pubkey,
but the pubkey determines the `CKeyID`, so those records stay distinct. The others key the
record by the map key, `m_proprietary` included, and `PSBTOutput` has no such field.

The test fails on master on both counts, and covers the merges that do not conflict as well.

I found this with a local assertion in the psbt fuzz target that a combined PSBT must
roundtrip. That assertion can go in a follow-up.

Tested:

```
./build/bin/test_bitcoin --run_test=psbt_tests
./build/bin/test_bitcoin --run_test=psbt_wallet_tests
./build/test/functional/test_runner.py rpc_psbt.py rpc_rawtransaction.py wallet_taproot.py wallet_signer.py feature_taproot.py wallet_basic.py
```

ACKs for top commit:
achow101:
ACK 1cb416397b7aa4addd714f9ab68ae0a861ff81c3
winterrdog:
re-ACK 1cb416397b7aa4addd714f9ab68ae0a861ff81c3

Tree-SHA512: 2599beffe701ba9b672e8dcc3d43844f3853ceeb3d86fc53798a428d3288aa8edd2e42f338b32a1046fa558d6cc5cfa5d55b3a0aaf960278b0f3002158381623

Merge bitcoin/bitcoin#35516: rpc: preserve global xpubs and proprietary fields in joinpsbts

436921eb469a1c4dde9f46e2fbad231da0e17e7a test: check joinpsbts preserves global xpubs and proprietary fields (Thomas)
011094b282b22484090213a20a4c04b32c0d90fc rpc: preserve global xpubs and proprietary fields in joinpsbts (Thomas)

Pull request description:

`joinpsbts` collects the global xpubs of all the joined PSBTs into `merged_psbt`, but returns a separately constructed `shuffled_psbt` into which only the inputs, outputs, and unknown fields are copied. The collected `PSBT_GLOBAL_XPUB` records are silently dropped, and `PSBT_GLOBAL_PROPRIETARY` records are not collected at all.

The xpub collection was added in #17034, which was written against a `joinpsbts` that still returned `merged_psbt`, but was merged after #16512 had introduced the `shuffled_psbt` rebuild, so the collected xpubs have never reached the result.

Shuffle the inputs and outputs of `merged_psbt` in place instead of rebuilding a new PSBT, so that all global data is preserved, and union the global proprietary records in the merge loop, matching the `combinepsbt` behavior from #34893.

ACKs for top commit:
jpk68:
ACK 436921eb469a1c4dde9f46e2fbad231da0e17e7a
achow101:
ACK 436921eb469a1c4dde9f46e2fbad231da0e17e7a
winterrdog:
tACK 436921eb469a1c4dde9f46e2fbad231da0e17e7a

Tree-SHA512: d9de34c25aecc29b6b4fb80d6584fa919cc5ff9b7ef2f4d8ce35c4043fe7638fefb8af10448f2cd14021f5d25e149f0efc8798c5b8c9bc8b5582c6152010e891

Merge bitcoin/bitcoin#34697: descriptor: fix musig() duplicate key checks and doubled PSBT origin paths

b42f7fade0cd9a6fb3d69fcdff71436a99ba0005 descriptor: don't prepend key origins twice (Shuvam Pandey)
7b15e2cb442c0bfec76d7d4b9abdd31c7a538da8 descriptor: fix duplicate check for hardened keys (Shuvam Pandey)

Pull request description:

Fixes #34273.

Importing a descriptor that uses the same `musig()` participants twice in one
tapleaf, with different musig subderivations, fails with
`is not sane: contains duplicate public keys`. It only fails when one of the
participants is a private key on a hardened path. The all-xpub version of the
same descriptor imports fine. That's what gave it away.

The duplicate check (`KeyCompare`) resolves each key expression to a pubkey and
compares the results. It does this at index 0, and the old code used an empty
signing provider. With that empty provider, a `musig()` expression can't resolve
when one of its participants is on a hardened path, because deriving that
participant needs its private key, so the whole aggregate key comes back empty.
Two different musig expressions both came back empty, so the check treated them
as duplicates. The fix derives against the signing provider populated during
parsing, which holds the private keys, and only compares the expression strings
when neither side resolves. 151henry151 had suggested looking at the empty
signing provider on the issue.

scgbckbone found a second, separate bug in the same descriptors. When another
expression that reuses those participants is handled in the same expansion, its
participant origin in the PSBT is added twice, so `m/86h/1h/0h` becomes
`m/86h/1h/0h/86h/1h/0h` in both the input and output Taproot BIP32 derivation
maps. `OriginPubkeyProvider::GetPubKey()` now derives into a temporary provider,
merges it, and writes the corrected origin once, so a later expression can't
prepend the same origin again.

Tested:
```
./build/bin/test_bitcoin --run_test=descriptor_tests
./build/bin/test_bitcoin --run_test=miniscript_tests
./build/bin/test_bitcoin --run_test=bip328_tests
./build/bin/test_bitcoin --run_test=psbt_wallet_tests
./build/test/functional/test_runner.py wallet_musig.py --jobs=1
```

ACKs for top commit:
achow101:
ACK b42f7fade0cd9a6fb3d69fcdff71436a99ba0005
scgbckbone:
ACK b42f7fade0cd9a6fb3d69fcdff71436a99ba0005

Tree-SHA512: ab36caa6bc484fa1fc3289c79e9a3d713278d82f80de478e53e1bdbe645037e07776ac798eba085733abc139c11a9dbf0d3f49d3c0eee9632e4ddf33d2242f92

Merge bitcoin/bitcoin#35933: psbt: don't abort on invalid MuSig2 derivations

73a94b45459a9433ffcf99aff3b044cc7166ee12 psbt: avoid aborting on invalid MuSig2 derivations (Lőrinc)
e3d1e75a519a3542a736c3097ce8ab3a2bd8dc5c test: characterize MuSig2 derivation aborts (Lőrinc)

Pull request description:

**Problem:** A PSBT may contain MuSig2 derivation metadata with a hardened child index or a path that derives to a different key.
The hardened index aborts during public derivation, while the mismatched key aborts at the result assertion.
`analyzepsbt`, `finalizepsbt`, and `descriptorprocesspsbt` all reach this code without a wallet.
Even the read-only `analyzepsbt` can force a co-signer service to restart its node after unexpected input.

**Fix:** Return failure when a MuSig2 derivation path contains a hardened child index, and skip only the current aggregate when the path derives to a different key so another matching aggregate can still be tried.

This follows [#35154](https://github.com/bitcoin/bitcoin/pull/35154), with the related contributions credited in the commits.

ACKs for top commit:
jeanpablojp:
ACK 73a94b45459a9433ffcf99aff3b044cc7166ee12
achow101:
ACK 73a94b45459a9433ffcf99aff3b044cc7166ee12
andrewtoth:
ACK 73a94b45459a9433ffcf99aff3b044cc7166ee12

Tree-SHA512: d8e28c5a4184154a4427c644ce62423cbcccdc3d82a6293f36fe99055fa04714598bc92c43b526fbcc7c99b231140669c2d0f1b853999d7dc33f949564c90504

Merge bitcoin/bitcoin#34075: fees: Introduce Mempool Based Fee Estimation to reduce overestimation

7f9c4e2928991704e9e2224ad32d3b159b2551ac doc: add release notes (ismaelsadeeq)
e18d392689da72d857fc590097889060a48d98e0 test: add mempool estimator i/o fuzz test (ismaelsadeeq)
970f02096d37447d04bf84993f2922a1e6f7d7a7 fees: persist mempool policy estimator data (ismaelsadeeq)
7dcb37989d25bccdb335aaa4aa429588cd26c874 fees: move fee_estimates.dat into fees directory (ismaelsadeeq)
0db2b69e6db7ad07d1d4edfcf3f84c3fde568992 rpc: add verbosity option to estimatesmartfee options (ismaelsadeeq)
06bb65730ebee2dd66bb79566dd29936a366e3eb fees: gate mempool estimates on recent block coverage (ismaelsadeeq)
cfe585df25c482c157e239fcd6c81b7afe3bab00 validation: emit block mempool removal signal from ConnectTip (ismaelsadeeq)
0d88558f95113a5041fa957432fa5d9c12314466 fees: return mempool estimates when it's lower than block policy (ismaelsadeeq)
693b1351aff422c4c75e0f4c3be7571193d236bf fees: add caching to MemPoolFeeRateEstimator (ismaelsadeeq)
c9bb3df29ff9ba2952af1e5349c93657a1483d3f fees: add MemPoolFeeRateEstimator class (ismaelsadeeq)
9cacf677a918ba89b6a47f4ae2fde8d62f1cf384 rpc: add fee_rate_estimator option to estimatesmartfee (ismaelsadeeq)
ba6c61bbdd32d01fb1d20ebcd1e48e3b32cf0932 fees: add FeeRateEstimatorManager class (ismaelsadeeq)
2cb6b831e0304c7caf7953a991b97c15c0c24550 fees: add EstimateFeeRate and MaximumTarget to CBlockPolicyEstimator (ismaelsadeeq)
5adb2ab084333c7a968895848123302b88cb4183 refactor: test block policy estimator directly (ismaelsadeeq)
9c8309a89097fac0128129cdfb2072c334b21a87 test: rename policy estimator tests to block policy estimator tests (ismaelsadeeq)
e3d5ef1b5fb282025751c894cd0f0e0494cd0641 fees: move StringForBlockPolicyEstimateReason to block policy estimator (ismaelsadeeq)
74245c20e05e6fb5844a9bdfca489f107c6c8aca fees: split wallet and estimator fee reasons (ismaelsadeeq)

Pull request description:

This PR is another attempt to fix #27995 using a better approach.

For background and motivation, see #27995 and the discussion in the Delving Bitcoin post [Mempool Based Fee Estimation on Bitcoin Core](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703).

This PR is currently limited to using the mempool only to lower what is recommended by the Block Policy Estimator.
Accurate and safe fee estimation using the mempool is challenging. There are open questions about how to prevent mempool games that are theoretically possible for miners [(a variant of the Finney attack)](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703/6).

This is one reason this PR uses the mempool only to lower the Block Policy Estimator result. The Block Policy Estimator itself is not gameable in this way, so the combined estimate is not susceptible to this attack increasing the returned feerate.

The underlying assumption is that, with the current tools and work done to make RBF and CPFP feasible and reliable (TRUC transaction relay, ephemeral anchors, cluster size 2 package RBF), underestimation is safer than overestimation. We now assume it is relatively easy to fee-bump later if a transaction does not confirm, whereas once a fee is overestimated there is no way to recover from that.

Another open question when using the mempool for fee estimation is how to account for incoming transaction inflow.
[Bitcoin Augur](https://github.com/block/bitcoin-augur) does this by using past inflow plus a constant expected inflow to predict future inflow. I find this unconvincing for fee estimation and potentially prone to more overestimation, as past conditions are not always representative of the future. See my [review of the Augur fee rate estimator and open questions](https://github.com/block/bitcoin-augur/issues/3).

This PR uses a much simpler approach based on current user behavior, similar to the widely used method employed by mempool.space: looking at the top block of the mempool and selecting a percentile feerate depending on whether the user is economical or conservative.

Empirical data from both myself and Clara Shikhelman shows that the 75th percentile feerate for economical users and the 50th percentile feerate for conservative users provide positive confirmation guarantees, hence this is what is used in this PR.

Parallel research by Rene Pickhardt and his student suggests that using the [average fee per byte of the block template performs well](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703/12).

All of these are constants that can be adjusted. There is parallel work exploring these constants and running benchmarks across fee estimators to find a sweet spot.

See also work in LND, the [LND Budget Sweeper](https://delvingbitcoin.org/t/lnds-deadline-aware-budget-sweeper/1512), which applies this idea successfully. Their approach is to estimate fees initially with bitcoind, then increment gradually as the confirmation deadline approaches, using a fixed fee budget.

Historical data indicates that this PR's approach can [reduce overestimation quite significantly (~29%)](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703/8).

This is particularly useful in scenarios where the Block Policy Estimator recommends a high feerate while the mempool is empty.

<img width="1800" height="1090" alt="56f3ba26c0184521c42bb82ec9d8c9f2224d4f8e" src="https://github.com/user-attachments/assets/c035c40c-8ece-42a7-b290-d29f1ac9bf4d" />

As seen in the image above, there is only one remaining unfixed case: when there is a sudden inflow of transactions and the feerate rises, the Block Policy Estimator takes time to reflect this. In that case, users will continue to see a low feerate estimate until it slowly updates. From the historical data linked above, [this occurs about ~26% of the time](https://delvingbitcoin.org/t/mempool-based-fee-estimation-on-bitcoin-core/703/8).

Overall, we observe a **73% success rate with 0% overestimation, and 26% underestimation** with this approach.

See https://bitcoincorefeerate.com/stats for recent running stats that have almost identical data.

This PR also includes refactors that enable this work. Rather than splitting the PR and implementing changes incrementally, I opted for an end-to-end implementation:

### 1. Refactors

* Split the mixed fee reason enum into separate wallet and block policy concepts. The wallet now has a `FeeReason` enum for why the wallet selected a fee rate (`FEE_RATE_ESTIMATOR`, `MEMPOOL_MIN`, `USER_SPECIFIED`, `FALLBACK`, `REQUIRED`), while the Block Policy Estimator uses `BlockPolicyEstimateReason` for its internal threshold details.
* Move `StringForBlockPolicyEstimateReason` to the Block Policy Estimator code, keeping the estimator-specific strings with the estimator.
* Move detailed Block Policy Estimator logging out of wallet transaction creation and into the estimator path. Wallet transaction creation now logs the selected fee and wallet fee reason instead of leaking estimator internals.
* Keep the wallet RPC `fee_reason` field name for compatibility, but update its meaning to report the wallet fee reason instead of the Block Policy Estimator's internal threshold reason.
* Rename policy estimator tests and files to block-policy-specific names where appropriate.
* Update Block Policy Estimator unit tests to be independent of the mempool and validation interface.

### 2. Introduce Mempool-Based Fee Estimator and Fee Estimator Manager

* Introduce `FeeRateEstimation` and `FeeRateEstimationError` as common estimator result types, avoiding new out-parameters for fee estimation results.
* Add `FeeRateEstimatorType` to identify the estimator that produced a result.
* Add `FeeRateEstimatorManager`, responsible for owning the Block Policy Estimator and Mempool Fee Rate Estimator.
* Update the node context to store a `std::unique_ptr` to `FeeRateEstimatorManager` instead of `CBlockPolicyEstimator`.
* Update `CBlockPolicyEstimator` to no longer subscribe directly to the validation interface; instead, `FeeRateEstimatorManager` subscribes and forwards relevant notifications.
* Add a mempool fee estimator that generates a block template when called, calculates a percentile feerate, and returns the 75th percentile for economical mode or the 50th percentile for conservative mode.
* When the selected estimate is below the node's fee floor, `estimatesmartfee` still returns at least the max of `mempoolminfee` and `minrelaytxfee`.
* Add caching to the mempool estimator so new estimates are generated at most every 7 seconds while the chain tip is unchanged, assuming enough [transactions have propagated](https://bitcoin.stackexchange.com/questions/125776/how-long-does-it-take-for-a-transaction-to-propagate-through-the-network/125777#125777) to make a meaningful difference.
This heuristic will likely be replaced by requesting block templates via the general-purpose block template cache proposed here: https://github.com/bitcoin/bitcoin/issues/33389
* Update `MempoolTransactionsRemovedForBlock` to receive the connected block as well as the transactions removed from the mempool.
* Track the weight of block transactions and mempool transactions removed due to block connection after each block connection.
This data is tracked for the last 6 mined blocks. A mempool feerate estimate is returned only when the ratio of mempool transaction weight removed due to block connection to block transaction weight is greater than 75% across the tracked window. This heuristic provides rough confidence that the node's mempool matches that of the majority of the hashrate. The 75% threshold is arbitrary and can be adjusted.

There is a caveat when transactions in the local mempool are consistently not mined by the network, as described in #27995 (e.g. due to filtering).
Accounting for these transactions during fee estimation is not necessary, as they should be evicted from the mempool itself (see #33510). Handling this again within fee estimation would be redundant.

* Persist statistics for the 6 most recent mined blocks to `fees/mempool_policy_estimator.dat` during periodic flushes and shutdown, so this data is available after restarts.
* Move Block Policy Estimator data from `fee_estimates.dat` to `fees/block_policy_estimates.dat`, migrating the legacy file during startup when needed.
* Add `fee_rate_estimator` to the `estimatesmartfee` options object. Supported values are `"none"` (default combined behavior), `"block_policy"` (use only the Block Policy Estimator), and `"mempool_policy"` (use only the Mempool Fee Rate Estimator). Unknown values are treated as `"none"`.
* Add `verbosity` to the `estimatesmartfee` options object. With `verbosity >= 2`, the RPC returns recent mempool health statistics.
* Expose the selected fee rate estimator in `estimatesmartfee` results when `fee_rate_estimator` is `"none"` and the estimate succeeds.
* Add unit, functional, and fuzz test coverage for the new estimator behavior, persistence, RPC options, and estimator I/O.

<details>
<summary>see example output</summary>

```bash
bitcoin-cli estimatesmartfee 1 economical '{"verbosity": 2, "fee_rate_estimator": "none"}'
```

```json
{
"feerate": 0.00002133,
"estimator": "mempool_policy",
"blocks": 2,
"mempool_health_statistics": [
{
"block_height": 927953,
"block_weight": 3991729,
"mempool_txs_weight": 3942409
}
]
}
```

</details>

ACKs for top commit:
willcl-ark:
reACK 7f9c4e2928991704e9e2224ad32d3b159b2551ac
jsarenik:
Approach ACK 7f9c4e2

Tree-SHA512: c35b423eea0eb34524cf5ad07822c0ab8d53e2ab78965b58c8738044c61c77352184822360ed077a51bfbf83d0226d221e988f7156b1948023707c7e1fb31495

Merge bitcoin/bitcoin#32958: wallet/refactor: Update SignPSBTInput to return util::Expected<void, PSBTError> and remove PSBTError:Ok

6cca38e2b92967b3c057407319e39c6aebadd032 refactor: remove unused PSBTError::Ok (kevkevinpal)
3660678b953c7de8c1d2c670296df90649630f5d refactor: SignPSBTInput now uses util:Expected (kevkevinpal)

Pull request description:

### Description
This is a follow-up to https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2092030045 and https://github.com/bitcoin/bitcoin/pull/31622#discussion_r2092035407

### What this changes
- Updates `SignPSBTInput` to return `util::Expected<void, PSBTError>`
- Removes `PSBTError:Ok` from the `PSBTError` Enum

ACKs for top commit:
achow101:
ACK 6cca38e2b92967b3c057407319e39c6aebadd032
rkrux:
lgtm ACK 6cca38e2b92967b3c057407319e39c6aebadd032
jeanpablojp:
tACK 6cca38e2b92967b3c057407319e39c6aebadd032

Tree-SHA512: c83b2e7e440aff01becc788e36a732756085f507d447eaa16dd1276d0d0b54070d0d8a51727d553d214043935072586111a175f34338a8d1886a6d5b251de7e4

Merge bitcoin/bitcoin#35161: consensus: document merkle mutation root invariant

ef501a63d9d65fce49bd633c424e0bbabead7ef6 consensus: document merkle mutation root invariant (Lőrinc)

Pull request description:

**Problem:** `ComputeMerkleRoot`'s optional mutation flag and the reasoning behind its per-level check are undocumented, and the behavior is only exercised indirectly by merkle_test through random duplications and old-vs-new comparisons, so a refactor could silently change it, as the discussions in #22046 and #28430 illustrate.

**Fix:** Document the flag on the function declaration, explain inside the inner loop why the mutation check runs at every tree level even after a duplicate is found, and add direct API coverage for the CVE-2012-2459 construction.

**Coverage check:** Both `merkle_test` and the new `merkle_test_mutated_return_value` would fail under a refactor that stops the outer reduction once mutation is detected, e.g.:
<details><summary>Hypothetical regression</summary>

```patch
diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp
index dfa23cf897..40bc3f8efa 100644
--- a/src/consensus/merkle.cpp
+++ b/src/consensus/merkle.cpp
@@ -59,6 +59,7 @@ uint256 ComputeMerkleRoot(std::vector<uint256> hashes, bool* mutated) {
if (hashes[pos] == hashes[pos + 1]) mutation = true;
}
}
+ if (mutation) break;
if (hashes.size() & 1) {
hashes.push_back(hashes.back());
}
```
</details>

Fixes #28457

ACKs for top commit:
optout21:
reACK ef501a63d9d65fce49bd633c424e0bbabead7ef6
achow101:
ACK ef501a63d9d65fce49bd633c424e0bbabead7ef6
w0xlt:
reACK ef501a63d9d65fce49bd633c424e0bbabead7ef6
hodlinator:
ACK ef501a63d9d65fce49bd633c424e0bbabead7ef6

Tree-SHA512: 5a54eed071079a0a37333d5ba7c2d8eb81ae318ee4c84e15e3c050198daea6282453d4f7727b75f7dab90696b3d9bc946b8b33e6456a7f190b2297c15aca390c

Merge bitcoin-core/gui#944: Fix out-of-bounds read in RPCParseCommandLine on empty command

fef99e6563ae284811904b980069166621c4fa22 qt: fix out-of-bounds read in RPCParseCommandLine on empty command (sayed nabhan)

Pull request description:

When a console line has no command name (it starts with `)`, or is `()`, `(`, or `,`), RPCParseCommandLine reaches the command-execution branch while the current argument frame is still empty, so `stack.back()[0]` reads out of bounds and the argument list built from `stack.back().begin() + 1` to `end()` is an invalid iterator range (throws std::length_error in practice, UBSan flags the null-pointer reference otherwise).

The `(` branch already guards the frame with `stack.back().size() > 0`, so I add the same check to the `)`/newline branch and the empty frame is skipped. To be clear, `(` alone isn't safe on master either: it fails via the `\n` branch, not via the `(` branch itself (the state there isn't `STATE_ARGUMENT`), and `,` alone fails the same way.

Since there's no command to run in any of these cases, the parser now returns `false` so the console reports an invalid command line, consistent with other fully-invalid input like a bare `'` or `"`, rather than silently ignoring it.

Regression cases added to rpcNestedTests for `)`, `()`, `(` and `,` (all abort on master without the guard), plus `getblockchaininfo)` which stays tolerated.

ACKs for top commit:
hebasto:
ACK fef99e6563ae284811904b980069166621c4fa22, tested on Ubuntu 26.04.

Tree-SHA512: 15822a0525402878483d5b2d0fe7b9e27916e514c1a8ad4a697f11b1e5cfe33f5fcd1f089efefb976bcf0d84cdf5166a58a070593924c2d4c2e1f8224d06590f

Merge bitcoin/bitcoin#35665: psbt: avoid duplicate global xpub keys when merging

6d387af562fc85eee003a269dbea10963e941985 psbt: remove write-only global xpub tracking set (Thomas)
3b7051c7e3388727402eeffe15a0df86e3c0de3c test: check combinepsbt with conflicting global xpub origins (Thomas)
7c632c0e2a2ef604cc5bca682935ee519cfbad8e psbt: avoid duplicate global xpub keys when merging (Thomas)

Pull request description:

Global xpubs are stored in a map of key origin to set of xpubs, while the serialization writes one record per xpub, keyed by the xpub. `Merge` unions the map origin-by-origin, so when the combined PSBTs provide different key origins for the same xpub, the result serializes the same `PSBT_GLOBAL_XPUB` key twice. BIP 174 declares PSBTs with duplicate keys invalid and the deserializer rejects them, so `combinepsbt` returns a PSBT that no RPC can parse again. This affects all releases since the merge loop was added in #17034 (v23.0).

<details><summary>Reproduction on master</summary>

The PSBTs share the unsigned transaction and xpub, and differ only in the master fingerprint of the global xpub record (`00000000` vs `11111111`):

```
$ A=cHNidP8BADwCAAAAAaqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqAAAAAAD/////AQAAAAAAAAAAAAAAAABPAQQ1h88AAAAAAAAAAACHPf+BwC9SViP9H+UWfqw6VaBJ3j0xS7Qu4if/7TfVCAM5o2ATMBWX2u9B++WToCzFE9C1VSfsLfEFDi6P9JyFwgQAAAAAAAAA
$ B=cHNidP8BADwCAAAAAaqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqAAAAAAD/////AQAAAAAAAAAAAAAAAABPAQQ1h88AAAAAAAAAAACHPf+BwC9SViP9H+UWfqw6VaBJ3j0xS7Qu4if/7TfVCAM5o2ATMBWX2u9B++WToCzFE9C1VSfsLfEFDi6P9JyFwgQRERERAAAA
$ bitcoin-cli -regtest decodepsbt "$(bitcoin-cli -regtest combinepsbt "[\"$A\",\"$B\"]")"
error code: -22
error message:
TX decode failed Duplicate Key, global key "01043587cf00...9c85c2" already provided: iostream error
```

</details>

Deduplicate by xpub when merging, keeping the origin that is already present: BIP 174 lets the Combiner "pick arbitrarily when conflicts occur", and conflicting unknown and proprietary records are already resolved the same way. The logic is shared between `combinepsbt` and `joinpsbts` through a new `MergeGlobalXPubs` helper. The second commit adds a test that fails on master with the error above, and the last commit removes the `global_xpubs` tracking set in `Unserialize`, write-only since the generic duplicate key check introduced in #21283 (1e2d146b47) replaced the explicit one.

Note: the xpub loop in `joinpsbts` currently has no observable effect, since the collected xpubs never reach the returned PSBT. My #35516 fixes that, so this PR should land first: on its own, #35516 would make the same duplicate key issue reachable through `joinpsbts`, while with the shared helper in place it never becomes reachable. I will rebase #35516 on top afterwards.

ACKs for top commit:
Bicaru20:
tACK 6d387af562.
achow101:
ACK 6d387af562fc85eee003a269dbea10963e941985
winterrdog:
tACK 6d387af562fc85eee003a269dbea10963e941985

Tree-SHA512: e2a9e02617eeec22a9240d7cf9386ee880a5f3639b143df7de4d8ea3e7b808f8c123f0b0410ff4a22e9a564bd86b2335a5c4aa3b2281af47d111484a6f1fd108

Merge bitcoin/bitcoin#35859: wallet: use unsigned KDF iteration count

cf36df070b4dfa954df78bb59c687de54b277a5a Wallet: Check crypter return values (benthecarman)
b76afff27490686cd35c9a8cf004f543f7d65a42 Wallet: Use unsigned KDF iteration count (benthecarman)

Pull request description:

CMasterKey::nDeriveIterations values are deserialized from wallet files
as unsigned 32-bit integers, but key derivation narrowed the count to a
signed int. A count above INT_MAX became negative in the conversion, and
the derivation loop counter then overflowed, which is undefined
behavior.

Keep the count unsigned through the derivation path to match the
serialized type, and add tests for zero and normal counts.

Also check key-derivation calibration failures and validate calculated
iteration counts before conversion. Keep the output master key unchanged
until derivation and encryption succeed, and mark fallible crypter
methods as [[nodiscard]].

ACKs for top commit:
l0rinc:
code review ACK cf36df070b4dfa954df78bb59c687de54b277a5a
achow101:
ACK cf36df070b4dfa954df78bb59c687de54b277a5a

Tree-SHA512: 95d5db2655fef8ca499af7da0f0258b4bee90975468286cb88e424257c4c5bf36407d2b2816b538e2ac3227e2a3a75c8d775211c65fe3ec6d12189da0b05fba1

Merge bitcoin/bitcoin#35069: Refactor keypath parser

777aee77d12069718f7a92c5d73a87eddc3e0924 refactor: deduplicate keypath element parsing (pythcoiner)
7d8fddfba2587a1b3481c380c826324311b74900 refactor: define BIP32_HARDENED and BIP32_UNHARDENED constants (pythcoiner)

Pull request description:

The codebase used raw `0x80000000` (and implicit `0`) as the bip32 hardened / unhardened flag.

`ParseHDKeypath` and `ParseKeyPathNum` were two separate parsers for BIP32 keypath elements, #32784 aligned their rules (both accept ' and h as hardened marker and reject indexes > 0x7FFFFFFF), but the parsing logic itself was still duplicated.

This PR:
- Define `BIP32_HARDENED_FLAG` / `BIP32_UNHARDENED_FLAG` constants to replace magic `0x80000000` and `0` literals.
- Add `ParseKeyPathElement` as bip32 parsing util and use it consistantly in `ParseHDKeyPath` and the descriptor keypath parser.

ACKs for top commit:
Sjors:
ACK 777aee77d12069718f7a92c5d73a87eddc3e0924
achow101:
ACK 777aee77d12069718f7a92c5d73a87eddc3e0924

Tree-SHA512: fb096eef82bb5a90baa7de41f5562b935665ee4b64c41586cf89e06c5633be44063a35a89d01a1432c8e63222d94bd06840bafc2a837f252013e317de0f5837a

Merge bitcoin/bitcoin#35980: contrib: reject divergent verify-commits history

465bca734ebf22dc27dd8667224f1cf2b590fc9f contrib: reject divergent verify-commits history (Lőrinc)
b3d1dca33882279a344c7cccc8f0795b35c29b91 contrib: fail on verify-commits ancestry errors (Lőrinc)

Pull request description:

**Problem:** `verify-commits.py` checks a Git commit's history for trusted signatures and tree hashes back to configured roots.
The documented workflow runs this check after fetching a commit and before checkout, proceeding only when the script succeeds.
A commit that is an ancestor of a configured root is intentionally accepted without checking earlier history.
The script also takes this success path after Git errors or for divergent commits, even though neither establishes that relationship.

**Fix:** Require Git to prove the ancestor relationship before taking this success path.

**Reproducers:** Each commit can be validated manually.
<details><summary>Manual reproducer: Git error</summary>

Run this on `master` and at this PR's head:

```bash
contrib/verify-commits/verify-commits.py 0000000000000000000000000000000000000000 && echo ❌ || echo ✅
```

`master` exits successfully without verifying the missing commit, while the PR head rejects the Git error.
</details>

<details><summary>Manual reproducer: divergent history</summary>

On `master` and at this PR's head, create an unreferenced sibling of the trusted root and run the verifier:

```bash
root=$(head -n1 contrib/verify-commits/trusted-git-root)
divergent_commit=$(git commit-tree "$root^{tree}" -p "$root^" -m 'divergent commit')
contrib/verify-commits/verify-commits.py "$divergent_commit" && echo ❌ || echo ✅
```

`master` exits successfully without verifying the sibling commit, while the PR head rejects divergent history.
</details>

This issue was also found and disclosed responsibly by the Red Team 🟥.

ACKs for top commit:
151henry151:
tACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f
jeanpablojp:
tACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f
achow101:
ACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f
sedited:
ACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f
maflcko:
review ACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f 🥜

Tree-SHA512: 72b8cd9902d881e59a1d99fda8e5d511806826fa27c05a2c21a7d2eb62b2a5a0b1b6bdc67e8d19d57f9171278f4858fd019eb7890b960df0475ba4713683f0ac

Merge bitcoin/bitcoin#36012: psbt: Remove unused `IsNull()` methods

2c16efbb7b5b5f0b188f320d9d4c92af602e9128 psbt: Remove unused IsNull() methods (nebula-21)

Pull request description:

This PR removes the `IsNull()` methods from `PartiallySignedTransaction`, `PSBTInput`, and `PSBTOutput`, along with their calls from the fuzz target.

This methods have no production callers, their only callers are the fuzz target. As such, keeping these methods seems not useful.

The motivation for this PR came from jeanpablojp's comment on [#35848](https://github.com/bitcoin/bitcoin/pull/35848#issuecomment-5274013825), added him as coauthor.

ACKs for top commit:
maflcko:
review ACK 2c16efbb7b5b5f0b188f320d9d4c92af602e9128 🥑
vicjuma:
ACK 2c16efbb7b5b5f0b188f320d9d4c92af602e9128
sedited:
ACK 2c16efbb7b5b5f0b188f320d9d4c92af602e9128

Tree-SHA512: 129933ae9803a2d053e340ee2a85efd1e5d9e5e38833fac0a0a9cbd467fec3157087959c464a8eeedb04ea99b1e5d7eba5a2fafde2e78a4fabcd247fac855477