💬 l0rinc commented on pull request "refactor: optimize: avoid allocations in script & policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#discussion_r2482103559)
I don't like the reuses, seems like a workaround instead of a fix, pushed an alternative to https://github.com/bitcoin/bitcoin/pull/33757, added @Raimo33 as a coauthor

💬 davidgumberg commented on pull request "ci: gha: Set debug_pull_request_number_str annotation":
(https://github.com/bitcoin/bitcoin/pull/33754#issuecomment-3474017856)
lgtm ack https://github.com/bitcoin/bitcoin/commit/fa9d0f994b45a94e3f26c01e395c58ff59f47f43

https://github.com/bitcoin/bitcoin/actions/runs/18971691417/job/54180745411?pr=33754#step:2:11

💬 Raimo33 commented on pull request "refactor: optimize: avoid allocations in script & policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#discussion_r2482120044)
what about the other commits? @l0rinc.
there are some intresting zero-cost optimizations. maybe not worth for a PR alone, but could be included as small refactorings in bigger PRs.

💬 Raimo33 commented on pull request "refactor: make script Solver's often-unused solutions parameter optional":
(https://github.com/bitcoin/bitcoin/pull/33757#issuecomment-3474029652)
Code Review ACK 2726abd896817c0dfd5b171a643bad6af015d070

I proposed the same in this private commit: 3ea121d49c01935f74673cbb825ed608179e43d5
would like a bit more clarity on the benchmarks referenced. are we talking about `CCoinsCaching`?

💬 ajtowns commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3474035674)
This change affects enforcement of `SCRIPT_VERIFY_WITNESS_PUBKEYTYPE` which afaics should not be weakened [bip143](https://github.com/bitcoin/bips/blob/master/bip-0143.mediawiki#restrictions-on-public-key-type)?

🚀 glozow merged a pull request: "[IBD] coins: increase default UTXO flush batch size to 32 MiB"
(https://github.com/bitcoin/bitcoin/pull/31645)

💬 ismaelsadeeq commented on pull request "node: add `BlockTemplate{Cache, Manager}`":
(https://github.com/bitcoin/bitcoin/pull/33421#discussion_r2482188424)
This is gone now.

💬 ismaelsadeeq commented on pull request "node: add `BlockTemplate{Cache, Manager}`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3474113647)
Forced pushed from 5f798b4a3b1a978b8ac954db54e9e8dfc7da6319 to 084bfbc1ec7f8f64f54d231bb641285622311b59 Compare diff [dfc7da6319..084bfbc1](https://github.com/bitcoin/bitcoin/compare/5f798b4a3b1a978b8ac954db54e9e8dfc7da6319..084bfbc1ec7f8f64f54d231bb641285622311b59)

Thanks for your thorough review. I've made significant changes following your feedback:

* Removed the first commit, as it is out of scope for now. [https://github.com/bitcoin/bitcoin/pull/33421#discussion_r2379497293](https://g
...

💬 roconnor-blockstream commented on pull request "Relax standardness rules regarding CHECKMULTISIG":
(https://github.com/bitcoin/bitcoin/pull/33755#issuecomment-3474132937)
@ajtowns I did consider that.

Firstly we should note that `SCRIPT_VERIFY_WITNESS_PUBKEYTYPE` and BIP-143 rules regarding compressed public keys are not consensus rules, but policy only. So at least it *can* be weakened.

Regarding as to whether it should be weakened: Currently Bitcoin Core's implementation choice for CHECKMULTISIG is such that once enough signatures match the pubkeys, no further pubkeys in the CHECKMULTISIG stack are inspected for well-formedness policy constraints. That
...

💬 ismaelsadeeq commented on pull request "node: add `BlockTemplate{Cache, Manager}`":
(https://github.com/bitcoin/bitcoin/pull/33421#discussion_r2482211458)
Yes this will be good for precision but In the current iteration of the PR I switched to using the exact block assembly time as save in the block `nTime` which is in seconds.

However I took the other suggestion of changing the name to maximum age.

⚠️ ismaelsadeeq opened an issue: "BlockTemplate Manager Tracking issue"
(https://github.com/bitcoin/bitcoin/issues/33758)
The main motivation and design rationale are discussed in #33389.

What to review

* #33421

### Plan

* [ ] **Introduce the Block Template Cache (Manager) #33421**
Implement a cache layer. All newly created block templates should be stored along with their respective configuration options. Client requests for block templates will specify the **maximum age** of the template in seconds; that is, how fresh they want the template to be:

* If a template with matching options exists in the cache
...

✅ ismaelsadeeq closed an issue: "RFC: Bitcoin Core Node `BlockTemplateManager`"
(https://github.com/bitcoin/bitcoin/issues/33389)

💬 ismaelsadeeq commented on issue "RFC: Bitcoin Core Node `BlockTemplateManager`":
(https://github.com/bitcoin/bitcoin/issues/33389#issuecomment-3474206427)
> Do you think https://github.com/bitcoin/bitcoin/issues/33756 would be compatible with this?

I have not take a look at that yet, I will do and revert thanks.

Closing this in favour of https://github.com/bitcoin/bitcoin/issues/33758 discussion can continue though.
Thanks for engaging.

💬 l0rinc commented on pull request "refactor: make script Solver's often-unused solutions parameter optional":
(https://github.com/bitcoin/bitcoin/pull/33757#issuecomment-3474213420)
> would like a bit more clarity on the benchmarks referenced

Was waiting for corecheck to finish, but it shows only `2.63%` speedup for `CCoinsCaching`.
I'll measure it properly, but in the meantime I'll leave this as a refactoring only since the resulting code is arguably cleaner.

👋 l0rinc's pull request is ready for review: "refactor: make script Solver's often-unused solutions parameter optional"
(https://github.com/bitcoin/bitcoin/pull/33757)

💬 ismaelsadeeq commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2482289948)
Yes.

💬 ismaelsadeeq commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2482293453)
Thanks will do when retouching.

💬 ismaelsadeeq commented on pull request "interfaces: enable cancelling running `waitNext` calls":
(https://github.com/bitcoin/bitcoin/pull/33676#discussion_r2482303861)
I’m a bit on the fence here a referenced parameter already signals that the value comes from elsewhere, also prevents nullptr input (must be initialized).

💬 ryanofsky commented on pull request "mining: check witness commitment in submitBlock":
(https://github.com/bitcoin/bitcoin/pull/33745#issuecomment-3474302360)
Code review ACK 6eaa00fe20206baedc0d8ade5bb8d066ea615704. Just documentation updates and test clarifications since last review, also splitting up a commit.

---

re: https://github.com/bitcoin/bitcoin/pull/33745#issuecomment-3472031005

> > and I think BlockTemplate accessors could be added so miners do not need to make hardcoded assumptions about the coinbase transaction? ([#33745 (comment)](https://github.com/bitcoin/bitcoin/pull/33745#discussion_r2477980297)).
>
> The Template Prov
...

🤔 ismaelsadeeq reviewed a pull request: "mining: check witness commitment in submitBlock"
(https://github.com/bitcoin/bitcoin/pull/33745#pullrequestreview-3405553194)
Concept ACK, will review.