Persistance for redesigned structures.#943
Closed
evanlinjin wants to merge 15 commits intobitcoindevkit:masterfrom
Closed
Persistance for redesigned structures.#943evanlinjin wants to merge 15 commits intobitcoindevkit:masterfrom
evanlinjin wants to merge 15 commits intobitcoindevkit:masterfrom
Conversation
8117189 to
4e57a9d
Compare
8 tasks
a49123d to
45d376a
Compare
d2ba50e to
d0e6e9c
Compare
20 tasks
bc7beab to
5ebedaf
Compare
Contributor
|
Concept ACK.. The new Persistence structure looks simple and good.. I will try to run some manual testing over the Electrum example., |
3 tasks
Comment on lines
+22
to
+24
| pub type RemoteTracker<K, A, O> = Tracker<K, A, RemoteChain<O>>; | ||
| pub type RemoteTrackerStore<K, A, O> = TrackerStore<K, A, RemoteChain<O>, RemoteChainChangeSet>; | ||
| pub type RemoteTrackerChangeSet<K, A> = ChangeSet<K, A, RemoteChainChangeSet>; |
Contributor
There was a problem hiding this comment.
Do we plan to have an example with RemoteChain? It could be interesting to measure performance with and without caching chaindata. Also to show how to impl ChainOracle on remote block sources directly.
This is similar to `keychain::Persist`, however, we allow for a generic tracker and changeset.
Similar to `KeychainStore`, this is intended to be a `PersistBackend`. The difference is that `Store` is more generic, allowing support for different changeset and tracker types. `Store` does not implement `PersistBackend` directly (it cannot), but can the logic can be easily reused to implement it.
`PersistBackend` is now two traits: * `PersistBackend` has method `write_changes()`. * `LoadablePersistBackend` has method `load_into_tracker()`. This is because `LoadablePersistBackend` requires a second generic `T` (for the tracker) which `Store` cannot implement for without introducing a new tracker trait, or having a concrete type for `T`. By having two traits, `Store` can still implement `PersistBackend` and the `LoadablePersistBackend` can be implemented individually with each concrete tracker type.
This is an `Anchor` implementation that also informs of the exact confirmation height of the transaction.
Essentially a copy of `keychain_tracker_example_cli` but uses the new data structures. This also requires us to create an example `Tracker`.
Instead of splitting `PersistBackend` into two traits, we introduce `Loadable` which is the mimimum functionality for a tracker to be able to be "loaded into" by the `PersistBackend`. `Loadable` also makes it easier to make trakers "composable". Typically, trackers would wrap other tracker implementations internally, and so the associated changeset would also be wrapped. Having a `Loadable` trait would allow the new tracker implementation to also have a corresponding `PersistBackend`.
Introduce field `Tracker::chain` which represents the best-chain history. It can either be `LocalChain` or `RemoteChain<O: ChainOracle>`. `Tracker::chain` implements `Loadable`, thus allowing persistance no matter the implementation. For `RemoteChain`s, we would want to persist the last-seen height so we know which height to starting syncing from for the next scan.
This creates a duplicate module of `electrum` as `electrum::v2`.
This is `keychain_tracker_electrum_example` using the redesigned structures.
It is not needed. Also moved `tracker_example_cli::RemoteChain` into it's own file.
4e5424e to
e4c1661
Compare
evanlinjin
commented
May 1, 2023
| pub anchor_block: BlockId, | ||
|
|
||
| /// The exact confirmation height of the transaction (if any). | ||
| pub confirmation_height: Option<u32>, |
Member
Author
There was a problem hiding this comment.
This shouldn't be an option.
d055f1a to
b6280a9
Compare
The internals are replaced with redesigned structures. It turns out that we need an `Tracker` struct. Fixes: * Fix `FinalExectrumUpdate::into_confirmation_time_update()` * The behaviour of `TxGraph::try_get_chain_position` is also changed so that unconfirmed txs with `last_seen` value of 0 can be part of the chain. Additional changes: * Also introduce `TxGraph::all_anchors` method.
b6280a9 to
7fe84f0
Compare
Member
Author
|
This PR is a mess. I will split this up into multiple PRs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is part of #895, extends the work done in #926, and replaces the work done in #937.
Description
Within the
bdk_chainmodule, the PR introduces a more genericPersiststructure andPersistBackendtrait. These allow for any tracker and changeset.Introduce
Storestructure for thebdk_chain_storemodule. Similar toKeychainStore, this is intended to be aPersistBackend. The difference is thatStoreis more generic, allowing support for different changeset and tracker types.Notes to the reviewers
I think the
Loadabletrait that I introduced is a bad idea. Bear with me while I figure out how to remove it!Changelog notice
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features: