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

Use chainId instead of networkId #196

Merged
merged 10 commits into from
Mar 27, 2019
Merged

Conversation

patitonar
Copy link

Instead of using networkId for detecting on which network the user is, now we use chainId. For that I had to update to latest version of web3 since there is a new getChainId method. I could use raw request to get the chainId for home and foreign networks, but I cannot access to the url used on the wallet, only by using the exposed web3 provider.

For e2e tests I had to update the parity version. The e2e tests are failing, it seems an issue between the new web3 version we are using and the web3 version used by the metamask extension we are using on the test. I'll try to update the extension to a newer version.

@ghost ghost assigned patitonar Mar 26, 2019
@netlify
Copy link

netlify bot commented Mar 26, 2019

Deploy preview for kind-kilby-95344f processing.

Building with commit 86c1bfa

https://app.netlify.com/sites/kind-kilby-95344f/deploys/5c9bbcaff76f4c0007f31e3e

@ghost ghost added the in progress label Mar 27, 2019
@patitonar patitonar marked this pull request as ready for review March 27, 2019 13:46
@coveralls
Copy link

Pull Request Test Coverage Report for Build 404

  • 1 of 31 (3.23%) changed or added relevant lines in 2 files are covered.
  • 31 unchanged lines in 8 files lost coverage.
  • Overall coverage increased (+5.9%) to 10.551%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/stores/utils/contract.js 1 12 8.33%
src/stores/utils/web3.js 0 19 0.0%
Files with Coverage Reduction New Missed Lines %
src/components/ProgressRing.js 1 0.0%
src/components/utils/yn.js 1 0.0%
src/components/index.js 1 0.0%
src/App.js 4 0.0%
src/components/BridgeNetwork.js 4 0.0%
src/stores/utils/testUtils.js 5 0.0%
src/stores/utils/gas.js 6 0.0%
src/stores/utils/web3.js 9 0.0%
Totals Coverage Status
Change from base Build 385: 5.9%
Covered Lines: 48
Relevant Lines: 436

💛 - Coveralls

@ghost ghost removed the in progress label Mar 27, 2019
@@ -41,8 +41,9 @@ class TxStore {
this.getTxReceipt(hash)
}).on('error', (e) => {
if(!e.message.includes('not mined within 50 blocks') && !e.message.includes('Failed to subscribe to new newBlockHeaders')){
this.alertStore.setLoading(false)
this.alertStore.pushError('Transaction rejected on wallet');
// False error is thrown by web3 https://github.com/ethereum/web3.js/issues/2542
Copy link
Author

Choose a reason for hiding this comment

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

@akolotov An issue from web3 it throwing error Error: Transaction has been reverted by the EVM: even if transaction was successful. The only workaround I found until it is fixed in a new version was to comment the code to stop the loading flow of the app when a transaction is submitted. The bad side is that if transaction is really reverted, the UI won't be notified

@ghost ghost added the in progress label Mar 27, 2019
@ghost ghost removed the in progress label Mar 27, 2019
@akolotov akolotov merged commit 4777813 into develop Mar 27, 2019
@ghost ghost removed the review label Mar 27, 2019
@akolotov akolotov deleted the use-chainId-instead-of-networkId branch March 27, 2019 19:14
@patitonar patitonar mentioned this pull request Mar 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants