⚠️ brunoerg opened an issue: "fuzz: connman, `m_nodes` is always empty"
(https://github.com/bitcoin/bitcoin/issues/27980)
`m_nodes` in `CConnman` is always empty in connman target (since we don't call `CreateNodeFromAcceptedSocket`/`OpenNetworkConnection` for an obvious reason). Because of this, the calls for `DisconnectNode`, `FindNode`, `ForEachNode`, `GetNodeStats` and many other ones seems to be "useless". That's the reason we're not getting coverage (at all) for many of these covered functions (See: https://marcofalke.github.io/b-c-cov/fuzz.coverage/src/net.cpp.gcov.html).

I suppose we could use `ConnmanTes
...

💬 sipa commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#discussion_r1242690354)
This can be written to make use of `AsBytes`, actually:

```diff
@@ -472,10 +472,10 @@ struct CustomUintFormatter
if (v < 0 || v > MAX) throw std::ios_base::failure("CustomUintFormatter value out of range");
if (BigEndian) {
uint64_t raw = htobe64(v);
- s.write({reinterpret_cast<const std::byte*>(&raw) + 8 - Bytes, Bytes});
+ s.write(AsBytes(Span{&raw, 1}).last(Bytes));
} else {
uint64_t raw = htole64(v);
-
...

💬 Crypt-iQ commented on issue "depends build fails macOS intel":
(https://github.com/bitcoin/bitcoin/issues/27977#issuecomment-1608144283)
Good point - the log snippet above is from an unmodified bitcoind. But even with fixing the dot, the make target can't find `setup.py`

💬 TheCharlatan commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1608156434)
I can confirm that my guix dmg runs on macOS 11.1.

👍 TheCharlatan approved a pull request: "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM"
(https://github.com/bitcoin/bitcoin/pull/27676#pullrequestreview-1499321116)
3df60704661cdb5e61ea2b999f468f3a1d16105f

🤔 mzumsande reviewed a pull request: "fuzz: addrman, add coverage for `network` field in `Select()`, `Size()` and `GetAddr()`"
(https://github.com/bitcoin/bitcoin/pull/27549#pullrequestreview-1499329509)
Code Review ACK 35a2175ad8bec92b409ae2202c124e39b2f3f838, haven't tested this yet, but I will let the fuzzer run for a while now.

💬 mzumsande commented on pull request "fuzz: addrman, add coverage for `network` field in `Select()`, `Size()` and `GetAddr()`":
(https://github.com/bitcoin/bitcoin/pull/27549#discussion_r1242705238)
> Next line calling AddrMan::GetAddr() function passes std::nullopt as the network argument

no, it now passes `network` as the network argument, which, depending on the fuzzing seed, could be `std::nullopt` or any network.

The goal of this PR is to give the fuzzer the possibility to reach network-specific code paths. If there was a bug within these, it wouldn't be able to find it before, because they just weren't invoked.

💬 hebasto commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1608171885)
ACK 3df60704661cdb5e61ea2b999f468f3a1d16105f

💬 brunoerg commented on pull request "fuzz: improve `coinselection`":
(https://github.com/bitcoin/bitcoin/pull/27585#issuecomment-1608188265)
Rebased

👍 ryanofsky approved a pull request: "wallet: Load database records in a particular order"
(https://github.com/bitcoin/bitcoin/pull/24914#pullrequestreview-1499361716)
Code review ACK 973d910c287867d8bb1c8041bca906c14596aeda

Looks good, probably ready for merge with a third ACK.

💬 ryanofsky commented on pull request "wallet: Load database records in a particular order":
(https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1242724102)
re: https://github.com/bitcoin/bitcoin/pull/24914#discussion_r1242640941

> In [dc1ef26](https://github.com/bitcoin/bitcoin/commit/dc1ef26e430abbf92ef39d2ce595adac651b949a):
>
> nit: this result is also being set to `CORRUPT` when `LoadToWallet` returns false with `corrupted_tx=true` (~30 lines of code below this line).

Nice catch. Would be nice to just eliminate the CORRUPTED_TX variable:

```diff
--- a/src/wallet/walletdb.cpp
+++ b/src/wallet/walletdb.cpp
@@ -1065,10 +1065,9 @@ st
...

📝 sipa opened a pull request: "Fix potential network stalling bug"
(https://github.com/bitcoin/bitcoin/pull/27981)
See https://github.com/ElementsProject/elements/issues/1233. There, it has been observed that if both sides of a P2P connection have a significant amount of data to send, a stall can occur, where both try to drain their own send queue before trying to receive. The same issue seems to apply to the current Bitcoin Core codebase, though I don't know whether it's a frequent issue for us.

The core issue is that whenever our optimistic send fails to fully send a message, we do subsequently not even
...

💬 sipa commented on pull request "Fix potential network stalling bug":
(https://github.com/bitcoin/bitcoin/pull/27981#issuecomment-1608209183)
cc @psgreco

💬 achow101 commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1242737788)
To clarify, the order matters because the ECDH is more than just plain ECDH - it hashes the ECDH secret with the pubkeys, and so ordering in the hashing serialization matters. BIP 324 specifies the preimage of this hash as `initiator pub || receiver pub || ecdh secret`, so we do this swapping so that we are computing this order correctly.

💬 ismaelsadeeq commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1242750105)
Could we perhaps consider relying on the check here? It seems like a duplicate of the same process in two different places if you agree.

💬 achow101 commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#issuecomment-1608232026)
ACK 3168b08043546cd248a81563e21ff096019f1521

💬 psgreco commented on pull request "Fix potential network stalling bug":
(https://github.com/bitcoin/bitcoin/pull/27981#issuecomment-1608257369)
concept ack 5e9237891df69876e1a6f81bf158aed2a683ffe2, this potential issue is not easy to trigger on demand in bitcoin, but it's relatively easy to trigger in elements, when the node is roughly 20/24 hours behind. Tested in elements a similar version of the patch it does solve the stalling

🚀 achow101 merged a pull request: "BIP324: ElligatorSwift integrations"
(https://github.com/bitcoin/bitcoin/pull/27479)

💬 tobtoht commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1608279863)
Guix builds:

```
109ed45706b8cbdc897b6c51efa0247f8e8f1e6bb35c57e69f30093b62189a51 guix-build-3df60704661c/output/aarch64-linux-gnu/SHA256SUMS.part
4e28463f1c074c7c121abd5fbad89aa2269c5978829ccd4898d33a21eedfeb4f guix-build-3df60704661c/output/aarch64-linux-gnu/bitcoin-3df60704661c-aarch64-linux-gnu-debug.tar.gz
f20d074f0529202274a40f0530f79f601f5c575f1cd0addc099418719982485e guix-build-3df60704661c/output/aarch64-linux-gnu/bitcoin-3df60704661c-aarch64-linux-gnu.tar.gz
414d7c4bf531a8d85
...

💬 achow101 commented on pull request "bumpfee: Allow the user to choose which output is change":
(https://github.com/bitcoin/bitcoin/pull/26467#discussion_r1242859628)
Done