📝 wnqqnw19 opened a pull request: "Wnqqnw19"
(https://github.com/bitcoin/bitcoin/pull/34007)
```
## Changes Made

1. **Renamed `format` parameter to `format_version`** (line 18)
- Avoids shadowing Python's built-in `format()` function
- Improves code clarity and follows Python best practices

2. **Removed redundant local assignment of `ADDRMAN_NEW_BUCKET_COUNT`** (line 35)
- Now uses the imported constant from `test_framework.netutil` directly
- Ensures consistency with the centralized constant definition
- If the constant changes in the future, this code will a
...

✅ fanquake closed a pull request: "Wnqqnw19"
(https://github.com/bitcoin/bitcoin/pull/34007)

💬 purpleKarrot commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3613330088)
> Some low-level code could benefit from being able to use `std::expected` from C++23.

Can you elaborate? How would it benefit? Over what alternative?

👍 ryanofsky approved a pull request: "mining: getCoinbase() returns struct instead of raw tx"
(https://github.com/bitcoin/bitcoin/pull/33819#pullrequestreview-3541130613)
Code review ACK 5761dac2f5481c27550483279a007c2b850bc990, with the two suggestions applied since last review.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589928992)
> Why not simply call `NumToOpenAdd(num_for_rebroadcast)`

This would be the change:

<details>
<summary>Use only num_for_rebroadcast</summary>

```diff
void PeerManagerImpl::ReattemptPrivateBroadcast(CScheduler& scheduler)
{
- // The following heuristic is subject to races, but that is ok: if it overshoots,
- // we will open some private connections in vain, if it undershoots, the stale
- // transactions will be picked on the next run.
-
- size_t active_connections{0
...

👍 sedited approved a pull request: "depends: update freetype and document remaining `bitcoin-qt` runtime libs"
(https://github.com/bitcoin/bitcoin/pull/33952#pullrequestreview-3541144045)
Guix build:
```
760ca6bb241b02b496785a0123cea4e9e49e03e27e2280824237bd4125ffb1d0 guix-build-41e657aacfa6/output/aarch64-linux-gnu/SHA256SUMS.part
432c63a2879ef00793c9378f64d5130752258c0d845b565c4fa7c708399be01a guix-build-41e657aacfa6/output/aarch64-linux-gnu/bitcoin-41e657aacfa6-aarch64-linux-gnu-debug.tar.gz
15c65f8720598ff04dba8d15b0b49e5ab9064f6798159b331681806a62f28426 guix-build-41e657aacfa6/output/aarch64-linux-gnu/bitcoin-41e657aacfa6-aarch64-linux-gnu.tar.gz
d53c1ef1fd1501f0452d
...

💬 maflcko commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3613425769)
> > Some low-level code could benefit from being able to use `std::expected` from C++23.
>
> Can you elaborate? How would it benefit? Over what alternative?

Sure. The alternatives were listed in doc diff in the commit, but I've pulled it out and put in the pull description as well.

💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2589984428)
Marking as resolved, checked with @andrewtoth on IRC, thanks!

🤔 jaonoctus reviewed a pull request: "init: point out -stopatheight may be imprecise"
(https://github.com/bitcoin/bitcoin/pull/33993#pullrequestreview-3541279438)
re-ACK ff06e2468a5d3eeebeffe781904c34c9d1b44385

💬 hebasto commented on pull request "ci: Add IWYU job":
(https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2590030400)
Thanks! Reworked.

📝 0xB10C opened a pull request: "log: don't rate-limit "new peer" with -debug=net"
(https://github.com/bitcoin/bitcoin/pull/34008)
Previously, when `debug=net` is enabled, we log "New [..] peer connected" for new inbound peers with `LogPrintf`. However, `LogPrintf` will get rate-limited since https://github.com/bitcoin/bitcoin/pull/32604. When we specifically turn on `debug=net`, we don't want these log messages to be rate-limited.

To fix this, use `LogDebug` if `debug=net` is enabled. Otherwise use `LogPrintf`. This means we don't rate-limit the message with `debug=net` (due to `LogDebug` not being rate-limited) but sti
...

💬 0xB10C commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2590031496)
I'm not super happy with this approach of duplicating the log statement, but I think it's the clearest to a reader. If someone feels strongly, I'm very happy to change it to something better! There are two alternatives that came to mind:

1. Use a macro to avoid duplicating the log statements: Not sure if we want to define a macro extra for this, but could be done. This was my initial approach I dismissed.
2. Extract the log message string and format it with e.g. `std::format()` first, then pass
...

💬 hebasto commented on pull request "ci: Add IWYU job":
(https://github.com/bitcoin/bitcoin/pull/33810#issuecomment-3613552394)
The [feedback](https://github.com/bitcoin/bitcoin/pull/33810#discussion_r2550408178) from @fanquake has been addressed.

💬 mzumsande commented on pull request "index: Deduplicate HashKey / HeightKey handling":
(https://github.com/bitcoin/bitcoin/pull/32997#issuecomment-3613627392)
> what do you think about wrapping all the code in `db_key.h` under a namespace like "util", "common", "index::util", or "util::index" (just threw some names), so it's clear when reading the code that these functions are defined elsewhere and shared, without needing to search around for the definition.

I did that in 5646e6c0d3581f12419913b88745f51c7a3161b9 - let me know what you think. I used `index_util`, turned out it was required a bit more often than I would have expected.

💬 ajtowns commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2590075362)
Perhaps

```c++
auto mapped_as_info = [&]() -> std::string {
const auto mapped_as{m_connman.GetMappedAS(pfrom.addr);
return (mapped_as ? strprintf(", mapped_as=%d", mapped_as) : "");
};
if (!pfrom.IsInboundconn()) {
LogInfo("New %s %s peer ...", ..., mapped_as_info());
} else {
LogDebug(BCLog::NET, "New %s %s ...", ..., mapped_as_info());
}
```

💬 darosior commented on issue "ASN-based bucketing of the network nodes":
(https://github.com/bitcoin/bitcoin/issues/16599#issuecomment-3613637707)
That makes sense, but maybe other mechanism could achieve the same without the same drawbacks. For instance is it still really necessary after https://github.com/bitcoin/bitcoin/pull/19858?

💬 purpleKarrot commented on pull request "Add util::Expected (std::expected)":
(https://github.com/bitcoin/bitcoin/pull/34006#issuecomment-3613640791)
> > > Some low-level code could benefit from being able to use `std::expected` from C++23.
> >
> > Can you elaborate? How would it benefit? Over what alternative?
>
> Sure. The alternatives were listed in doc diff in the commit, but I've pulled it out and put in the pull description as well.

What is wrong with the most obvious approach to error handling?

💬 ajtowns commented on pull request "log: don't rate-limit "new peer" with -debug=net":
(https://github.com/bitcoin/bitcoin/pull/34008#discussion_r2590081585)
Or could make it `auto conn_details = [&]() ...` and `LogDebug(BCLog::NET, "New %s peer connected: %s", pfrom.ConnectionTypeAsString(), conn_details())` and move all the transport, version, mapped_as stuff into the lambda, moving the transport type to after the colon.

💬 hebasto commented on pull request "guix: Use UCRT runtime for Windows release binaries":
(https://github.com/bitcoin/bitcoin/pull/33593#issuecomment-3613693155)
> Given it's dropping the other CI, I think this PR should be completing #30210, dropping workarounds, updating any docs, and completing the migration.

More commits have been added to complete the migration.

🤔 ryanofsky reviewed a pull request: "mining: add getMemoryLoad() and track template non-mempool memory footprint"
(https://github.com/bitcoin/bitcoin/pull/33922#pullrequestreview-3541246611)
Code review e8f8f7f677bcde0179526be3ed9a657c44998b93. This looks good except for a thread safety issue I think you can address by adding a mutex.

re: https://github.com/bitcoin/bitcoin/pull/33922#issuecomment-3611468373

> > Would be good if this said templates are also released
>
> Added a sentence to the PR description.

Sorry, I should have made a more specific suggestion. The problem is is that this sentence is not accurate: "templates are not released until the client disconnects or calls
...