Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 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
...
💬 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
...