💬 davidgumberg commented on pull request "ci: Retry image building once on failure":
(https://github.com/bitcoin/bitcoin/pull/33677#issuecomment-3458345585)
crACK https://github.com/bitcoin/bitcoin/pull/33677/commits/5555bce994b648f836c35a02570f22ae9ad36da3

Concept ACK on moving the CI script from bash to python and crACK https://github.com/bitcoin/bitcoin/pull/33677/commits/fabe516440c96bb7a6a6902195684d3802d64139 and https://github.com/bitcoin/bitcoin/pull/33677/commits/5555bce994b648f836c35a02570f22ae9ad36da3 as refactors with no behavior change, but those changes seem orthogonal to the PR description and linked issue and I think they would i
...

💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for `src/init` and `src/policy`":
(https://github.com/bitcoin/bitcoin/pull/33725#discussion_r2470959705)
Thanks! Reworked.

💬 SatsAndSports commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458378187)
I've updated the description now, linking to some of the evidence provided by others in this thread, and mentioning the policy that you referenced. I'm not an expert at writing PRs, I hope my updated description is concise and helpful

> @SatsAndSports it would be helpful if you could update the PR description with some of the more specific motivations.

@davidgumberg , thanks for pointing out the README change. I've updated it now too in this PR

💬 TheCharlatan commented on pull request "build: Introduce internal kernel library":
(https://github.com/bitcoin/bitcoin/pull/28690#issuecomment-3458382072)
Rebased 78ed83ba337c97798134f4bd0f9307facde3a59d -> a9ec7cdc0a1f6d4a3bd532649e79459074ed9f3b ([kernelInternalLib_20](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_20) -> [kernelInternalLib_21](https://github.com/TheCharlatan/bitcoin/tree/kernelInternalLib_21), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelInternalLib_20..kernelInternalLib_21))

* Fixed conflict with #33218

💬 instagibbs commented on pull request "Cluster mempool":
(https://github.com/bitcoin/bitcoin/pull/33629#discussion_r2450652757)
there's a single lasting reference to "package-mempool-limits" in the comment in this file

💬 davidgumberg commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2470986340)
+1 to the way @instagibbs pointed out for honest peers sending unsolicited CMPCTBLOCK's making this a bad idea. And with this, any way now or in the future that you could trick a node into sending an unsolicited CMPCTBLOCK becomes an attack vector. Given that there is little an attacker could accomplish by sending us an unsolicited compact block if we drop unsolicited compact blocks on the ground, I think we don't gain much by marking them as misbehaving or disconnecting them.

💬 niteshbalusu11 commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458426762)
I ran a test a few times, my results.

<img width="1416" height="505" alt="Screenshot 2025-10-28 at 4 47 44 PM" src="https://github.com/user-attachments/assets/3bf09a9e-8132-4ac4-8198-b1687c7c4e69" />

💬 davidgumberg commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2471008758)
@ajtowns I'm not sure which suggestion you mean, I missed the suggestion to drop the newline, but in the current branch (5caaefd8d4174cfc27d5538e7f2d3a1c2bdb77d8) this is a `CMPCTBLOCK` message and IP's are logged: https://github.com/bitcoin/bitcoin/blob/5caaefd/src/net_processing.cpp#L2484

💬 onyxcoyote commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458447799)
There is another important reason why I ACK this...

Separation of duties, which is both a best practice, and even legally required in some industries (like banks).

Any one individual should not cross over multiple checks and balances. For example, they should not be able to develop, test, review, and deploy code to production (or really any 2-3 of those things). It does not matter if they are a rockstar developer and 100% trusted, or even if it could offend them. In my previous life I rem
...

💬 ajtowns commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2471037172)
```
$ git rev-parse HEAD
5caaefd8d4174cfc27d5538e7f2d3a1c2bdb77d8
$ grep not.marked net_processing.cpp
4416: LogDebug(BCLog::NET, "Peer %d, not marked as high-bandwidth, sent us an unsolicited compact block!\n", pfrom.GetId());
```

💬 SatsAndSports commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458485281)
Thanks @niteshbalusu11 and @john-moffett for your detailed breakdown of the versions. Can you share the code or system you used?

I just updated the description of this PR with a small script I made, with `dig` and `nmap`, also showing that recent versions are not represented in this seed. But it would be great to have your code and data described, as mine data is very basic

💬 maflcko commented on pull request "ci: Retry image building once on failure":
(https://github.com/bitcoin/bitcoin/pull/33677#issuecomment-3458487836)
> refactors with no behavior change, but those changes seem orthogonal to the PR title and linked issue and I think they would ideally be in a separate PR.

thx, i'll open a dedicated follow-up for the remaining refactors. Though, I'll leave this one as-is for now, as it already has 3 acks and seems rfm.

💬 davidgumberg commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2471044173)
@ajtowns Sorry, my mistake, see my edit above.

💬 davidgumberg commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#discussion_r2471047407)
I should've counted a few more Mississippi's before commenting 😁

💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3458530990)
> Don't have access to a Windows device at the moment but I'll try and test this at some point, would there be a reasonable unit/functional test that `fsbridge::fopen` and argv can still handle non-ascii stuff on Windows? Might be nice to have, but not blocking by an means.

I believe it is handled by unit and functional tests:
```
$ git grep ₿_🏃
src/test/dbwrapper_tests.cpp: fs::path ph = m_args.GetDataDirBase() / "test_runner_₿_🏃_20191128_104644";
src/test/fs_tests.cpp: std::stri
...

💬 hebasto commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2471072312)
Let me leave this change for a follow-up to avoid invalidating 3 acks.

💬 maflcko commented on issue "ci: short read: expected xxxxxxxxx bytes but got xxxxxxxxx: unexpected EOF":
(https://github.com/bitcoin/bitcoin/issues/33640#issuecomment-3458556346)
I presume the same issue exists for the lint task. Though, the caching there does not seem to work at all. So it could make sense to make the caching work there, and then also add a retry loop there.


Otherwise, there could be Ubuntu APT timeouts like https://github.com/bitcoin/bitcoin/actions/runs/18861389218/job/53820273763?pr=33247#step:4:1053

💬 furszy commented on pull request "http: replace WorkQueue and single threads handling for ThreadPool":
(https://github.com/bitcoin/bitcoin/pull/33689#discussion_r2471097523)
Follow-up to this convo after some great back-and-forth via DM with Andrew.
Usually, RAII works best when we don’t care exactly when an object gets destroyed. In this case, though, we actually do care about that moment (or, in our terms, when it’s stopped), since it marks the point where there are no pending tasks and it’s safe to tear down the backend handlers.
If we don’t enforce that explicitly, the shutdown procedure could continue while workers are still active, leading them to access nul
...

💬 maflcko commented on pull request "Log ZMQ bind error at Error level, abort startup on ZMQ init error (#33715)":
(https://github.com/bitcoin/bitcoin/pull/33727#discussion_r2471102635)
pointers are nullable, so why wrap twice?

💬 maflcko commented on pull request "ci: use pycapnp 2.2.1":
(https://github.com/bitcoin/bitcoin/pull/33693#issuecomment-3458604240)
lgtm ACK 53b34c80c631ee3f5ae652315592924f6935e0f1