💬 l0rinc commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1764507421)
Done, thanks!

👋 l0rinc's pull request is ready for review: "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests"
(https://github.com/bitcoin/bitcoin/pull/30906)

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764514580)
This should use the preset `dev-mode`. Otherwise, deps may be missed.

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764520558)
```suggestion
// for posix_fallocate, in cmake/introspection.cmake we check if it is present after this
```

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764514975)
```suggestion
This can be checked by building the `translate` target with `cmake` ([instructions](translation_process.md)), then verifying that `bitcoin_en.ts` remains unchanged.
```

💬 maflcko commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#issuecomment-2357683381)
review ACK bc4a929cd716cd2b412c70754749d4fda0ca2a10

✅ maflcko closed an issue: "CI timeouts"
(https://github.com/bitcoin/bitcoin/issues/30851)

💬 maflcko commented on issue "CI timeouts":
(https://github.com/bitcoin/bitcoin/issues/30851#issuecomment-2357742653)
Closing for now. Possibly commit a95e742b69207d7541afc6903b3fd72131f4d6de (x86_64-apple-darwin22.6.0 -> arm64-apple-darwin23.6.0) may have switched to using more recent hardware, which may be less likely to be corrupt.

In any case, if someone is still seeing a CI timeout (that is a CI failure caused by a 2 hour timeout), or a CI task that takes longer than half of the timeout (1h), it would be good to mention it either in this issue, or in a new issue.

The 2 hour CI timeout is picked, so t
...

🤔 hodlinator reviewed a pull request: "refactor: migrate `bool GetCoin` to return `optional<Coin>`"
(https://github.com/bitcoin/bitcoin/pull/30849#pullrequestreview-2311839788)
Drive-by minimum-context, admittedly low-PoW review, prompted by author.

Dislike adding of new TODO's but if full-context reviewers accept, I guess it's okay.

💬 hodlinator commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1764547631)
++braces, or remove the `else`.

💬 hodlinator commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1764551012)
\*taps sign\*

💬 hodlinator commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1764529470)
developer-notes.md:
> If an `if` only has a single-statement `then`-clause, it can appear
on the same line as the `if`, without braces. In every other case,
braces are required, and the `then` and `else` clauses must appear
correctly indented on a new line.

💬 hodlinator commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1764556394)
\*taps sign\*

💬 hodlinator commented on pull request "refactor: migrate `bool GetCoin` to return `optional<Coin>`":
(https://github.com/bitcoin/bitcoin/pull/30849#discussion_r1764538629)
Avoid inflating the line count?
```suggestion
std::optional<Coin> coin;
if (!mempool || !mempool->isSpent(vOutPoint)) coin = view.GetCoin(vOutPoint);
hits.push_back(coin.has_value());
if (coin) outs.emplace_back(std::move(*coin));
```

💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1764589040)
Fixed.

💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1764589112)
You're right. I dropped this line and checked again that anytime `fDisconnect` is set to `true` emits a log message.

💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2357781244)
Rebased and dropped duplicate message in `CConnman::DisconnectNodes`.

💬 Sjors commented on pull request "p2p: Increase tx relay rate":
(https://github.com/bitcoin/bitcoin/pull/28592#issuecomment-2357789198)
I've been running this patch for almost a year now. Didn't do very extensive monitoring, but at least it didn't crash the node or noticeably upset the machine.

💬 Sjors commented on pull request "Remove Taproot activation height":
(https://github.com/bitcoin/bitcoin/pull/26201#issuecomment-2357794441)
Removed ~ from the description (wasn't relevant anymore anyway), since @jonatack pointed out in #28358 that these don't get rendered nicely in merges.

💬 Sjors commented on pull request "validation: log which peer sent us a header":
(https://github.com/bitcoin/bitcoin/pull/27826#issuecomment-2357797639)
Post cmake rebase, just in case.