💬 ryanofsky commented on issue "Default ASmap file path is not used unless -asmap is set":
(https://github.com/bitcoin/bitcoin/issues/33386#issuecomment-3299667325)
re: fjahr https://github.com/bitcoin/bitcoin/issues/33386#issuecomment-3298756998

> Can you pinpoint which part of the docs gave you this impression? Was it the RPC help or maybe files.md (https://github.com/bitcoin/bitcoin/blob/master/doc/files.md)? Is there a different place where you think this behavior could be better documented?

Since I noted the same issue in https://github.com/bitcoin/bitcoin/pull/30529#pullrequestreview-2516002191, the part of the doc which gave me that impression was
...

👍 ryanofsky approved a pull request: "cmake: Install `bitcoin` manpage"
(https://github.com/bitcoin/bitcoin/pull/33407#pullrequestreview-3231004906)
Code review ACK 7584a4fda95d004d31c2df15fdb6f3a7f9654348.

If I'm understanding this right, #31375 added the executable without a manpage, #33348 added the manpage but did not install it, so it would not be included with release binaries, and this PR installs it so it is included.

💬 mzumsande commented on pull request "net: do not apply whitelist permissions to onion inbounds":
(https://github.com/bitcoin/bitcoin/pull/33395#discussion_r2353211410)
Done with latest push, the section will now no longer be omitted (in the specific case of onion inbounds, I think it's not possible to have other prior implicit permissions here so it also won't actually be executed, but I agree it's a cleaner approach).

💬 mzumsande commented on pull request "net: do not apply whitelist permissions to onion inbounds":
(https://github.com/bitcoin/bitcoin/pull/33395#issuecomment-3299745335)
[e46a7a5](https://github.com/bitcoin/bitcoin/commit/e46a7a547371317a4d116b9b1a314917508ea480) to [f563ce9](https://github.com/bitcoin/bitcoin/commit/f563ce90818d486d2a199439d2f6ba39cd106352): Addressed @vasild suggestion.

💬 mzumsande commented on pull request "net: do not apply whitelist permissions to onion inbounds":
(https://github.com/bitcoin/bitcoin/pull/33395#discussion_r2353219694)
think I'll leave this for a refactoring PR that can apply this more systematically.

👍 darosior approved a pull request: "net: do not apply whitelist permissions to onion inbounds"
(https://github.com/bitcoin/bitcoin/pull/33395#pullrequestreview-3231159963)
ACK f563ce90818d486d2a199439d2f6ba39cd106352

🤔 furszy reviewed a pull request: "net: do not apply whitelist permissions to onion inbounds"
(https://github.com/bitcoin/bitcoin/pull/33395#pullrequestreview-3231188034)
ACK f563ce90818d486d2a199439d2f6ba39cd106352

Comments are nits, not-blocking. The code is good as is.

💬 furszy commented on pull request "net: do not apply whitelist permissions to onion inbounds":
(https://github.com/bitcoin/bitcoin/pull/33395#discussion_r2353308719)
nano nit:
usually, it is better to be explicit with the empty optionals:
```suggestion
AddWhitelistPermissionFlags(permission_flags, inbound_onion ? std::nullopt : std::make_optional(addr), vWhitelistedRangeIncoming);
```
but it is a non-blocking comment.

💬 furszy commented on pull request "net: do not apply whitelist permissions to onion inbounds":
(https://github.com/bitcoin/bitcoin/pull/33395#discussion_r2353315252)
nano nit for later:
could skip the loop when `addr == std::nullopt`

👍 vasild approved a pull request: "net: do not apply whitelist permissions to onion inbounds"
(https://github.com/bitcoin/bitcoin/pull/33395#pullrequestreview-3231237773)
ACK f563ce90818d486d2a199439d2f6ba39cd106352

🤔 enirox001 reviewed a pull request: "log: show reindex progress in `ImportBlocks`"
(https://github.com/bitcoin/bitcoin/pull/33353#pullrequestreview-3231320525)
Concept ACK d7de5b1

I tested this locally, the percentage logging provides a clear indication of progress during reindex and is useful visibility while the node processes block files.

I have left a few non-blocking nits for consideration.

💬 enirox001 commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2353411309)
Would it make sense to log `nFile+1/total_files` (e.g. `4/125`) alongside the percent? That gives clearer, more granular progress than an integer percent alone.

💬 enirox001 commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2353404361)
Use a defensive guard when computing percent, e.g. `total_files ? (100 * (nFile + 1) / total_files) : 0`, to avoid accidental divide-by-zero after future refactors.

💬 enirox001 commented on pull request "log: show reindex progress in `ImportBlocks`":
(https://github.com/bitcoin/bitcoin/pull/33353#discussion_r2353403899)
Consider computing the progress as `(nFile + 1) * 100 / total_files` so the indicator advances past 0% on the first file. Is there a reason we intentionally show 0% initially?

💬 musaHaruna commented on pull request "rpc: add scan_utxoset option to getbalance(s) to verify wallet balance accuracy":
(https://github.com/bitcoin/bitcoin/pull/33392#issuecomment-3300125137)
> I have not looked into making it an option for importdecriptors, but I will look into it, and get back to you on that.

Yes, I have looked into adding the option to importdescriptors by introducing a scan_utxoset flag and it's possible. The idea is that when enabled, the wallet would scan the UTXO set immediately after import to verify balances against chainstate, and if a discrepancy is detected (for example due to an incorrect birthdate), it could automatically trigger a full rescan from h
...

💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3300132443)
Rebased to refresh the CI.

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2353518252)
6000fc8c3fcc0446b3665f7fccf102068919fa3c

Didn't nail down exactly when this would become a concern, but do note that this call can trigger `tx.vin.size()` number staged parents and `GetAncestors` calls based on those parents.

Think this is potentially a huge number of clusters with each up to 64 transactions at a time.

💬 theStack commented on pull request "key: use static context for libsecp256k1 calls where applicable":
(https://github.com/bitcoin/bitcoin/pull/33399#discussion_r2353526094)
Makes sense yes, done. Packed the scope reduction in the same commit as I think the diff is still digestible, but happy to divide it into two commits if that's preferable for reviewers.

💬 theStack commented on pull request "key: use static context for libsecp256k1 calls where applicable":
(https://github.com/bitcoin/bitcoin/pull/33399#issuecomment-3300168851)
Thanks for the reviews, fixed the typo in commit message and PR description (s/paramter/parameter/) and reduced the scope of `secp256k1_context_sign` in the fuzz test as suggested (h/t @l0rinc). Didn't do any renaming of the context object, as I haven't stumbled upon a name yet where I was convinced that it's much better (I agree with @real-or-random though that's it's imprecise).

🤔 danielabrozzoni reviewed a pull request: "coins: warn on oversized `-dbcache`"
(https://github.com/bitcoin/bitcoin/pull/33333#pullrequestreview-3230365817)
tACK 13fa0d0a433722f294854001b0561c079db12dbc

I tested locally and reproduced the warning:

```
❯ ./build/bin/bitcoind -dbcache=64000
...
2025-09-16T13:57:22Z [warning] A 64000 MiB dbcache may be too large for a system memory of only 64219 MiB.
Warning: A 64000 MiB dbcache may be too large for a system memory of only 64219 MiB.

❯ ./build/bin/bitcoin-cli getblockchaininfo
{
[...]
"warnings": [
"A 64000 MiB dbcache may be too large for a system memory of only 64219 MiB.",

...