Change Details

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. 1. 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. 2. 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.

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. 1. 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. 2. 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

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. 1. 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. 2. 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