💬 l0rinc commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2353681440)
Thanks @enirox001, I think the 0% is fine and we've just calculated `total_files`, no need to guard and the percentage is the same as the `nFile+1/total_files`, I don't see the need to duplicate it.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353682010)
It's to avoid having to include and link the libsecp module in a bunch of irrelevant places.

💬 hebasto commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-3300386434)
@m3dwards

Could you please rebase this PR to refresh the CI?

💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2353687423)
I have explained it in the commit message
> The added test checks if the value is roughly correct by checking if the CI platforms are returning any value and if the value is at least 1 GiB and not more than 1 TiB.

The max value is only validated on 64 bits, since it's not unreasonable for 32 bits to have max memory, but on 64 bits it's likely an error.
Do you think it needs more explanation than that?

💬 willcl-ark commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3300404149)
> To be clear, this will start skipping the job in [bitcoin-core/gui](https://github.com/bitcoin-core/gui?rgh-link-date=2025-09-12T09%3A39%3A51Z) ?

That is currently correct, as I understand it.

Though I thought I heard that @m3dwards had been successful in persuading Cirrus to allow us to share our runners with both orgs? In which case it would automagically work there when USE_CIRRUS_RUNNERS was set.

🤔 enirox001 reviewed a pull request: "test: don't throw from the destructor of DebugLogHelper"
(https://github.com/bitcoin/bitcoin/pull/33388#pullrequestreview-3231754426)
Concept ACK 7f87cdd

💬 enirox001 commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353692055)
The `message` parameter was optimized to `const std::string&`, but `match` still uses pass-by-value. For consistency, shouldn't `match` also be `const MatchFn&`?

💬 enirox001 commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353697142)
This commit mixes exception safety fixes with parameter optimization. Like @l0rinc mention perhaps consider splitting the parameter changes into a separate commit for cleaner git history and easier debugging.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2353702784)
After reviewing the mempool eviction unit test that was failing in an intermediate commit, I ended up reworking the test altogether, so this should be better now.

💬 sdaftuar commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2353704384)
Took this patch, thanks.

💬 l0rinc commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353708443)
I prefer passing both by value and moving into the members, since most call sites pass temporaries from string literals for the message.

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353725515)
The callers all pass a `script_pubkey`. It is also more descriptive because it is actually the pubkey that appears in the script, which is not necessarily the same as the aggregate.

💬 enirox001 commented on pull request "test: don't throw from the destructor of DebugLogHelper":
(https://github.com/bitcoin/bitcoin/pull/33388#discussion_r2353727844)
Good point. Using pass-by-value + move for both parameters would indeed solve the consistency issue while being optimal for the common case of string literals. That approach makes both parameters follow the same pattern.

💬 vostrnad commented on issue "Default ASmap file path is not used unless -asmap is set":
(https://github.com/bitcoin/bitcoin/issues/33386#issuecomment-3300460071)
To clarify, the docs that confused me was the CLI help text:
```
-asmap=<file>
Specify asn mapping used for bucketing of the peers (default:
ip_asn.map). Relative paths will be prefixed by the net-specific
datadir location.
```
As ryanofsky said, this seems to imply that the default value is used when the option is not specified. I would personally be fine with that behavior (it certainly didn't raise any eyebrows when I thought that was how it worked), but maybe you're ri
...

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353855601)
Done

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353855886)
Done

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353856020)
Done

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353856126)
Done

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353856628)
Done with a different approach

💬 achow101 commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2353856957)
Done