Conversation
| // Starting geth a second time this app session which will | ||
| // require an app restart, so show restart modal | ||
| this.showSwitchOffModal() | ||
| handleFornoToggle = (fornoMode: boolean) => { |
There was a problem hiding this comment.
Fixes if else- if switching off forno mode, show the modal if need be, else just turn it off. If switching on forno mode, show switch on modal.
Previously, if switching off forno mode and modal not necessary, this would switch forno on (opposite of intended behavior)
| yield put(setGethConnected(true)) | ||
| } | ||
|
|
||
| export function* gethSagaIfNecessary() { |
There was a problem hiding this comment.
Only start geth saga if using geth
| isGethConnectedSelector, | ||
| getNetworkConnected, | ||
| (gethConnected, networkConnected) => gethConnected && networkConnected | ||
| (fornoEnabled, gethConnected, networkConnected) => |
There was a problem hiding this comment.
Only care if gethConnected if geth is being using
| render() { | ||
| const { t, appConnected, appSynced } = this.props | ||
| const { t, appConnected, appSynced, fornoEnabled } = this.props | ||
| const appSyncedIfNecessary = appSynced || fornoEnabled |
There was a problem hiding this comment.
Only care if app is synced if geth is being used
|
|
||
| switchWeb3ProviderForSyncMode(false) | ||
| // Ensure web3 is fully synced using new provider | ||
| yield take(GethActions.SET_GETH_CONNECTED) |
There was a problem hiding this comment.
Wait for geth to be connected before starting web3 sync
There was a problem hiding this comment.
Should we use waitForGethConnectivity here like this line: https://github.com/celo-org/celo-monorepo/blob/master/packages/mobile/src/web3/saga.ts#L133
Codecov Report
@@ Coverage Diff @@
## master #2584 +/- ##
==========================================
+ Coverage 73.88% 74.01% +0.12%
==========================================
Files 558 559 +1
Lines 13859 13932 +73
Branches 1433 1378 -55
==========================================
+ Hits 10240 10312 +72
+ Misses 3337 3336 -1
- Partials 282 284 +2
Continue to review full report at Codecov.
|
jeanregisser
left a comment
jeanregisser
left a comment
There was a problem hiding this comment.
Looks great 👍 Thanks for making the naming consistent!
jmrossy
left a comment
jmrossy
left a comment
There was a problem hiding this comment.
Looks great! Just one question about waitForGethConnectivity
|
Rearranged some forno-related state:
|
3 participants
Description
Fixes a couple bugs when the app that only appear when the app is build with
FORNO_ENABLED_INITIALLY=true. The geth saga starts unnecessarily, and the web3 sync can fail if geth isn't yet connected. This fixes it by only starting gethSaga when not in forno mode, and by waiting for geth to be connected before checking web3 sync.Note that the bulk of this PR is the forno/zeroSync rename so I have annotated the non rename changes to make it easier to review. This rename is necessary because while I had initially used zeroSync mode to make it clear the app is not syncing, we have been using the term forno internally (not zeroSync) so wanted to make the codebase consistent.
Tested
Tested a default forno build turning on and off forno. Tested a default geth build turning on and off forno.
Related issues