💬 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.

🚀 ryanofsky merged a pull request: "psbt: Check non witness utxo outpoint early"
(https://github.com/bitcoin/bitcoin/pull/29855)

🤔 paplorinc reviewed a pull request: "Don't empty dbcache on prune flushes: >30% faster IBD"
(https://github.com/bitcoin/bitcoin/pull/28280#pullrequestreview-2164003242)
nice, I like this version a lot more.

I haven't investigated it yet, but now that we have a circular list, does the double linking still offer a measurable advantage or could we do the same with a single link and maybe 2 iterations instead (and would it be worth it, if it's possible)?