Bitcoin Core

Merge bitcoin/bitcoin#36059: test: make index crash test check saved state

7ea36e985a900b2291ce549e468f6baab5324dc6 test: preserve index crash test state (Lőrinc)
5aa15df60c49aacd3b3dafe13a4ceded9cec07cc test: expose missing index crash checkpoint (Lőrinc)

Pull request description:

**Problem:** #35847 moved the unclean-shutdown test into the shared base index tests, but it checked only that each index could reopen and start background sync.
Both checks also pass when the index reopens at height 0, so they do not verify that a height-100 checkpoint was saved before the simulated crash and reloaded afterward.

**Fix:** The first commit records the existing false positive by asserting that each index reopens at height 0 before background sync.
The second commit establishes a durable checkpoint at height 100, drains its setup notification, and changes the same assertion to the pre-crash height.

ACKs for top commit:
jeanpablojp:
tACK 7ea36e985a900b2291ce549e468f6baab5324dc6
mzumsande:
ACK 7ea36e985a900b2291ce549e468f6baab5324dc6

Tree-SHA512: 0dca2bdd978c5df4acbb01692bb2058e74efa70da3d7191687628075a680a57848ddda8087629b5d743d9a1648d7dc849fda9ff487252136a0c91cdfda33ba32

Merge bitcoin/bitcoin#36063: refactor: [test] Remove deprecated SetMockTime(i64) alias

fad1e6bf238ef4693464faf4102934c5b2dc7cb2 util: refactor: Remove deprecated SetMockTime(i64) alias (MarcoFalke)
faf87c3535c51ab2d5c64ed9660d7b3846975469 test: refactor: Use FakeNodeClock over manual/global SetMockTime (MarcoFalke)

Pull request description:

The deprecated test-only alias is only used in a few places and required in none.

In fact, it is incorrectly used in two unit tests, so first fixup those, and then remove it.

ACKs for top commit:
rustaceanrob:
ACK fad1e6bf238ef4693464faf4102934c5b2dc7cb2

Tree-SHA512: 1fa49e363bf8ccad07d61a77d3bc55c84724cd4cf034756b534cdb56614f334c0009d62dbdb08a940ddf46e0143ec6ecd6b4608baa2f8f581f56a6eef0f0abb8

util: refactor: Remove deprecated SetMockTime(i64) alias

The deprecated test-only alias is only used in three places and required
in none.

So remove it.

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#35884: util: set os-level thread names on Windows

dd669f40b98bb864bb9713673f0c38d946040591 util: set os-level thread names on Windows (ViniciusCestarii)

Pull request description:

Update SetThreadName to set os-level thread names on Windows too.

This is useful for debugging-ergonomics on Windows. Threads currently show up unnamed in debuggers, crash dumps on Windows and mismatch what's documented under https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#threads.

Tested with the mingw cross build running on Windows 11, print from WinDbg:

<img width="713" height="631" alt="image" src="https://github.com/user-attachments/assets/05e03383-c9b1-4e6b-91f3-9088b2fc7e90" />

ACKs for top commit:
l0rinc:
code review ACK dd669f40b98bb864bb9713673f0c38d946040591
hebasto:
ACK dd669f40b98bb864bb9713673f0c38d946040591, tested Guix-built `bitcoind.exe` on Windows 11 Pro using WinDbg:
winterrdog:
utACK dd669f40b98bb864bb9713673f0c38d946040591

Tree-SHA512: 3632584f5f0612414a53ad6d868b9f832e8e7f1fad19f212f292172a4a6516f6e0055ec6ac8fbb71b22acdd003d09c9a5f97c0c15a137e1ad242580f997aca6f

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#36018: test: [refactor] Properly use BOOST_CHECK_EXCEPTION

fa0fe212f52ad261bfc59683dd8309a7a0cf3a51 test: [refactor] Properly use BOOST_CHECK_EXCEPTION (MarcoFalke)

Pull request description:

The exception checking in unit tests is partly verbose, fragile, inconsistent and thus confusing.

Fix all those issues by using `BOOST_CHECK_EXCEPTION` consistently:

* The test code is less bloated and follows a standard pattern; Extra state and dead code like `exceptionThrown = false;` or `BOOST_CHECK(0)` can be removed.
* The checks are more strict, because they use `HasReason{...}` or a similar predicate.

ACKs for top commit:
l0rinc:
ACK fa0fe212f52ad261bfc59683dd8309a7a0cf3a51
janb84:
ACK fa0fe212f52ad261bfc59683dd8309a7a0cf3a51
jonatack:
Light ACK fa0fe212f52ad261bfc59683dd8309a7a0cf3a51

Tree-SHA512: f3a9abfe02988299f6d2604643feb630e078c2177287899d4fcc191802536d41d09e6365d81d0184bf3533583987969cb1b85ab3e112e7364fdb5b85a3405cb7

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

Merge bitcoin/bitcoin#35968: test: sync funding block before isolating nodes

8454fb2bd74cd0e43b447dd8f49d388b8a25d2d7 test: sync funding block before isolating nodes (shaurya2k06)

Pull request description:

Fixes #35967

test_alternate_witness_tx mines the taproot funding output on node0 with
sync_fun=self.no_op and immediately disconnects. node1 later includes the
script-path spend via generateblock. If the funding block has not reached
node1, that call fails with bad-txns-inputs-missingorspent.

Drop the no_op so generate() uses the default sync_all before the partition.
Later generate* calls keep no_op because the nodes are then disconnected.

Seen twice this week in hebasto bitcoin-core-nightly NetBSD jobs:
https://github.com/hebasto/bitcoin-core-nightly/actions/runs/31350308484/job/93339698854
https://github.com/hebasto/bitcoin-core-nightly/actions/runs/31765546925/job/94660585799

The modified test is test/functional/wallet_listtransactions.py. I ran it
locally three times with build/test/functional/wallet_listtransactions.py.

ACKs for top commit:
achow101:
ACK 8454fb2bd74cd0e43b447dd8f49d388b8a25d2d7
furszy:
utACK 8454fb2bd74cd0e43b447dd8f49d388b8a25d2d7

Tree-SHA512: 6b8fdcdc9ce9c57c2939caff34850e88450864c909fe226ba9b6e02ffcefa3625623589b8ecd2f09b9ca16bcaec3a64bd07642ff2c47ae6e8d250de478d34b53

Merge bitcoin/bitcoin#32162: depends: Switch from multilib to platform-specific toolchains

de9b436ba36576903744feb17e4fae7b1842c75b depends: Switch from multilib to platform-specific toolchains (Hennadii Stepanov)

Pull request description:

Using the multilib GCC toolchain, as currently documented in [`depends/README.md`](https://github.com/bitcoin/bitcoin/blob/4c1906a500cacab385b09e780b54271b0addaf4b/depends/README.md), has several issues, such as:

1. The [`g++-multilib`](https://packages.ubuntu.com/noble/g++-multilib) package conflicts with platform-specific cross-compiler packages. This means it is not possible to cross compile for `i686` and other platforms using the same set of installed packages.

2. The [`g++-multilib`](https://packages.ubuntu.com/noble/g++-multilib) package is not available for `arm64`:
```sh
$ sudo apt install g++-multilib
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
E: Unable to locate package g++-multilib
```

3. Managing the multilib GCC toolchain requires additional code in both depends and Guix scripts.

This PR addresses all the issues mentioned above by switching from multilib to platform-specific toolchains.

Also see https://github.com/bitcoin/bitcoin/pull/22456.

---

Here are examples of building for different scenarions:

- Linux, `x86_64` or `arm64`, building with depends natively:
```sh
$ gmake -C depends -j $(nproc)
$ cmake -B build --toolchain depends/$(./depends/config.sub $(./depends/config.guess))/toolchain.cmake
$ cmake --build build -j $(nproc)
```

- Linux, `x86_64` or `arm64`, cross compiling for `i686-pc-linux-gnu`:
```sh
$ sudo apt install g++-i686-linux-gnu binutils-i686-linux-gnu
$ export HOST=i686-linux-gnu
$ gmake -C depends -j $(nproc)
$ cmake -B build-${HOST} --toolchain depends/${HOST}/toolchain.cmake
$ cmake --build build-${HOST} -j $(nproc)
```

- Linux, `x86_64`, cross compiling for `arm64`:
```sh
$ sudo apt install g++-aarch64-linux-gnu binutils-aarch64-linux-gnu
$ export HOST=aarch64-linux-gnu
$ gmake -C depends -j $(nproc)
$ cmake -B build-${HOST} --toolchain depends/${HOST}/toolchain.cmake
$ cmake --build build-${HOST} -j $(nproc)
```

- Linux, `arm64`, cross compiling for `x86_64`:
```sh
$ sudo apt install g++-x86-64-linux-gnu binutils-x86-64-linux-gnu
$ export HOST=x86_64-linux-gnu
$ gmake -C depends -j $(nproc)
$ cmake -B build-${HOST} --toolchain depends/${HOST}/toolchain.cmake
$ cmake --build build-${HOST} -j $(nproc)
```

ACKs for top commit:
fanquake:
ACK de9b436ba36576903744feb17e4fae7b1842c75b
BrandonOdiwuor:
ACK de9b436ba36576903744feb17e4fae7b1842c75b

Tree-SHA512: 453b4744974cdf56d6edfdbe93bb11e3bae3f9bc9cd99b9c57aee74e65fcdd3ac011a1dcc19f485ea3be427f4e9c6c6b0d704881369f719620f0cb299123e561

test: [refactor] Properly use BOOST_CHECK_EXCEPTION

Merge bitcoin/bitcoin#36008: wallet: WalletBatch->WriteVersion respect argument

ec5d19665b8935eabac36df4ec1ba2e19ee05c25 wallet: WalletBatch->WriteVersion respect argument. (David Gumberg)

Pull request description:

> Previously would use global `CLIENT_VERSION` no matter what, but this is one sense a refactor since all of the places where WriteVersion is called currently call it with `CLIENT_VERSION` anyways. The `client_version` argument is kept since future test code may want to write other versions.

> Addresses a review comment from [#32636](https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2356299627):

This was originally pointed out in https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2356299627, and the followup (#34490) was never merged. However I think it's confusing to have functions that take arguments but ignore them (and it's dead code), so I've cherry-picked the fix up from #34490.

ACKs for top commit:
achow101:
ACK ec5d19665b8935eabac36df4ec1ba2e19ee05c25
pablomartin4btc:
ACK ec5d19665b8935eabac36df4ec1ba2e19ee05c25
w0xlt:
ACK ec5d19665b8935eabac36df4ec1ba2e19ee05c25

Tree-SHA512: 3ad82d979493ac14704975bef504c791ac72fe7910c25097f0019bf61e3d384b0e1646f90c730d2f655733428b14ccd0196c333df2d6153ff5b55ab41918a6dd

Merge bitcoin/bitcoin#35797: psbt: support output metadata updates before inputs are added

c0792889673da175661f29138a318614b33636cd psbt: update output metadata without inputs (Lőrinc)
4f5712476a3cf66d3b4f225b883eeca2deee657b test: characterize P2WSH miniscript output (Lőrinc)
e24e8fa2a689d7479262540dba6da3f3e21fbaa8 test: characterize PSBT output metadata (Lőrinc)

Pull request description:

**Problem:** PSBTv2 permits outputs to be added before inputs.
An authenticated `descriptorprocesspsbt` request can abort the node while updating metadata for one of those outputs because `UpdatePSBTOutput()` traverses the output script with a signature creator for input index 0.
ECDSA signing or a miniscript timelock check can then access the missing input.

**Fix:** Make `UpdatePSBTOutput()` traverse output scripts with a temporary one-input transaction while continuing to take the output from the PSBT's unsigned transaction.
`MutableTransactionSignatureCreator` continues to require a valid input index.
Output metadata traversal still records scripts and key origins, allowing outputs to be updated before inputs are added.

ACKs for top commit:
jeanpablojp:
tACK c0792889673da175661f29138a318614b33636cd
achow101:
ACK c0792889673da175661f29138a318614b33636cd
w0xlt:
ACK c0792889673da175661f29138a318614b33636cd
polespinasa:
ACK c0792889673da175661f29138a318614b33636cd

Tree-SHA512: 0d8cda74b8a56c0f4713b2669e5a3e5b0551ecda4fdfceb38a80e5b98a9d208d447f2045a1cc9fee74fe33b2fc8f7a60997cd60b2961de5cea871f53831895fe

Merge bitcoin/bitcoin#35946: rpc: Improve some type specs for openrpc

e07d826e0ebd9507793fe033236e5f0f12ba5732 rpc: Fix type in ApplyTypeStrOverride (Shuvam Pandey)
c94074fa1b1396e310ab94955f5d04c9bda61b64 rpc: Surface OBJ_USER_KEYS description for openrpc (sedited)
c020c21d543a14268b98995d1a9d1878f3d95ec2 rpc: Handle skip type args for openrpc (sedited)

Pull request description:

This was initially motivated by testing the dump of the schema against open-rpc-generator, which crashed with:

```
open-rpc-generator generate -t client -l rust -n bitcoin_client -d ./openrpc.gen.json -o ./generated
There was error at generator runtime:
TypeError: Cannot convert undefined or null to object
```

The changes here fix this crash (albeit perfectly valid existing schema), but I think creating a more complete output is helpful on its own. The openrpc schema dumps can eventually be re-used for the rpc docs and to track rpc interface changes more accurately. Adding the CreateTxDoc outputs section seems useful for that.

Also includes a type tightening from number to integer in `ApplyTypeStrOverride` to reflect the actual behaviour in the rpc calls, where only integers are accepted.

ACKs for top commit:
achow101:
ACK e07d826e0ebd9507793fe033236e5f0f12ba5732
willcl-ark:
ACK e07d826e0ebd9507793fe033236e5f0f12ba5732

Tree-SHA512: d0454a71b4f1dab1daf8a0d5b1e5bf1c1b8f1a16d26638d4a64a2652402ad74230366cabf0cf4135a16d0bdab584d3d4b2a47f2968a4eff605348ce85e8dbadb

Merge bitcoin/bitcoin#35955: wallet: remove orphaned GetAffectedKeys and LegacyScriptPubKeyMan declarations

02de12b1e614035a73cbae58250920c843a17196 wallet: remove remaining LegacyScriptPubKeyMan references (Laxman Acharya)
d194be69d601322a79444e79ffebe475e58e5f77 wallet: remove orphaned GetAffectedKeys declaration (Laxman Acharya)

Pull request description:

Follow-up to #28710, which removed `GetAffectedKeys()` and `LegacyScriptPubKeyMan` but left their declarations behind.

Remove both orphaned declarations, rename `SetupLegacyScriptPubKeyMan()` to `SetupLegacyDataSPKM()`, and update related comments and logging to reflect the minimal `LegacyDataSPKM` retained for legacy wallet loading and migration.

## Testing

```bash
cmake --build build --target bitcoin_wallet -j 8
cmake --build build --target test_bitcoin -j 8
build/bin/test_bitcoin --run_test=scriptpubkeyman_tests,walletdb_tests,wallet_tests --catch_system_errors=no --log_level=error
```

ACKs for top commit:
achow101:
ACK 02de12b1e614035a73cbae58250920c843a17196
polespinasa:
ACK 02de12b1e614035a73cbae58250920c843a17196

Tree-SHA512: 0d340291c969f013fd6ec55158f6797d4a1478ff7a819c2d250d1b3f345131db99a8b7b4b3ff0da1fa0e06182701abed31dcd317f8ef12c81837c1aed57b21a5

Merge bitcoin/bitcoin#35972: fuzz: Fix assertion in `txorphan`

01dde6b20578616af7620a2180ee6b5db0ca4793 fuzz: Fix assertion in txorphan (marcofleon)

Pull request description:

`EraseTx()` calls `LimitOrphans()`, which may evict announcements from a peer that didn't announce the erased transaction, causing that peer's usage to decrease. Relax the assertion in the `EraseTx()` branch that claimed usage of a non-announcer peer should be unchanged. Also, add assertions for the other cases.

ACKs for top commit:
dergoegge:
utACK 01dde6b20578616af7620a2180ee6b5db0ca4793
instagibbs:
ACK 01dde6b20578616af7620a2180ee6b5db0ca4793

Tree-SHA512: 2e597b85fd41058c2fa79fa55f0d37e12505065b5e27aba7b9680e0c249a5450e6fa97b45394d6ffe1318f42538134ffa9c423b126c455f6f8e6d8ca59eed4b6

Merge bitcoin/bitcoin#35735: Add state to HTTPRequest

9954aa77280ecd67816e784815c6478a973f6635 http: don't parse any new requests from a client if m_req_busy = true (Matthew Zipkin)
c7db3ae1f907a10748e608cd69899c7586bfc210 test: cover HTTPRequest state machine (Matthew Zipkin)
90676e24ad1aa44346aed586f19ba1c68c477394 Add state to HTTPRequest to avoid duplicate work over I/O cycles (Matthew Zipkin)
507e528e845034583dd21b884e6debb1ff5307e3 http: reuse HTTPHeaders to parse chunked trailer (Matthew Zipkin)
902d8908c94dbe837d712a408d63dcbac634d4c5 http: only read one HTTPRequest at a time per client (Matthew Zipkin)

Pull request description:

This PR reduces the memory consumption of the HTTP Server when reading data from connected clients, and improves performance especially when requests are large (i.e. requiring multiple TCP packets).

In https://github.com/bitcoin/bitcoin/pull/35182 the server copies as much data as it can from the socket into application memory, and then tries to parse as many complete HTTP requests as possible from that data. If a request is discovered to be incomplete, the in-progress request is abandoned. The server tries again on the next I/O cycle to read the same data from the buffer, duplicating work as many times as it takes before the client finishes sending the request (or times out).

This PR implements two improvements to this:
1. Only parse one request at a time from the receive buffer. The server processes requests from each client in series anyway.
2. Add state to `HTTPRequest` so it can be filled with data from the receive buffer over multiple I/O loop iterations without losing progress.

If a client sends large or multiple requests, that data will sit in the kernel's socket buffer instead of the application memory. Eventually the socket buffer will fill up and TCP backpressure will kick in, dropping the TCP window to 0 and blocking the client from sending any more.

A state machine for `HTTPRemoteClient` was [discussed previously](https://github.com/bitcoin/bitcoin/pull/35182#pullrequestreview-4322490068) to control resource consumption. Another nice benefit of this model (for a follow-up PR) will be to insert the RPC authentication check after reading 8kB-limited headers but before the 32MB-limited request body.

ACKs for top commit:
winterrdog:
re-ACK 9954aa77280ecd67816e784815c6478a973f6635
janb84:
re ACK 9954aa77280ecd67816e784815c6478a973f6635
frankomosh:
ACK 9954aa77280ecd67816e784815c6478a973f6635.
fjahr:
ACK 9954aa77280ecd67816e784815c6478a973f6635

Tree-SHA512: b7c913114283fbf1f360b40f6c65a01390a26731bf3b166f460ec260f9206f25d738b3a06887bfa839911c1c6aaf634448181da47a752a9a881aebd907e44868