(Feature) - Replaces createProxy with createProxyWithNonce method no safe creation by Agupane · Pull Request #1630 · 5afe/safe-react

Agupane · 2020-11-18T14:47:01Z

Closes #1577 by:

Replacing the createProxy method with createProxyWithNonce that lets us add some salt for randomizing the created safeAddress

github-actions · 2020-11-18T14:47:19Z

CLA Assistant Lite All Contributors have signed the CLA.

github-actions · 2020-11-18T14:51:41Z

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	23	0
Ignored	2	N/A

Result: ✅ success
Annotations: 23 total

[warning] @typescript-eslint/explicit-module-boundary-types

Require explicit return and argument types on exported functions' and classes' public class methods

src/routes/open/components/ReviewInformation/index.tsx Line 147 - Argument 'controls' should be typed.
src/routes/open/components/ReviewInformation/index.tsx Line 147 - Argument 'props' should be typed.
src/routes/open/components/fields.ts Line 7 - Missing return type on function.
src/routes/open/components/fields.ts Line 7 - Argument 'index' should be typed.
src/routes/open/components/fields.ts Line 8 - Missing return type on function.
src/routes/open/components/fields.ts Line 8 - Argument 'index' should be typed.
src/routes/open/components/fields.ts Line 10 - Missing return type on function.
src/routes/open/components/fields.ts Line 10 - Argument 'values' should be typed.
src/routes/open/container/Open.tsx Line 57 - Missing return type on function.
src/routes/open/container/Open.tsx Line 57 - Argument 'values' should be typed.
src/routes/open/container/Open.tsx Line 57 - Argument 'userAccount' should be typed.
src/routes/open/utils/safeDataExtractor.ts Line 6 - Missing return type on function.
src/routes/open/utils/safeDataExtractor.ts Line 6 - Argument 'values' should be typed.
src/routes/open/utils/safeDataExtractor.ts Line 14 - Missing return type on function.
src/routes/open/utils/safeDataExtractor.ts Line 14 - Argument 'values' should be typed.
src/routes/open/utils/safeDataExtractor.ts Line 22 - Argument 'names' should be typed.
src/routes/open/utils/safeDataExtractor.ts Line 22 - Argument 'addresses' should be typed.
src/routes/open/utils/safeDataExtractor.ts Line 28 - Missing return type on function.
src/routes/open/utils/safeDataExtractor.ts Line 28 - Argument 'values' should be typed.
src/routes/open/utils/safeDataExtractor.ts Line 30 - Missing return type on function.
src/routes/open/utils/safeDataExtractor.ts Line 30 - Argument 'values' should be typed.
src/routes/open/utils/safeDataExtractor.ts Line 32 - Missing return type on function.
src/routes/open/utils/safeDataExtractor.ts Line 32 - Argument 'values' should be typed.

^{Report generated by eslint-plus-action}

ghost · 2020-11-18T15:10:33Z

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

fernandomg

I'm finding quite odd that we are calculating the randomSalt value twice during the creation of the Safe.

I think we should pass the value as a parameter to the functions.

src/logic/contracts/safeContracts.ts

…create2' into feature/1577-use-proxy-creation-create2

Agupane · 2020-11-18T16:26:28Z

I'm finding quite odd that we are calculating the randomSalt value twice during the creation of the Safe.

I think we should pass the value as a parameter to the functions.

Yes make sense, I updated the code to do that

nicosampler · 2020-11-18T16:32:32Z

src/logic/contracts/safeContracts.ts

    .encodeABI()

-  return proxyFactoryMaster.methods.createProxy(safeMaster.options.address, gnosisSafeData)
+  return proxyFactoryMaster.methods.createProxyWithNonce(safeMaster.options.address, gnosisSafeData, safeCreationSalt)


DO you know if all the proxyFactory versions have this method implemented?

Mmm I'm not sure, @rmeissner could you confirm please?

