💬 ryanofsky commented on issue "RFC: Bitcoin Core Node `BlockTemplateManager`":
(https://github.com/bitcoin/bitcoin/issues/33389#issuecomment-3299100671)
Interesting discussion. Especially interesting to know about possibility of leaking transaction information if creation times aren't tracked and used correctly. And to know how this could take more advantage of cluster mempool. I also like idea of calling this BlockTemplateCache instead of BlockTemplateManager to give it more focus.

Overall, notion of a shared cache seems useful. In terms of the implementation approach, I think it'd be nice if new code were integrated with existing code sooner
...

💬 Sjors commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#issuecomment-3299129244)
On second thought I don't think mutating the block is a problem.

> I think this assumes that the provided coinbase tx includes the coinbase witness, which is different to how `submitblock` behaves. (In particular `submitblock` includes a call to `chainman.UpdateUncommittedBlockStructures`, whereas `AddMerkleRootAndCoinbase` doesn't) That seems like it might be a source of bugs -- might at least be worth adding a check and having an explicit log message if the coinbase requires a witness but d
...

💬 Raimo33 commented on pull request "key: use static context for libsecp256k1 calls where applicable":
(https://github.com/bitcoin/bitcoin/pull/33399#issuecomment-3299155068)
Concept ACK

🤔 marcofleon reviewed a pull request: "p2p: Correct unrealistic headerssync unit test behavior"
(https://github.com/bitcoin/bitcoin/pull/32579#pullrequestreview-3230419017)
code review ACK cc5dda1de333cf7aa10e2237ee2c9221f705dbd9

I don't have a strong opinion on the refactors or commit order, although I can see how this PR might not be the most straightforward to review. Overall, the core changes in 7b00643ef5f932116ee303af9984312b27c040f1 and cc5dda1de333cf7aa10e2237ee2c9221f705dbd9 lgtm. Making the headers sync params configurable ensures that the new test can cover the redownload buffer boundary logic.

💬 TheCharlatan commented on pull request "index: initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#discussion_r2352862088)
I made a small fuzz test now. If you think it is useful, and want to add it:

```c++
// Copyright (c) 2025-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <util/threadpool.h>

#include <test/fuzz/FuzzedDataProvider.h>
#include <test/fuzz/fuzz.h>

namespace {

struct ExpectedException : std::runtime_error {
using std::runtime_error::runtime_er
...

💬 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