Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 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.
💬 hebasto commented on pull request "Replace dead gnome link notificator.cpp":
(https://github.com/bitcoin/bitcoin/pull/32629#discussion_r2113852140)
> Agreed, the link doesn't directly help understand the code Should I remove it?

I think so.
💬 stickies-v commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#discussion_r2113855067)
I hadn't considered the extra vector allocation, yeah I'm okay leaving this as-is then.

I did nerd-snipe myself into looking into having `AccessCoins` return a view would be quite elegant in some ways, allowing lazy evaluation, but seems like it would require quite a bit of overhaul and might have other drawbacks I've not considered yet.

```cpp
auto AccessCoins(const CTransaction& tx) const
{
return tx.vin | std::views::transform([this](const CTxIn& input) -> const Coin
...
👍 hebasto approved a pull request: "[28.x] Backport guix: accomodate migration to codeberg"
(https://github.com/bitcoin/bitcoin/pull/32639#pullrequestreview-2878132664)
ACK 5c2ba9f583e28460e618cc1119a10c06b868377c.

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

I also observed random connection issues.
💬 hebasto commented on pull request "windows: Use predefined `RC_INVOKED` macro instead of custom one":
(https://github.com/bitcoin/bitcoin/pull/32633#issuecomment-2919271405)
My Guix build:
```
aarch64
90c59c671509f759cc862b4376b374ff36707a323eda619fed3f91cf322c0de5 guix-build-55f1c2ac8beb/output/aarch64-linux-gnu/SHA256SUMS.part
890e9ac6b9c445bc1fe799378d3c7f6a9ac3c6c2d650e70d1a251a23cca88394 guix-build-55f1c2ac8beb/output/aarch64-linux-gnu/bitcoin-55f1c2ac8beb-aarch64-linux-gnu-debug.tar.gz
13e21adc0e0f487e19649d468753c3565f60b20d5e021576cd27cdf6aad16640 guix-build-55f1c2ac8beb/output/aarch64-linux-gnu/bitcoin-55f1c2ac8beb-aarch64-linux-gnu.tar.gz
ee0faf9e
...
💬 fanquake commented on pull request "windows: Use predefined `RC_INVOKED` macro instead of custom one":
(https://github.com/bitcoin/bitcoin/pull/32633#issuecomment-2919295253)
Guix Build (win):
```bash
512632ba356a2e858091dca086221a1d16b9b3318cb44c0b513dde6548379888 guix-build-55f1c2ac8beb/output/dist-archive/bitcoin-55f1c2ac8beb.tar.gz
18079f9eaa6b541fc2d4d425de55e10a8fec1fc7a549f4ac7dcf25275d632e0e guix-build-55f1c2ac8beb/output/x86_64-w64-mingw32/SHA256SUMS.part
0d548fc0b8de0a087644930b52e8ef8b6fce93e73e1a4ff30076efee9075993e guix-build-55f1c2ac8beb/output/x86_64-w64-mingw32/bitcoin-55f1c2ac8beb-win64-codesigning.tar.gz
50dfa21fba79537a7f1ddb9e870931f72fc96
...
👍 fanquake approved a pull request: "windows: Use predefined `RC_INVOKED` macro instead of custom one"
(https://github.com/bitcoin/bitcoin/pull/32633#pullrequestreview-2878165242)
ACK 55f1c2ac8beb5ebebcaef0707d8cc3003e41d80e
🚀 fanquake merged a pull request: "windows: Use predefined `RC_INVOKED` macro instead of custom one"
(https://github.com/bitcoin/bitcoin/pull/32633)
💬 strmfos commented on pull request "Replace dead gnome link notificator.cpp":
(https://github.com/bitcoin/bitcoin/pull/32629#issuecomment-2919401771)
@hebasto I removed the outdated link (comment), if I can do anything else, I'll be glad
Thank you
💬 hebasto commented on pull request "Replace dead gnome link notificator.cpp":
(https://github.com/bitcoin/bitcoin/pull/32629#issuecomment-2919418223)
Please [squash](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits) your commits and update the PR title and description.
💬 ismaelsadeeq commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2919479775)
> I'm interested in seeing benchmarks of this on various systems, especially high-end/modern ones (as their performance is likely most predictive of what future hardware will be like).

Below are benchmarks on MacBook M2 PRO 2023.
Compiled using clang with only build bench cmake option.
```
cmake -B build -DBUILD_BENCH=ON && cmake --build build
```

<details>
<summary>master 9a887baadebce7c2e76df831fad54c5fa81d309e</summary>

```terminal
abubakarismail@Abubakars-MacBook-Pro ~/D/W/b/b
...
💬 sipa commented on pull request "Replace cluster linearization algorithm with SFL":
(https://github.com/bitcoin/bitcoin/pull/32545#issuecomment-2919488733)
@ismaelsadeeq Not an apples-to-apples comparison I'm afraid, because the benchmark cases are changed in this PR too. Can you run the `LinearizeBoundedExample?` benchmarks too?
💬 ismaelsadeeq commented on pull request "cluster mempool: add TxGraph reorg functionality":
(https://github.com/bitcoin/bitcoin/pull/31553#issuecomment-2919520179)
reACK e1f501cab224608004e3a1eee7a48eec3cea25fc [deab145ab8..e1f501cab2](https://github.com/bitcoin/bitcoin/compare/538e9ff804f8dfba6f6a808e83572fdeab145ab8..e1f501cab224608004e3a1eee7a48eec3cea25fc)

Following the last light ACK, review comments by @instagibbs were addressed:
- Increased fuzz test coverage
- Clearer and corrected comments
- An Assume statement was added