We are getting that info from the @gnosis.pm/safe-contracts package. I think it's safe to assume that that method will be available.

our proxy factory always exposes this method ;)

nicosampler · 2020-11-18T16:36:00Z

src/routes/open/components/Layout.tsx

  )
 }
-
-export default Layout


is there a reason to remove default?

Yep, check this: https://basarat.gitbook.io/typescript/main-1/defaultisbad

Interesting, I disagreed with some points and some others are interesting.
But I think we should all agreed on this because right now almost all the project is making use of default export.

If we define to start using named exports always, we should define a linter rule. wdyt?

Yep it's a good idea for me

ghost · 2020-11-18T16:40:17Z

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

ghost · 2020-11-18T16:40:33Z

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

ghost · 2020-11-18T17:03:41Z

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

ghost · 2020-11-18T17:03:53Z

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

ghost · 2020-11-18T18:53:40Z

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

ghost · 2020-11-18T18:54:05Z

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

Agupane added 2 commits November 18, 2020 11:36

Replaces createProxy with createProxyWithNonce method no safe creation

534f4f2

Updates estimationGas method

1f262f3

Agupane requested review from dasanra, fernandomg and nicosampler November 18, 2020 14:47

Merge branch 'development' into feature/1577-use-proxy-creation-create2

594a330

Agupane self-assigned this Nov 18, 2020

fernandomg reviewed Nov 18, 2020

View reviewed changes

src/logic/contracts/safeContracts.ts Outdated Show resolved Hide resolved

Agupane added 3 commits November 18, 2020 13:20

Moves safeCreationSalt as a parameter

e9d2b1a

Fix default export

25cfa93

Merge remote-tracking branch 'origin/feature/1577-use-proxy-creation-…

2595d07

…create2' into feature/1577-use-proxy-creation-create2

nicosampler reviewed Nov 18, 2020

View reviewed changes

Merge branch 'development' into feature/1577-use-proxy-creation-create2

81aff37

fernandomg approved these changes Nov 18, 2020

View reviewed changes

Merge branch 'development' into feature/1577-use-proxy-creation-create2

d596aee

dasanra approved these changes Nov 19, 2020

View reviewed changes

Agupane merged commit 486bb4b into development Nov 19, 2020

Agupane deleted the feature/1577-use-proxy-creation-create2 branch November 19, 2020 12:19

github-actions bot locked and limited conversation to collaborators Nov 19, 2020

Conversation

Agupane commented Nov 18, 2020

Uh oh!

github-actions bot commented Nov 18, 2020

Uh oh!

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

Uh oh!

ESLint Summary View Full Report

[warning] @typescript-eslint/explicit-module-boundary-types

Uh oh!

ghost commented Nov 18, 2020

Uh oh!

fernandomg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Agupane commented Nov 18, 2020

Uh oh!

nicosampler Nov 18, 2020

Choose a reason for hiding this comment

Uh oh!

Agupane Nov 18, 2020

Choose a reason for hiding this comment

Uh oh!

fernandomg Nov 18, 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!

rmeissner Nov 18, 2020

Choose a reason for hiding this comment

Uh oh!

nicosampler Nov 18, 2020

Choose a reason for hiding this comment

Uh oh!

Agupane Nov 18, 2020

Choose a reason for hiding this comment

Uh oh!

nicosampler Nov 18, 2020

Choose a reason for hiding this comment

Uh oh!

Agupane Nov 18, 2020

Choose a reason for hiding this comment

Uh oh!

ghost commented Nov 18, 2020

Uh oh!

ghost commented Nov 18, 2020

Uh oh!

ghost commented Nov 18, 2020

Uh oh!

ghost commented Nov 18, 2020

Uh oh!

ghost commented Nov 18, 2020

Uh oh!

ghost commented Nov 18, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Nov 18, 2020 •

edited

Loading

fernandomg Nov 18, 2020 •

edited

Loading