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

Add Gnosis Safe connector #588

Merged
merged 18 commits into from
Aug 13, 2021
Merged

Add Gnosis Safe connector #588

merged 18 commits into from
Aug 13, 2021

Conversation

TomAFrench
Copy link
Contributor

@TomAFrench TomAFrench commented Jul 20, 2021

Description

This PR adds a new Gnosis Safe connector which allows loading the interface through the Gnosis website to directly connect to a Safe.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Other

How should this be tested?

The main issue which needs to be tested is that we correctly pick up when a transaction confirms when using a Gnosis safe with only one signer. This is because we don't talk directly with the RPC but through a Gnosis service which sometimes can not know about a transaction that's in flight for some time.

Testing that you can do a single transaction and not have the app get stuck in an infinite load should be sufficient but we should probably test each transaction type just to be thorough.

You can load the app on either mainnet or polygon

To open as a safe app:

  • Load a Safe on https://polygon.gnosis-safe.io and go to the Apps tab.
  • Click the first tile on that page which allows loading a custom safe app.
  • Set the most recent preview URL as the URL and the name+icon should autofill. Click add.
  • You'll be taken to the app running in an iframe (It's possible that your wallet will autoload at this point, just log out if so)
  • Connect again using the new option with the name "Gnosis Safe".

If you're trying to run this on mainnet then you need to run the app locally as passwords on Vercel prevent it from loading properly

Known issues:

  • Transactions in the "recent activity" menu use the "safeTxHash" rather than the real hash so the links are broken. Addressed
  • It's possible to connect to metamask on the Gnosis interface so we should detect this case and prevent connecting to other wallets. Addressed

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code where relevant, particularly in hard-to-understand areas
  • My changes generate no new console warnings
  • The base of this PR is master if hotfix, develop if not

@vercel
Copy link

vercel bot commented Jul 20, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

kovan-app – ./

🔍 Inspect: https://vercel.com/balancer/kovan-app/BPW4B9GsCzwM6jUQhgSYYHkNEEfF
✅ Preview: Canceled

[Deployment for cad68fb canceled]

app – ./

🔍 Inspect: https://vercel.com/balancer/app/2mrN4CtRsQDDmVsGY7FzQt7wAxCx
✅ Preview: Canceled

[Deployment for cad68fb canceled]

staging-app – ./

🔍 Inspect: https://vercel.com/balancer/staging-app/8Lam1SwxkEGWQVmDZV71PWBfydXf
✅ Preview: https://staging-app-git-safe-app-balancer.vercel.app

staging-kovan-app – ./

🔍 Inspect: https://vercel.com/balancer/staging-kovan-app/EGT6BiYfYooStLfMHcutXgTe15QB
✅ Preview: https://staging-kovan-app-git-safe-app-balancer.vercel.app

gnosis – ./

🔍 Inspect: https://vercel.com/balancer/gnosis/Dc5dNNiWUXPVcLwVDuuPembjdHNv
✅ Preview: https://gnosis-git-safe-app-balancer.vercel.app

polygon – ./

🔍 Inspect: https://vercel.com/balancer/polygon/3w2s8SFiCAJWGEoFxaAYikXgAo7J
✅ Preview: https://polygon-git-safe-app-balancer.vercel.app

[Deployment for cad68fb canceled]

beta-polygon – ./

🔍 Inspect: https://vercel.com/balancer/beta-polygon/9bP1UVdk36Qqe5aqLtpY6GLyzPe5
✅ Preview: https://beta-polygon-git-safe-app-balancer.vercel.app

@vercel vercel bot temporarily deployed to Preview – app July 20, 2021 19:22 Inactive
@vercel vercel bot temporarily deployed to Preview – kovan-app July 20, 2021 19:22 Inactive
@vercel vercel bot temporarily deployed to Preview – kovan-app July 20, 2021 20:10 Inactive
@vercel vercel bot temporarily deployed to Preview – app July 20, 2021 20:10 Inactive
@vercel vercel bot temporarily deployed to Preview – kovan-app July 20, 2021 20:11 Inactive
@vercel vercel bot temporarily deployed to Preview – app July 20, 2021 20:18 Inactive
@vercel vercel bot temporarily deployed to Preview – staging-app July 20, 2021 20:33 Inactive
@vercel vercel bot temporarily deployed to Preview – kovan-app July 20, 2021 20:33 Inactive
@vercel vercel bot temporarily deployed to Preview – staging-kovan-app July 20, 2021 20:33 Inactive
@vercel vercel bot temporarily deployed to Preview – app July 20, 2021 20:33 Inactive
@TomAFrench
Copy link
Contributor Author

@garethfuller can you sign off on this? I can't merge without a rereview

Copy link
Collaborator

@garethfuller garethfuller left a comment

Choose a reason for hiding this comment

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

Looks good, sorry for the delay.

@TomAFrench
Copy link
Contributor Author

No worries, this got pushed down the priority list so I didn't even notice it was still locked until this morning.

@TomAFrench TomAFrench merged commit 351abc2 into develop Aug 13, 2021
@TomAFrench TomAFrench deleted the safe-app branch August 13, 2021 13:22
This was referenced Aug 24, 2021
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