🤔 ismaelsadeeq reviewed a pull request: "Cluster linearization: separate tests from tests-of-tests"
(https://github.com/bitcoin/bitcoin/pull/30605#pullrequestreview-2328851273)
Code review bf1b1b09ca4dd18b7848fb1807df8533cf930df7

This is a nice refactor, really improve the cluster linearize test a lot.

💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775578705)
I believe this new behavior is correct, as it reduces `iteration_left` when work is being done. This aligns with the behavior in `SearchCandidateFinder`.

On a side note, I think "iterations" might be a misnomer here. A more accurate term could be "maximum optimization" or "optimizations left".

💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775638585)
This can be in the same loop above?

Why not just compare with the result of ancestor finder?

💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775677456)
Should we also assert that the transactions in the set are the same?

💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775651129)
We could wrap this in a helper to be dry, as same code is used above.

💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775682420)
Maybe indicate that we only compare `SimpleCandidateFinder` with `ExhaustiveCandidateFinder` when we optimally found a subset and the cluster size is small.

🤔 ismaelsadeeq reviewed a pull request: "init: Remove retry for loop"
(https://github.com/bitcoin/bitcoin/pull/30968#pullrequestreview-2329121486)
Nice Concept ACK

💬 ismaelsadeeq commented on pull request "Cluster linearization: separate tests from tests-of-tests":
(https://github.com/bitcoin/bitcoin/pull/30605#discussion_r1775732905)
could be in it's own commit?

💬 achow101 commented on pull request "validation: Disable CheckForkWarningConditions for background chainstate":
(https://github.com/bitcoin/bitcoin/pull/30962#issuecomment-2374863380)
ACK c0a0c72b4d68a4f0c53c2c4b95f4d6e399f8e4ee

✅ achow101 closed an issue: "`invalidateblock` during background validation"
(https://github.com/bitcoin/bitcoin/issues/30958)

🚀 achow101 merged a pull request: "validation: Disable CheckForkWarningConditions for background chainstate"
(https://github.com/bitcoin/bitcoin/pull/30962)

💬 Oly6 commented on something "":
(https://github.com/bitcoin/bitcoin/commit/39219fe145e5e6e6f079b591e3f4b5fea8e71804#commitcomment-147202742)
good

💬 achow101 commented on pull request "[28.x] backports and finalize (or rc3)":
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2374879207)
Added #30962

I think it's trivial enough to not require rc3 as well.

💬 achow101 commented on pull request "validation: Disable CheckForkWarningConditions for background chainstate":
(https://github.com/bitcoin/bitcoin/pull/30962#issuecomment-2374879640)
Backport in #30959

💬 fjahr commented on issue "Embedded ASMap data Tracking Issue":
(https://github.com/bitcoin/bitcoin/issues/28794#issuecomment-2374881703)
> Is there no way to query AS Maps directly from the BGP tables?

Who's BGP table? Can you be a bit more specific what you are thinking of in terms of architecture and trust assumptions?

🤔 ryanofsky reviewed a pull request: "refactor: Implement missing error checking for ArgsManager flags"
(https://github.com/bitcoin/bitcoin/pull/16545#pullrequestreview-2329185957)
Updated 87b6d4cb5a5c04f6c8542c633c3bfa5f76901d43 -> 5a945600451037693a032e6df94f99a666dd581f ([`pr/argcheck.41`](https://github.com/ryanofsky/bitcoin/commits/pr/argcheck.41) -> [`pr/argcheck.42`](https://github.com/ryanofsky/bitcoin/commits/pr/argcheck.42), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/argcheck.41..pr/argcheck.42)) with suggested formatting & grammar tweaks.

re: https://github.com/bitcoin/bitcoin/pull/16545#pullrequestreview-2325050760

Thanks for the review. To
...

💬 ryanofsky commented on pull request "refactor: Implement missing error checking for ArgsManager flags":
(https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1775793996)
re: https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1773221823

> nit: you're using both `can not` and `cannot` in the errors

Seems like cannot is better so switched to that, thanks!

💬 ryanofsky commented on pull request "refactor: Implement missing error checking for ArgsManager flags":
(https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1775792589)
re: https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1773293766

Happy to change, but I'm not really sure what to do here. I always just look at plaintext comments, not generated documentation, and I do think having breaks between list items makes the plaintext comment more readable since most of the items are paragraphs. I could get rid of the break before the first item while keeping the breaks before the other items, though, if that works and would be preferred.

💬 ryanofsky commented on pull request "refactor: Implement missing error checking for ArgsManager flags":
(https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1775766244)
re: https://github.com/bitcoin/bitcoin/pull/16545#discussion_r1773291395

Thanks, applied suggestion

👍 ryanofsky approved a pull request: "init: Remove retry for loop"
(https://github.com/bitcoin/bitcoin/pull/30968#pullrequestreview-2329376198)
Code review ACK e9d60af9889c12b4d427adefa53fd234e417f8f6. Nice change to make AppInitMain shorter and more understandable.