Skip to content
This repository was archived by the owner on Nov 10, 2023. It is now read-only.

Support Circles Safes in frontend on xDai #1636

Merged
dasanra merged 16 commits intodevelopmentfrom
issue-1578
Nov 26, 2020
Merged

Support Circles Safes in frontend on xDai #1636
dasanra merged 16 commits intodevelopmentfrom
issue-1578

Conversation

@nicosampler
Copy link
Contributor

@nicosampler nicosampler commented Nov 19, 2020

Closes #1578 #1636.

Sorry but this PR contains mixed changes.

  1. I fixed a problem with prettier, it was badly configured and was omitting some folders. I fixed prettierignore and fixed the errors shown by the linter.
  2. I fixed Support Circles Safes in frontend on xDai  #1636. When a safe is loaded, we delegate the validation to determine if the address corresponds to a valid safe to the backed.
  3. Now, we support to load Circles accounts as safes.

@nicosampler nicosampler self-assigned this Nov 19, 2020
@github-actions
Copy link

CLA Assistant Lite All Contributors have signed the CLA.

@github-actions
Copy link

github-actions bot commented Nov 19, 2020

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 5 0
Ignored 12 N/A
  • Result: ✅ success

  • Annotations: 5 total


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

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


Report generated by eslint-plus-action

@nicosampler nicosampler marked this pull request as ready for review November 20, 2020 16:59
@ghost
Copy link

ghost commented Nov 24, 2020

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

@ghost
Copy link

ghost commented Nov 24, 2020

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

@ghost
Copy link

ghost commented Nov 25, 2020

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

@ghost
Copy link

ghost commented Nov 25, 2020

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

context?: unknown
methods: Array<string | {method: string, type?: string, args: MethodsArgsType }>
}
methods: Array<string | { method: string; type?: string; args: MethodsArgsType }>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why shouldn't we use the simple coma here , ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was prettier who changed it

export const DEFAULT_FALLBACK_HANDLER_ADDRESS = '0xd5D82B6aDDc9027B22dCA772Aa68D5d74cdBdF44'
export const SAFE_MASTER_COPY_ADDRESS_V10 = '0xb6029EA3B2c51D09a50B53CA8012FeEB05bDa35A'

const web3 = getWeb3()
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure that doing this may lead to potential errors. I think it was ok by calling getWeb3 from within every function.

If the provider changes during runtime, this pointer won't get updated.

Also, by memoizing you're doing the same thing, you're getting an instance of a Contract instantiated with a particular provider. If it changes during runtime, you'll try to send a transaction, and it will fail.

Please, correct me if I'm wrong.

Also, if you're just using a contract instance only to call a method, it's better to use the web3ReadOnly exported ref.

@ghost
Copy link

ghost commented Nov 25, 2020

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

@ghost
Copy link

ghost commented Nov 25, 2020

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

@ghost
Copy link

ghost commented Nov 26, 2020

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

@ghost
Copy link

ghost commented Nov 26, 2020

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

const getMasterCopyInfo = async () => {
const masterCopies = await fetchMasterCopies()
const masterCopyAddress = await getMasterCopyAddressFromProxyAddress(safeAddress)
const masterCopy = masterCopies?.find((mc) => mc.address === masterCopyAddress)
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpicking, but can we use the sameAddress function instead of ===?

@Agupane Agupane self-requested a review November 26, 2020 16:27
@ghost
Copy link

ghost commented Nov 26, 2020

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

@ghost
Copy link

ghost commented Nov 26, 2020

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

@dasanra dasanra merged commit 0ecd70f into development Nov 26, 2020
@dasanra dasanra deleted the issue-1578 branch November 26, 2020 17:00
@github-actions github-actions bot locked and limited conversation to collaborators Nov 26, 2020
@francovenica
Copy link
Contributor

I created an account in circles.garden and loaded a safe with the normal workflow
I exported the owner account into the MM extension with the PK located in devtools > Applications > Storage

The safe in the Release branch:
https://pr1657--safereact.review.gnosisdev.com/xdai/app/#/safes/0xF85469e0B454f3D6D5c31eBd80Fa071FaC62785a/transactions

I had a small amount of xDai and the 50 CRC tokens already in my wallet
I was able to send some CRC to the owner of the safe just fine

Something to note is that I received 0.5 CRC at some point, but is not the same CRC token I originally had. It seems even that there are a ton of different CRC tokens for this type of safe
image.png
image.png

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support Circles Safes in frontend on xDai

5 participants