💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1669007098)
I'm assuming that it's only used for stronger invariants checks in the fuzz/test harness

💬 fjahr commented on issue "test: Write assumeutxo tests":
(https://github.com/bitcoin/bitcoin/issues/28648#issuecomment-2214756801)
Or rather, the problem is that the issue here is not specific and so none of the issues can close this issue because it's unclear which will be the last one to get merged. I will recommend in the PRs that they mention the issue here as related to so it's at least clear what is being worked on where.

💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669012557)
Good catch! Yes, since the full blocks are never being submitted to any node (just the headers to node 1), nothing failed. Fixed now.

💬 ryanofsky commented on pull request "Fix issues with CI on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2214759087)
@maflcko is your "lgtm ACK" more of a partial ACK, or do you think this is good to merge? I think practically speaking this looks ok to merge, but I wouldn't want to merge it myself if I'm the only full reviewer.

I guess there are also number of possible followups to this PR if it is merged:

- Maybe preventing github actions from running twice on PRs, [#29274 (comment)](https://github.com/bitcoin/bitcoin/pull/29274#issuecomment-2188840483).
- Maybe restoring ability to skip cirrus ARM job
...

💬 fjahr commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669014412)
Could also mention #28648 as related in description because of this.

💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669014970)
Done, in a slightly modified way. The alternative chain is just created in python, node 0 is only used to fetch the previous block it builds on.

💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669015125)
took those

💬 fjahr commented on pull request "Assumeutxo: bugfix on loadtxoutset with a divergent chain + test":
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2214762372)
nit: Could mention #28648 as related in the PR description.

💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#discussion_r1669015915)
Removed both TODOs.

💬 mzumsande commented on pull request "assumeutxo: Don't load a snapshot if it's not in the best header chain":
(https://github.com/bitcoin/bitcoin/pull/30320#issuecomment-2214767684)
[29ab88d ](https://github.com/bitcoin/bitcoin/commit/29ab88dfe1c43af3620480c99cba844aa414023c)to [ab478c5](https://github.com/bitcoin/bitcoin/commit/ab478c5fa16427b496e8a93e4780770d4c270214):
Addressed feedback by @fjahr (thanks!) and rebased.

👋 stickies-v's pull request is ready for review: "C++20 std::views::reverse"
(https://github.com/bitcoin/bitcoin/pull/28687)

💬 stickies-v commented on pull request "C++20 std::views::reverse":
(https://github.com/bitcoin/bitcoin/pull/28687#issuecomment-2214772241)
With https://github.com/bitcoin/bitcoin/pull/30263 merged, this PR is now ready for review 🥳

📝 achow101 converted_to_draft a pull request: "wallet: Remove IsMine from migration code"
(https://github.com/bitcoin/bitcoin/pull/30328)
The legacy wallet `IsMine` code will be removed with the legacy wallet, but is still partially needed for migration. Instead of using `IsMine` directly in migration, equivalent checks are performed by migration.

Builds on #26596

👍 instagibbs approved a pull request: "locks: introduce mutex for tx download, flush rejection filters on UpdatedBlockTip"
(https://github.com/bitcoin/bitcoin/pull/30111#pullrequestreview-2163961806)
reACK 17d41e7a547f16f0dd3802dac73a63772ca000b2

biggest change is changing the new function to `ActiveTipChange` and making ibd a no-op

reviewed via `git range-diff master 0e0c422 17d41e7`

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1669031104)
fixed

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1669032119)
thanks accepted your suggestion with a slight modification.

💬 ismaelsadeeq commented on pull request "Fee Estimation: Ignore all transactions that are CPFP'd":
(https://github.com/bitcoin/bitcoin/pull/30079#discussion_r1669032477)
yes, this is gone now.

💬 theStack commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#issuecomment-2214797055)
Thanks for the reviews! I force-pushed following [@dergoegge's suggestion](https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1668290789) of doing the version message push in `SendMessages` instead, with the advantage of not needing to change the `NetEventsInterface` (sorry for invalidating the ACK @maflcko).
The previous version is still available on the following branch: https://github.com/theStack/bitcoin/tree/202407-p2p-fix_selfdetection_racecond_oldversion

💬 theStack commented on pull request "net: fix race condition in self-connect detection":
(https://github.com/bitcoin/bitcoin/pull/30394#discussion_r1669035582)
Done as suggested.

💬 mzumsande commented on pull request "fuzz: improve utxo_snapshot target":
(https://github.com/bitcoin/bitcoin/pull/30329#issuecomment-2214820310)
> maybe someone can confirm that everything is deterministic and the snapshot can be loaded now (e.g. by adding an assert and setting `base_blockheight` and `m_coins_count` to 200 it should crash almost immediately).

FYI, confirmed for myself on another computer that this was just an error, and the snapshot can be loaded by the fuzzer.