👍 hebasto approved a pull request: "depends: qt 5.15.14 and fix macOS build with Clang 18"
(https://github.com/bitcoin/bitcoin/pull/30198#pullrequestreview-2090665641)
ACK 0a3631fc352eda849290db940844e5ef723436df, a new patch indeed fixes cross-compiling on Ubuntu 24.04 with `FORCE_USE_SYSTEM_CLANG=1`.

💬 maflcko commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1622387886)
Clearing the cache helped, so likely it was a corrupt compilation unit, cached by ccache, which also explains why a re-run didn't help.

💬 maflcko commented on pull request "depends: consolidate dependency docs":
(https://github.com/bitcoin/bitcoin/pull/30204#issuecomment-2142122297)
ACK a27e1ceb9f9c9239af9b0a151c84a57573ea646a

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1622428800)
I agree with the "making it more confusing". Given this is an edge case, I'll remove the outbound count and we can account for it if someone has a better way of doing so, or if someone opens an issue about it.

📝 dergoegge opened a pull request: "fuzz: Make FuzzedSock fuzz friendlier"
(https://github.com/bitcoin/bitcoin/pull/30211)
`FuzzedSock` has a few issues that block a fuzzer from making progress. See commit messages for details.

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1622453453)
Matches!

💬 maflcko commented on pull request "lint/contrib/doc updates":
(https://github.com/bitcoin/bitcoin/pull/30084#discussion_r1622454655)
When fixing typos, it could make sense to cherry-pick the real ones (non-subtree, non-release notes) from the closed https://github.com/bitcoin/bitcoin/pull/30188/files. Otherwise, people will continue to open pull requests to fix them.

💬 dergoegge commented on pull request "fuzz: Make FuzzedSock fuzz friendlier":
(https://github.com/bitcoin/bitcoin/pull/30211#issuecomment-2142220760)
These issues were discovered by Marco (@marcofleon) and me while he was working on #28803. He will have a PR for that up soon and this PR is a prerequisite (although it also helps with other harnesses that use `FuzzedSock`).

cc @vasild perhaps you are interested in reviewing this.

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#issuecomment-2142248272)
> ACK [fa42bf7](https://github.com/bitcoin/bitcoin/commit/fa42bf7ee89999b33d4af0600b0d8da1275e5d5e)
>
> This is already an improvement over `master`. Would be nice to resolve [#30065 (comment)](https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1613422626) as well.

Addressed all comments now

💬 sr-gi commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#discussion_r1622471290)
I've rolled back this. After some of the refactoring, this looked like:

```
std::clamp(available_fds - min_required_fds, 0, nUserMaxConnections);
```

However, `available_fds` cannot be smaller than `min_required_fds`, and `nUserMaxConnections` cannot be negative. So calling `std::min` should be enough.

📝 willcl-ark opened a pull request: "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN"
(https://github.com/bitcoin/bitcoin/pull/30212)
Closes: #19363

Renaming these two errors improves clarity around the returned error when:

1. a transactions' outputs are already found in the utxo set (`ALREADY_IN_CHAIN -> ALREADY_IN_UTXO_SET`)
2. a transactions' inputs are missing from the utxo set, which could also be due to all inputs having already been spent (`MISSING_INPUTS -> INPUTS_MISSING_OR_SPENT`)

💬 willcl-ark commented on pull request "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN":
(https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2142284206)
This addresses #19363 's concern:

> If a transaction is already in the block chain, and all of its outputs are spent, the BroadcastTransaction() fails to detect the correct transaction status. It returns TransactionError::MISSING_INPUTS instead of TransactionError::ALREADY_IN_CHAIN.

...by now returning `TransactionError::INPUTS_MISSING_OR_SPENT`. The [translation](https://github.com/bitcoin/bitcoin/blob/62f7f59ff495fbcbfc10c25e97bb0dc032647abf/src/util/error.cpp#L19) for this error message
...

🚀 fanquake merged a pull request: "depends: qt 5.15.14 and fix macOS build with Clang 18"
(https://github.com/bitcoin/bitcoin/pull/30198)

🚀 fanquake merged a pull request: "depends: consolidate dependency docs"
(https://github.com/bitcoin/bitcoin/pull/30204)

📝 Sjors opened a pull request: "net: have GetReceivedMessage() return bytes"
(https://github.com/bitcoin/bitcoin/pull/30213)
Fixes #30209.

Previously `GetReceivedMessage()` returned a CNetMessage. Have the caller process the bytes instead. Similarly `SetMessageToSend` needed a `CSerializedNetMsg`, give it bytes instead.

This makes the `Transport` class usable for future message types that don't inherit from CNetMessage.

TODO:
- [ ] clean up
- [ ] fix tests
- [ ] implement `SetMessageToSend`

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1622525854)
@theuni Nice idea, done.

💬 Sjors commented on issue "Make Transport independent of CNetMessage and CSerializedNetMsg":
(https://github.com/bitcoin/bitcoin/issues/30209#issuecomment-2142404400)
Easier said than done... very rough draft in #30213

💬 Sjors commented on pull request "net: have GetReceivedMessage() return bytes":
(https://github.com/bitcoin/bitcoin/pull/30213#issuecomment-2142415999)
v2 is fairly straight-forward, but v1 is not.

💬 sipa commented on pull request "net: have GetReceivedMessage() return bytes":
(https://github.com/bitcoin/bitcoin/pull/30213#discussion_r1622531541)
Please don't do this; the whole point of having separate transport classes is hiding the complexity of dealing with the difference from the rest of the network code.

💬 sipa commented on pull request "net: have GetReceivedMessage() return bytes":
(https://github.com/bitcoin/bitcoin/pull/30213#issuecomment-2142430707)
Would an alternative approach be that `V1Transport` and `V2Transport` always just treat the first 12 bytes of a message as the command type (as in on decode they'd produce a message with "length" 12+payload_len, etc). That would mean the `Transport` interface works purely on bytes, and could be usable for protocols that don't deal with things that have a command type.

Another option is having two layers: a "raw transport" interface that works on bytes, and a layer "command-based transport" in
...