💬 Satoshin-Btc commented on issue "Net split meta issue":
(https://github.com/bitcoin/bitcoin/issues/33958#issuecomment-3629209877)
so i agree with the things you pointed out. the rushed merger was a complete mess the last time in 2022 and didn't bode well either. i get the node and process thread issue for validated peers for txs blocks and so forth. my only thought on solution is something similar to the net split but that causes its own issues. so with dedicated nodes back to where the we're before allowing calls from the different containers helped the process a lot more than the jumbled up system we are dealing with. th
...

💬 sedited commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#issuecomment-3629213020)
Updated beff6ff964c5c1b4a96af89daf6b2ddaede3464a -> 8ca9997e48bb2067ea83fabb1c640af72178f97c ([rmMinPowChecked_0](https://github.com/TheCharlatan/bitcoin/tree/rmMinPowChecked_0) -> [rmMinPowChecked_1](https://github.com/TheCharlatan/bitcoin/tree/rmMinPowChecked_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/rmMinPowChecked_0..rmMinPowChecked_1))

* Addressed [comment](https://github.com/bitcoin/bitcoin/pull/34022#discussion_r2599070189), and [comment](https://github.com/bitcoin/
...

💬 l0rinc commented on pull request "validation: Remove min_pow_checked arg in ProcessNewBlockHeaders":
(https://github.com/bitcoin/bitcoin/pull/34022#issuecomment-3629227687)
reACK 8ca9997e48bb2067ea83fabb1c640af72178f97c

only doc updates since last ACK

👍 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
...