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

📝 ryanofsky opened a pull request: "refactor: Improve assumeutxo state representation"
(https://github.com/bitcoin/bitcoin/pull/30214)
This PR contains the first part of #28608, which tries to make assumeutxo code more maintainable, and improve it by not locking `cs_main` for a long time when the snapshot block is connected, and by deleting the snapshot validation chainstate when it is no longer used, instead of waiting until the next restart.

The changes in this PR are just refactoring. They make `Chainstate` objects self-contained, so for example, it is possible to determine what blocks to connect to a chainstate without q
...

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1622550401)
Cool! Would you mind sharing the current disk usage here https://github.com/setavenger/blindbit-oracle/issues/15
it's been a while since I've done a full mainnet sync.