💬 hebasto commented on issue "cmake: Replace f`ile(GLOB ...)` command with an explicit list of `*.ts` files":
(https://github.com/bitcoin/bitcoin/issues/32653#issuecomment-2930114468)
> Using a glob is kind of fragile here, harder to ensure the build matches git when there's loose `.ts` files around.

True. Please see https://github.com/bitcoin/bitcoin/issues/32653.

🤔 hebasto reviewed a pull request: "depends: don't install & then delete sqlite pkgconf"
(https://github.com/bitcoin/bitcoin/pull/32656#pullrequestreview-2887975315)
My Guix build:
```
aarch64
545db2ab3bf28143eda2307089bb7266ae64802ea4a15371bacccf4574ee2c67 guix-build-72a5aa9b791e/output/aarch64-linux-gnu/SHA256SUMS.part
660030fb8c70d09e11f35e2ccdde4187f6bd90009bc1da2826be1cf8a73e6f4b guix-build-72a5aa9b791e/output/aarch64-linux-gnu/bitcoin-72a5aa9b791e-aarch64-linux-gnu-debug.tar.gz
e4a702b1d4f589cd9a1d6a0534eee7ddab725a9aa753b5a276510ee9b956ed94 guix-build-72a5aa9b791e/output/aarch64-linux-gnu/bitcoin-72a5aa9b791e-aarch64-linux-gnu.tar.gz
932b1525
...

👍 hebasto approved a pull request: "depends: don't install & then delete sqlite pkgconf"
(https://github.com/bitcoin/bitcoin/pull/32656#pullrequestreview-2887977405)
ACK 72a5aa9b791e80a6204990c27c8165ce8aa81dd7.

🚀 hebasto merged a pull request: "depends: don't install & then delete sqlite pkgconf"
(https://github.com/bitcoin/bitcoin/pull/32656)

💬 hebasto commented on pull request "deps: Bump lief to 0.16.6":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2930192028)
FWIW, https://codeberg.org/guix/guix/pulls/353 has been landed recently.

⚠️ Sjors opened an issue: "wallet: render BIP388 wallet policies"
(https://github.com/bitcoin/bitcoin/issues/32659)
[BIP 388](https://github.com/bitcoin/bips/blob/master/bip-0388.mediawiki) wallet policies were introduced by @bigspider to make it easier to verify descriptors on a signing device.

A first step towards supporting them could be to add a `policy` and `keys` field to the `getdescriptorinfo` and `listdescriptors` RPC results. Only for the subset of descriptors that BIP388 allows.

These fields can be manually passed into HWI using this (somewhat old) PR https://github.com/bitcoin-core/HWI/pull/647
...

💬 hebasto commented on pull request "Add bitcoin-{node,gui} to release binaries for IPC":
(https://github.com/bitcoin/bitcoin/pull/31802#issuecomment-2930369097)
> @ryanofsky CI complains about `std::move` in `libmultiprocess`: https://cirrus-ci.com/task/6187773452877824, e.g.:
>
> ```
> /ci_container_base/src/ipc/libmultiprocess/include/mp/proxy-io.h:252:16: error: forwarding reference passed to std::move(), which may unexpectedly cause lvalues to be moved; use std::forward() instead [bugprone-move-forwarding-reference,-warnings-as-errors]
> [05:29:23.932] 252 | m_fn = std::move(fn);
> [05:29:23.932] | ^~~~~~~~~
> [
...

📝 maflcko opened a pull request: "rpc: Use type-safe HelpException"
(https://github.com/bitcoin/bitcoin/pull/32660)
The current "catch-all" `catch (const std::exception& e)` in `CRPCTable::help` is problematic, because it could catch exceptions unrelated to passing the help string up.

Fix this by using a dedicated exception type.

💬 maflcko commented on pull request "rpc: Use type-safe HelpException":
(https://github.com/bitcoin/bitcoin/pull/32660#issuecomment-2930384842)
Can be tested by adding a bug, like

```diff
diff --git a/src/rpc/signmessage.cpp b/src/rpc/signmessage.cpp
index 5597f8d237..d62893bf0c 100644
--- a/src/rpc/signmessage.cpp
+++ b/src/rpc/signmessage.cpp
@@ -19,7 +19,7 @@ static RPCHelpMan verifymessage()
return RPCHelpMan{"verifymessage",
"Verify a signed message.",
{
- {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The bitcoin address to use for the signature."},
+ {"addres\ns", R
...

💬 maflcko commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `unused(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#discussion_r2120966114)
> type-safe

fixed in https://github.com/bitcoin/bitcoin/pull/32660

💬 fanquake commented on pull request "cmake: Replace deprecated `qt6_add_translation` with `qt6_add_lrelease`":
(https://github.com/bitcoin/bitcoin/pull/32651#issuecomment-2930387771)
Guix Build:
```bash
6ec55ec67ac33aeb89eae88bd55525e36e05d3c768eb98cf9c718cc8221de76e guix-build-18cf727429e9/output/aarch64-linux-gnu/SHA256SUMS.part
f3e260544a199b5b06f3f5de4352fcff61f6a3e36fd12a827317c2ba168ab3a8 guix-build-18cf727429e9/output/aarch64-linux-gnu/bitcoin-18cf727429e9-aarch64-linux-gnu-debug.tar.gz
259b8bd0260adc1517c11a8579983210501f6625083b780f3451c5bf6d531c52 guix-build-18cf727429e9/output/aarch64-linux-gnu/bitcoin-18cf727429e9-aarch64-linux-gnu.tar.gz
6772df343eb3f628
...

💬 hebasto commented on pull request "depends: sqlite 3.50.0; switch to autosetup":
(https://github.com/bitcoin/bitcoin/pull/32655#discussion_r2120976295)
How does this affect the overall dependency graph?

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120826619)
(Incorporated most of this, breaking out commits, expanding function prototypes into multiple lines, even removing the `HeadersGeneratorSetup` type in favor of free functions).

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120864823)
Yeah, seems src/ipc/ really went all in on `-`. Holding off on changing for now to see what others think.

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120817742)
Ah, the Sonarcloud recommendation was actually for the `uint64_t`-ctor, similar to my response here: https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2115831187.

Tested out making it explicit, modifying 13 additional files. It did uncover some implicit casts that went from `int64_t Params::nPowTargetTimespan` to `arith_uint256`... But the rest felt a bit forced. I think it is intentional and makes sense for u64s to implicitly cast to u256.

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120844393)
Using `const vector&` communicates that we are sending in the entire chain, not some subset. I like the specificity. `const vector`s feel more neutered than spans IMO.

💬 hodlinator commented on pull request "headerssync: Keep tests ahead of increasing params":
(https://github.com/bitcoin/bitcoin/pull/32579#discussion_r2120833106)
Is there anything to catch wrong order?
```C++
const auto [pow_validated_headers, success, request_more] = ...
```
vs
```C++
const auto [pow_validated_headers, request_more, success] = ...
```
(There's clang-tidy logic to catch `/*paramname=*/true` mismatches for function calls).

⚠️ ismaelsadeeq opened an issue: "wallet, node: Redundant opt-in RBF wallet config and RPC parameters"
(https://github.com/bitcoin/bitcoin/issues/32661)

Following the removal of the `-mempoolfullrbf` node option in [#30592](https://github.com/bitcoin/bitcoin/pull/30592), the Bitcoin Core wallet still retains the `-walletrbf` option.
This option determines whether transactions are marked as opt-in RBF (i.e., using sequence numbers `MAX - 1` or `MAX - 2`).

With full-RBF now the default behavior, `-walletrbf` is effectively redundant and should be removed.

However, removing `-walletrbf` alone is not sufficient, as several wallet RPCs also suppor
...

🤔 dergoegge reviewed a pull request: "p2p: Add mutation check inside compact block's FillBlock"
(https://github.com/bitcoin/bitcoin/pull/32646#pullrequestreview-2888127199)
Concept ACK

💬 dergoegge commented on pull request "p2p: Add mutation check inside compact block's FillBlock":
(https://github.com/bitcoin/bitcoin/pull/32646#discussion_r2120969666)
nit

```suggestion
const CBlockIndex* prev_block{Assume(m_chainman.m_blockman.LookupBlockIndex(partialBlock.header.hashPrevBlock))};
```