Bitcoin Core Github
44 subscribers
120K links
Download Telegram
📝 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
...
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214968788)
> With only a single link, how would you delete arbitrary map entries?

My naive thinking was that maybe we don't actually need random-element deletion. Based on how `ClearFlags` and `erase` are currently called from loops that already have the previous node, we could often call `ClearFlags(*prev)`.
The destructor is trickier. Maybe we can handle this in the `CCoinsViewCache` destructor instead, batch-destructing entries there, which would involve a single extra $\mathcal{O}(n)$ traversal.
I
...
💬 petertodd 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-2214969086)


On July 8, 2024 8:42:50 PM GMT+02:00, Luke Dashjr ***@***.***> wrote:
>>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 cost reduction is a natural one, in line with the byte reduction.

>> The creating and spending of keyless anchor outputs is completely fixed, with no ability to encode any data. It
...
💬 sipa commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214976515)
@paplorinc When UTXOs are spent while FRESH, they get deleted from the cache; this is quite possibly the most significant performance optimization Bitcoin Core has during IBD, as it avoids the need for those UTXOs to ever end up on disk. It's done based on `COutPoint` lookup, not during iteration.
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2214993679)
Makes sense, thanks for explaining @sipa!