-
Notifications
You must be signed in to change notification settings - Fork 12
feat: add Kyoto #296
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
feat: add Kyoto #296
Conversation
Hi @reez, I created two new extensions for Please let me know what you think. If needed, I can refactor the protocol into a concrete type. |
} | ||
} | ||
|
||
func loadWalleFromBackup() throws -> Wallet { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling, missing "t"
|
||
needsFullScan = true | ||
func getSyncMode() -> SyncMode? { | ||
try? keyClient.getSyncMode() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whats the reason we save syncmode to keychain? Its probably something obvious I'm missing but I can't think of it at the moment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to store the SyncMode—either Kyoto or Esplora—somewhere so we can restore it the next time the app is opened. I chose to save it in the Keychain to avoid having to manage another way of persisting data in the app. Another option would be to store it in UserDefaults
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah the keychain part is not a problem, but the reason for needing to know the sync mode when the app is opened im still trying to chase down
func stopService() async throws | ||
} | ||
|
||
extension BDKSyncService { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't remember, what was the reason for removing a lot of this logic from BDKService and moving it into a BDKSyncService that helps the Kyoto part?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to manage the Wallet
instance in both Kyoto and Esplora, so I centralized the common logic—such as buildWallet, buildBackupInfo, deleteWallet, etc.—into BDKSyncService, allowing both KyotoService
and EsploraService
to use it without duplicating code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok let me think on that a bit
A lot of good changes, not all strictly necessary for Kyoto feature, I propose we keep this branch, have this branch available on the main repo for folks to use already (since seeing this all work is really cool), and then migrate this PR into
I’m happy to do this since you’ve already done a great job getting this to work! I’ve already started by pushing your branch (with |
Totally agree with you @reez. If that’s okay with you, I can start creating other PRs with those additions (StorageUtils, URL+Extension, etc). |
Sure, you rule! |
Im going to close this PR (we can always re-open if needed) because we've got the branch live for people to use https://github.com/bitcoindevkit/BDKSwiftExampleWallet/tree/experiment/kyoto and we will be spinning this PR out into other PRs |
Description
This PR proposes to demonstrate an implementation of Kyoto while keeping the Esplora server.
[issue] #293
Notes to the reviewers
This implementation proposes adding Kyoto synchronization while keeping the Esplora server, to demonstrate to the user how we can support both synchronization methods.
To encapsulate Kyoto and Esplora, it introduces a protocol called BDKSyncService, which is used by the BDKService client.
Checklists
All Submissions:
.swift-format
fileNew Features:
Bugfixes: