💬 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

📝 stringintech opened a pull request: "test: Add bitcoin-chainstate test for assumeutxo functionality"
(https://github.com/bitcoin/bitcoin/pull/33728)
This PR adds functional test coverage for the bitcoin-chainstate tool loading a datadir initialized with an assumeutxo snapshot.

The PR also includes:
- Fix for assertion crash in `ActivateExistingSnapshot()` when active chainstate has no initialized mempool (required for the test to pass)
- `-regtest` flag support for bitcoin-chainstate to enable the testing

This work started while experimenting with how the current state of the kernel API (#30595) behaves when loading a datadir contain
...

💬 niteshbalusu11 commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458719646)
@SatsAndSports some basic python vibe coding. most of the code is to make it pretty looking, core logic is really small.

https://gist.github.com/niteshbalusu11/582f450104a0493ed78a0d0edb54a928

💬 l0rinc commented on pull request "refactor: unify container presence checks":
(https://github.com/bitcoin/bitcoin/pull/33192#issuecomment-3458728275)
Thanks for the feedback, I have [reverted](https://github.com/bitcoin/bitcoin/compare/d870caca33ac9f0fdd76969a7341535c5722417e..9445f3955cefaaf7113eb520c5fa9ca905182450) the mining and mempool related coflicting changes and [rebased](https://github.com/bitcoin/bitcoin/compare/9445f3955cefaaf7113eb520c5fa9ca905182450..b97db6f06185a1d63828f958b33a7e6780aee73c) in a separate commit to simplify review.

I agree that these refactors shouldn't conflict with other important changes, it's why I have r
...

💬 davidgumberg commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#discussion_r2471194836)
Agreed

💬 davidgumberg commented on pull request "Modernize use of UTF-8 in Windows code":
(https://github.com/bitcoin/bitcoin/pull/32380#issuecomment-3458739956)
> > 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
...

💬 l0rinc commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458773128)
ACK c18f9d1fd5d4e1fc255d83ed7f808e2571c4ed9f

Please update the PR title now that the README was also updated.
And I personally would suggest a different phrasing, we're not removing this entry because it's "unnecessary" but because of breach of trust.

💬 kevkevinpal commented on pull request "refactor/doc: Add blockman param to GetTransaction doc comment":
(https://github.com/bitcoin/bitcoin/pull/33683#issuecomment-3458778733)
reACK [1a1f46c](https://github.com/bitcoin/bitcoin/pull/33683/commits/1a1f46c2285994908df9c11991c1f363c9733087)

💬 l0rinc commented on pull request "refactor: Return uint64_t from GetSerializeSize":
(https://github.com/bitcoin/bitcoin/pull/33724#issuecomment-3458799452)
Concept ACK (only reviewed it lightly) - thanks for taking care of this, I will get back and review it more thoroughly!

💬 hebasto commented on pull request "guix: Build for macOS using Clang only":
(https://github.com/bitcoin/bitcoin/pull/32764#issuecomment-3458812746)
Rebased to resolve a conflict with the merged bitcoin/bitcoin#33185.

💬 A-Manning 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_r2471241081)
`nullptr` is already used to indicate that no ZMQ interface was initialized. This is not the same as failing to initialize a ZMQ interface.

💬 stickies-v commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458831359)
re-ACK c18f9d1fd5d4e1fc255d83ed7f808e2571c4ed9f

💬 jlopp commented on pull request "Remove unnecessary seed from chainparams.cpp":
(https://github.com/bitcoin/bitcoin/pull/33723#issuecomment-3458846272)
> Please update the PR title now that the README was also updated.

Since any given DNS seed is strictly unnecessary, I'd suggest changing "unnecessary" to "unreliable."

🚀 hebasto merged a pull request: "Modernize use of UTF-8 in Windows code"
(https://github.com/bitcoin/bitcoin/pull/32380)

👍 hebasto approved a pull request: "ci: use pycapnp 2.2.1"
(https://github.com/bitcoin/bitcoin/pull/33693#pullrequestreview-3391123943)
ACK 53b34c80c631ee3f5ae652315592924f6935e0f1.

👍 hebasto approved a pull request: "build: Bump clang minimum supported version to 17"
(https://github.com/bitcoin/bitcoin/pull/33555#pullrequestreview-3391131782)
ACK fa0fa0f70087d08fe5a54832b96799bd14293279.

💬 l0rinc commented on pull request "ci: use pycapnp 2.2.1":
(https://github.com/bitcoin/bitcoin/pull/33693#issuecomment-3458893049)
crACK 53b34c80c631ee3f5ae652315592924f6935e0f1

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