π¬ 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
(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
...
(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?
(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
(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
...
(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
(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
...
(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
(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
...
(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.
(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)
(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!
(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.
(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.
(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
(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."
(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)
(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.
(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.
(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
(https://github.com/bitcoin/bitcoin/pull/33693#issuecomment-3458893049)
crACK 53b34c80c631ee3f5ae652315592924f6935e0f1