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

Align deployment of Safe to be of same version as Safe UI-deployed and having fallback handler attached #1375

Merged
merged 4 commits into from
Feb 26, 2024

Conversation

mudrila
Copy link
Contributor

@mudrila mudrila commented Feb 26, 2024

Description

This PR swaps all the GnosisSafe usages to GnosisSafeL2 and adds fallbackHandler to the create Safe flow, right into setup function call.

Notes

Issue

Closes #1361

Testing

  1. Deploy brand new Safe via Fractal
  2. Create new Safe via Safe UI, go to Safe's Settings page
  3. Go to Safe UI and find Fractal-created Safe, go to Safe's Settings page
  4. Compare Safe created via Fractal and Safe created via Safe UI - their version should be exactly the same (1.3.0+L2) and attached Fallback Handler (could be found under "Modules" tab) also should be exactly the same. See screenshots below for references on how it should look like

Screenshots (if applicable)

Знімок екрана 2024-02-26 о 19 06 43 Знімок екрана 2024-02-26 о 19 43 23

@mudrila mudrila added bug Something isn't working enhancement New feature or request labels Feb 26, 2024
@mudrila mudrila requested review from adamgall and Da-Colon February 26, 2024 18:45
@mudrila mudrila self-assigned this Feb 26, 2024
Copy link

netlify bot commented Feb 26, 2024

Deploy Preview for fractal-framework-interface-dev ready!

Name Link
🔨 Latest commit dffbb6a
🔍 Latest deploy log https://app.netlify.com/sites/fractal-framework-interface-dev/deploys/65dcf0e5a95b7500083c1d37
😎 Deploy Preview https://deploy-preview-1375--fractal-framework-interface-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Contributor

@Da-Colon Da-Colon left a comment

Choose a reason for hiding this comment

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

Surprised we still have to rely on the typechain files for these. But tested and it indeed works. I see that the Contract+L2 is correctly displayed in the settings and the CompatibilityFallbackHandler has been set up as a module.

Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

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

Whenever using the safe-deployments package to get singleton addresses, the filter simply confirms that the contract has been deployed on the given network.

It's still up to the caller to dig into the networkAddresses object of the result to find the official address for a given chain, do not rely on the defaultAddress!

This applies to the new CompatibilityFallbackHandler code (which seems good for Sepolia and Polygon, but might as well do it for Mainnet too), as well as the actual Master Safe Singleton deployment address as well (for ALL THREE networks).

src/providers/NetworkConfig/networks/mainnet.ts Outdated Show resolved Hide resolved
src/providers/NetworkConfig/networks/mainnet.ts Outdated Show resolved Hide resolved
src/providers/NetworkConfig/networks/polygon.ts Outdated Show resolved Hide resolved
src/providers/NetworkConfig/networks/sepolia.ts Outdated Show resolved Hide resolved
…t it retrieved from networkAddresses and not defaultAddress
@mudrila mudrila requested a review from adamgall February 26, 2024 20:14
@adamgall
Copy link
Member

Surprised we still have to rely on the typechain files for these. But tested and it indeed works. I see that the Contract+L2 is correctly displayed in the settings and the CompatibilityFallbackHandler has been set up as a module.

Pretty lucky that we had those GnosisSafeL2 types in that usul folder, huh! It looks like the only reason we had those types in the project, before, this PR, was to get the Multisend typing.

Copy link
Member

@adamgall adamgall left a comment

Choose a reason for hiding this comment

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

LGTM!

@adamgall adamgall merged commit 9b2551e into develop Feb 26, 2024
5 checks passed
@adamgall adamgall deleted the fix/#1361-deploy-safe-up-to-spec-with-safe-dapp branch February 26, 2024 20:36
@adamgall adamgall added maintenance Keep the lights on and removed bug Something isn't working enhancement New feature or request labels Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Keep the lights on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fractal UI deploys out-of-spec Safes
3 participants