💬 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)?

💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1669052113)
it seems to me that `next` remains unchanged after clear:
```C++
auto next{node->second.Next()};
node->second.ClearFlags();
assert(next == node->second.Next());
node = next;
```
so if that's intentional, we can simplify to:
```C++
node->second.ClearFlags();
node = node->second.Next();
```

💬 mzumsande commented on issue "RPC `getblock` resulted in 500 and ReadBlockFromDisk: OpenBlockFile failed for FlatFilePos(nFile=-1, nPos=0)":
(https://github.com/bitcoin/bitcoin/issues/20978#issuecomment-2214874858)
If the block is not available anymore, we already throw a specific error: "Block not available (pruned data)": [rpc](https://github.com/bitcoin/bitcoin/blob/1f9d30744d32d24ad3128721cf5bd65a3f1543e8/src/rpc/blockchain.cpp#L586-L588) / [rest](https://github.com/bitcoin/bitcoin/blob/1f9d30744d32d24ad3128721cf5bd65a3f1543e8/src/rest.cpp#L310-L312)

So I think it would only be consistent to add a check for `BLOCK_HAVE_DATA`, not call `ReadRawBlockFromDisk` and return a similar error (e.g. "Block n
...

💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214881383)
@paplorinc With only a single link, how would you delete arbitrary map entries? Barring large hacks like putting the data in the next entry rather than the entry which the key points to, I don't see how you could delete entries in less than $\mathcal{O}(n)$ time.

💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#discussion_r1669095380)
this was unintentional. Disallowed it and added coverage.

💬 instagibbs commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2214900126)
@theStack took test coverage suggestion, and yes "fees" is intentional since it will be in any net's address

@luke-jr I think @petertodd has stated what I would say. It's a pattern people can already do, and it's trivial and cheap to clean up.

💬 luke-jr commented on pull request "policy: Add PayToAnchor(P2A), `OP_TRUE <0x4e73>` as a standard output script for spending":
(https://github.com/bitcoin/bitcoin/pull/30352#issuecomment-2214922582)
>This proposal is merely a way of doing something that is already done - signed anchor outputs - in a more efficient way with less overall bytes consumed.

And at a reduced cost. Unless this proposal increases vsize to make it identical?

> The creating and spending of keyless anchor outputs is completely fixed, with no ability to encode any data. It has nothing to do with "spam".

What is the 0x4e73 for?

These outputs bloat the UTXO set, without sufficient value to incentivise cleanup
...