Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Syncing/Rescanning super issue #1195

Closed
LLFourn opened this issue Nov 6, 2023 · 4 comments
Closed

Syncing/Rescanning super issue #1195

LLFourn opened this issue Nov 6, 2023 · 4 comments
Assignees
Labels
Milestone

Comments

@LLFourn
Copy link
Contributor

LLFourn commented Nov 6, 2023

There seems to be quite a few independent efforts to define and address problems with the syncing/scanning API. I thought it would be nice to organize them in one place and have a single PR that does all of them.

  1. Rename "scan" to "rescan": Anything that is using stop_gap style scanning must necessarily be assuming that another wallet must have given out addresses prior to this one. The name rescan communicates this better (h/t @danielabrozzoni). Related scan_without_keychain and scan's names should be inverted #1112.
  2. ([wallet] Consider having a higher-level method for syncing #1153) We should have data structures defined in bdk_chain that define bundles of things a Wallet (or any other system) could be interested in. I think these should be:
/// A list of blockchain entities for which we want to receive any related transaction data.
struct SyncRequest {
    /// transactions that spend from or two these script pubkeys
    spks: Vec<ScriptBuf>,
    /// Transactions with these txids 
    txids: Vec<Txid>,
    /// Transactions with these outpoints or spend from these outpoints
    outpoints: Vec<OutPoint>,
    /// The local chain checkpoint. The sync process will return a new chain that extends this one.
    checkpoint: Option<CheckPoint>
}

/// Script pubkeys indexed by their keychain.
struct RescanRequest<K, I> {
    /// Iterators of script pubkeys indexed by the keychain index
    spks_by_keychain: BTreeMap<K,I>,
   /// The local chain checkpoint. The scan process will return a new chain that extends this one.
    checkpoint: Option<CheckPoint>
}
  1. Now the problem with the above is that we want to be able to easy log when a new spk etc is drawn by the blockchain. So really we can't have these fields public -- internally each thing should be Box<dyn Iterator<Item = ...>>. Then when you add a new items to the SyncRequest it should be something like SyncRequest::append_spks(&mut self, spks: impl IntoIterator<Item=ScriptBuf>) which would append the new iterator onto the existing one with chain and replace it.
  2. Then we need APIs in wallet that return these structs (Syncing/Rescanning super issue #1195)
@notmandatory notmandatory added the enhancement New feature or request label Nov 6, 2023
@notmandatory notmandatory added this to the 1.0.0-alpha.3 milestone Nov 6, 2023
@notmandatory
Copy link
Member

We will discuss on this on Tues at the call but your recommendations above all look good to me. I'll start updating #1194 unless anyone has already started working on it or thinks it'd be better to start a new PR.

@vladimirfomene
Copy link
Contributor

Given Evan's PR (#1178 ), I believe the checkpoint fields on both structs don't have to be an option anymore.

@notmandatory notmandatory modified the milestones: 1.0.0-alpha.3, 1.0.0-alpha.4 Nov 13, 2023
@notmandatory notmandatory self-assigned this Nov 15, 2023
@chrisguida
Copy link

Adding this draft PR to the discussion: vladimirfomene#2

@nondiremanuel nondiremanuel modified the milestones: 1.0.0-alpha.4, 1.0.0 Jan 6, 2024
@notmandatory notmandatory added the api A breaking API change label Feb 15, 2024
@notmandatory notmandatory added module-blockchain module-wallet and removed enhancement New feature or request labels Mar 18, 2024
@notmandatory
Copy link
Member

completed with #1403

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

No branches or pull requests

5 participants