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

Fix Desync issue #322

Merged
merged 4 commits into from
Mar 20, 2019
Merged

Fix Desync issue #322

merged 4 commits into from
Mar 20, 2019

Conversation

SebastienGllmt
Copy link
Contributor

Premise

  • Yoroi (Browsers) doesn't implement bip44 correctly. Notably, it doesn't scan 20 addresses ahead of the last used address. This causes desyncs when you use Yoroi either on multiple computers with the same wallet or you use the same wallet on Chrome + mobile

Solution

We use the same solution Yoroi-Mobile uses:

  • Keep track of the next 20 "External" and "Internal" addresses (yes, internal too but this is what mobile seems to do) inside the DB
  • Explicitly keep track of how many addresses the user has generated that way we can display only those addresses on the "receive" screen
  • When you need an address, simply use the next one available in the DB (don't generate a new address through Rust)
  • When a tx is received, generate new addresses to maintain the bip44 gap

Migration system

Old wallets have two problems:

  1. They are missing the 20 address gap for bip44 compliance
  2. They transaction history may be incorrect (due to desync issue)

To fix this, I introduced a migration system. This is generic enough to help us perform migrations to new versions to maintain compatibility in the future also

To keep track of migrations, we now save LAST-LAUNCH-VER in localstorage. That way whenever you launch the app, you know how many migration steps need to be applied to become compatible with the latest version.

Since these steps may become long in the future, I exposed all the loading steps through the LoadingStore. This means in the future we can have a more fine-grained loading message (instead of just "loading..." we can say

loading Rust...
Done.
Processing migrations...
Done

Misc

I removed generateSTxs.js as it was used to stress-test the backend while Yoroi was still being developed and it's no longer used (and I didn't want to migrate the code to use the new bip44 setup)

* Due to desync issue caused by the incorrect bip44 implementation
*/
reset();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

await syncAddresses(account, 'External');

await refreshChains();
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

@SebastienGllmt SebastienGllmt merged commit 2703757 into develop Mar 20, 2019
@SebastienGllmt SebastienGllmt deleted the feature/desync branch March 20, 2019 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug (confirmed) Bug with repro steps or logs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants