💬 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.

💬 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
...