(Feature) [xDai] - Create network-based configurations by fernandomg · Pull Request #1420 · 5afe/safe-react

fernandomg · 2020-09-30T19:51:28Z

Closes #1414, by:

setting config files for mainnet, rinkeby and xdai.
defining types for config files

A special note for reviewers: we added GAS_PRICE

Co-authored-by: Matias Dastugue <matias.dastugue@altoros.com>

github-actions · 2020-09-30T19:51:46Z

CLA Assistant Lite All Contributors have signed the CLA.

github-actions · 2020-09-30T19:56:23Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	5	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

…ai-generic-configs

ghost · 2020-09-30T21:46:34Z

Travis automatic deployment:
https://pr1420--safereact.review.gnosisdev.com/app

ghost · 2020-10-01T15:24:57Z

Travis automatic deployment:
https://pr1420--safereact.review.gnosisdev.com/app

Co-authored-by: Matias Dastugue <matias.dastugue@altoros.com>

ghost · 2020-10-01T17:16:56Z

Travis automatic deployment:
https://pr1420--safereact.review.gnosisdev.com/app

Co-authored-by: Matias Dastugue <matias.dastugue@altoros.com>

ghost · 2020-10-01T19:38:27Z

Travis automatic deployment:
https://pr1420--safereact.review.gnosisdev.com/app

…ai-generic-configs

ghost · 2020-10-01T23:45:06Z

Travis automatic deployment:
https://pr1420--safereact.review.gnosisdev.com/app

nicosampler · 2020-10-02T11:04:57Z

package.json

      "!src/**/test/**/*",
-      "!src/**/assets/**",
-      "!src/config/**/*"
+      "!src/**/assets/**"


not to be done now, but should we change the version to v3.0.0?

Something to consider. We should talk about this when preparing release

nicosampler · 2020-10-02T11:07:57Z

src/config/networks/network.d.ts

@@ -0,0 +1,59 @@
+import { ETHEREUM_NETWORK } from 'src/logic/wallets/getWeb3'


can we do the opposite? I mean defining the networks here and import them in 'src/logic/wallets/getWeb3'

I agree, but we preferred doing it later (in a different PR) because there are lots of files that import this enum, and it will make a lot of noise.

And probably we should wrap the ETHEREUM_NETWORK[getNetwork()] into a function.

Don't forget to create a ticket for this

@dasanra, @nicosampler it's done in 4e5e8bf (#1422)

src/config/networks/network.d.ts

nicosampler · 2020-10-02T11:15:29Z

src/config/networks/network.d.ts

