Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[wallet]Run geth in an infura-like mode #1108

Merged
merged 2 commits into from
Oct 2, 2019
Merged

[wallet]Run geth in an infura-like mode #1108

merged 2 commits into from
Oct 2, 2019

Conversation

ashishb
Copy link
Contributor

@ashishb ashishb commented Sep 25, 2019

Description

Run mobile app without running a local geth instance. Connect to {testnet}-infura.celo-testnet.org for example, https://integration-infura.celo-testnet.org`

Tested

  1. Set DEFAULT_TESTNET=integration and DEFAULT_SYNC_MODE=-1 in packages/mobile/.env file.
  2. Run the app with yarn run dev.
  3. Invite yourself with celotooljs account invite -e integration --phone <phone number> to get SMS.
  4. Perform the verification, send money, exchange gold, ensure that all of this works.

Related issues

@ashishb ashishb changed the title [WIP]Run geth in an infura-like mode [WIP][wallet]Run geth in an infura-like mode Sep 26, 2019
@ashishb ashishb force-pushed the ashishb/geth_free branch 2 times, most recently from 8544cbe to 54a24c1 Compare September 27, 2019 00:22
@ashishb ashishb changed the title [WIP][wallet]Run geth in an infura-like mode [wallet]Run geth in an infura-like mode Sep 27, 2019
Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! See comments below, mostly small stuff

@jmrossy jmrossy assigned ashishb, cmcewen and annakaz and unassigned cmcewen and annakaz Sep 27, 2019
Copy link
Contributor

@annakaz annakaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally with yarn run dev and build the integration sdk to test. Can import a wallet, exchange gold and dollars both directions, and send dollars.

Note that the transaction feed is not loading on integration, but it is not loading on master either and seems untouched in this PR.

@ashishb ashishb force-pushed the ashishb/geth_free branch from 0a6351d to ea2690c Compare October 1, 2019 20:50
@ashishb
Copy link
Contributor Author

ashishb commented Oct 1, 2019

Tested again. Can't get SMS verification to work but that's broken on the master as well.

@ashishb ashishb force-pushed the ashishb/geth_free branch from ea2690c to ef9d930 Compare October 1, 2019 21:22
@ashishb ashishb added the automerge Have PR merge automatically when checks pass label Oct 1, 2019
@ashishb ashishb force-pushed the ashishb/geth_free branch from ef9d930 to 8fc4a75 Compare October 1, 2019 21:57
* Remove private key from logging

* Make infura setup work with https
I was trying to use infura-like setup with `ws` (Web sockets). Since
`ws` sends plain-text, it does not work in prouduction. We cannot use
`wss` since it is not supported natively by geth
 - ethereum/go-ethereum#16423
Even K8s only supports https. To avoid this issue, I shifted to https.
Subscriptions don't work with https, so, a crude polling mechanism to
check for transaction confirmations.

Also, in this commit, change the nonce count to include "pending"
transactions, so that, multiple transactions can be sent without waiting
for completion.

* Handle nonce related to simultaneous transactions

Handle the case when too many transactions are sent close to each other.

Geth has `web3.eth.getTransactionCount(account, 'pending')` but even
that only seems to work once the transactions have made to the
transaction pool. Therefore, for transactions sent too close to each
other, which is the case for SMS verification transactions, maintain a
local nonce and ensure that new Nonce is equal or greater than local
nonce.

* Increment nonce before sending transaction

Otherwise, the next transaction might not get the right nonce.

* Fix some issues pointed by Rossy
* Remove artifical wait
@ashishb ashishb force-pushed the ashishb/geth_free branch 3 times, most recently from e4c57ac to f19a6fb Compare October 2, 2019 05:58
@ashishb
Copy link
Contributor Author

ashishb commented Oct 2, 2019

I don't know why __mocks__/web3.js was not working. I had to re-repeat some of that code in src/invite/saga.test.ts to make the unit tests pass.

@ashishb ashishb force-pushed the ashishb/geth_free branch from f19a6fb to 4d1497e Compare October 2, 2019 06:32
@ashishb ashishb force-pushed the ashishb/geth_free branch from 4d1497e to e104181 Compare October 2, 2019 06:32
@codecov
Copy link

codecov bot commented Oct 2, 2019

Codecov Report

Merging #1108 into master will decrease coverage by 0.76%.
The diff coverage is 20.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1108      +/-   ##
==========================================
- Coverage   66.83%   66.06%   -0.77%     
==========================================
  Files         261      261              
  Lines        7492     7564      +72     
  Branches      503      437      -66     
==========================================
- Hits         5007     4997      -10     
- Misses       2387     2471      +84     
+ Partials       98       96       -2
Flag Coverage Δ
#mobile 66.06% <20.18%> (-0.77%) ⬇️
Impacted Files Coverage Δ
packages/mobile/src/identity/verification.ts 63.77% <ø> (ø) ⬆️
packages/mobile/src/web3/actions.ts 81.48% <ø> (ø) ⬆️
packages/mobile/src/transactions/saga.ts 51.61% <0%> (ø) ⬆️
packages/mobile/src/missingGlobals.ts 0% <0%> (ø)
packages/mobile/src/web3/contracts.ts 0% <0%> (-68.97%) ⬇️
packages/mobile/src/escrow/saga.ts 29.93% <10%> (-1.06%) ⬇️
packages/mobile/src/web3/testnets.ts 100% <100%> (ø) ⬆️
packages/mobile/src/geth/network-config.ts 100% <100%> (ø) ⬆️
packages/mobile/src/geth/consts.ts 100% <100%> (ø) ⬆️
packages/mobile/src/config.ts 94.44% <100%> (+0.15%) ⬆️
... and 8 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 4416f95...e104181. Read the comment docs.

@ashishb ashishb merged commit 579c199 into master Oct 2, 2019
@ashishb ashishb deleted the ashishb/geth_free branch October 2, 2019 06:49
aaronmgdr added a commit that referenced this pull request Oct 4, 2019
* master:
  [Protocol] Fix network id for alfajores in truffle configs (#1211)
  When resetting and upgrading a VM testnet, new tx-nodes are included in the new instance group (#771)
  Upload static VM testnet nodes, add stackdriver logging (#750)
  Revert "Make packages depend on git vesrion (not npm)" (#1201)
  Make packages depend on git vesrion (not npm) (#1192)
  [contractkit] Document methods (#1195)
  [ck] consistent send tx object in kit (#1191)
  Move docker images to use node v10 (#1183)
  [ContractKit]Fill more fields before web3 signing (#1133)
  [codecov]Fix codecov errors (#1147)
  [Wallet] Add support for address pasting in send input field (#1180)
  Fix verification pool validation (#1176)
  Improve QR Code scan ability (#1036)
  Add CLI commands around identity metadata (#1167)
  [wallet]Run geth in an infura-like mode (#1108)

# Conflicts:
#	yarn.lock
jmrossy added a commit that referenced this pull request Oct 7, 2019
* Implement new import wallet flow designs including empty wallet warning
* Improve responsiveness of import wallet screen
* More robust error handling for import saga
* Add tests for import saga
* Cleanup redundant web3 zerosync mocks introduced in #1108
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[wallet]Users SBAT to run the app in an infura-like setup
4 participants