Bitcoin Core Github
44 subscribers
122K links
Download Telegram
🤔 i-am-yuvi reviewed a pull request: "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage"
(https://github.com/bitcoin/bitcoin/pull/32516#pullrequestreview-2877825924)
Post-Merge ACK 84aa484d45e2fb3c1149941ef23779e4adb983d9
💬 fanquake commented on pull request "build: add a depends dependency provider":
(https://github.com/bitcoin/bitcoin/pull/32595#issuecomment-2919014673)
> A fallback provider: This will use system packages when they are found and fall back to compiling vendored depends. Using this provider, find_package will always succeed, but it will not compile any "unnecessary" code. This approach may be preferred by developers or for quick experiments.

I can't tell from this if you are saying to use depends to "fill in" any missing system packages, or, if all the required system packages aren't available, we would switch to using depends (unclear if that
...
💬 hebasto commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2919073897)
I believe the manifest could be further simplified:
```diff
--- a/contrib/guix/manifest.scm
+++ b/contrib/guix/manifest.scm
@@ -18,7 +18,7 @@
((gnu packages python-build) #:select (python-poetry-core))
((gnu packages python-crypto) #:select (python-asn1crypto))
((gnu packages python-science) #:select (python-scikit-build-core))
- ((gnu packages python-xyz) #:select (python-pydantic-2 python-pydantic-core))
+ ((gnu package
...
💬 hebasto commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2113740976)
Could we defer such amendments to follow-ups until feedback from real-world CI usage has been collected?
💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-2919095363)
@hebasto @davidgumberg I believe all comments have been addressed?
💬 marcofleon commented on pull request "fuzz: Add target for coins database":
(https://github.com/bitcoin/bitcoin/pull/32602#discussion_r2113749842)
I think the comment makes sense here, even if it is a bit verbose. Happy to change it once a fix such as https://github.com/bitcoin/bitcoin/pull/32313 gets merged.
💬 marcofleon commented on pull request "fuzz: Add target for coins database":
(https://github.com/bitcoin/bitcoin/pull/32602#discussion_r2113750187)
Done
💬 marcofleon commented on pull request "fuzz: Add target for coins database":
(https://github.com/bitcoin/bitcoin/pull/32602#discussion_r2113750716)
Done.
💬 marcofleon commented on pull request "fuzz: Add target for coins database":
(https://github.com/bitcoin/bitcoin/pull/32602#discussion_r2113750909)
Done.
👍 hebasto approved a pull request: "ci: remove 3rd party js from windows dll gha job"
(https://github.com/bitcoin/bitcoin/pull/32513#pullrequestreview-2877956479)
ACK ff2e35f6315cc6b912f68fe85103575d751bd2bc.

While touching this code, it seems reasonable to also address the other @davidgumberg's [comment](https://github.com/bitcoin/bitcoin/pull/32634#issuecomment-2918081325):
> Might be nice to have a CI check like [davidgumberg@1b98160](https://github.com/davidgumberg/bitcoin/commit/1b9816052b1d8ad1d9c17945530e14ead79fce33) to prevent future regressions, but OK for a separate PR if out of scope.
💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-2919114726)
> While this approach introduces some duplication at present, it will ultimately enable us to consolidate how the IWYU tool is executed, allowing for a choice between using the iwyu_tool.py script or CMake's built-in functionality.

Reading the PR description, it's not really clear why we'd need to retain using both tools? Either the CMake built-in should be better in some way, or at least equivalent in functionality, in which case we should just switch to using it?
💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2113759162)
Is there an issue open for this upstream?
💬 fanquake commented on pull request "ci, iwyu: Treat warnings as errors for specific targets":
(https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2113759342)
Same here?
💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-2919117368)
> While touching this code, it seems reasonable to also address the other @davidgumberg's [comment](https://github.com/bitcoin/bitcoin/pull/32634#issuecomment-2918081325):

I don't mind adding that.
👍 hodlinator approved a pull request: "windows: Use predefined `RC_INVOKED` macro instead of custom one"
(https://github.com/bitcoin/bitcoin/pull/32633#pullrequestreview-2877972468)
re-ACK 55f1c2ac8beb5ebebcaef0707d8cc3003e41d80e

Built x86_64-w64-mingw32 Guix build locally.
💬 hodlinator commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#issuecomment-2919144295)
I suspect checking for manifests will currently fail for bench_bitcoin.exe as it doesn't yet have a manifest. Not sure about other binaries. Wouldn't be against adding missing manifests unless it slows down the builds noticeably.
💬 fanquake commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2919196828)
> I believe there is a regression in parsing big endian PPC64 headers in LIEF: https://github.com/lief-project/LIEF/issues/1217, but I've added a work-around to contrib/guix/security-check.py for this.

You should be able to pull in https://github.com/lief-project/LIEF/commit/a1ec0f815fb7ddefc8b53af9f7bb990884655964.
👍 TheCharlatan approved a pull request: "fuzz: Add target for coins database"
(https://github.com/bitcoin/bitcoin/pull/32602#pullrequestreview-2878087402)
ACK cfc42ae5b7ef6d3892907cfe36dc3ae923495d37
📝 fanquake opened a pull request: "[28.x] Backport guix: accomodate migration to codeberg"
(https://github.com/bitcoin/bitcoin/pull/32639)
See https://guix.gnu.org/blog/2025/migrating-to-codeberg/.

When interacting with the old repo you may now also see:
```bash
warning: redirecting to https://codeberg.org/guix/guix/
```

Github-Pull: #32439
Rebased-From: c8d9baae942c94d64ce47ae8f67d3710e6a296bd
💬 fanquake commented on pull request "guix: accomodate migration to codeberg":
(https://github.com/bitcoin/bitcoin/pull/32439#issuecomment-2919256360)
Backported to 28.x in #32639.