+}
+
+type GasPrice = {
+  gasPrice: number


I'm not sure about this, I thought it was going to be a URL or NULL, if a URL is provided, then the service in charge to resolve the gas calculation will do all it's needed to return a value. If it's NULL the service will return a hardcoded value.

If you don't like that alternative and would suggest typing this value like so:
gasPriceInfo: number | string

mhh... but, where will you define the hardcoded value depending on the network you're using?

In regard to the type, why will you need a string type for that value?

I don't see the problem here. This may be necessary to be different depending on the network, so the fallback value should be configurable

Any more feedback to add @nicosampler ?

no, that's fine

nicosampler · 2020-10-02T11:21:18Z

src/config/networks/__tests__/networks.test.ts

+import { ETHEREUM_NETWORK } from 'src/logic/wallets/getWeb3'
+import { isValidURL } from 'src/utils/url'
+
+describe('Network config file test', () => {


shouldn't it be Networks config files?

yes, makes sense

nicosampler · 2020-10-02T11:27:27Z

src/config/networks/__tests__/networks.test.ts

+      const { color } = networkConfig.network
+
+      // Then
+      const s = new Option().style


just curious, is there a reason to use an Option instead of another element?

yes... totally

nicosampler · 2020-10-02T11:39:28Z

src/config/networks/network.d.ts

+}
+
+export type EnvironmentSettings = GasPrice & {
+  txServiceUri: string


didn't know the difference between URL and URI, so I found this: https://danielmiessler.com/study/difference-between-uri-url/#:~:text=A%20URI%20is%20an%20identifier,as%20HTTPs%20%2C%20FTP%20%2C%20etc.

Seems like we should use URL instead of URI.

ghost · 2020-10-02T12:33:59Z

Travis automatic deployment:
https://pr1420--safereact.review.gnosisdev.com/app

src/config/rinkeby.ts

src/config/networks/network.d.ts

ghost · 2020-10-02T15:36:25Z

Travis automatic deployment:
https://pr1420--safereact.review.gnosisdev.com/app

add network based configs

9ab2060

Co-authored-by: Matias Dastugue <matias.dastugue@altoros.com>

fernandomg requested review from Agupane, dasanra and nicosampler September 30, 2020 19:51

fernandomg assigned matextrem and unassigned matextrem Sep 30, 2020

fernandomg assigned fernandomg and matextrem Sep 30, 2020

fernandomg added 2 commits September 30, 2020 17:30

Merge branch 'feature/#1353-xDai-compatibility' into feature/#1414-xD…

fd48894

…ai-generic-configs

Merge branch 'feature/#1353-xDai-compatibility' into feature/#1414-xD…

8adeaf0

…ai-generic-configs

Rename dev config by baseConfig

79fdcd9

fernandomg and others added 4 commits October 1, 2020 13:55

use ETHEREUM_NETWORK enum to define the network id

5b90fa9

Co-authored-by: Matias Dastugue <matias.dastugue@altoros.com>

change config keys from CONSTANT_CASE to upperCase

c0859fd

Co-authored-by: Matias Dastugue <matias.dastugue@altoros.com>

replace Host and Url with Uri in config keys

1b39ee1

Co-authored-by: Matias Dastugue <matias.dastugue@altoros.com>

move network config files into a networks directory

79e2a70

Co-authored-by: Matias Dastugue <matias.dastugue@altoros.com>

add tests for config files

ab69f74

Co-authored-by: Matias Dastugue <matias.dastugue@altoros.com>

Merge branch 'feature/#1353-xDai-compatibility' into feature/#1414-xD…

45cb541

…ai-generic-configs

nicosampler reviewed Oct 2, 2020

View reviewed changes

src/config/networks/network.d.ts Outdated Show resolved Hide resolved

nicosampler reviewed Oct 2, 2020

View reviewed changes

fix test description

9e2e755

Agupane suggested changes Oct 2, 2020

View reviewed changes

src/config/rinkeby.ts Outdated Show resolved Hide resolved

src/config/networks/network.d.ts Outdated Show resolved Hide resolved

fernandomg added 3 commits October 2, 2020 11:58

change features' type to boolean

dfd1b2a

replace Uri with Url

dc26462

add networkExplorerName key

35604e7

fernandomg requested review from Agupane and nicosampler October 2, 2020 15:21

Agupane approved these changes Oct 2, 2020

View reviewed changes

dasanra approved these changes Oct 2, 2020

View reviewed changes

nicosampler approved these changes Oct 2, 2020

View reviewed changes

fernandomg merged commit 6a4e353 into feature/#1353-xDai-compatibility Oct 2, 2020

fernandomg deleted the feature/#1414-xDai-generic-configs branch October 2, 2020 19:52

github-actions bot locked and limited conversation to collaborators Oct 2, 2020

		@@ -0,0 +1,59 @@
		import { ETHEREUM_NETWORK } from 'src/logic/wallets/getWeb3'

Conversation

fernandomg commented Sep 30, 2020

Uh oh!

github-actions bot commented Sep 30, 2020

Uh oh!

github-actions bot commented Sep 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ESLint Summary View Full Report

Uh oh!

ghost commented Sep 30, 2020

Uh oh!

ghost commented Oct 1, 2020

Uh oh!

ghost commented Oct 1, 2020

Uh oh!

ghost commented Oct 1, 2020

Uh oh!

ghost commented Oct 1, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fernandomg Oct 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Oct 2, 2020

Uh oh!

Uh oh!

Uh oh!

ghost commented Oct 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Sep 30, 2020 •

edited

Loading

fernandomg Oct 2, 2020 •

edited

Loading