💬 achow101 commented on pull request "Update libmultiprocess subtree to fix build issues":
(https://github.com/bitcoin/bitcoin/pull/33241#issuecomment-3221668831)
ACK 323b3fd27283282f2f8eb1096f56f23d8230f2d6

Verified the subtree update and that it fixes the Ubuntu issue (after removing the 0.8.0 restriction).

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299137075)
I don't think any of those are actually useful things to assert. The point of the offline wallet is that it doesn't have access to the blockchain, so actually `txcount`, and `lastprocessedblock` should be different. The keypool sizes may be different depending on the node startup options, and birthtime may be different if something was used before being imported to the offline wallet.

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299144047)
Done

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299144625)
Added extra text to the help.

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299145052)
Renamed to `funder`.

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299145315)
Done

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299145463)
Done

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2299146384)
Done, however there should not be an expectation that the topup will always work as it does not work for wallets that are encrypted.

🚀 achow101 merged a pull request: "Update libmultiprocess subtree to fix build issues"
(https://github.com/bitcoin/bitcoin/pull/33241)

👍 l0rinc approved a pull request: "Revert compact block cache inefficiencies"
(https://github.com/bitcoin/bitcoin/pull/33253#pullrequestreview-3152574449)
Concept ACK, excellent find.

I redid the benchmarks, got more modest changes with GCC and Clang:

<details>
<summary>C++ compiler .......................... GNU 14.2.0</summary>

```bash
echo "> C++ compiler .......................... GNU $(gcc -dumpfullversion)" && rm -rf build && cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ >/dev/null 2>&1 && cmake --build build -j$(nproc) >/dev/null 2>&1 && build/bin/bench_bitcoin -filter
...

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298811715)
nit: newline at the very end is customary for the linter and formatter to not complain

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298827191)
nit: direct iteration would be slightly simpler:
```suggestion
for (auto& tx : refs) {
AddTx(tx, /*fee=*/tx->vout[0].nValue, pool);
}
```

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298808174)
nit: `std::ranges::shuffle` might be simpler:
```suggestion
std::ranges::shuffle(refs, rng);
```

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298810753)
we could consume the result to make sure the benchmark isn't optimized away

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298820059)
alternatively this may be slightly more descriptive than `11` and it could even be used to init the sigspam:
```suggestion
FastRandomContext rng{/*fDeterministic=*/true};
auto sigspam{rng.randbytes<200>()};
```

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298807140)
nit: we might as well reserve `50'000` here

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298886174)
I redid the a8203e94123b6ea6e4f4a6320e3ad20457f44a28 cherry-pick, this line and the top comment are new additions (+ some conflicts were resolved slightly differently to how I have, but the result seems correct to me)

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2298873346)
nit:
```suggestion
BenchCBHAST cmpctblock{rng, /*txs=*/3'000};
```

💬 l0rinc commented on pull request "Revert compact block cache inefficiencies":
(https://github.com/bitcoin/bitcoin/pull/33253#discussion_r2299068765)
as mentioned by @instagibbs and @achow101, we could add two benches here, for empty extra and with some (partially overlapping) one, something like:
```C++
std::vector<std::pair<Wtxid, CTransactionRef>> extra_txn;
// ...
extra_txn.reserve(extra_tx_count);
for (size_t i{0}; i < extra_tx_count; ++i) {
size_t index{i < (extra_tx_count / 2) ? mempool_size + i : mempool_size - i}; // some overlap with mempool
auto tx{CreateTestTransaction(sigspam
...

💬 achow101 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2299170764)
In a4f2144facb5d4dc2f749494ea65744fffcb628b "rpc: Handle -named argument parsing where '=' character is used"

This seems unnecessary, it's the same thing `CRPCConvertParam`.