[Cashtab] Cashtab 2.0.0 - Migrate to new wallet management API
Summary:
Cashtab implemented multi-wallet support years ago and without automated integration testing. The current architecture is bad. So bad that I have found it almost impossible to change or test without completely rebuilding it.
So, here, it is completely rebuilt (and tested).
Before
- Active wallet stored at localforage key wallet and state var wallet
- savedWallets stored at localforage key savedWallets and not in state, so every time config changes it, it is getting and setting localforage directly
- When we activate a wallet...we are moving that wallet from savedWallets and into the wallet key, in state and localforage. Also we check if it needs to migrate only on activation (so savedWallets may have n invalid wallets at different stages of migration).
After this diff
- All wallets are stored at wallets key of localforage and cashtabState.wallets in state
- We migrate all invalid wallets on Cashtab startup
- wallets[0] is the active wallet
- We add new wallets to the end of wallets on creation, so they are easy to find. wallets is sorted alphabetically on Cashtab bootup (except active wallet which is at wallets[0]).
- Integration tests for all expected wallet manipulation from Configure.js (feature parity from the old way, but now tested).
- Migration support from legacy wallet mgmt to new wallets method
This will make it possible to improve the structure of the cashtab wallet object, enabling things like HD wallets and better token mgmt in in-node chronik.
Attempting to fix the technical debt related to how savedWallet and wallet are managed proved impractical without addressing and resolving another issue in Cashtab.
- We were running the update method in useWallet.js at a regular interval. This was causing re-entrancy issues. Now that we have reliable websocket connections, there is no need for this type of interval. Instead, we only call update
- when an active wallet is loaded
- when a wallet is changed
- when a tx to the active wallet is sent or received
This greatly simplified integration testing and dramatically improves the speed of switching wallets and loadin the app overall.
- We were experiencing some state / localforage leakage in between integration tests due to cashtabState being a variable-defined object. Convert it (and its dependent objects) to a Class, so it can be initialized as new.
While it is probably possible to separate these changes into smaller diffs, I do not think this offers better impact. I would need to design and implement shims to preserve the existing technical debt. Then remove them like bad scaffolding.
Instead, I think it is best to do this all in one major version bump change. Tests are in place for migration, and the diff does not overwrite legacy storage if we need to revert for some reason.
The key changes implemented in this diff are in Configure.js and useWallet.js
Test Plan:
npm test
The prod version of this diff is available at https://cashtab-local-dev.netlify.app/
While the integration tests cover the changes introduced by this diff, it is worth spending some time on the deployed version trying to break things. Add wallets. Delete wallets. Rename wallets to existing names. Switch wallets rapidly, a lot. Confirm you are still subscribed to the right scripts in the websocket.
Reviewers: #bitcoin_abc, emack
Reviewed By: #bitcoin_abc, emack
Subscribers: emack
Differential Revision: https://reviews.bitcoinabc.org/D15612