💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2115807522)
What's the point of making a constructor that takes no arguments `explicit`?

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2115857753)
Agree on the split but what would be the point of changing the type?

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2115879398)
The new comment is replacing the first (mention of python file) and last comments here:
https://github.com/bitcoin/bitcoin/blob/11a2d3a63e90cdc1920ede3c67d52a9c72860e6b/src/headerssync.cpp#L12-L25

Your comment would probably be enough but I'd rather keep some more context.

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2115884389)
Thanks, missed that!

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2116580384)
Started renaming but then realized that we already have *headerssync-params.py*. It's not obvious to me that underscores would be more correct for C++ files. We do have other C++ files with hyphens.

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2116614226)
Hm.. my `std::optional<ProcessingResult>` suggestion would not work here:

https://github.com/bitcoin/bitcoin/blob/a7b9cc4a0af0dbe4031ec5848d403f61dc9c3356/src/test/headers_sync_chainwork_tests.cpp#L127-L136

Success seems to mean that PRESYNC- and REDOWNLOAD-headers matched, even if the chain is too short.

🤔 hebasto reviewed a pull request: "deps: Bump lief to 0.16.6"
(https://github.com/bitcoin/bitcoin/pull/32431#pullrequestreview-2882489162)
`ninja` is already [listed](https://github.com/bitcoin/bitcoin/blob/4b1d48a6866b24f0ed027334c6de642fc848d083/contrib/guix/manifest.scm#L15) as a dependency in `manifest.scm`, so patching it out doesn't seem necessary:
```diff
--- a/contrib/guix/manifest.scm
+++ b/contrib/guix/manifest.scm
@@ -167,21 +167,13 @@ chain for " target " development."))
(url "https://github.com/lief-project/LIEF")
(commit version)))
(file-name (git-file-na
...

💬 fanquake commented on pull request "cmake, qt: Process `*.qrc` files manually":
(https://github.com/bitcoin/bitcoin/pull/32648#issuecomment-2923517649)
Thanks for taking a look at this. I'm not sure the complexity of having to manually process, and then still having to supress warnings anyway, is worth it. I was thinking the fix here would be to change the tools / Qt to just produce files that don't have the issue. Seems like something we could fix upstream?

📝 hebasto opened a pull request: "cmake: Replace deprecated `qt6_add_translation` with `qt6_add_lrelease`"
(https://github.com/bitcoin/bitcoin/pull/32651)
See Qt docs:
- https://doc.qt.io/archives/qt-6.2/qtlinguist-cmake-qt-add-translation.html
- https://doc.qt.io/archives/qt-6.2/qtlinguist-cmake-qt-add-lrelease.html

💬 fanquake commented on pull request "cmake: Replace deprecated `qt6_add_translation` with `qt6_add_lrelease`":
(https://github.com/bitcoin/bitcoin/pull/32651#discussion_r2116704527)
If we're touching it already, can we fix the TODO, rather than move it? Or, put it into an issue, so that someone might pick it up?

📝 roconnor-blockstream opened a pull request: "wallet: add codex32 argument to addhdkey"
(https://github.com/bitcoin/bitcoin/pull/32652)
Rebase of #27351 onto #29136.

WIP. Testcases don't pass yet.

This PR introduces the ability to import [BIP 93/codex32](https://github.com/bitcoin/bips/blob/master/bip-0093.mediawiki) master seeds with the `addhdkey` command. It currently expects seeds to be provided in either as a single seed or as a list of shares which can be assembled via Shamir Secret Sharing.

✅ pinheadmz closed an issue: "Bitcoin frog"
(https://github.com/bitcoin/bitcoin/issues/32221)

💬 ajtowns commented on pull request "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer":
(https://github.com/bitcoin/bitcoin/pull/32606#issuecomment-2923667684)
> My main concern here is if this disables a FIBRE future; FIBRE apparently used unsolicited compact blocks

The fact that you're enabling FIBRE on a connection can just be taken as implicitly soliciting compact blocks on that connection, so I don't think this is a problem.

💬 hodlinator commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2923670457)
re-ACK a189d636184b1c28fa4a325b56c1fab8f44527b1

Changes since previous review (https://github.com/bitcoin/bitcoin/pull/32406#pullrequestreview-2870311100):
* Resolved silent merge conflict with 47894367b583c06c53d568dc4a984d27bac8f742 by same author.

💬 hebasto commented on pull request "cmake: Replace deprecated `qt6_add_translation` with `qt6_add_lrelease`":
(https://github.com/bitcoin/bitcoin/pull/32651#issuecomment-2924462495)
My Guix build:
```
aarch64
58c6cdfd914bfd2f2646640166692a0a02ec515809d5fa05cb3c9ff511617958 guix-build-d73400e06bef/output/aarch64-linux-gnu/SHA256SUMS.part
51fdf3f3ddb16b5aaf571f33f0da5b95f0b728c7d07863df48dfd3c2d02480dc guix-build-d73400e06bef/output/aarch64-linux-gnu/bitcoin-d73400e06bef-aarch64-linux-gnu-debug.tar.gz
cd2198dbde7ea6af923cc24c4a7363a3355e18f14a397994684db3c1b1662108 guix-build-d73400e06bef/output/aarch64-linux-gnu/bitcoin-d73400e06bef-aarch64-linux-gnu.tar.gz
855c93e3
...

⚠️ hebasto opened an issue: "cmake: Replace f`ile(GLOB ...)` command with an explicit list of `*.ts` files"
(https://github.com/bitcoin/bitcoin/issues/32653)
From CMake [docs](https://cmake.org/cmake/help/latest/command/file.html#glob):
> We do not recommend using GLOB to collect a list of source files from your source tree. If no CMakeLists.txt file changes when a source is added or removed then the generated build system cannot know when to ask CMake to regenerate.

So the following case have to be reworked:https://github.com/bitcoin/bitcoin/blob/4b1d48a6866b24f0ed027334c6de642fc848d083/src/qt/CMakeLists.txt#L57-L60

💬 hebasto commented on pull request "cmake: Replace deprecated `qt6_add_translation` with `qt6_add_lrelease`":
(https://github.com/bitcoin/bitcoin/pull/32651#discussion_r2117397285)
> Or, put it into an issue, so that someone might pick it up?

https://github.com/bitcoin/bitcoin/issues/32653.

💬 romanz commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2117417395)
Will prepare a benchmark to compare both formats :)

💬 fanquake commented on pull request "cmake: Replace deprecated `qt6_add_translation` with `qt6_add_lrelease`":
(https://github.com/bitcoin/bitcoin/pull/32651#discussion_r2117446696)
Delete it from here now?

⚠️ hMsats reopened an issue: "bitcoind 29.0 much slower than 28.0 on my system: cause found"
(https://github.com/bitcoin/bitcoin/issues/32455)
I always compile the bitcoin software myself (`Ubuntu 24.04.2 LTS`). For `bitcoin-29.0` I compiled it (of course) for the first time with `cmake` (with and without `berkeley-db`).

For both (with and without `berkeley-db`), `bitcoind-29.0` is much slower than `bitcoin-28.0` which gives me problems on my server.

To investigate I wrote a shell script that determines the time difference in seconds between "Saw new header" and "UpdateTip":

```
2025-05-09T05:24:11Z Saw new header hash=000000000000
...