Skip to content

feat(backup & sync): use entropySourceId to sync accounts for Multi-SRP #5753

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mirceanis
Copy link
Contributor

@mirceanis mirceanis commented May 2, 2025

Explanation

This is a first pass at syncing account data for each SRP.
This commit adds an entropySourceId param to all methods that might use it.

Following #5618 we have extra options available on InternalAccount objects; entropySource and derivationPath.
We can use these to properly segregate account data for multi-SRP.

At the moment there are some caveats:

  • entropySourceId for accounts is NOT available until the wallet is unlocked at least once.
    • This is true for secondary SRPs too!!!! - it's a blocker, since defaulting to the primary SRP for account renames will rename the WRONG ACCOUNT!!!
    • To fix this we need to make sure the clients call accountsController.updateAccounts(); after onboarding and after importing an SRP
    • We also need to make sure the accountAdded event grabs the proper metadata from somewhere. I think we can populate the entropySourceId for it but I'm not sure about the derivationPath because these events are computed on diffs, not on the full list of accounts where we would know the index. This is best done in another PR, though.
  • big sync is not segregated yet
    • I added a (currently unused) listEntropySources method, not knowing where else to grab the actual list
    • we also need to create the mapping Record<entropySourceId, internalAccounts> after calling AccountsController.listInternalAccounts() to be able to work with the segregated data
    • it would take me a few more hours to adjust the big sync too
  • rename events work if we remove the primary SRP filter ("don't explode filters")
    • "work" means if I rename an account for a secondary SRP it gets reflected on another instance where that account is on a primary SRP
    • BUT only after an unlock; see first comment
  • adding is broken because the event does not provide the keyring metadata
  • LATER: it might be easier to replace the spray of entropySourceId params in the user-storage sdk/controller with multiple this.config.auth instances, or something along those lines.
    • the SRP JWT bearer authenticator can have multiple instances; each instance could use a class member entropySourceId instead of needing an update to all method params
    • the Auth controller is a singleton, this means its methods will need to accept entropySourceId parameters
    • not sure yet if these can be reconciled; perhaps the controller can maintain multiple instances of the SDK authenticators
    • will solve these in later passes

References

Changelog

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed, highlighting breaking changes as necessary
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

…art 1

This is a first pass at syncing account data for each SRP.
This commit adds an entropySourceId param to all methods that might use it.
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.

1 participant