💬 mzumsande 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-2407928676)
[973cd01 ](https://github.com/bitcoin/bitcoin/commit/973cd0174ae44ff9cbc3b41912323f71f8eafca4)to [5e1ff82](https://github.com/bitcoin/bitcoin/commit/5e1ff82251283c07274c62e1b65d5aff9be80212):
- added more detailed coverage by @instagibbs (thanks!)
- provided missing block and check that sync finishes in the end in `near_tip_stalling` subtest
- remove unused `total_bytes_recv_for_blocks` in first commit

> Just for brainstorming, also made a modified branch that reduces parallel compact blo
...

💬 mzumsande 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#discussion_r1797299338)
added, by connecting a peer that provides the missing block in the end, and checking that the sync finishes.

💬 satsie commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#discussion_r1797311701)
The thinking behind this is because it is a 4D array, all the nesting required to create it can be a headache to look at. Lines 725 - 727 are the array sizes, the second arguments needed for `std::array` creation. Each one of these lines up with the `std::array` they are part of.

For example, `NUM_DIRECTIONS` finishes the declaration off the parent level `std::array` that all the others are nested in. For that reason, `NUM_DIRECTIONS` lines up with the space after that array's opening `<`.

...

💬 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-2407953923)
> Is it a problem that on slow connections or new connections with unfilled mempools the parallel compact blocks download (which is triggered immediately instead of having a timeout) would hit more often than necessary?

I think that's already the case, a saving grace is that since we only trigger parallel cb with high-bandwidth cb peers it won't happen until you've seen a few blocks at tip at least.

