💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490689248)
Can we do it for the shorter ones as well? it's easier to compare them and more consistent with the rest

💬 laanwj commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-3486217821)
> A year later, adoption is now almost at ~70% according to https://21.ninja/reachable-nodes/node-service-share/ - could update the OP with that info.

Concept ACK. i was critical of this in the past, but i'm more convinced that it makes some sense now, and believe it outweighs the implementation/maintenance cost of such a feature.

💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490696786)
The test became more complicated than it was before (which was my main critique of it, seems arbitrary).
Could we simplify it? I think we're testing too many things, a single median check should likely suffice in my opinion.

💬 l0rinc commented on pull request "test: Add test on skip heights in CBlockIndex":
(https://github.com/bitcoin/bitcoin/pull/33661#discussion_r2490699922)
what does `--` mean here?

💬 frankomosh commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2490694848)
Only 1 coin is created (~1 BTC), but the target is 8 BTC. So my superficial thought is that `result_b` might never reach the target, therefore, the resulting failure would not be because of `TOTAL_TRIES` being hit, but simply insufficient funds. Or might I be missing some context here?

💬 frankomosh commented on pull request "test: add case where `TOTAL_TRIES` is exceeded yet solution remains":
(https://github.com/bitcoin/bitcoin/pull/33701#discussion_r2490695744)
nit: I think there's a double space after `=`. (maybe should be single spaced for consistency?)

💬 Sjors 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-3486236896)
Note that there's other seeds run by people who are not active contributors. That shouldn't be a reason to remove a seed, unless we're consistent about that.

Active hostility is a different matter. I don't read that in "no longer support the Bitcoin Core project", I do see it in social media. The question is whether that's sufficient grounds for removing a seed, or whether something additional should happen.

💬 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`.