💬 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?

💬 Sjors commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1203997112)
Lost in rebase?

💬 instagibbs commented on pull request "Parallel compact block downloads, take 3":
(https://github.com/bitcoin/bitcoin/pull/27626#discussion_r1204280315)
hmmm appears so

💬 MarcoFalke commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1204288987)
Some ideas (feel free to ignore):

* `-DLLVM_USE_LINKER=lld` and/or ` -DLLVM_PARALLEL_LINK_JOBS=1` to reduce change of OOM?
* `-DLLVM_INCLUDE_BENCHMARKS=OFF -DLLVM_INCLUDE_TESTS=OFF` to reduce CPU?

👍 MarcoFalke approved a pull request: "ci: compile Clang and compiler-rt in msan jobs"
(https://github.com/bitcoin/bitcoin/pull/27737#pullrequestreview-1442083675)
lgtm. Longer term it may be better to try to go back to using the pre-compiled one from debian, or alternatively try a different Linux distro for the msan tasks?

💬 stickies-v 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_r1204313559)
I had similar considerations to e.g. wrap the `EraseLastKElements` call in a lambda fn, but it's probably not worth the LoC change.

I think this case is slightly different in that it better highlights that if there are no inbound NoBan peers, the result is always `std::nullopt`.

💬 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_r1204318480)
good point, i'll add it. one line i think in this case will improve readability

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

I did block the peer right after I posted the latest logs four days ago, and after that the block timeouts dropped to 2-3 per day, but like I mentioned in the bug report, since Core doesn't normally log the IP of disconnected incoming peers you pretty much have to enable debug=net logging to find the IP of the misbehaving peer. Which on this particular node is ~1GB of logs per 2-3 hours.

⚠️ Sjors opened an issue: "Spurious (?) valgrind failure for p2p_compactblocks.py"
(https://github.com/bitcoin/bitcoin/issues/27741)
Ubuntu 23.04 with g++ 12.2.0 and valgrind 3.19.0. Built without depends and without `--enable-debug`.

The failure is introduced by:
https://github.com/bitcoin/bitcoin/pull/27626/commits/cce96182ba2457335868c65dc16b081c3dee32ee

Offending line in `RemoveBlockRequest`:

```cpp
if (from_peer && *from_peer != node_id) {
```

It seems to believe `from_peer` is uninitialized. Adding a log statement that prints `from_peer` makes the error go away. As does compiling with `--enable-debug`. Ot
...

💬 fanquake commented on issue "Spurious (?) valgrind failure for p2p_compactblocks.py":
(https://github.com/bitcoin/bitcoin/issues/27741#issuecomment-1561375155)
Can you make it clear what version of the code you are actually running valgrind on? Are you compiling cce96182ba2457335868c65dc16b081c3dee32ee or are you compiling master (if so, which commit).

💬 theuni commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#issuecomment-1561389825)
Concept ACK. I'm not so familiar with the history here, but the changes seem sane to me.

The libc++ changes here are especially interesting. I assume they work with no issues because there's no SDK to take into consideration, but (as @fanquake suggested in an offline discussion yesterday) I'm going to see if we can maybe port this over to our depends builds with the SDK included. Unsure if it'll work 🤷 but it would be much cleaner if it did.

📝 glozow opened a pull request: "[NO MERGE] BIP331 Ancestor Package Relay"
(https://github.com/bitcoin/bitcoin/pull/27742)
**WORK IN PROGRESS.** Please do not run it for any use other than testing.

This PR is not meant for merge. This branch exists to help reviewers see what package relay would look like all together. I will open separate PRs for these components and expect to add more tests, docs, and polish along the way. This PR contains all of the functionality built in a linear manner. However, since there are pieces in various areas of the codebase and they can make progress in parallel, commits don't nec
...

💬 stickies-v commented on pull request "kernel: Remove util/system from kernel library, interface_ui from validation.":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1204062504)
nit: I think you also need to `#include <util/translation.h>` for `bilingual_str`