👍 stickies-v approved a pull request: "log: don't rate-limit "new peer" with -debug=net"
(https://github.com/bitcoin/bitcoin/pull/34008#pullrequestreview-3554269912)
ACK 8f553ac9df91c03a6e753cce3e7cf2e46b977bdb

Wholesale downgrading inbound connections to `LogDebug` seems sensible to me. Even with ratelimiting, we should still make sure that unconditional logs can't easily be triggered by an attacker.

I think this would benefit from a brief release note. People might be confused why they're no longer seeing inbound connections in their logs?

💬 stickies-v commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2600414530)
Is there a reason why the message is split across the log statement and the `new_peer_info` lambda? That feels a bit clunky to me?

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

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index a823b2c2e2..54528ff3d0 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3659,23 +3659,23 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
return;
}


...

💬 stickies-v commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2600308329)
clang-format-nit

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

```diff
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index a823b2c2e2..802f68e98f 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -3661,11 +3661,12 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,

auto new_peer_info = [&]() {
const auto mapped_as{m_connman.GetMappedAS(pfrom.addr)};
- return strprintf("transpo
...

💬 stickies-v commented on pull request "log: exempt all category-specific logs from ratelimiting when running with debug":
(https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3629382646)
Closing this in favour of https://github.com/bitcoin/bitcoin/pull/34008, I think that PR's new approach of downgrading the inbound log to `LogDebug` is superior.

I think the approach in this PR makes sense if there are other places in the code with a similar issue (modulo addressing [this comment](https://github.com/bitcoin/bitcoin/pull/34018#issuecomment-3621967700)), but until that use case pops up, there's no point keeping this PR open.

✅ stickies-v closed a pull request: "log: exempt all category-specific logs from ratelimiting when running with debug"
(https://github.com/bitcoin/bitcoin/pull/34018)

🤔 hebasto reviewed a pull request: "guix: use GCC 14.3.0 over 13.3.0"
(https://github.com/bitcoin/bitcoin/pull/33775#pullrequestreview-3554458851)
My Guix build:
```
aarch64
02de261cf6c90f15a69d42b43e63485a6c0cdb3d745b5916b6834278a1d63dbc guix-build-a72ff31b879f/output/aarch64-linux-gnu/SHA256SUMS.part
cf2d9615338be34f5082940d7d2b1a4c085226a410a75dceec764e42cd5df5ee guix-build-a72ff31b879f/output/aarch64-linux-gnu/bitcoin-a72ff31b879f-aarch64-linux-gnu-debug.tar.gz
e18f34ce40036367f611451ae4dfe39e7dedeb53e549f229cff118bf2c2836a8 guix-build-a72ff31b879f/output/aarch64-linux-gnu/bitcoin-a72ff31b879f-aarch64-linux-gnu.tar.gz
17cd0a5fad2a4e
...

💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2600468539)
True, changed to `ABCStep`. The point is that we want to sometimes stop here (and possibly call `CheckBlockIndex` etc.), because whenever we would release cs_main, the same would be possible from another thread.

💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2600470202)
makes sense, removed the condition

💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2600474557)
right, that changed recently with #29640

💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#discussion_r2600486783)
looks good, I'll do some fuzzing with it / try to understand the non-determinism before I include it.

💬 mzumsande commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3629461809)
[7927473](https://github.com/bitcoin/bitcoin/commit/7927473c97b688d0c7162e4de294d137b8779568) to [62edcea](https://github.com/bitcoin/bitcoin/commit/62edceaf6d331cf7c06e277338b37831c910b1f7):
Addressed feedback (didn't include `getblockfrompeer` behavior of re-downloading prune blocks yet, see above).

🤔 hebasto reviewed a pull request: "guix: use GCC 14.3.0 over 13.3.0"
(https://github.com/bitcoin/bitcoin/pull/33775#pullrequestreview-3554616850)
Approach ACK a72ff31b879f164ca7875c0121953af93d799aee.

💬 hebasto commented on pull request "guix: use GCC 14.3.0 over 13.3.0":
(https://github.com/bitcoin/bitcoin/pull/33775#discussion_r2600556460)
Why not using the same minor version for both packages `gcc-14` and `gcc-toolchain-14`?

🚀 ryanofsky merged a pull request: "Add util::Expected (std::expected)"
(https://github.com/bitcoin/bitcoin/pull/34006)

💬 ryanofsky commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3629757028)
I merged this since it seems like a clear improvement that other changes can build on. Also took "whichever happens earlier" (https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599410546) as a hint to mean the author is happy to see this merged as is. Followup suggestion are listed in: https://github.com/bitcoin/bitcoin/pull/34006#pullrequestreview-3552578391

💬 ryanofsky commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2600677706)
re: https://github.com/bitcoin/bitcoin/pull/34006#discussion_r2599410546

FWIW, I think an explicit `(void)connection_client.release()` statement would make it more obvious that `connection_client` is null than the other other alternatives of calling `release()` inline or it assigning to a `_` variable. Similarly, I don't think would be very useful to declare a `bootstrap_cap` variable that is immediately moved from and unusable. But any of the approaches seems ok.

💬 hebasto commented on pull request "guix: Build for macOS using Clang only":
(https://github.com/bitcoin/bitcoin/pull/32764#issuecomment-3629761373)
> Looks like this still using the GNU binutils for building native tools.

Should be fixed now.

🤔 ryanofsky reviewed a pull request: "test: interface_ipc.py minor fixes and cleanup"
(https://github.com/bitcoin/bitcoin/pull/34003#pullrequestreview-3555280211)
Thanks for the reviews!

<!-- begin push-3 -->
Updated 6ef092c0e4343fc421c323ff09d3c791fb4bc33a -> c358910b8cd6d1db51c10becb0cbcb58709b738f ([`pr/pyipc.2`](https://github.com/ryanofsky/bitcoin/commits/pr/pyipc.2) -> [`pr/pyipc.3`](https://github.com/ryanofsky/bitcoin/commits/pr/pyipc.3), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/pyipc.2..pr/pyipc.3))<!-- end --> applying suggestions and adding a new commit to test actions during waitNext calls instead of before them.

I tried to
...

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2600997265)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598844209

Added a new commit to make this check work as originally intended and call generate during the wait instead of before.

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2601000748)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2592842483

Make sense, applied the suggestion

💬 ryanofsky commented on pull request "test: interface_ipc.py minor fixes and cleanup":
(https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2600998615)
re: https://github.com/bitcoin/bitcoin/pull/34003#discussion_r2598858386

Added a new commit to make this check work as originally intended and send the transaction during the wait instead of before.