👍 alfonsoromanz approved a pull request: "AssumeUTXO: Don't Assume m_chain_tx_count in GuessVerificationProgress"
(https://github.com/bitcoin/bitcoin/pull/30909#pullrequestreview-2307534418)
tACK 9be080abfa87543652c415af4e239a6ba4d2b2e1

The test passes locally, and I see that all the CI checks have passed, so it looks good to me.

It took me some time today to get familiar with the CMake-based build system, but I managed to get it working.

<img width="982" alt="Screenshot 2024-09-16 at 16 05 09" src="https://github.com/user-attachments/assets/bbe337ef-f789-49f0-a414-2661a17dbfb7">


Thanks!

💬 jonatack commented on pull request "AssumeUTXO: Don't Assume m_chain_tx_count in GuessVerificationProgress":
(https://github.com/bitcoin/bitcoin/pull/30909#discussion_r1761740170)
Question, is logging this enough, e.g. in the various RPCs (and CLI -getinfo) that return a "verificationprogress" field, are there any where it would be a good idea to describe the what/why of a returned 0.0 value.

💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#issuecomment-2353722966)
> I have opened a separate PR #30909 with my suggested fix and your test commit cherry-picked on top of it @alfonsoromanz .

Thanks a lot @fjahr !

Should I modify this PR now to be on top of yours, including only the second test? Or would it be better to wait until your PR gets merged and then modify mine?

💬 achow101 commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2353732588)
ACK a93c171faa7b4426823466e972c8f24260918bbf

💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#issuecomment-2353739606)
@achow101 see #30510

🚀 achow101 merged a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440)

💬 achow101 commented on pull request "refactor: add clang-tidy `modernize-use-starts-ends-with` check":
(https://github.com/bitcoin/bitcoin/pull/30868#issuecomment-2353758047)
ACK fc7b507e9a595a7bf91f4e0f42b4d842af8b93f3

💬 theuni commented on pull request "build: speed up by flattening the dependency graph":
(https://github.com/bitcoin/bitcoin/pull/30911#issuecomment-2353778200)
Looks like this is hitting a CMake bug: https://gitlab.kitware.com/cmake/cmake/-/issues/24058

Converting to draft while I investigate.

📝 theuni converted_to_draft a pull request: "build: speed up by flattening the dependency graph"
(https://github.com/bitcoin/bitcoin/pull/30911)
These changes speed up my build (default config/options/targets) by roughly 10%. I suspect the difference may be more significant in other build configs.

Before:
> $ time cmake --build build -j24
> real 3m26.932s

After:
> $ time cmake --build build -j24
> real 3m7.556s

Generally they allow for jobservers (either `make -jX` or `ninja`) to be better utilized. This can be verified using `top` while building and looking at the number of compiles running at any given time before/after th
...

💬 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