💬 achow101 commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2353779137)
ACK bb3b980dfd96d9a89e6b04aef2859ce0555c6807

🚀 achow101 merged a pull request: "refactor: add clang-tidy `modernize-use-starts-ends-with` check"
(https://github.com/bitcoin/bitcoin/pull/30868)

💬 romanz commented on pull request "refactor: add clang-tidy `modernize-use-starts-ends-with` check":
(https://github.com/bitcoin/bitcoin/pull/30868#issuecomment-2353787731)
Many thanks!

💬 furszy commented on pull request "test: Introduce ensure helper":
(https://github.com/bitcoin/bitcoin/pull/30893#discussion_r1761796887)
> > And, in fact, we could implement it using the wait_until function too
>
> Yes, I tried this and decided against it, see the description of the PR.

Ah, missed it. Sorry.

> > I’d suggest simply waiting for the entire duration without intermediate checks.
>
> I have also considered this and decided against it because I don't think it's better. I have also described this in the PR description already. Since the polling doesn't seem to lead to a real performance issue I think it's bet
...

✅ achow101 closed an issue: "Raise maximum -dbcache setting"
(https://github.com/bitcoin/bitcoin/issues/28249)

🚀 achow101 merged a pull request: "Drop -dbcache limit"
(https://github.com/bitcoin/bitcoin/pull/28358)

🤔 jonatack reviewed a pull request: "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment"
(https://github.com/bitcoin/bitcoin/pull/30666#pullrequestreview-2307594542)
Approach ACK 321097a685b67f82c32f53652227b7177fe97a95 on first look

💬 jonatack commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761770703)
perhaps more clear
```suggestion
//! best header is no longer valid nor guaranteed to be the most-work
```

💬 jonatack commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761774657)
For references to cs_main, are we still generally preferring to use the scope resolution operator? (you do use it in line 1330)


```suggestion
void SetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
```

💬 jonatack commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#discussion_r1761767261)
"to a all" -> unclear what this means

💬 achow101 commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2353813095)
ACK 6a1aa510e31e8b77793341473aa5afc9d023a6e3

✅ achow101 closed an issue: "RPC `getblock` resulted in 500 and ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0)"
(https://github.com/bitcoin/bitcoin/issues/20978)

🚀 achow101 merged a pull request: "rpc, rest: Improve block rpc error handling, check header before attempting to read block data."
(https://github.com/bitcoin/bitcoin/pull/30410)

💬 davidgumberg commented on pull request "streams: cache file position within AutoFile":
(https://github.com/bitcoin/bitcoin/pull/30884#discussion_r1761938767)
That makes sense, I didn't consider pipes/fifos.

I agree on this approach given that a bad `m_position` value is only an issue when XOR'ing, and your branch has checks for an unknown `m_position` before any XOR operations. And in general, it seems to me, `ftell`'s are followed by file operations that fail if the `ftell` fails for any reason other than the file being a pipe/fifo (`ESPIPE`).

I think ideally `errno` is checked here for values other than `ESPIPE`, but the current approach is
...

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#issuecomment-2353995596)
Rebased after merge of #30286.

💬 jonatack commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761946005)
`s/for a non-debug build by running build with/by building/`

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

(nano-nit, "cmake" is written without the backticks in other documents; could be consistent, also regarding referring to it as "cmake" or "CMake")

💬 achow101 commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#issuecomment-2354004288)
ACK e4e3b44e9cc7227b3ad765397c884999f57bac2e

💬 jonatack commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1761947070)
```suggestion
print("Re-compile with the -DBUILD_DAEMON=ON build option")
```

🚀 achow101 merged a pull request: "net: call `Select` with reachable networks in `ThreadOpenConnections`"
(https://github.com/bitcoin/bitcoin/pull/29436)