💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1204069957)
Switched back to Jammy in next push.

📝 MarcoFalke opened a pull request: "ci: Add missing set -e to 01_base_install.sh"
(https://github.com/bitcoin/bitcoin/pull/27739)
Otherwise errors are silently ignored

💬 MarcoFalke commented on pull request "ci: Add missing set -e to 01_base_install.sh":
(https://github.com/bitcoin/bitcoin/pull/27739#issuecomment-1561108254)
Looks like it is working: https://cirrus-ci.com/task/4766589006905344?logs=build#L2788


Added another unrelated commit to re-trigger CI and to give more yummy review to reviewers.

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1561118892)
Ah, this is more useful:

```
==2894643== Thread 25 b-msghand:
==2894643== Conditional jump or move depends on uninitialised value(s)
==2894643== at 0x24A6EC: (anonymous namespace)::PeerManagerImpl::RemoveBlockRequest(uint256 const&, std::optional<long>) [clone .isra.0] (in /home/sjors/dev/bitcoin/src/bitcoind)
==2894643== by 0x25AAA1: (anonymous namespace)::PeerManagerImpl::ProcessBlock(CNode&, std::shared_ptr<CBlock const> const&, bool, bool) (in /home/sjors/dev/bitcoin/src/bitcoin
...

💬 MarcoFalke commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1561137418)
Maybe open a separate issue? (There are already too many comments in this thread).

Make sure to include details, otherwise it will be harder to do something. Note that not all compiler versions are compatible with all valgrind versions.

💬 justingoldberg commented on issue "Frequent "Timeout downloading block" with 24.1":
(https://github.com/bitcoin/bitcoin/issues/27705#issuecomment-1561165980)
Can you block this peer?

👋 ryanofsky's pull request is ready for review: "refactor: Replace `std::optional<bilingual_str>` with `util::Result`"
(https://github.com/bitcoin/bitcoin/pull/25977)

💬 instagibbs commented on issue "Notes on Block-In-Flight Handling":
(https://github.com/bitcoin/bitcoin/issues/16172#issuecomment-1561195606)
As of https://github.com/bitcoin/bitcoin/pull/27626 this case should be covered, you can close

💬 ismaelsadeeq commented on issue "25.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/27736#issuecomment-1561196420)
Thank you @theStack for pointing this out, these are important features, I will add them to the guide.

✅ fanquake closed an issue: "Notes on Block-In-Flight Handling"
(https://github.com/bitcoin/bitcoin/issues/16172)

💬 mzumsande commented on issue "Parallel compact block download":
(https://github.com/bitcoin/bitcoin/issues/25258#issuecomment-1561244418)
should this stay open for a while as per https://github.com/bitcoin/bitcoin/pull/27626#issuecomment-1557224231?

💬 instagibbs commented on issue "Parallel compact block download":
(https://github.com/bitcoin/bitcoin/issues/25258#issuecomment-1561246246)
We could open a new issue with fresh context as well, summarizing the discussion to this point?

💬 fanquake commented on issue "Parallel compact block download":
(https://github.com/bitcoin/bitcoin/issues/25258#issuecomment-1561246426)
> should this stay open

The PR shouldn't stay open, but we can open a tracking issue if needed.

💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1561260881)
> Do you happen to know what the difference to the debian package is? Maybe it can be fixed upstream instead?

I'm not completely sure, the answer will be in here somewhere: https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/blob/snapshot/debian/rules (also taking into account [all of the patches](https://salsa.debian.org/pkg-llvm-team/llvm-toolchain/-/tree/snapshot/debian/patches) they apply). They should be doing a multi stage build, and building everything with the 2nd-built Clang, but
...

💬 MarcoFalke commented on pull request "refactor: Replace `std::optional<bilingual_str>` with `util::Result`":
(https://github.com/bitcoin/bitcoin/pull/25977#issuecomment-1561270092)
Looks like there is a clang bug. Could be fixed by either bumping it again, see https://github.com/bitcoin/bitcoin/pull/27682, but I am not sure how to do that for macOS. Or by replacing `return addrman;` with `return {std::move(addrman)};`, or something like that (temporarily).

⚠️ instagibbs opened an issue: "Parallel compact blocks bandwidth reduction or improvements"
(https://github.com/bitcoin/bitcoin/issues/27740)
Opening a tracking issue post https://github.com/bitcoin/bitcoin/issues/25258

To summarize a few potential strategies:

1) It may be beneficial to reduce/eliminate concurrent block fetches from high-bandwidth outbound peers. That's at tension with the potential block relay speedup for non-listening peers, but at least non-listening peers aren't as vulnerable to sybil nodes.
2) Could reduce number of concurrent to 2, instead of 3, for non-listening only, or both modes, to reduce worst-case
...

💬 instagibbs commented on issue "Parallel compact block download":
(https://github.com/bitcoin/bitcoin/issues/25258#issuecomment-1561271505)
opened a new issue: https://github.com/bitcoin/bitcoin/issues/27740

💬 pinheadmz commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1204269104)
Good point, although "if empty, return now" logic would make sense after each step in this function (why bother calling `EraseLastKElements()` with an empty array?) even without this PR. One thing we could do is add that logic to the beginning of `EraseLastKElements()` itself, but I dunno how much time is really wasted in there (`sort`, `min`, `erase` ... ?)

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204276234)
This is the line valgrind trips over. Adding something like `fprintf(stderr, "from_peer=%ld\n", *from_peer);` above it makes it stop complaining. Maybe this is just a valgrind a bug?

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1203951678)
cce96182ba2457335868c65dc16b081c3dee32ee: maybe do `if (!mapBlocksInFlight.count(hash) {` first and then construct the range after the `Assume` below?