Bitcoin Core Github
43 subscribers
123K links
Download Telegram
πŸ’¬ l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#issuecomment-3162361510)
I ran the 3 versions you mentioned with hyperfine, 10 runs each:
```python
COMMITS="d767503b6a2618e0c99407acf98f3bd19fb7defd 5bb380ec87af93ed5543ba10e1f8465aca7bcfe7 35e91b899dac689b24a8a95fcef083affc695d1e"; \
PARALLEL=200; RUNS=10; \
CC=gcc; CXX=g++; \
...
πŸ’¬ sipa commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#issuecomment-3162395975)
The changes in `src/qt/bitcoin_szl.ts` look like they contain Rust source code?
πŸ’¬ Ataraxia009 commented on pull request "Removing Bitcoin core text where unnecessary":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3162400137)
> You are free to fork the code and change whatever you like, but the burden of maintaining your fork falls on you not the contributors of Bitcoin Core.

good call, there was a problem with using binary.name in the new python lief version. We should be fine now, just pushed a fix.

However i cant run the drahbot build, if somebody could add the label for building, would be helpful
πŸ’¬ Ataraxia009 commented on pull request "Removing Bitcoin core text where unnecessary":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3162407197)
> You are free to fork the code and change whatever you like, but the burden of maintaining your fork falls on you not the contributors of Bitcoin Core.

The contributors and builders of the bitcoin network do have a responsibility towards helping decentralise the network, which leads to overall network health.

In my humble opinion, helping support more diversity in client choice for node runners should be, if not already is, a core principle of the network.

Also this leads to less hardc
...
πŸ’¬ maflcko commented on pull request "Removing Bitcoin core text where unnecessary":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3162656637)
> Can we merge this, the builds seem fine

No, it didn't. The build failed and you haven't tested this yourself at all?



> However i cant run the drahbot build, if somebody could add the label for building, would be helpful

A passing CI (or build) is not sufficient for a merge. Especially here, because a full guix build (with signing) isn't run by any CI. The burden to explain the changes (and why they are safe and correct, and how to test them) is on the pull request author. Instead
...
πŸ’¬ maflcko commented on pull request "cmake: Switch to generated `ts_files.cmake` file":
(https://github.com/bitcoin/bitcoin/pull/33115#issuecomment-3162661955)
> The changes in `src/qt/bitcoin_szl.ts` look like they contain Rust source code?

Yes, see also https://github.com/maflcko/b-c-gui-translations-review/blob/fbeee68d9e07215e3ab0dc2e17ddf41ed9af7ce5/reviews/szl.md
πŸ’¬ maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#issuecomment-3162681579)
> So this is obviously faster for many tiny tests, will run the same for the full test suite and do a full code review later.

Nice. Thanks for testing and confirming that the code without any sleep is the fastest of the 3 versions. I think benchmarking the full test suite may be difficult, because it has internal noise higher than the savings here. At least back in https://github.com/bitcoin/bitcoin/pull/13384#issuecomment-394335449, it was hard to measure, but maybe there is less noise now a
...
πŸ’¬ optout21 commented on pull request "merkle: pre‑reserve leaves to prevent reallocs with odd vtx count":
(https://github.com/bitcoin/bitcoin/pull/32497#issuecomment-3162698716)
LGTM!
Minor comments:
- Maybe add a note to the `ComputeMerkleRoot` declaration, that the `hashes` input is expected to have one extra capacity (in case size is odd). Minor, as this function is not used directly much.
- Even more, it could debug-assert that capacity is even, but this may be a bit of a stretch.
πŸ€” maflcko reviewed a pull request: "Add Docker support with multi-arch build and user permissions handling"
(https://github.com/bitcoin/bitcoin/pull/33139#pullrequestreview-3095800749)
now you are just copy-pasting random files from the internet without even mentioning the source?
πŸ’¬ maflcko commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#discussion_r2259396974)
copy-paste from https://github.com/hundehausen/tor-hidden-service-docker/blob/main/Dockerfile
πŸ’¬ maflcko commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#discussion_r2259458592)
This needs a proper workflow documented on how to bump it and when. Otherwise, it would be confusing to have a 30.0 git release tag which contains a dockerfile with a hard-coded 29.0 tag.
πŸ’¬ maflcko commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#discussion_r2259402491)
copy-paste from https://github.com/willcl-ark/bitcoin-core-docker/blob/master/29.0/alpine/Dockerfile
βœ… fanquake closed a pull request: "doc:Fix typos"
(https://github.com/bitcoin/bitcoin/pull/33148)
πŸ’¬ fanquake commented on pull request "doc:Fix typos":
(https://github.com/bitcoin/bitcoin/pull/33148#issuecomment-3162996627)
PIcked the relevant change into #33125.
πŸ’¬ fanquake commented on pull request "ci: Use mlc `v1` and fix typos":
(https://github.com/bitcoin/bitcoin/pull/33125#discussion_r2259479844)
Dropped this for now.
πŸ’¬ maflcko commented on pull request "Add Docker support with multi-arch build and user permissions handling":
(https://github.com/bitcoin/bitcoin/pull/33139#issuecomment-3163010102)
For reference, for development, the dockerfile should look more like https://github.com/fanquake/core-review/blob/0bb70124a23807e3638f29ebd76c2bebc4880fd7/podman/debian.imagefile, just containing the install commands copy-pasted from the docs. But the development and deployment workflows differ significantly enough to not mix them up as the same.

My recommendation would be to just pick *one* problem/goal and not try to interleave multiple in the same pull request.
πŸ’¬ maflcko commented on pull request "ci: Use mlc `v1` and fix typos":
(https://github.com/bitcoin/bitcoin/pull/33125#issuecomment-3163012735)
Is this ready for review?
πŸ’¬ maflcko commented on pull request "tests: cover abortrescan() in-flight True path with dynamic-tail retry":
(https://github.com/bitcoin/bitcoin/pull/33131#discussion_r2259496837)
I am sure on Linux you could halt execution of a process (e.g. by adding a gdb breakpoint), but I am not sure if this is possible on Windows, or if this something worth trying.

Could mark as draft while CI is red?
πŸ’¬ maflcko commented on issue "Release Schedule for 30.0":
(https://github.com/bitcoin/bitcoin/issues/32275#issuecomment-3163039235)
> 2025-08-06 🚧

Looks like the translation update was missed?
πŸ“ bitcoin0158 opened a pull request: "Bitcoin v1"
(https://github.com/bitcoin/bitcoin/pull/33149)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...