💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r2469713813)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r2454538331

This is an interesting idea because you are right that basically everywhere the `Update()` method is used here and in follow-up PRs (#25722 and #29700), result state rarely goes from failure->success.

There is one place it happens in this PR though: when a failed LoadChainstate operation is [retried](https://github.com/ryanofsky/bitcoin/blob/8b892d41fdeb5756fd83f6050f27a170338d260a/src/init.cpp#L1753-L1758).

We cou
...

💬 dergoegge commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457355840)
ACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e

Don't think a lot of damage can be done by controlling just one of the seeds but it's just a matter of time before Luke pulls some random shenanigans.

💬 pinheadmz commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457377979)
> Don't think a lot of damage can be done by controlling just one of the seeds but it's just a matter of time before Luke pulls some random shenanigans.

This is going to be a hard thread to moderate because personal attacks are forbidden, but this is one of the very few lines of code that literally point to a specific central entity.

The PR author is also a first-time contributor to the project and I'm worried this PR is going to get brigaded by people susceptible to social media influence
...

👍 stickies-v approved a pull request: "Remove unnecessary seed from chainparams.cpp"
(https://github.com/bitcoin/bitcoin/pull/33723#pullrequestreview-3389712658)
ACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e

Operators of DNS seeds have a (limited) ability to be harmful to users, e.g. by filtering results or logging requests. Luke has shown multiple cases of hostile and unpredictable behaviour towards the Bitcoin Core project, so removing this seed seems like the responsible thing to do, even if we can't guarantee the reliability of other DNS seeds.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470247644)
Originally we flushed, so if we still want to, we may want to extend this to assert that behavior. Or avoid flushing and simplify the bench. Or add the flushing behavior above to a test, etc.

💬 pinheadmz commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457396673)
I think it is far more mature to wait until the service becomes unreliable or violates a stated policy rule before removing. Otherwise the project is merely reacting preemptively to someone's tweets.

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470254148)
Even consensus invalid ones? Or did I misunderstand the context here?

💬 l0rinc commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470260510)
Yeah, but we want to understand where the differences are coming from, otherwise we'd have the "faster-on-my-machine" syndrome. If you disagree, just resolve the issue.

💬 dergoegge commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457411161)
> "must be run by entities which have some minimum level of trust within the Bitcoin community"

Can only speak for myself but that doesn't reflect my current sentiment regarding Luke.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470278073)
We avoid flushing now.

💬 maflcko commented on issue "GetSerializeSize's return type should not be platform dependent":
(https://github.com/bitcoin/bitcoin/issues/33709#issuecomment-3457434288)
> ... LLM-generated. I'm not sure how to go about that. I am not comfortable with the idea of an LLM making changes to consensus-critical code. On the other hand, if the code is correct just closing the PR seems inappropriate.

I looked at the PR, and while it appears "correct", it is randomly only changing `size_t` to `uint64_t` in half the places, even if they are on adjacent lines (https://github.com/bitcoin/bitcoin/pull/33712/files#diff-a9bf14cec48b76a03a785de76b8bc71c5db17d494c7f02c29dfde26
...

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2470280988)
Yeah, InputFetcher doesn't know if a block is consensus valid or not yet. It hasn't passed `ConnectBlock` yet before entering here.

💬 darosior commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3457444677)
Concept ACK.

💬 brunoerg commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2470293205)
Perfect, I wasn't aware of `UpdateModifiedFee`, I just took a look and this is a saturated addition, so it won't have any effect. This can be ignored.

💬 willcl-ark commented on pull request "guix: build for Linux HOSTS with `-static-libgcc`":
(https://github.com/bitcoin/bitcoin/pull/33181#issuecomment-3457495113)
No match.

```
❯ uname -m; find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
x86_64
76d395814824929ef63caf98b3f5c0d4be2aa310c91c86cdd4fa7f4835a2462e guix-build-a99bbec14500/output/aarch64-linux-gnu/SHA256SUMS.part
4261545a6177bd944b93e1590d3d5fed5466bcc7aa5cb8f34be48d5aba7f0286 guix-build-a99bbec14500/output/aarch64-linux-gnu/bitcoin-a99bbec14500-aarch64-linux-gnu-debug.tar.gz
50c7bdd8f65ffafc5dbe303705ab3e0b2d80f3834a0
...

🤔 hebasto reviewed a pull request: "refactor: Return uint64_t from GetSerializeSize"
(https://github.com/bitcoin/bitcoin/pull/33724#pullrequestreview-3389834738)
Concept ACK.

💬 maflcko commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-3457573218)
Looks like the new valgrind fuzz task is failing due to a false positive warning, which needs to be disabled like in the other GCC tasks:

```
$ git grep maybe-uninitialized ./ci
ci/test/00_setup_env_arm.sh:export BITCOIN_CONFIG="-DREDUCE_EXPORTS=ON -DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=maybe-uninitialized'"
ci/test/00_setup_env_win64.sh:-DCMAKE_CXX_FLAGS='-Wno-error=maybe-uninitialized'"

📝 hebasto opened a pull request: "ci, iwyu: Treat warnings as errors for `src/init` and `src/policy`"
(https://github.com/bitcoin/bitcoin/pull/33725)
This PR continues the work from bitcoin/bitcoin#31308 by extending the treatment of IWYU warnings as errors to the `src/init` and `src/policy` directories.

💬 reardencode commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457597324)
utACK 9c3291f

Given recent developments, it seems that including Bitcoin Knots' maintainer's seed as a trusted bootstrap source for bitcoin core users is no longer advisable.

💬 delta1 commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3457601403)
ACK 9c3291ff9bd713e1a92dae2c388a83e4a107bf7e

Luke considers Bitcoin Core v30 to be "malware", so he probably doesn't want to provide a DNS seed for Core nodes any longer.

<img width="313" height="139" alt="image" src="https://github.com/user-attachments/assets/f3512f65-95ce-47ef-940f-88e763295331" />