feat: add blockchain traits module with esplora_async impl#1139
feat: add blockchain traits module with esplora_async impl#1139notmandatory wants to merge 1 commit intobitcoindevkit:masterfrom
Conversation
4ce9011 to
8856611
Compare
|
Concept NACK.
|
| /// Defines the options when syncing spks with an Electrum or Esplora blockchain client. | ||
| #[derive(Debug)] | ||
| pub struct SpkSyncMode { | ||
| /// Sync all spks the wallet has ever derived | ||
| pub all_spks: bool, | ||
| /// Sync only derived spks that have not been used, only applies if `all_spks` is false | ||
| pub unused_spks: bool, | ||
| /// Sync wallet utxos | ||
| pub utxos: bool, | ||
| /// Sync unconfirmed transactions | ||
| pub unconfirmed_tx: bool, | ||
| } | ||
|
|
||
| impl Default for SpkSyncMode { | ||
| fn default() -> Self { | ||
| Self { | ||
| all_spks: false, | ||
| unused_spks: true, | ||
| utxos: true, | ||
| unconfirmed_tx: true, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
From our discussion in FFI bindings meeting: can this be an enum?
|
I want to @LLFourn's points that re-introducing separate async/blocking traits might make it hard to switch between syncing methods during runtime. If these are introduced, I'd prefer if we could make sure all relevant traits are implemented for all syncing methods (i.e., also have async bitcoind/electrum implementations, for instance). |
|
Is the purpose of introducing these traits to make it esier to implement FFI bindings? |
c6573e8 to
78c2e2a
Compare
|
The initial reason I started looking into this, as @evanlinjin mentioned above, is so the mobile bindings and Rust wallet users don't need to rewrite the same sync logic themselves. Instead I thought I could just throw the the scan/sync logic from the
Based on below I can see even if I could do what I'm trying to do without a Mutex it isn't the right approach.
Yes this is a problem and I agree we don't want to get back into all the MSRV and feature problems.
Long term I'd like to see users using CBF too, but most users today need esplora/electrum so I want to make their lives as easy as possible.
I'm going to close this PR, but I like your suggestion for how to add some sort of helper traits in the |
Awesome. I'd say start with something like this: #1153 (comment) which simplifies it down to three lines. Shouldn't need any traits either. That approach is better than what I was hinting at above. |
Description
WIP to make a simpler API for users to do common sync operations.
Notes to the reviewers
Changelog notice
Checklists
All Submissions:
cargo fmtandcargo clippybefore committingNew Features:
Bugfixes: