💬 danielabrozzoni commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3480341842)
In my last push: re-introduce Assert in header sync tests, see https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2485972867

Sorry for another push after the acks, and thanks for the reviews

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2486365567)
Generally yes, but here I wanted to copy the [original code](https://github.com/bitcoin/bitcoin/pull/33637/files#diff-2e16a7ac99a65c1617112c3c326fba499b0010fe6c48b92b59bbc9696d883585L104) as closely as possible

💬 optout21 commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#issuecomment-3480347281)
utACK 5cbb9a40873203ea5a4dd0aa7127c9756da2f607

An evident, localized micro-optimization in a hot-path. I haven't run measurements.

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2486378183)
Pushed anyway, thanks

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2486378880)
Pushed anyway, thanks

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#discussion_r2486379425)
done, thanks

💬 fanquake commented on pull request "guix: Build for macOS using Clang only":
(https://github.com/bitcoin/bitcoin/pull/32764#issuecomment-3480370515)
Looks like this still using the GNU binutils for building native tools.

💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3480373301)
Code review reACK 9a387e9175d34d1eaf83b9008f5e857d1eb04b8e

Finalized the pointer to reference migration since last ack.

💬 optout21 commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#issuecomment-3480376742)
utACK 237eec0f7ca095bc60e40ee94ba1a160a7064753

Re-acked (after 7 mins :D ), only minor/formatting changes since last review

👍 hodlinator approved a pull request: "refactor: Header sync optimisations & simplifications"
(https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3410905600)
re-ACK 9a387e9175d34d1eaf83b9008f5e857d1eb04b8e

Changes since previous review (https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3409949311):

* Re-added `Assert` in response to https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2485972867
I was slightly worried the lock lambda would become more likely to return by-value, instead of by-reference, but `CBlockIndex` has `protected`/`delete`d copying:
https://github.com/bitcoin/bitcoin/blob/9a387e9175d34d1eaf83b9008f5e857d
...

🤔 sipa reviewed a pull request: "refactor: remove dead branches in `SingletonClusterImpl`"
(https://github.com/bitcoin/bitcoin/pull/33768#pullrequestreview-3410906177)
ACK 2d23820ee11678d567c75f94c40011ed9f0e274f

💬 stickies-v commented on issue "log: "Replaying blocks" / "Rolling forward" logging is rate-limited":
(https://github.com/bitcoin/bitcoin/issues/33769#issuecomment-3480392336)
Is there a good reason to unconditionally log this statement? It seems to me like a single unconditional "Replaying blocks from hash (height)" before starting the loop, and a conditional (debug) per-iteration statement would be sensible?

💬 optout21 commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2486416019)
Indeed, the input 2 also gives 0 as result. The intent may have been that got indices 0 and 1 it makes no sense to jump, for the rest follow the computation (but 2 accidentally also gives 0).
In any case, I don't want to change product behavior to the slightest in this PR.

💬 murchandamus commented on pull request "mini miner: enable `Linearize` return package feerates":
(https://github.com/bitcoin/bitcoin/pull/33216#issuecomment-3480414914)
Concept ACK

💬 murchandamus commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-3480418753)
Concept ACK

💬 murchandamus commented on pull request "rpc: print P2WSH and P2SH redem Script in getrawtransaction":
(https://github.com/bitcoin/bitcoin/pull/31252#issuecomment-3480436431)
Concept ACK

💬 A-Manning commented on pull request "rest: Query predecessor headers using negative count param":
(https://github.com/bitcoin/bitcoin/pull/33752#issuecomment-3480452826)
>Request the starting hash with GET /rest/blockhashbyheight/<HEIGHT>.<bin|hex|json> and then request headers upward with /rest/headers/?

We currently use this approach at L2L. This strategy is far from ideal - any of the upward requests may return headers from a different active chain, in the event of a re-org. It is necessary to check that the final block hash is the same as the tip hash we want to sync to, and if it is not, then the only way to retrieve the headers would be requesting heade
...

📝 ryanofsky opened a pull request: "init: Require explicit -asmap filename"
(https://github.com/bitcoin/bitcoin/pull/33770)
Currently, if `-asmap` is specified without a filename bitcoind tries to load `ip_asn.map` data file.

This change now requires `-asmap=ip_asn.map` or another filename to be specified explicitly.

The change is intended to make behavior of the option explicit avoid confusion reported https://github.com/bitcoin/bitcoin/issues/33386 where documentation specifies a default file which is not actually loaded by default. It was originally implemented in
https://github.com/bitcoin/bitcoin/pull/336
...

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2486501704)
Hmm right I was not clearing the containers, so the sorting was dominating. I retried with clearing and the unordered_map is still slightly faster. In your benchmark you are creating and reserving the map inside the benchmark instead of before. In this implementation the reserved memory is kept in between blocks, so reserving and creating outside makes more sense.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >20% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2486506578)
Why would short ids matter here though? Isn't that for keeping the bandwidth smaller for compact blocks?