💬 00w1 commented on pull request "Remove unreliable seed from chainparams.cpp, and the associated README":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458908994)
NACK

1. This pull request is a desperate attempt to remove the DNS seed without clarifying anything with Luke.
2. I do not see any policy that requires the inclusion of latest releases in the DNS seed results.

> Given that Luke was hacked a couple years ago, arguably his DNS seed should have been removed at that time. Furthermore, his personal website still has the following disclaimer, even after 3 years:

> We also know that Luke has been in contact with the FBI regarding this hack, a
...

💬 l0rinc commented on pull request "refactor: optimize block index comparisons (1.4-6.8x faster)":
(https://github.com/bitcoin/bitcoin/pull/33637#issuecomment-3458941571)
Rebased after https://github.com/bitcoin/bitcoin/pull/29640 - only change was adjusting the comment

💬 l0rinc commented on pull request "[IBD] coins: increase default UTXO flush batch size to 32 MiB":
(https://github.com/bitcoin/bitcoin/pull/31645#issuecomment-3458961773)
Thank you for the reviews!

rfm?

💬 davidgumberg commented on pull request "Remove unreliable seed from chainparams.cpp, and the associated README":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458988382)
ACK https://github.com/bitcoin/bitcoin/commit/c18f9d1fd5d4e1fc255d83ed7f808e2571c4ed9f

I think you should squash the two commits, and change the commit description to match the PR description.

@00w1

> I do not see any policy that requires the inclusion of latest releases in the DNS seed results.

[`/doc/dnsseed-policy.md`](https://github.com/bitcoin/bitcoin/blob/3bb30658e631ed45b6c8609292facc7ae3dd0f61/doc/dnsseed-policy.md#expectations-for-dns-seed-operators)

> 1. The DNS seed r
...

💬 john-moffett commented on pull request "Remove unreliable seed from chainparams.cpp, and the associated README":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3459024346)
ACK c18f9d1fd5d4e1fc255d83ed7f808e2571c4ed9f

@SatsAndSports I used a custom hacked-together script that switched VPNs to several different countries, re-ran, and de-duped the IPs. This is the [code](https://gist.github.com/john-moffett/fb6f38af787e7231c135c1b1a865d961) with the VPN related stuff removed.

💬 00w1 commented on pull request "Remove unreliable seed from chainparams.cpp, and the associated README":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3459050985)
> Modifying the DNS seed results to exclude functioning Bitcoin nodes on the public network because of their version clearly violates this expectation, or do you have a different interpretation of the words "fairly selected and functioning Bitcoin nodes from the public network"?

[Version based filtering](https://delvingbitcoin.org/t/dns-seed-node-filtering/570) is done by other DNS seeds as well. It’s not a new practice or a violation of the policy.

Older versions are **functioning bitcoin
...

💬 achow101 commented on pull request "Remove unreliable seed from chainparams.cpp, and the associated README":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3459143043)
> Its interesting to note that @achow101's comments contradict her previous comments about security: [#29149 (comment)](https://github.com/bitcoin/bitcoin/pull/29149#issuecomment-1872189682)

To clarify, there has been new information since that comment was made. In particular, we were notified of the FBI's (continued) involvement in 2024. Additionally, it's now been 3 years since the hack and the disclaimer is still on his website. This suggests that no further action has been done to ensure
...

💬 Ademan commented on pull request "Remove unreliable seed from chainparams.cpp, and the associated README":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3459168786)
> > Modifying the DNS seed results to exclude functioning Bitcoin nodes on the public network because of their version clearly violates this expectation, or do you have a different interpretation of the words "fairly selected and functioning Bitcoin nodes from the public network"?
>
> [Version based filtering](https://delvingbitcoin.org/t/dns-seed-node-filtering/570) is done by other DNS seeds as well. It’s not a new practice or a violation of the policy. Older versions are **functioning bitc
...

💬 hebasto commented on pull request "random: scope the environ extern to macOS":
(https://github.com/bitcoin/bitcoin/pull/33714#issuecomment-3459211092)
> Testing on some other platforms: [hebasto/bitcoin-core-nightly#79](https://github.com/hebasto/bitcoin-core-nightly/pull/79).

Builds on the *BSD and illumos systems failed.

💬 hebasto commented on pull request "guix: Build for macOS using Clang only":
(https://github.com/bitcoin/bitcoin/pull/32764#issuecomment-3459238801)
My Guix build:
```
aarch64
a8613aaa30cc598f8f193932478e57381299399fb1a6a3e8cd27d41ed58e1a2c guix-build-7c97bae595b0/output/aarch64-linux-gnu/SHA256SUMS.part
6b32bd6a25e2c8b2823efc9deb8d9dc1fd4da7713dc00992ee721171500ebbf0 guix-build-7c97bae595b0/output/aarch64-linux-gnu/bitcoin-7c97bae595b0-aarch64-linux-gnu-debug.tar.gz
fbda951a67c0b02a9c4ed8014b3371ecacf7c41f2f0338653963c2f765e29f7f guix-build-7c97bae595b0/output/aarch64-linux-gnu/bitcoin-7c97bae595b0-aarch64-linux-gnu.tar.gz
d99914fd
...

💬 mzumsande commented on pull request "Remove unreliable seed from chainparams.cpp, and the associated README":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3459305391)
In my opinion it doesn't really matter whether a single DNS seeds returns nodes running the newest version or not. DNS seeds are important for the initial bootstrap, they are ideally are queried exactly once in the lifetime of a node. After that, nodes will automatically learn about thousands of other peers, so they will get a representative sample of whatever mix of nodes anyway. Besides, newer Knots versions based on 29.x aren't returned either.

I think the other issues brought up [above](h
...

👍 polespinasa approved a pull request: "refactor: Header sync optimisations & simplifications"
(https://github.com/bitcoin/bitcoin/pull/32740#pullrequestreview-3391487971)
Code Review ACK 50e61f448eedfe0ccfa07f3124aeef9c7762a69b

This PR cleans and simplifies the code in many ways like avoiding calling unneeded functions, avoiding constructing temporary objects, removing unused params, among others...

Just left a single comment with a question :)

btw 552b9c3a565c1817074468bf71fb4526f3a47f42 and 67aa62c80e7d2faa485ba020cefe262f479190f4 are pretty :)

💬 polespinasa commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2471574938)
I don't get this change, what's the use-case for explicit here? and why is better to use the list instead of a "normal" constructor?

💬 delta1 commented on pull request "Remove unreliable seed from chainparams.cpp, and the associated README":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3459606922)
reACK https://github.com/bitcoin/bitcoin/commit/c18f9d1fd5d4e1fc255d83ed7f808e2571c4ed9f

💬 shyrwall commented on pull request "Remove unreliable seed from chainparams.cpp, and the associated README":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3459616234)
> I just queried all the DNS seeds and compared responses with BitNodes API, using `jq` to isolate the user agent:
>
> https://gist.github.com/pinheadmz/ff6057f14aa6b9aa642db2c98f331a1a
>
> ~No real anomolies to speak of,~ except that `seed.mainnet.achownodes.xyz` returned the **most** knots nodes 🤷
>
> update: `dnsseed.bitcoin.dashjr-list-of-p2p-nodes.us` is not returning v29 or v30 nodes, apparently

You can just check https://luke.dashjr.org/programs/bitcoin/files/charts/data/seed
...

📝 Ashok8601 opened a pull request: "Fix formatting in windows-app.manifest.in"
(https://github.com/bitcoin/bitcoin/pull/33729)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 frankomosh commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2471886228)
If user passes 3 args where `argv[2] != "-regtest"`(or generally malformed argument combinations), could it silently set `use_regtest = false` without warning?

If thats the case then I think there should atleast be a warning

💬 frankomosh commented on pull request "test: Add bitcoin-chainstate test for assumeutxo functionality":
(https://github.com/bitcoin/bitcoin/pull/33728#discussion_r2471889658)
Is this now weaker that the original assertion, because I think it will now silently accepts a null mempool pointer?

🤔 frankomosh reviewed a pull request: "test: Add bitcoin-chainstate test for assumeutxo functionality"
(https://github.com/bitcoin/bitcoin/pull/33728#pullrequestreview-3391873761)
Concept ACK
I think its a nice addition as it adds valuable functional coverage for bitcoin-chainstate.
Few nits and inquiries inline;

✅ maflcko closed a pull request: "Fix formatting in windows-app.manifest.in"
(https://github.com/bitcoin/bitcoin/pull/33729)