💬 fanquake commented on pull request "rpc: add cpu_load to getpeerinfo":
(https://github.com/bitcoin/bitcoin/pull/31672#issuecomment-2598328420)
Seems lihe there was some very brief discussion in #31033, and the conclusion ["I guess this should start with planting some metrics"](https://github.com/bitcoin/bitcoin/issues/31033#issuecomment-2394932171), but I'm not sure putting changes into Bitcoin Core is the right first step.

Before changing our API, it'd be good to atleast show some usage that indicates that the changes here are useful. I assume you've already been running this locally, and collecting the data, so it'd be good to kno
...

💬 bigspider commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#issuecomment-2598333028)
> @bigspider thanks for your [reply](https://groups.google.com/g/bitcoindev/c/OPx9qvmr5Nc/m/8mp8VHjQBQAJ) on the mailinglist. So I guess another way to test this PR is to patch HWI to not do the conversion between PSBT 0 and 2, and then make (any?) transaction with a Ledger that runs at least 2.0.0.

I suppose so, although it feels somewhat circular as bitcoin-core was always the reference for the tests of the Ledger Bitcoin app!

💬 hebasto commented on pull request "cmake: Introduce `WITH_PYTHON` build option":
(https://github.com/bitcoin/bitcoin/pull/31669#issuecomment-2598357568)
Rebased due to the conflict with the merged #31651.

💬 fanquake commented on pull request "cmake: Introduce `WITH_PYTHON` build option":
(https://github.com/bitcoin/bitcoin/pull/31669#issuecomment-2598358694)
NACK - based on the reasoning above.

💬 hebasto commented on pull request "cmake: Introduce `WITH_PYTHON` build option":
(https://github.com/bitcoin/bitcoin/pull/31669#issuecomment-2598364343)
> NACK - based on the reasoning above.

> Not sure. Why does this need another build option? Given that this is just for working around silent CMake warnings in the CI, couldn't this just be fixed by using explicit `*_TEST` options that otherwise fail if Python is missing?

What "explicit `*_TEST`" do you mean?

💬 Sjors commented on pull request "cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets":
(https://github.com/bitcoin/bitcoin/pull/30901#issuecomment-2598371523)
ACK 4185ad1cb12060337c2d66a9a66e37be1d10f4ce

My cmake knowledge is also limited, but I like that this removes duplication. E.g instead of `base58_encode_decode.json.h` and `data/base58_encode_decode.json` now `test/CMakeLists.txt` only contains the latter.

The build log looks the same, before and after:

```
[ 76%] Generating data/base58_encode_decode.json.h
```

💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2598377454)
Rebased due to the conflicts with the merged #31621 and #31651.

💬 Sjors commented on pull request "init: Lock blocksdir in addition to datadir":
(https://github.com/bitcoin/bitcoin/pull/31674#issuecomment-2598382415)
> This caused an incompatibility with blocksdirs from previous versions of Core. Setting to draft whlie I play around with a fix.

Once you figure that out, you can use a previous release to demonstrate that this works. See e.g. `mempool_compatibility.py`.

💬 fanquake commented on pull request "cmake: Introduce `WITH_PYTHON` build option":
(https://github.com/bitcoin/bitcoin/pull/31669#issuecomment-2598383717)
> What "explicit *_TEST" do you mean?

(Introducing) An option that does anything test related, that also requires Python, but I don't think this solution is better than #31665, because it forces builders to start opting out of things, and doesn't solve #31476 generically, compared to #31665.

💬 fanquake commented on pull request "build: enable libc++ and libstdc++ hardening":
(https://github.com/bitcoin/bitcoin/pull/31424#discussion_r1920209528)
libstdc++

💬 fanquake commented on pull request "build: Make config warnings fatal if -DWERROR=ON":
(https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2598403146)
> Other projects use the same or similar build option only to turn compiler's warnings into errors.

Sure, but should that dictate what we do? Having an option to turn warnings into errors, that turns slightly more warnings into errors compared to other projects, and is mostly used in our CI where we want to surface all warnings, seems reasonable. Given that CMake lacks any native functionality to acheive the same thing, using `-DWERROR` seems ok, especially if the alternative is implement `N
...

💬 fanquake commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2598407008)
Guix Build (x86_64):
```bash
a22efc082966018f6ddd17d1a7092ffda56900895dd4694080244257e1c5c974 guix-build-6f0513613a98/output/aarch64-linux-gnu/SHA256SUMS.part
ad65ac733f93931c5fcfa09afc2a1f8bed76cd5280be9b569ff9bdf15dec3abe guix-build-6f0513613a98/output/aarch64-linux-gnu/bitcoin-6f0513613a98-aarch64-linux-gnu-debug.tar.gz
abc9037335e719927525c0474a7959db57a6f3f64d9522474ede4d88e721f73c guix-build-6f0513613a98/output/aarch64-linux-gnu/bitcoin-6f0513613a98-aarch64-linux-gnu.tar.gz
fdb0186
...

🚀 fanquake merged a pull request: "depends: Use base system's `sha256sum` utility on FreeBSD"
(https://github.com/bitcoin/bitcoin/pull/31626)

🚀 fanquake merged a pull request: "cmake: Always provide `RPATH` on NetBSD"
(https://github.com/bitcoin/bitcoin/pull/31543)

💬 maflcko commented on pull request "ci: Skip read-write of default env vars":
(https://github.com/bitcoin/bitcoin/pull/31678#discussion_r1920262852)
For reference, the exclusion is still needed to avoid the error from https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1564122573. Diff to reproduce the error:


```diff
diff --git a/ci/test/02_run_container.sh b/ci/test/02_run_container.sh
index 4181e68..168ab51 100755
--- a/ci/test/02_run_container.sh
+++ b/ci/test/02_run_container.sh
@@ -12,7 +12,7 @@ set -o errexit -o pipefail -o xtrace
if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then
# Export all env vars to avoid missing som
...

💬 scgbckbone commented on pull request "Implement BIP 370 PSBTv2":
(https://github.com/bitcoin/bitcoin/pull/21283#issuecomment-2598502303)
Coldcard Mk4/Q also support PSBT v2 if you need to test ( from October 2023 https://github.com/Coldcard/firmware/blob/master/releases/History-Mk4.md#520---2023-10-10)

but as @bigspider, I also used core PR to implement this

💬 maflcko commented on pull request "ci: Turn CentOS task into native one":
(https://github.com/bitcoin/bitcoin/pull/31651#discussion_r1920283682)
thx, fixed in https://github.com/bitcoin/bitcoin/pull/31593

👋 maflcko's pull request is ready for review: "ci: Bump centos stream 10"
(https://github.com/bitcoin/bitcoin/pull/31593)

💬 maflcko commented on pull request "ci: Bump centos stream 10":
(https://github.com/bitcoin/bitcoin/pull/31593#issuecomment-2598504619)
> Upstream tracking issue for branching and adding `dash` into EPEL 10 is here: https://bugzilla.redhat.com/show_bug.cgi?id=2335416

Thx, used ksh for now (temporarily)

💬 hebasto commented on pull request "ci: Bump centos stream 10":
(https://github.com/bitcoin/bitcoin/pull/31593#issuecomment-2598523720)
Concept ACK.