👍 hebasto approved a pull request: "guix: use GCC 13 to build releases"
(https://github.com/bitcoin/bitcoin/pull/29881#pullrequestreview-2363408909)
ACK f9e0010de394ad6ae5eb6a22ead7ebc116a16fd4.

Guix build (on both `x86_64` and `aarch64`):
```
d6a2d55943232d35bc56c36341b8501af7637c1f337a1e49f2ac34baced7d884 guix-build-f9e0010de394/output/aarch64-linux-gnu/SHA256SUMS.part
3bbb9714ea3196a2fca7f2d8db8a309bc298b43ace4397f11ff8acc8b6d829d3 guix-build-f9e0010de394/output/aarch64-linux-gnu/bitcoin-f9e0010de394-aarch64-linux-gnu-debug.tar.gz
6b5402883ebef604c6c1da21d81d9414e731be41bf294a3a92f5fb5ff5be300b guix-build-f9e0010de394/output/aar
...

💬 hebasto commented on pull request "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled":
(https://github.com/bitcoin/bitcoin/pull/30685#issuecomment-2407976069)
Apparently, I haven't had time to address feedback and rebase this PR recently. Perhaps it could be labelled "Up for grabs"?

📝 hebasto converted_to_draft a pull request: "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled"
(https://github.com/bitcoin/bitcoin/pull/30685)
CET is Intel’s [Control-flow Enforcement Technology](https://www.intel.com/content/www/us/en/developer/articles/technical/technical-look-control-flow-enforcement-technology.html).

The current GCC implementation of the `-fcf-protection` option is [based](https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html#index-fcf-protection) on CET for `x86_64-linux-gnu`.

However, on the master branch @ d79ea809d28197b1b4e3748aa1715272b53601d0, the release binaries are not marked as CET-enable
...

📝 hebasto converted_to_draft a pull request: "Release `LockData::dd_mutex` before calling `*_detected` functions"
(https://github.com/bitcoin/bitcoin/pull/26781)
Both `double_lock_detected()` and `potential_deadlock_detected()` functions call `LogPrintf()` which in turn implies locking of the `Logger::m_cs` mutex. To avoid a deadlock, the latter must not have the `Mutex` type (see https://github.com/bitcoin/bitcoin/pull/16112).

With this PR the mentioned restriction has been lifted, and it is possible now to use our regular `Mutex` type for the `Logger::m_cs` mutex instead of a dedicated `StdMutex` type (not introducing that change here, as its diff i
...

📝 hebasto converted_to_draft a pull request: "test, subprocess: Improve coverage report correctness"
(https://github.com/bitcoin/bitcoin/pull/30075)
As a child process uses the [`execvp()`](https://linux.die.net/man/3/execvp) call, an explicit dumping of the collected profile information is required.

Coverage:
- on the master branch:

- with this PR:

This PR mostly follows [gcc/libgcc/libgcov-interface.c#L320-L332](https://github.com/gc
...

🤔 LarryRuane requested changes to a pull request: "netinfo: add peer services column and outbound-only peers list"
(https://github.com/bitcoin/bitcoin/pull/30930#pullrequestreview-2363290423)
I think you also should update line 93 (change "4" to "5")
```
argsman.AddArg("-netinfo", "Get network peer connection information from the remote server. An optional integer argument from 0 to 4 can be passed for different peers listings (default: 0). Pass \"help\" for detailed help documentation.", ArgsManager::ALLOW_ANY, OptionsCategory::CLI_COMMANDS);
```

💬 LarryRuane commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#discussion_r1797252340)
683b558a020f1632b0a3cbdaa165adbd1423281c
```suggestion
str += (s == "NETWORK_LIMITED") ? 'l' : ((s == "P2P_V2") ? '2' : ToLower(s[0]));
```
(readability, nonblocking nit, if you happen to retouch)

📝 ryanofsky opened a pull request: "util: Check bilingual_str format strings at compile time"
(https://github.com/bitcoin/bitcoin/pull/31074)
Replace `tinyformat::format` overload that used to accept `bilingual_str` runtime format strings with an overload that only accepts `bilingual_lit` literal format strings that are checked for correctness at compile time.

This PR is alternative to #31061 that has somewhat messier syntax, but a cleaner implementation and a less confusing API. Unlike #31061, the `_` and `Untranslated` functions continue to return `bilingual_str` structs with `original` and `translated` members instead of keeping
...

💬 jonatack commented on pull request "netinfo: add peer services column and outbound-only peers list":
(https://github.com/bitcoin/bitcoin/pull/30930#issuecomment-2408020616)
> I think you also should update line 93 (change "4" to "5")

Well spotted 👍 I'm thinking of taking @vasild's outonly suggestion, and will fix that if not.

💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1797358825)
re: https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1796975768

> Not sure about any of this, though, would need to experiment.

To follow up, I was able to implement an alternate approach in #31074 that lets `_` and `Untranslated` always return `bilingual_str` instances that you can access with `.original` and `.translated` instead of `.lit` and `translate()`. The resulting API seems more clean and consistent, but it does require messier syntax when calling strprintf (implmented)
...

👍 BrandonOdiwuor approved a pull request: "test: Print CompletedProcess object on error"
(https://github.com/bitcoin/bitcoin/pull/31067#pullrequestreview-2363531344)
Code Review ACK fa43c4f93ca5b40734ec9b3ff91b74acf3ed7cf2

👍 TheCharlatan approved a pull request: "lint: commit-script-check.sh: echo to stderr"
(https://github.com/bitcoin/bitcoin/pull/31063#pullrequestreview-2363533818)
ACK fac6cfe5ac06547c90da6f976d7c8bed20da8bac

The change is fine, but what are you going to do with this?

💬 kevkevinpal 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_r1797449674)
Thank you for the review! I've updated in [4d59b9a](https://github.com/bitcoin/bitcoin/pull/31040/commits/4d59b9a517afb5eb661b7bc179c51141769658a3)

the second one looked good to me! I prefer asserting that each individual tx made it into the orphanage

💬 kevkevinpal 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_r1797449789)
Thanks! updated in [4d59b9a](https://github.com/bitcoin/bitcoin/pull/31040/commits/4d59b9a517afb5eb661b7bc179c51141769658a3)

💬 kevkevinpal 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_r1797449852)
Thank you! updated in [4d59b9a](https://github.com/bitcoin/bitcoin/pull/31040/commits/4d59b9a517afb5eb661b7bc179c51141769658a3)

💬 kevkevinpal 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_r1797449904)
agreed! Updated in [4d59b9a](https://github.com/bitcoin/bitcoin/pull/31040/commits/4d59b9a517afb5eb661b7bc179c51141769658a3)