✅ fanquake closed an issue: "stringop-overflow warning with GCC 14"
(https://github.com/bitcoin/bitcoin/issues/30114)

🚀 fanquake merged a pull request: "wallet, tests: Avoid stringop-overflow warning in PollutePubKey"
(https://github.com/bitcoin/bitcoin/pull/30131)

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609899492)
Getting a mismatch on the first block:

```sh
curl -s localhost:8000/tweak-index/709632?dustLimit=992 | jq
```

```json
[
"0282559668ac461407e39d78b0d604e19c88ec7d1b4eb1b07636a2fe10a00f9a74",
"0360b336c0121e9a1f64e5644a0519f2f6fd37ae28bc6558c604a63b1aac251a7c",
"03a1fbba688d7d1940910d2a40fdb08fa6c03e56ae3f1484941ae2b9451aa9672b",
"03e0b6b04bc4529c376dd8215dac77bb4d7696df34e097b28a7948686c2bc70b4c",
"02266e6da2790ffa1e47640eca18735281177b75e3435df731f41c42cc9646bdc0",
"03
...

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609910257)
835dd4ba1ca4cc1abfd338e6bc38e25a116f5f8b: oh this is wrong, it finds the _lowest_ output amount, oops.

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609915406)
This branch should have a sorted index. We can now find the txid a lot easier, I think.

💬 theuni commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1609919080)
@TheCharlatan Thanks for playing around with it :)

I don't have the internal clang knowledge to know if that's correct or not. Looks fine to me, but I agree with @maflcko that we could just remove the empty dtor in that case.

Also, to be clear, that TODO is an upstream comment, not one from me: https://github.com/llvm/llvm-project/blob/main/clang-tools-extra/clang-tidy/bugprone/UnusedRaiiCheck.cpp#L18

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609932428)
I added a fix for https://github.com/bitcoin/bitcoin/pull/28241/files#diff-1b4444ba5939fff2ab9f70d1d0d29fc2653b09a332e462166e01891e261c9fa9 and rebuilding the index now.

💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609936760)
Isn't there some kind of special handling needed for large values that don't fit into a byte after the shift (i.e. if txout.nValue >4080 sats)?

💬 theStack commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1609948787)
I'd suggest to introduce a fast `CScript::IsPayToTaproot` helper. It can be used both in `MaybeSilentPayment` and directly in the loop for finding the largest output amount (commit 337471e4acff439100488c16876d9f110ef04ef7), without the need to involve `Solver`:

<details>
<summary>patch to add `CScript::IsPayToTaproot()`</summary>

```diff
diff --git a/src/script/script.cpp b/src/script/script.cpp
index 80e8d26bcf..d0e677aaa4 100644
--- a/src/script/script.cpp
+++ b/src/script/script.cp
...

👍 theuni approved a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150#pullrequestreview-2071218768)
ACK fa3e1151a28345edff8f371283745bdd647f9a74

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1609954791)
bebfafcf98d8ed1432407b7603991d54f7cc26c2: it would be useful to have a quick recap here of which strategy is used by `QueryDefaultGateway` for which OS.

I think it will be (slightly) more readable to have a single `QueryDefaultGateway` implementation which then calls `QueryDefaultGatewayWindows`, `QueryDefaultGatewayMac` and `QueryDefaultGatewayLinuxBSD`.

It can start with:

```cpp
Assume(network == NET_IPV4 || network == NET_IPV6)
```

and end with

```cpp
#else return std::nullo
...

💬 TheCharlatan commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1609967684)
> I don't have freeBSD to test this, but are you sure it doesn't crash when the dtor is there but empty?

It looks like it should work to me. If a method has a body it should also contain a compound statement, or at least this [book](https://github.com/PacktPublishing/LLVM-Techniques-Tips-and-Best-Practices-Clang-and-Middle-End-Libraries) seems to suggest as much.

> Also, to be clear, that TODO is an upstream comment, not one from me: https://github.com/llvm/llvm-project/blob/main/clang-too
...

👍 AngusP approved a pull request: "contrib/signet/miner: increase miner search space"
(https://github.com/bitcoin/bitcoin/pull/30130#pullrequestreview-2071241565)
utACK 1cf174a2954b434e9623afe86f3643be8eec2d70

👍 TheCharlatan approved a pull request: "Add clang-tidy check for thread_local vars"
(https://github.com/bitcoin/bitcoin/pull/30146#pullrequestreview-2071241175)
ACK ace5b35fb435ca2646bb8738e6578187ae2e8720

💬 TheCharlatan commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1609977454)
Nit: Four spaces for indents like in the rest of the code?

🚀 fanquake merged a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150)

💬 theuni commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#issuecomment-2124847802)
Updated to resolve @TheCharlatan's nit, and also specified where `hasNonTrivialDestructor` came from.

👍 TheCharlatan approved a pull request: "Add clang-tidy check for thread_local vars"
(https://github.com/bitcoin/bitcoin/pull/30146#pullrequestreview-2071277257)
Re-ACK 34c9cee380e7276cf3f85f2ce56a293e57afd676

💬 furszy commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1610019226)
> nit: As you remove the `version` check, I wonder if this one can be removed as well for the same reason? The final `pong` test should be necessary and sufficient already.

I removed the `version` check because `verack`s are direct responses to `version` messages at the protocol level. I'm not sure we should rely on the `pong` alone because that might change over time?

💬 maflcko commented on pull request "Add clang-tidy check for thread_local vars":
(https://github.com/bitcoin/bitcoin/pull/30146#discussion_r1610026534)
> > I don't have freeBSD to test this, but are you sure it doesn't crash when the dtor is there but empty?
>
> It looks like it should work to me. If a method has a body it should also contain a compound statement, or at least this [book](https://github.com/PacktPublishing/LLVM-Techniques-Tips-and-Best-Practices-Clang-and-Middle-End-Libraries) seems to suggest as much.

Oh, I didn't mean clang-tidy to crash, but the program that uses thread_local in reference to https://bugs.freebsd.org/bug
...

💬 furszy commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#discussion_r1610031890)
> The debug logs could be added when needed. There are already two log lines, so a third (when added) shouldn't matter too much, I'd say.
>
> For example:
>
> ```
> node1 2024-05-01T17:15:16.401946Z (mocktime: 2024-05-01T16:46:25Z) [httpworker.0] [rpc/request.cpp:187] [parse] [rpc] ThreadRPCServer method=getpeerinfo user=__cookie__
> node1 2024-05-01T17:15:16.453092Z (mocktime: 2024-05-01T16:46:25Z) [http] [httpserver.cpp:306] [http_request_cb] [http] Received a POST request for / fro
...