[Wallet] Prompt user to restart app when turning off Forno a second time #1708

annakaz · 2019-11-14T02:14:06Z

Description

Currently, when users start geth twice in one app session (for example, Forno -> geth -> Forno -> geth, or geth -> Forno -> geth), the app fails to switch to geth the second time. Now, the app shows a modal prompting users to restart the app when they are starting geth for the second time in a session. When the app restarts geth works fine.

For some reason starting geth a second time in a single app session leads to a minimum gas price error, because it cannot find the Gold Token address as geth does not think it is deployed. Had done some geth debugging with help from Kevin but no clear solution, so adding this modal as a workaround for now.

Also fixes an issue where switching on Forno leads to the app thinking it's disconnected until the app is restarted. This was not caught before because the disconnect behavior was conflated with long block times.

Tested

On pilot:
Started in Forno, sent a transaction, turned it off, turned it on, turned it off, modal pops up- restarted app and sent transaction in geth mode.

Other changes

Remove some unused icons
Added snapshot tests for GethAwareButton, FeeExchangeIcon

Related issues

Backwards compatibility

Yes

…fault-gradle-properties

gradle.properties

…z/forno-fix

…po into annakaz/forno-modal

jmrossy

Mostly looks good! Just a few small comments

jmrossy · 2019-11-14T16:37:11Z

packages/mobile/src/geth/saga.ts

