💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490707151)
I don't understand where `16` and the `67%` come from.
Can we just keep the very last check of the median instead?

💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490708490)
k, please resolve the comment

💬 brunoerg commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#discussion_r2490759547)
cfeb160baec1369452c42d05c51f2a6af76ed077: nit: Perhaps we could remove this `(default: none)` to maintain consistency with other `=<file>` options?

👍 brunoerg approved a pull request: "init: Require explicit -asmap filename"
(https://github.com/bitcoin/bitcoin/pull/33770#pullrequestreview-3416811104)
code review ACK cfeb160baec1369452c42d05c51f2a6af76ed077

👍 TheCharlatan approved a pull request: "init: Require explicit -asmap filename"
(https://github.com/bitcoin/bitcoin/pull/33770#pullrequestreview-3416860885)
ACK cfeb160baec1369452c42d05c51f2a6af76ed077

💬 brunoerg commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#issuecomment-3486388292)
I left the `block_template_cache` fuzz target running overnight and generated a coverage report for it: https://brunoerg.xyz/bitcoin-core-coverage/33421/coverage_report/

💬 laanwj commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#discussion_r2490831082)
do we want a `DEFAULT_DISABLE_V1CONN_CLEARNET`?
(would be symmetric to the other options)

💬 gmaxwell commented on issue "dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us appears to be violating DNS seed policy":
(https://github.com/bitcoin/bitcoin/issues/33734#issuecomment-3486413945)
@luke-jr Can you point out where your filtering of recent versions was discussed in public in advance of implementation (per the policy, e.g. "Behavior outside of these expectations may be reasonable in some situations but should be discussed in public in advance")? It's both surprising to me and not consistent with what I knew. Filtering out sufficiently old versions is not news to me, at least to the extent it was filtering out things that actually might not work right (that it goes as rece
...

💬 fanquake commented on pull request "depends: disable variables, rules and suffixes.":
(https://github.com/bitcoin/bitcoin/pull/33045#issuecomment-3486418579)
Guix build (x86_64):
```bash
e0edc03339b5b32705b8eacbc213a6fd6e5b787caba574671bb3c18340a141ec guix-build-52b1595850f6/output/aarch64-linux-gnu/SHA256SUMS.part
55778a17e64cabdb200679240fa004b5c5c9a684ecae773e8497997ab7c968d6 guix-build-52b1595850f6/output/aarch64-linux-gnu/bitcoin-52b1595850f6-aarch64-linux-gnu-debug.tar.gz
6a2f49dbc8238438d40d46c3ce699814ae1d9eb6d74dd0b4ff273db6648b181c guix-build-52b1595850f6/output/aarch64-linux-gnu/bitcoin-52b1595850f6-aarch64-linux-gnu.tar.gz
2fce927
...

👍 theStack approved a pull request: "crypto: Use secure_allocator for `AES256_ctx`"
(https://github.com/bitcoin/bitcoin/pull/31774#pullrequestreview-3416932085)
Code-review ACK 8bdcd12d3bcbeaf922fa10dc2a261848e3900cfd

🚀 fanquake merged a pull request: "depends: disable variables, rules and suffixes."
(https://github.com/bitcoin/bitcoin/pull/33045)

💬 laanwj commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#discussion_r2490867244)
Would prefer to generalize this function name to `DisableV1OnNetwork`. That it happens to be about clearnet is an implementation, not an interface detail.

💬 brunoerg commented on pull request "node: add `BlockTemplateCache`":
(https://github.com/bitcoin/bitcoin/pull/33421#discussion_r2490867264)
084bfbc1ec7f8f64f54d231bb641285622311b59: I think the CI failure is related to this `std::chrono::seconds::max()` (long long) because the `maximum_template_age` is used in `IntervalElapsed`as the interval which is a `uint32_t`.

💬 l0rinc commented on pull request "random: scope environ extern to macOS, BSDs and Illumos":
(https://github.com/bitcoin/bitcoin/pull/33714#issuecomment-3486434684)
ACK 79d6f458e2300e1f47b94467cda233e1c761f8be

Thanks @hebasto for checking the other platforms.

💬 ismaelsadeeq commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3486443571)
reACK 6c7a34f3b0bd39ef7a1520aac56e12f78e5cc969 👾

There have been numerous changes to the header API since my last ACK.
I've reviewed the changes using the [compare diff](https://github.com/bitcoin/bitcoin/compare/a755e00a13541b3b5a707cf385f1cbec0449c6a9..6c7a34f3b0bd39ef7a1520aac56e12f78e5cc969)

<details>
<summary>Changes</summary>


* Fixed typo in `Write`
* `LoggingConnection` now does not take Logging Options; a clear error message is returned or thrown when the logger connectio
...

⚠️ Sjors opened an issue: "Mining interface tracking issue"
(https://github.com/bitcoin/bitcoin/issues/33777)
The mining interface is defined in:
- https://github.com/bitcoin/bitcoin/blob/master/src/interfaces/mining.h
- https://github.com/bitcoin/bitcoin/blob/master/src/ipc/capnp/mining.capnp

Clients that are known to depend on it:
- https://github.com/stratum-mining/sv2-tp

At this state it's OK to make breaking changes, such as deleting a method or changing its signature. But ideally we should do so once in a major release, collecting a bunch of fixes.

Non-breaking changes can be made any time, suc
...

🚀 hebasto merged a pull request: "random: scope environ extern to macOS, BSDs and Illumos"
(https://github.com/bitcoin/bitcoin/pull/33714)

💬 fanquake commented on pull request "kernel: Introduce C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-3486609787)
ACK 6c7a34f3b0bd39ef7a1520aac56e12f78e5cc969 - soon we'll be running bitcoin (kernel)

💬 theStack commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-3486623443)
Concept ACK

Nit for the future: putting the move-only changes on the existing code (i.e. introduction of the `validate_snapshot_import` and `complete_background_validation` methods) into an own refactoring commit would make reviewing this a bit easier, I think.

🚀 fanquake merged a pull request: "kernel: Introduce C header API"
(https://github.com/bitcoin/bitcoin/pull/30595)