💬 jonatack commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#discussion_r1796992532)
Would it make sense to put these 2 `ConnectionType` utility methods in `node/connection_types.{h,cpp}` where they could be more widely used?

💬 jonatack commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#discussion_r1796174636)
nit, this prints as:

`(json array, optional, default=empty (no aggregation)) An array of keywords for aggregating the results.`

Perhaps avoid the double braces with

`(json array, optional, default=empty, no aggregation) An array of keywords for aggregating the results.`

```suggestion
/*fallback=*/RPCArg::DefaultHint{"empty, no aggregation"},
```

💬 jonatack commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#discussion_r1796995737)
Note to self, these 2 `Network` methods might be candidates for extraction in https://github.com/bitcoin/bitcoin/pull/27385.

💬 jonatack commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#discussion_r1796987675)
Would there be any benefit to moving this class outside of `net.{h,cpp}`? It's quite a lot of code.

(Though, much of its verbosity is the formatting style used that often reads top-to-bottom rather than left-to-right. I prefer the latter.)

🤔 maflcko reviewed a pull request: "scripted-diff: Replace strprintf(Untranslated(...)) with Untranslated(strprintf(...))"
(https://github.com/bitcoin/bitcoin/pull/31072#pullrequestreview-2362888700)
I don't mind the changes here, but I don't think the motivation is correct and it seems misleading.

I don't think it matters much whether this is merged or not, but you could just replace the motivation with:

"This replacement makes the implementation of some compile-time checks in the future easier. Also, it makes the code a bit more consistent in that `Untranslated` isn't allowed as a format-string anymore, only plain `std::string`s, or translated strings. The downside would be that deve
...

💬 maflcko commented on pull request "scripted-diff: Replace strprintf(Untranslated(...)) with Untranslated(strprintf(...))":
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1797005420)
in commit " refactor: Use + instead of strformat to concatenate Untranslated strings ":

I still don't think this is correct. You are using `+` to concatenate `Untranslated` and `_()` (translated), possibly mixing English strings with non-English ones. Also, this compiles, so there are no compile-time checks preventing the "mix".

💬 ryanofsky commented on pull request "scripted-diff: Replace strprintf(Untranslated(...)) with Untranslated(strprintf(...))":
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1797036507)
> I still don't think this is correct.

What part of this is not correct? Is there a specific sentence or something?

> You are using `+` to concatenate `Untranslated` and `_()` (translated), possibly mixing English strings with non-English ones. Also, this compiles, so there are no compile-time checks preventing the "mix".

There is nothing in this commit message (19f49f138c4756b5219e715d8749d15be07e2c81) which mentions mixing English or non-english strings.

As a whole, this PR is upda
...

💬 fanquake commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407528118)
> What would be the downside of only having glibc 2.27 release binaries for x86_64 (and/or aarch64), as opposed to having a separate old-glibc build?

If what you're suggesting is :
`x86_64-linux-gnu`: 2.27
`arm-linux-gnueabihf`: 2.modern
`aarch64-linux-gnu`: 2.27
`riscv64-linux-gnu`: 2.modern
`powerpc64-linux-gnu`: 2.modern
`powerpc64le-linux-gnu`: 2.modern (when re-enabled)

Some might be:
* We can't ship modern (hardening) features to users running `x86_64` or `aarch64`. i.e #24123
...

💬 sipa commented on issue "Consider making 27.x Long-Term Support (LTS)":
(https://github.com/bitcoin/bitcoin/issues/31068#issuecomment-2407533799)
@fanquake Fair points, though your second and third point also apply to having a separate old-glibc build (in addition to having modern x86_64/aarch64 builds).

💬 maflcko commented on pull request "scripted-diff: Replace strprintf(Untranslated(...)) with Untranslated(strprintf(...))":
(https://github.com/bitcoin/bitcoin/pull/31072#discussion_r1797046367)
> > I still don't think this is correct.
>
> What part of this is not correct? Is there a specific sentence or something?

Sorry for being unclear. The commit title says

"refactor: Use + instead of strformat to concatenate Untranslated strings".

It should say:

"refactor: Use + instead of strformat to concatenate bilingual strings, which may be Untranslated or translated (_)"

💬 danielabrozzoni commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1797035913)
nit: I don't think `verbosity=2` is needed

💬 danielabrozzoni commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1797036268)
nit: same as above, verbosity shouldn't be needed

💬 danielabrozzoni commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1797035626)
This line shouldn't be necessary, you're not using the resulting `orphanage` anywhere

💬 danielabrozzoni commented on pull request "test: Assert that when we add the max orphan amount that we cannot add anymore and that a random orphan gets dropped":
(https://github.com/bitcoin/bitcoin/pull/31040#discussion_r1797055987)
The test is currently a bit slow, and I think it might be because you're using `send_and_ping`, which waits for the other peer to send back a pong in each iteration of the loop.

You could send the messages in the for loop and then ping only at the end:
```python
for i in range(MAX_ORPHANS):
tx_parent_1 = self.wallet.create_self_transfer()
tx_child_1 = self.wallet.create_self_transfer(utxo_to_spend=tx_parent_1["new_utxo"])
peer_1.send_message(m
...

📝 dergoegge opened a pull request: "ci: Split out native fuzz jobs for macOS and windows"
(https://github.com/bitcoin/bitcoin/pull/31073)
Split out two new CI jobs (for native macOS and windows) that run the fuzz tests on the qa-assets input corpora.

In both jobs the `fuzz` binary is built with `-DBUILD_FOR_FUZZING` to enable `Assume` assertions as well as `FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION`.

👋 dergoegge's pull request is ready for review: "ci: Split out native fuzz jobs for macOS and windows"
(https://github.com/bitcoin/bitcoin/pull/31073)

💬 instagibbs commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2407691628)
Just for brainstorming, also made a modified branch that reduces parallel compact block downloads to a count of 2, which would allow one or more backoff blocks at tip at a less aggressive pace: https://github.com/instagibbs/bitcoin/commits/mzumsande_202403_near_tip_stalling_2/

💬 theuni commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2407700684)
> > Running Bitcoin Core on unsupported OSes may expose users to security issues.
> > macOS Big Sur 11 received its last security update ([11.7.10](https://support.apple.com/en-us/106338)) over a year ago.
>
> If this is the reasoning we are going to use, shouldn't this be bumping to `10.13`, given `10.12` is also "unsupported" in terms of security updates?

Yes, good point. I didn't realize 12.0 was eol'd. We shouldn't let qt dictate what we support.

Agree that 13 would make more sense
...

💬 LarryRuane commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2407842364)
Force pushed Jon's suggestion, thanks - https://github.com/bitcoin/bitcoin/pull/30859#discussion_r1795456841

💬 jonatack commented on pull request "doc: cmake: prepend "build" to functional/test_runner.py":
(https://github.com/bitcoin/bitcoin/pull/30859#issuecomment-2407851530)
ACK e64b2f1a16e8d0ad2cd181d84e3b70312e3cdf33