Skip to content

Simplify chain update#9

Open
LLFourn wants to merge 8 commits into
evanlinjin:clean_esplorafrom
LLFourn:clean_esplora
Open

Simplify chain update#9
LLFourn wants to merge 8 commits into
evanlinjin:clean_esplorafrom
LLFourn:clean_esplora

Conversation

@LLFourn

@LLFourn LLFourn commented Apr 2, 2024

Copy link
Copy Markdown

No description provided.

These methods allow us to query for checkpoints contained within the
linked list by height. This is useful to determine checkpoints to fetch
for chain sources without having to refer back to the `LocalChain`.

Currently this is not implemented efficiently, but in the future, we
will change `CheckPoint` to use a skip list structure.
We impl `PartialEq` on `CheckPoint` instead of directly on `LocalChain`.
We also made the implementation more efficient.
This creates a checkpoint linked list which contains all blocks.
Previously, we would update the `TxGraph` and `KeychainTxOutIndex`
first, then create a second update for `LocalChain`. This required
locking the receiving structures 3 times (instead of twice, which
is optimal).

This PR eliminates this requirement by making use of the new `query`
method of `CheckPoint`.

Examples are also updated to use the new API.
These methods are no longer needed as we can determine missing heights
directly from the `CheckPoint` tip.
This gets the genesis hash of the env blockchain.
We ensure that calling `finalize_chain_update` does not result in a
chain which removed previous heights and all anchor heights are
included.
I felt things didn't have to be so complicated. The chain could easily
be updated in one function rather than spread across two. The logic
needs at least one less loop.
@evanlinjin evanlinjin force-pushed the clean_esplora branch 4 times, most recently from 91e8c91 to bf74f18 Compare April 2, 2024 13:45

@evanlinjin evanlinjin left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good. Thank you for showing me a much cleaner approach. My only concern is the following:

https://github.com/bitcoindevkit/bdk/blob/e5c81a231b15118f41a1fcb97c527008efc4c5a9/crates/esplora/src/blocking_ext.rs#L146-L150

The reason why I split chain_update_... into two methods is we want to fetch latest blocks before getting transaction update. We are using the local tip to determine "last-scanned block" and there are users which want to alternate between Esplora and block-by-block sources.

I will try update this to fetch latest blocks before getting transactions.

@LLFourn

LLFourn commented Apr 15, 2024

Copy link
Copy Markdown
Author

This looks really good. Thank you for showing me a much cleaner approach. My only concern is the following:

https://github.com/bitcoindevkit/bdk/blob/e5c81a231b15118f41a1fcb97c527008efc4c5a9/crates/esplora/src/blocking_ext.rs#L146-L150

The reason why I split chain_update_... into two methods is we want to fetch latest blocks before getting transaction update.

I don't think so. We want these two things to be completely decoupled. A chain update is orthogonal to the transaction update except that you want to try and grab heights that transactions seem to exist at. I didn't understand the motivation you gave. Why can't you switch between block-by-block and esplora here?

@evanlinjin evanlinjin force-pushed the clean_esplora branch 4 times, most recently from a504a1d to 77d3595 Compare April 17, 2024 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants