💬 zaidmstrr commented on pull request "test: Add submitblock test in interface_ipc":
(https://github.com/bitcoin/bitcoin/pull/33380#issuecomment-3299291247)
Tested ACK [0a26731](https://github.com/bitcoin/bitcoin/pull/33380/commits/0a26731c4cc182e887ce959cdd301227cdc752d7)

🤔 zaidmstrr reviewed a pull request: "test: Add submitblock test in interface_ipc"
(https://github.com/bitcoin/bitcoin/pull/33380#pullrequestreview-3230558328)
Tested ACK [0a26731](https://github.com/bitcoin/bitcoin/pull/33380/commits/0a26731c4cc182e887ce959cdd301227cdc752d7)

💬 plebhash commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#issuecomment-3299309843)
> The current ambition is to have the Template Provider communicate this value (eventually, no rush).

this would require a change to the Sv2 spec so that `NewTemplate` message carries a new field called `coinbase_witness `.

but unfortunately, changing Sv2 spec is usually received with resistance from the current players.

if/when a softfork happens and we start committing stuff to the coinbases' `witness reserved value`, that could be stronger motivation.

but until that day comes, th
...

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

Happy to re-ACK if you take Vasil's suggestion. I don't think there is any functional difference, but the separation of concerns between `-whitelist` and `-whitebind` permissions is slightly neater.

💬 hebasto commented on issue "build: `test_bitcoin` link failure with `-flto=thin` & address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/33147#issuecomment-3299457712)
Using a [supported linker](https://clang.llvm.org/docs/ThinLTO.html#linkers), such as `lld` on Ubuntu, helps.

💬 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-3299464744)
> Have you looked into making this an option of importdescriptors?
I have not looked into making it an option for `importdecriptors`, but I will look into it and check the performance, and get back to you on that.

Thanks for the suggestion. I really appreciate it.

💬 Sjors commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#issuecomment-3299548132)
@plebhash indeed this PR just introduces the extra method and clarifies in the documentation that `submitSolution` and `applySolution` are slightly different from the `submitsolution` RPC.

👋 willcl-ark's pull request is ready for review: "WIP: Backport Cirrus runners to 29.x"
(https://github.com/bitcoin/bitcoin/pull/33403)

🚀 glozow merged a pull request: "test: Add submitblock test in interface_ipc"
(https://github.com/bitcoin/bitcoin/pull/33380)

💬 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.