👍 hebasto approved a pull request: "clang-format: Set PackConstructorInitializers: CurrentLine"
(https://github.com/bitcoin/bitcoin/pull/33912#pullrequestreview-3490063992)
ACK fad0c76d0a109ab7b063f0d405588cf1e6c15e4d.

🤔 mzumsande reviewed a pull request: "test: assumeutxo: add missing tests in wallet_assumeutxo.py"
(https://github.com/bitcoin/bitcoin/pull/30455#pullrequestreview-3490160325)
Code Review ACK 55c6a69f777ac08a14e61b98efea00e5c8f98a5f

🤔 w0xlt reviewed a pull request: "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)"
(https://github.com/bitcoin/bitcoin/pull/33540#pullrequestreview-3490254189)
This command works on master but not in this PR:
`./build/bin/bitcoin-cli --datadir=/tmp/btc1 --signet getblockhash 1000`

Could it be related to GetCommandArgs()?

🤔 w0xlt reviewed a pull request: "argsman, cli: GNU-style command-line option parsing (allows options after non-option arguments)"
(https://github.com/bitcoin/bitcoin/pull/33540#pullrequestreview-3490620415)
Restructuring `ProcessOptionKey` as below seems to solve the problem of `./build/bin/bitcoin-cli --datadir=/tmp/btc1 --signet getblockhash 1000`.

```cpp
bool ArgsManager::ProcessOptionKey(std::string& key, std::optional<std::string>& val, std::string& error, const bool found_after_non_option) {
bool double_dash{false};
const std::optional<std::string> original_val{val};
std::string original_input{key};
if (val) original_input += "=" + *val;

// Normalize leading das
...

🤔 janb84 reviewed a pull request: "test: Retry download in get_previous_releases.py"
(https://github.com/bitcoin/bitcoin/pull/33915#pullrequestreview-3491910763)
ACK fad06f3bb436a97683e8248bfda1bd0d9998c899

PR adds one retry to download a prev. release.

The single retry is somewhat crude solution but a good first step to solve the intermitted download issues, good KISS sollution. If this isn't enough we can always fallback on more elegant solutions, but this also increases the complexity.

🤔 hebasto reviewed a pull request: "Fix bitcoin-qt visual glitches on Wayland"
(https://github.com/bitcoin-core/gui/pull/904#pullrequestreview-3492565990)
Tested 095f920629207b5ec4c50de7b454dfced0eafefb, it breaks UX.

For example, on Fedora 43 with Gnome 49 and Wayland, follow these steps:
1. Run `bitcoin-qt`.
2. Hide the main window using the "Hide" command in context menu.
3. Click on "Receive" in the system tray icon menu.

On the master branch, the main window reappears.

With this PR, however, the main window remains hidden.

🤔 fanquake requested changes to a pull request: "doc: Document compiler configuration for native depends packages"
(https://github.com/bitcoin/bitcoin/pull/33902#pullrequestreview-3492766868)
See https://github.com/bitcoin/bitcoin/pull/33902#discussion_r2549756641.

🤔 l0rinc reviewed a pull request: "coins: use number of dirty cache entries in flush warnings/logs"
(https://github.com/bitcoin/bitcoin/pull/33512#pullrequestreview-3492784897)
Thanks for the comments @optout21, since I wasn't invalidating any ACKs, I have applied your suggestions (with rebase) - let me know if this is what you meant.

> keeping a count like the dirty count is very efficient, but prone to going out of sync

yeah, we have a few of these - but the sanitizers and sanity checks and fuzzers usually reveal the problems (sanitizers break on underflow and the sanity checks recalculate everything during testing).

👍 maflcko approved a pull request: "ci: Add Windows + UCRT jobs for cross-compiling and native testing"
(https://github.com/bitcoin/bitcoin/pull/33764#pullrequestreview-3492941007)
lgtm

🤔 hodlinator reviewed a pull request: "build: Embedded ASMap [3/3]: Build binary dump header file"
(https://github.com/bitcoin/bitcoin/pull/28792#pullrequestreview-3492585142)
### Concept ACK 1dcd2e8a983406d12009e41bac5dbaeb12f330be

I do understand the distaste at adding 1MB+ binary blobs into a reproducible build. From that perspective, it would be cleaner to include the ASMap blob as a separate file included in our Bitcoin Core releases.

But having a separate file opens up the can of worms of where to install it on various operating systems and distributions, how to encourage package maintainers to include and ensure it ends up in the right locations, and making B
...

👍 maflcko approved a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3493037953)
lgtm. makes sense to just mention the stuff that is known to work and remove the stuff that is known to not work

🤔 fanquake requested changes to a pull request: "ci: Add Windows + UCRT jobs for cross-compiling and native testing"
(https://github.com/bitcoin/bitcoin/pull/33764#pullrequestreview-3493060075)
https://github.com/bitcoin/bitcoin/pull/33764#discussion_r2549961718

🤔 l0rinc requested changes to a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3493026362)
Concept ACK, but if we claim best-effort, we should cover the M series as well with latest AppleClang and latest LLVM.

🤔 hodlinator reviewed a pull request: "rest: allow reading partial block data from storage"
(https://github.com/bitcoin/bitcoin/pull/33657#pullrequestreview-3492740506)
Concept ACK e14650967dc95da5c10e0d6183b6eac3e8243fe5

#32541 contained some useful tricks that seem to be useful to shrink other index implementations. Thanks for letting it go in favor of this less invasive change though!

👍 ismaelsadeeq approved a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3493432014)
ACK 6e37b3b4861733ca97ec5d27d0bf52d187b6a2c9

🤔 TumaBitcoiner reviewed a pull request: "wallet/refactor: change PSBTError to PSBTResult and remove std::optional<common::PSBTResult> and return common::PSBTResult"
(https://github.com/bitcoin/bitcoin/pull/32958#pullrequestreview-3493400959)
I made some suggestions to keep consistency between naming and files. Sometimes `result` was used, other times `err`.

🤔 ismaelsadeeq reviewed a pull request: "mining: add getMemoryLoad() and track template non-mempool memory footprint"
(https://github.com/bitcoin/bitcoin/pull/33922#pullrequestreview-3493499544)
Concept ACK

I think it would be better if we have internal memory management for the mining interface IPC, since we hold on to the block templates.

I would suggest the following approach:

- Add memory budget for the mining interface.

- Introduce a tracking list of recently built block templates and total memory usahe.

- Add templates to the list and increment the memory usage after every `createnewblock` or `waitnext` return.

- Whenever the memory budget is exhausted, we should release tem
...

🤔 brunoerg reviewed a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3493836797)
Concept ACK

👍 darosior approved a pull request: "doc: clarify and cleanup macOS fuzzing notes"
(https://github.com/bitcoin/bitcoin/pull/33921#pullrequestreview-3494282906)
ACK 6e37b3b4861733ca97ec5d27d0bf52d187b6a2c9

nit: i think it would be good to have a sentence that fuzzing support is only maintained for Linux platforms as the first line of the document, so readers know what to expect. It's fine if it's also repeated in the "MacOS notes" section.

🤔 hodlinator reviewed a pull request: "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count"
(https://github.com/bitcoin/bitcoin/pull/32497#pullrequestreview-3493993983)
Concept ACK de6a28396b8bf7a3fd1124418b4cd5beba1906d6

`ComputeMerkleRoot(std::vector<uint256> hashes, ...)` takes a vector by value, but luckily call sites already move into it, and since the underlying array/memory is transferred on move, the capacity "transfers as well" - neat!

---

> Sorry to be this guy again, but it seems to me unreasonable to touch such critical consensus code for such marginal benefits. Unless observable benefits can be shown, ...

Thanks for being that guy!

Going by ht
...