Persist state and sync between tabs

[lukaw3d]

Part of https://github.com/oasisprotocol/oasis-wallet-web/issues/792

Temporarily deployed to https://lukaw3d.github.io/oasis-wallet-web/ce527a5/#/

Old preview of 22369e0

Preview of ce527a5

TODO

• [x] don't store fatal errors
• [x] syncing multiple tabs and cleanup, e.g. clearAccount open account in two tabs http://localhost:3000/account/oasis1qzwl8nn8lt8cv4gxh85skv9rw3h0raudws66q4y5 navigate to home on one tab (dispatches clearAccount) other tab loses data and shows loading
• [x] skip login:
  • [x] refactor to make networkSaga maintainable: always show unlock modal and if profile: on unlock emit setUnlockedRootState if profile: on skip emit event like setUnlockedRootState to use in networkSaga if no profile: auto click skip login emit event like setUnlockedRootState to use in networkSaga
• [x] walletId global var
• [x] close wallet = lock
• [x] change password
• [x] choose password and create profile when importing
• [ ] test ui
• [ ] test persistence

Deferred and moved to issue:

• auto lock timeout
• multiple profiles
• migrate old storage @metamask/browser-passworder
• test syncing tabs

[github-actions[bot]]

🦙 MegaLinter status: ✅ SUCCESS

Descriptor	Linter	Files	Fixed	Errors	Elapsed time
✅ EDITORCONFIG	editorconfig-checker	52		0	0.09s
✅ HTML	htmlhint	1		0	0.26s
✅ JSON	eslint-plugin-jsonc	1	0	0	0.93s
✅ JSON	jsonlint	1		0	0.23s
✅ JSON	prettier	1	0	0	0.8s
✅ JSON	v8r	1		0	1.47s
✅ REPOSITORY	checkov	yes		no	14.77s
✅ REPOSITORY	git_diff	yes		no	0.0s
✅ TSX	eslint	11	0	0	6.7s
✅ TYPESCRIPT	eslint	34	0	0	7.75s

See errors details in artifact MegaLinter reports on CI Job page Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by

[lukaw3d]

I temporarily deployed it to https://lukaw3d.github.io/oasis-wallet-web/ce527a5/#/

[nikolaglumac]

Great work @lukaw3d 🎉 I will do a proper review later today or tomorrow - one thing that I want to mention now (and perhaps that is something we can address in a separate PR) is that we should think about password rules. Having users set a single-character password is definitely not what we should allow.

[nikolaglumac]

Hi @lukaw3d - the flow works pretty well! I think it is a good first version - we can later iterate on it and improve it over time (e.g. by adding "remove-account" options etc).

There is this minor thing I found:

It should be an easy fix.

Thanks!

[lukaw3d]

Minimal edge-cases to write tests for:

• [x] lock while viewing an account causes error with clearTransaction (fixed in https://github.com/oasisprotocol/oasis-wallet-web/pull/975/commits/966b15cfbd6da72192098c21b8cb132d5abd34cd)
• [x] quickly unlock + lock: caused error with network fetching action (fixed in https://github.com/oasisprotocol/oasis-wallet-web/pull/975/commits/966b15cfbd6da72192098c21b8cb132d5abd34cd)
• [x] create wallet, enable "Store private keys", type password, disable "Store private keys", next. Expect not to be stored.
• [x] create profile, erase profile, open a new account, causes an error (fixed in https://github.com/oasisprotocol/oasis-wallet-web/pull/975/commits/787037b34b146485eb2760971c8b23c74d2bf79d)

Write test harness for interactions in multiple tabs (probably in playwright?) (defer to separate PR)

[lukaw3d]

@nikolaglumac There is this minor thing I found: Once you create a profile, regardless of if you opened it later or not, "Close wallet" button is always visible

That was intentional so users can get back to login screen after clicking "Continue without the profile" (without it: user would need to create a wallet to get that same button)

[buberdds]

1. Loading state issue

• open an account
• open a new tab with the same url
• close wallet
• in the previous tab loading state "hangs"

2. Cannot open any "foreign" account anymore when one account is open

• I have a tab with oasis1foo url
• In another tab I open oasis1bar account
• the first tab is not longer showing oasis1foo even if I type url there's a redirect to oasis1bar

3. Extension layout is broken when I click "Store private keys locally, protected by a password" due to hardcoded password input width.

4. Extension "Locked" UI looks weird

[lukaw3d]

@buberdds Thanks for finding those!

• [x] 1. oh no :O (to reproduce: don't have a persisted profile. There's no bug for unlocked and skipped)
• [x] 2. is probably acceptable for foreign accounts since we removed searching. But there are similar bugs I'm deferring to https://github.com/oasisprotocol/oasis-wallet-web/issues/823:
  • import an account in tab1
  • middle click on staking to open in tab2: opens account view
  • type /create-wallet or open from bookmark: opens account view
• [x] 3. oof I'll make it max-width and revert https://github.com/oasisprotocol/oasis-wallet-web/commit/05f138126c3dc05058fdca6da86e09ad6dc8b1db
• [x] 4. :+1: Fixed the tooltip transparency part in https://github.com/oasisprotocol/oasis-wallet-web/pull/1098

[lukaw3d]

Fixing 1. is annoying

• I could refactor resetRootState to keep some slices, instead of resetting everything but: it doesn't feel correct to keep state after user clicks Close Wallet; and it probably introduces state combinations that previously weren't reachable and handled.
• Another option: I could redirect home on resetRootState, but only in webapp takeLatest(resetRootState, () => { if (runtimeIs === 'webapp' && selectUnlockedStatus === 'emptyUnpersisted') location.href = '/' }). Luckily extension with saga in background page won't support multiple tabs / popups yet. And that still needs navigate('/'); dispatch(persistActions.lockAsync()) on click.

[csillag]

I see that it's possible to use two different accounts in two different tabs, but I can't use Mainnet on one tab and Testnet on the other. Is that an intentional choice?

[lukaw3d]

Rebased and squashed commits. Before 3d615aa, after e287d32

[codecov[bot]]

Codecov Report

Merging #975 (2d4e1eb) into master (32fd270) will increase coverage by 0.94%. The diff coverage is 90.94%.

@@            Coverage Diff             @@
##           master     #975      +/-   ##
==========================================
+ Coverage   84.54%   85.48%   +0.94%     
==========================================
  Files         129      140      +11     
  Lines        2387     2638     +251     
  Branches      576      648      +72     
==========================================
+ Hits         2018     2255     +237     
- Misses        369      383      +14     

Flag	Coverage Δ
cypress	56.48% <88.15%> (+4.32%)	:arrow_up:
jest	74.06% <42.50%> (-3.74%)	:arrow_down:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/app/pages/HomePage/index.tsx	100.00% <ø> (ø)
src/store/sagas.ts	100.00% <ø> (ø)
src/app/state/persist/syncTabs.ts	73.68% <73.68%> (ø)
src/app/state/wallet/index.ts	93.75% <75.00%> (+14.80%)	:arrow_up:
src/app/components/Persist/UnlockForm.tsx	76.47% <76.47%> (ø)
src/app/components/Sidebar/index.tsx	90.00% <81.81%> (+0.97%)	:arrow_up:
src/app/state/selectIsLockableOrCloseable.ts	85.71% <85.71%> (ø)
src/app/state/persist/saga.ts	87.83% <87.83%> (ø)
src/app/state/network/saga.ts	96.29% <91.66%> (-3.71%)	:arrow_down:
src/app/state/persist/index.ts	92.85% <92.85%> (ø)
... and 23 more

[lukaw3d]

Fixed everything, maybe. Deployed to https://lukaw3d.github.io/oasis-wallet-web/32fbe2b/#/

[buberdds]

Routing bug still occurs and it's not related to foreign accounts. Repro steps: import two accounts using mnemonic, try to open the second opened account in the second tab / fail. However, I can open the second account using Account Switcher in the second tab, but selectedWallet value will be now incorrect in the first tab, right? If this is something we care about we can track it in Clickup or GH issue.

[lukaw3d]

@buberdds I'm not able to reproduce this Can you make more specific steps (when to open another tab, what URL, choosing persistence?)

I tried:

• tab1: http://localhost:3000/create-wallet, open acc1 + acc2 without persistence
• tab2: http://localhost:3000/, switch to acc2 without persistence, send transaction from acc2
• tab1: send transaction from acc1 .
• tab1: http://localhost:3000/create-wallet, open acc1 + acc2 with persistence
• tab2: http://localhost:3000/, switch to acc2, send transaction from acc2
• tab1: send transaction from acc1 .
• tab1: http://localhost:3000/create-wallet, open acc1 without persistence
• tab2: http://localhost:3000/create-wallet, open acc2 without persistence, send transaction from acc2
• tab1: send transaction from acc1 .
• tab1: http://localhost:3000/create-wallet
• tab2: http://localhost:3000/
• tab1: open acc1 without persistence
• tab2: home, create wallet, open acc2 without persistence, send transaction from acc2
• tab1: send transaction from acc1

[buberdds]

• tab1 open acc1 + acc2 via mnemonic without persistence - acc1 is open
• open tab2 - enter acc 2 url - FAIL As a workaround in tab2 I can open acc2 using Account Switcher, but if wallet slice is synced is value of selectedWallet in tab1 correct?

[lukaw3d]

Ah yeah, that is currently treated as a foreign account :/

Workaround should work: wallet slice (including selectedWallet) is synced from tab1 to tab2 when it opens. But after initializing, selectWallet action isn't synced: https://github.com/oasisprotocol/oasis-wallet-web/blob/32fbe2b08ffa05a8893e03fd05f2527ba7967c03/src/app/state/persist/syncTabs.ts#L41-L49

So tab1 will continue to have selectedWallet=acc1 after tab2 switched accounts

[pro-wh]

oh uh

[iSQL]

Hello,

Is there any info when will this be published?

[lukaw3d]

@iSQL I'm not sure. We'll want to deploy it after we move wallet to a new domain (so there's no confusion about saved data on current domain)

and perhaps finish more of https://github.com/oasisprotocol/oasis-wallet-web/issues/792#issuecomment-1269273318

[lukaw3d]

@iSQL it is finally published