@@ -43,6 +43,7 @@ export function* waitForGethConnectivity() {

 function* waitForGethInstance() {
  const zeroSyncMode = yield select(zeroSyncSelector)
+  Logger.error(TAG, `zeroSync mode: ${zeroSyncMode}`)


why .error here?

Removed this log 👍

jmrossy · 2019-11-14T16:39:16Z

packages/mobile/src/web3/saga.ts

@@ -372,7 +376,7 @@ export function* addAccountToWeb3Keystore(key: string, currentAccount: string, p
    )
    yield put(setAccountInWeb3Keystore(account))
  } catch (e) {
-    Logger.debug(TAG + '@addAccountToWeb3Keystore', 'Failed to import raw key')
+    Logger.debug(TAG + '@addAccountToWeb3Keystore', `Failed to import raw key: ${e}`)


Should we use .error here which can take e as the 3rd param? It formats the exception messages nicely

jmrossy · 2019-11-14T16:39:31Z

packages/mobile/src/web3/saga.ts

@@ -417,17 +421,29 @@ export function* ensureAccountInWeb3Keystore() {
 }

 export function* switchToGethFromZeroSync() {
+  Logger.debug(TAG + 'Switching to geth from zeroSync..')


jmrossy · 2019-11-14T16:39:54Z

packages/mobile/src/web3/saga.ts

+      // there is an issue where it cannot find deployed contracts.
+      // Restarting the app fixes this issue.
+      yield call(navigate, Screens.WalletHome)
+      Logger.debug(TAG + 'Showed message')


jmrossy · 2019-11-14T16:42:58Z

packages/mobile/src/account/CeloLite.tsx

+  }
+
+  onPressRestartModal = () => {
+    this.props.toggleZeroSyncMode(false)


The naming has me a bit confused here. Any reason we dispatch this action here instead of restarting the app as the name says? I see below that the restart logic is in the saga instead, any reason we don't have it here?

Good point. I updated the name to onPressToggleWithRestartModal.

The restart logic is in the saga because we have to wait for geth to be turned on before restarting the app. Putting the restart logic here would require dispatching an action after the syncMode is done switching, and listening for that to know when to restart.

codecov · 2019-11-14T16:58:15Z

Codecov Report

Merging #1708 into master will decrease coverage by 0.16%.
The diff coverage is 37.93%.

@@            Coverage Diff             @@
##           master    #1708      +/-   ##
==========================================
- Coverage   74.43%   74.26%   -0.17%     
==========================================
  Files         279      277       -2     
  Lines        7589     7617      +28     
  Branches      666      669       +3     
==========================================
+ Hits         5649     5657       +8     
- Misses       1825     1845      +20     
  Partials      115      115

Flag	Coverage Δ
#mobile	`74.26% <37.93%> (-0.17%)`	⬇️

Impacted Files	Coverage Δ
packages/mobile/test/schemas.ts	`100% <ø> (ø)`	⬆️
packages/mobile/src/images/Images.ts	`100% <ø> (ø)`	⬆️
packages/mobile/src/web3/reducer.ts	`47.05% <0%> (-2.95%)`	⬇️
packages/mobile/src/utils/AppRestart.ts	`47.05% <0%> (-2.95%)`	⬇️
packages/mobile/src/app/ErrorScreen.tsx	`92.3% <100%> (ø)`	⬆️
packages/mobile/src/web3/selectors.ts	`83.33% <100%> (+3.33%)`	⬆️
packages/mobile/src/app/AppLoading.tsx	`81.81% <100%> (ø)`	⬆️
packages/mobile/src/geth/saga.ts	`23.25% <16.66%> (-0.56%)`	⬇️
packages/mobile/src/app/saga.ts	`58.33% <25%> (-0.71%)`	⬇️
packages/mobile/src/account/CeloLite.tsx	`79.41% <61.11%> (-20.59%)`	⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 236ce42...afca2ab. Read the comment docs.

…po into annakaz/forno-modal

annakaz · 2019-11-14T17:50:32Z

Added snapshot tests for some unrelated components (GethAwareButton, FeeExchangeIcon) to increase code coverage

* master: (81 commits) Make styling more consistent in validator quick start and add password prompt to account:unlock (#1731) New Version for contractkit (#1727) Remove end-to-end attestations test from circle (#1739) Added helper function to get the list of current validators (#1713) Fix flaky end-to-end transfer, protocol unit tests (#1734) Add AccountClaim to Metadata (#1663) [Wallet] Set default gradle properties (#1629) Update genesis block after adding parent signatures to block header (#1732) Removed end-to-end-geth-integration-sync-test job in workflow (#1730) Change the event we emit when selecting issuers (#1706) [Wallet] Prompt user to restart app when turning off Forno a second time (#1708) Check in spanish verification translations (#1726) [Wallet] Style fixes on iOS for verification and backup flows (#1718) [Wallet] Reset isRedeemingInvite on rehydrate (#1716) Adjust e2e transfer and governance tests to match new fee distribution and eliminate ProposerFraction (#1585) [Wallet] Add more local currencies (#1698) Switch to correct cluster when fauceting (#1687) [Wallet] Use the country of the phone number for determining the default local currency (#1684) [Wallet] Limit QR code scanner to 1 code per second (#1676) Update Dark backgrounds text color (#1677) ... # Conflicts: # packages/web/pages/_app.tsx # packages/web/src/dev/BuildPage.tsx # packages/web/src/dev/Cover.tsx # packages/web/src/dev/StackSection.tsx # yarn.lock

* master: (70 commits) Make styling more consistent in validator quick start and add password prompt to account:unlock (#1731) New Version for contractkit (#1727) Remove end-to-end attestations test from circle (#1739) Added helper function to get the list of current validators (#1713) Fix flaky end-to-end transfer, protocol unit tests (#1734) Add AccountClaim to Metadata (#1663) [Wallet] Set default gradle properties (#1629) Update genesis block after adding parent signatures to block header (#1732) Removed end-to-end-geth-integration-sync-test job in workflow (#1730) Change the event we emit when selecting issuers (#1706) [Wallet] Prompt user to restart app when turning off Forno a second time (#1708) Check in spanish verification translations (#1726) [Wallet] Style fixes on iOS for verification and backup flows (#1718) [Wallet] Reset isRedeemingInvite on rehydrate (#1716) Adjust e2e transfer and governance tests to match new fee distribution and eliminate ProposerFraction (#1585) [Wallet] Add more local currencies (#1698) Switch to correct cluster when fauceting (#1687) [Wallet] Use the country of the phone number for determining the default local currency (#1684) [Wallet] Limit QR code scanner to 1 code per second (#1676) Update Dark backgrounds text color (#1677) ...

annakaz and others added 16 commits October 31, 2019 10:55

Debugging in progress

7b56cac

Clear geth cache, rework sagas

ef07a7a

Add modal, modal UI unfinished

817aa26

Merge branch 'master' into annakaz/forno-fix

0a40b7d

Merge branch 'master' into annakaz/forno-fix

be91cff

Set default gradle properties

703baad

Update clearGethCache

79d8078

Merge remote-tracking branch 'origin/master' into jeanregisser/set-de…

878f684

…fault-gradle-properties

Testing reduced jvm options since we're setting them in

768ca7a

gradle.properties

Merge branch 'master' into annakaz/forno-fix

539c257

Merge branch 'jeanregisser/set-default-gradle-properties' into annaka…

79bd864

…z/forno-fix

Update modal

4e1a772

Update modal, track when restart needed

6bc8733

Add comments and clean up

c1462ff

Update copy, remove unused images

5163edf

Revert jean's gradle changes

cb22a47

annakaz requested review from cmcewen and jmrossy as code owners November 14, 2019 02:14

annakaz assigned jmrossy and cmcewen Nov 14, 2019

jmrossy assigned annakaz and unassigned jmrossy Nov 14, 2019

annakaz and others added 4 commits November 14, 2019 08:12

Merge branch 'master' into annakaz/forno-modal

9e8e10c

Fix lint checks

bea53dc

Merge branch 'annakaz/forno-modal' of github.com:celo-org/celo-monore…

88812dc

…po into annakaz/forno-modal

Update CeloLite snapshot

0b91945

jmrossy reviewed Nov 14, 2019

View reviewed changes

Make changes based on review

3554c4c

Changes based on review

5f535b2

annakaz and others added 3 commits November 14, 2019 09:33

Merge branch 'master' into annakaz/forno-modal

1d599e8

Add snapshot tests :)

a2129d9

merge branch 'annakaz/forno-modal' of github.com:celo-org/celo-monore…

9306c8e

…po into annakaz/forno-modal

annakaz assigned jmrossy and unassigned annakaz Nov 14, 2019

Put back in deleted hide modal

615496a

jmrossy approved these changes Nov 14, 2019

View reviewed changes

jmrossy assigned annakaz and unassigned jmrossy Nov 14, 2019

Merge branch 'master' into annakaz/forno-modal

afca2ab

annakaz added the automerge Have PR merge automatically when checks pass label Nov 14, 2019

celo-ci-bot-user merged commit 7feadc0 into master Nov 14, 2019

celo-ci-bot-user deleted the annakaz/forno-modal branch November 14, 2019 19:23

annakaz mentioned this pull request Nov 15, 2019

When on Celo Lite mode, should not see 'Connecting to Celo...' #1745

Closed

snyk-bot mentioned this pull request Nov 18, 2020

[Snyk] Fix for 1 vulnerabilities denissnykio/celo-monorepo#324

Open

leojoy95 mentioned this pull request Dec 19, 2023

[Snyk] Fix for 8 vulnerabilities leojoy95/celo-monorepo#810

Open

lili2311 mentioned this pull request Dec 20, 2023

[Snyk-dev] Fix for 10 vulnerabilities lili2311/lerna-celo-monorepo#238

Open

leojoy95 mentioned this pull request May 16, 2024

[Snyk] Fix for 52 vulnerabilities leojoy95/celo-monorepo#975

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Wallet] Prompt user to restart app when turning off Forno a second time #1708

[Wallet] Prompt user to restart app when turning off Forno a second time #1708

annakaz commented Nov 14, 2019 •

edited

Loading

jmrossy left a comment

jmrossy Nov 14, 2019

annakaz Nov 14, 2019

jmrossy Nov 14, 2019

jmrossy Nov 14, 2019

jmrossy Nov 14, 2019

jmrossy Nov 14, 2019

annakaz Nov 14, 2019

jmrossy Nov 14, 2019

codecov bot commented Nov 14, 2019 •

edited

Loading

annakaz commented Nov 14, 2019

[Wallet] Prompt user to restart app when turning off Forno a second time #1708

[Wallet] Prompt user to restart app when turning off Forno a second time #1708

Conversation

annakaz commented Nov 14, 2019 • edited Loading

Description

Tested

Other changes

Related issues

Backwards compatibility

jmrossy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Nov 14, 2019 • edited Loading

Codecov Report

annakaz commented Nov 14, 2019

annakaz commented Nov 14, 2019 •

edited

Loading

codecov bot commented Nov 14, 2019 •

edited

Loading