💬 ajtowns commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348345510)
An OP_TRUE signet would presumably get both for whatever that's worth

💬 fanquake commented on issue "Assertion hit on shutdown of bitcoin-node with connected mining interface client":
(https://github.com/bitcoin/bitcoin/issues/33387#issuecomment-3291201455)
@Sjors

💬 fanquake commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#issuecomment-3291221804)
Backported to 30.x in #33356.

👍 0xB10C approved a pull request: "depends: systemtap 5.3"
(https://github.com/bitcoin/bitcoin/pull/33373#pullrequestreview-3223791227)
ACK 28efd724b47841a16d0630cec4b59563cda54a81

I agree that the package sha256 hash is `966a360fb73a4b65a8d0b51b389577b3c4f92a327e84aae58682103e8c65a69a`. The diff is minimal and looks fine. The guix build hashes match.

```bash
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
860c39a3db7eb7e8aaf8165cd6834d2fd87afc69b82b20203a013c7ad7e7b057 guix-build-28efd724b478/output/aarch64-linux-gnu/SHA256SUMS.part
841aea1c04fd996bc437a
...

⚠️ ismaelsadeeq opened an issue: "RFC: Bitcoin Core Node `BlockTemplateManager`"
(https://github.com/bitcoin/bitcoin/issues/33389)
Bitcoin Core Node `BlockTemplateManager` main use should be handling block template creation for other components of the node.
It should use the low-level block assembler to do that. This `BlockTemplateManager` should be initialized with a pointer to the mempool and a `ChainstateManager` reference. It should have access to the default block creation options and be instantiated during node startup and destroyed when the node is shutting down.
This `BlockTemplateManager` should be available via t
...

🤔 Eunovo reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3223525430)
Concept ACK https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85
Left some comments.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348447122)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: I think it's best to take out this TODO from the commit. You should also adjust the commit message to focus on what this commit is adding specifically.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348463061)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: A comment here will also be beneficial.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348538334)
https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85:
should be "block not on best header chain"

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348503453)
https://github.com/bitcoin/bitcoin/pull/33336/commits/6222d8cced4828b83a9dde20b9da9df44848b9bd:
Why not `AssumeValid::DISABLED` instead of `AssumeValid::CHECKED` and `AssumeValid::ENABLED` instead of `AssumeValid:SKIPPED`?
I find the current names to be confusing. I think using `AssumeValid::ENABLED` to indicate that assume_valid is enabled is better.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348449770)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: This should probably still be set to `3` since the 4th node is not used in this commit.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348462197)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: A comment explaining this check will be beneficial.

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348540085)
https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85:
consider "block not part of assumevalid chain"

💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348292328)
https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85:
+1 on returning "unknown reason" instead of aborting.

💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3291494499)
Rebased on master and added a comment explaining why we bother to skip minimumchainwork, even though we expect minimumchainwork to match assumevalid block height most times.

💬 0xB10C commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3291550859)
Running v30.0rc1 on a couple of [peer.observer](https:///public.peer.observer) nodes - will report if I see something suspicious.

👍 ismaelsadeeq approved a pull request: "fuzz: enhance wallet_fees by mocking mempool stuff"
(https://github.com/bitcoin/bitcoin/pull/33210#pullrequestreview-3224092444)
Code review ACK c9a7a198d9e81e99de99a2aaff1687d13d6674e8
with a comment and nit happy to reACK when you amend my suggestion

💬 ismaelsadeeq commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2348652999)
In "fuzz: mock CBlockPolicyEstimator in wallet_fuzz" ff10a37e99271125a9ece92bae571f7b78fb9e22

It would be nice if `FuzzedBlockPolicyEstimator` were renamed to `MockedBlockPolicyEstimator`.
It could then be moved to `src/test/util/` so that it can be reused by other components that might need a fee rate estimate as well.

💬 ismaelsadeeq commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2348701041)
In "fuzz: create FeeEstimatorTestingSetup to set fee_estimator" adf67eb21baf39a222b65480e45ae76f093e8f66

nit: It will be nice if we don't have to set the fee estimator and just initialize it in the `FeeEstimatorTestingSetup` constructor, but we have to find a way to pass the fuzz data provider as well.

💬 ryanofsky commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348807001)
re: https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348144204

I think it makes sense if method order order in `.capnp` files just matches the method order in `.h` files so the declarations are easy to compare.

Since we haven't needed binary compatibility before, we've just renumbered when making any changes, so numbers have been in order up to this point. But if we want to start having binary compatibility, we won't be able to reassign numbers anymore, and it'll be normal for th
...