👍 marcofleon approved a pull request: "[28.x] 28.3rc2"
(https://github.com/bitcoin/bitcoin/pull/33557#pullrequestreview-3330818813)
ACK 8e4651bb33b8f97940bf2ad6a2f97932198f6765

💬 fanquake commented on pull request "CPack":
(https://github.com/bitcoin/bitcoin/pull/33455#discussion_r2425811942)
> CMake does not provide options for all NSIS attributes. For attributes that are strictly required,

If migrating to do this the "CMake way" means less flexibility or giving up functionality because CMake doesn't support something, that tradeoff that should be mentioned in the PR description. This also means that any future option we might want to use, may also require workarounds, assuming that CMake support for new options will lag NSIS releases by some amount (or not be added at all), and
...

💬 fanquake commented on pull request "CPack":
(https://github.com/bitcoin/bitcoin/pull/33455#issuecomment-3396786226)
> Adjust the Guix build script as well?

Yea, this wont Guix build:
```bash
HOSTS="x86_64-w64-mingw32" ./contrib/guix/guix-build
<snip>
[100%] Built target bitcoin-qt
Running symbol and dynamic library checks...
[100%] Built target check-symbols
make: *** No rule to make target 'deploy'. Stop.
```

💬 hebasto commented on issue "Remove remaining mention of datacarriersize being deprecated":
(https://github.com/bitcoin/bitcoin/issues/33605#issuecomment-3396835542)
> Best to remove that too?

https://github.com/bitcoin/bitcoin/blob/d0f6d9953a15d7c7111d46dcb76ab2bb18e5dee3/src/qt/bitcoinstrings.cpp#L1

In the case of the source string removed in https://github.com/bitcoin/bitcoin/pull/33453, this line won’t affect the translation at runtime.

✅ hebasto closed an issue: "Remove remaining mention of datacarriersize being deprecated"
(https://github.com/bitcoin/bitcoin/issues/33605)

💬 maflcko commented on pull request "ci: fix buildx gha cache authentication on forks":
(https://github.com/bitcoin/bitcoin/pull/33508#issuecomment-3396867870)
> > (You can see an outline here if you're interested [master...willcl-ark:bitcoin:docker-ci-containers](https://github.com/bitcoin/bitcoin/compare/master...willcl-ark:bitcoin:docker-ci-containers) from march. This attempt tried to preserve the ability to run on bare metal too, but if we didn't want that then most of the scripts could be moved into dockerfiles too, further simplifying/locking us in to docker 😋 .)
>
> bare metal is required for the macOS CI, so I don't think we can go pure do
...

👍 hebasto approved a pull request: "depends: Use $(package)_file_name when downloading from the fallback"
(https://github.com/bitcoin/bitcoin/pull/33580#pullrequestreview-3331004206)
ACK 671b774d1b58c491b53f2b2f6ee42fb6b65a0e71, tested with the following patch:
```diff
--- a/depends/packages/native_capnp.mk
+++ b/depends/packages/native_capnp.mk
@@ -1,6 +1,6 @@
package=native_capnp
$(package)_version=1.2.0
-$(package)_download_path=https://capnproto.org/
+$(package)_download_path=https://capnproto.org.FAIL/
$(package)_download_file=capnproto-c++-$($(package)_version).tar.gz
$(package)_file_name=capnproto-cxx-$($(package)_version).tar.gz
$(package)_sha256_hash
...

🚀 hebasto merged a pull request: "depends: Use $(package)_file_name when downloading from the fallback"
(https://github.com/bitcoin/bitcoin/pull/33580)

👍 stickies-v approved a pull request: "[28.x] 28.3rc2"
(https://github.com/bitcoin/bitcoin/pull/33557#pullrequestreview-3331061515)
ACK 8e4651bb33b8f97940bf2ad6a2f97932198f6765 modulo 1 link update required (see comment)

All backports clean except:
- 9c911f7e2dcb6dc26b5824bdae2d389cc931607e backported from abf4a6eeaee116917dafd56eb9caee03e13048d2: merge conflict due to qt being bumped to `5.15.16` on `master` and still at `5.15.14` on `28.x`.

Getting identical manpages, and release notes look complete.

💬 stickies-v commented on pull request "[28.x] 28.3rc2":
(https://github.com/bitcoin/bitcoin/pull/33557#discussion_r2425963692)
Download link on l3 needs to be updated:

<details>
<summary>git diff on 8e4651bb33</summary>

```diff
diff --git a/doc/release-notes.md b/doc/release-notes.md
index b8b90475c8..f446c14cc5 100644
--- a/doc/release-notes.md
+++ b/doc/release-notes.md
@@ -1,6 +1,6 @@
Bitcoin Core version 28.3rc2 is now available from:

- <https://bitcoincore.org/bin/bitcoin-core-28.3/test.rc1/>
+ <https://bitcoincore.org/bin/bitcoin-core-28.3/test.rc2/>

This release includes various bug fixe
...

👍 stickies-v approved a pull request: "[28.x] 28.3rc2"
(https://github.com/bitcoin/bitcoin/pull/33557#pullrequestreview-3331102990)
re-ACK 44d05b2fb25b0a5f14e7487c792ac25ad5f5c284

👍 willcl-ark approved a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33416#pullrequestreview-3331104875)
ACK aee53d60d32ec800357336fc8c8602405f767c34

Backports look good.

> Should also backport #31407

Do you want to respond to @achow101's suggestion to backport this #31407? I guess an argument against doing so is that it's apretty large change, and 27.x is out of maintenance?

💬 fanquake commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33416#issuecomment-3397051344)
Yea, I'm not planning on adding any further changes to this PR (which will likely be the last to the `27.x` branch).

💬 hebasto commented on pull request "Update `minisketch` subtree and switch to its build script":
(https://github.com/bitcoin/bitcoin/pull/32856#discussion_r2426003276)
> Why?

Because this PR is not supposed to change behaviour.

> We should not interfere with `CMAKE_EXPORT_COMPILE_COMMANDS`. If developers set this option globally in their dotfiles because it will give them code completion in their IDE, setting it to `OFF` here will break their setup.

When the decision was made to disable the `EXPORT_COMPILE_COMMANDS` property for subtree targets during the migration from Autotools to CMake, this use case was not considered, but only the “tidy” CI job,
...

🤔 marcofleon reviewed a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33416#pullrequestreview-3331161979)
lgtm ACK aee53d60d32ec800357336fc8c8602405f767c34

🚀 fanquake merged a pull request: "[27.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33416)

💬 fanquake commented on pull request "[27.x] Backports":
(https://github.com/bitcoin/bitcoin/pull/33416#issuecomment-3397094073)
I've closed the [27.3 milestone](https://github.com/bitcoin/bitcoin/milestone/76), as there's not currently a plan to make a `27.3` release.

💬 ismaelsadeeq commented on pull request "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator.{h,cpp}`":
(https://github.com/bitcoin/bitcoin/pull/33218#issuecomment-3397095596)
> nit: if it's already in a fees/ subfolder i think naming them estimator.{h,cpp} would be as clear and thrice as short?

Hmm, I wanted it to be specific, thats why I named it block policy estimator and the file being in the fees directory was meant to indicate it is estimating fees.

💬 ismaelsadeeq commented on pull request "refactor: rename `fees.{h,cpp}` to `fees/block_policy_estimator.{h,cpp}`":
(https://github.com/bitcoin/bitcoin/pull/33218#discussion_r2426029167)
will fix nits when retouching.

💬 marcofleon commented on pull request "[28.x] 28.3rc2":
(https://github.com/bitcoin/bitcoin/pull/33557#issuecomment-3397098352)
Nice, re ACK 44d05b2fb25b0a5f14e7487c792ac25ad5f5c284