Feat(Multichain): use contract addresses from safe-deployments [SW-169] #4262

jmealy · 2024-09-26T16:01:22Z

What it solves

Resolves: https://www.notion.so/safe-global/Use-officially-deployed-lib-contracts-ee4ff0091ae946799c37dde88531a070

How this PR fixes it

Replaces hardcoded temporary contract addresses with the proper addresses from safe-deployments

How to test it

Ensure safe creation works as before.
Ensure migration of invalid mastercopy safes works as before.
It should work on all the chains where the new contracts are deployed.
Creation and migration transactions should not be flagged by blockaid.

Checklist

I've tested the branch on mobile 📱
I've documented how it affects the analytics (if at all) 📊
I've written a unit/e2e test for it (if applicable) 🧑‍💻

notion-workspace · 2024-09-26T16:02:19Z

🔥 Use officially deployed lib contracts

github-actions · 2024-09-26T16:02:27Z

Branch preview

✅ Deploy successful!

Website:
https://multichain_use_official_deployments--walletweb.review.5afe.dev/home?safe=eth:0xA77DE01e157f9f57C7c4A326eeE9C4874D0598b6

Storybook:
https://multichain_use_official_deployments--walletweb.review.5afe.dev/storybook/

github-actions · 2024-09-26T16:08:23Z

📦 Next.js Bundle Analysis for safe-wallet-web

This analysis was generated by the Next.js Bundle Analysis action. 🤖

⚠️ Global Bundle Size Increased

Page	Size (compressed)
`global`	`1003.17 KB` (🟡 +48.54 KB)

Details

The global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

If you want further insight into what is behind the changes, give @next/bundle-analyzer a try!

Sixteen Pages Changed Size

The following pages changed size from the code in this PR compared to its base branch:

Page	Size (compressed)	First Load
`/`	`511 B` (🟢 -24.37 KB)	1003.67 KB
`/address-book`	`26.09 KB` (🟡 +5 B)	1.01 MB
`/apps`	`50.18 KB` (🟡 +174 B)	1.03 MB
`/apps/custom`	`41.81 KB` (🟡 +174 B)	1.02 MB
`/apps/open`	`54.74 KB` (🟡 +37 B)	1.03 MB
`/balances`	`31 KB` (🟡 +73 B)	1.01 MB
`/balances/nfts`	`19.18 KB` (-1 B)	1022.35 KB
`/home`	`60.96 KB` (🟡 +1.24 KB)	1.04 MB
`/new-safe/advanced-create`	`36.49 KB` (🟡 +1.36 KB)	1.02 MB
`/new-safe/create`	`35.75 KB` (🟡 +1.36 KB)	1.01 MB
`/new-safe/load`	`16.41 KB` (🟡 +10 B)	1019.58 KB
`/settings/notifications`	`27.02 KB` (-2 B)	1.01 MB
`/settings/safe-apps`	`25.56 KB` (🟡 +38 B)	1 MB
`/settings/setup`	`35.98 KB` (🟡 +5 B)	1.01 MB
`/transactions/tx`	`21.07 KB` (-2 B)	1 MB
`/welcome`	`6.77 KB` (🟢 -30 B)	1009.94 KB

Details

Only the gzipped size is provided here based on an expert tip.

First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If next/link is used, subsequent page loads would only need to download that page's bundle (the number in the "Size" column), since the global bundle has already been downloaded.

Any third party scripts you have added directly to your app using the <script> tag are not accounted for in this analysis

Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this.

github-actions · 2024-09-26T16:10:28Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	78.2% (-0.03% 🔻)	12420/15883
🔴	Branches	58.18% (+0.01% 🔼)	3230/5552
🟡	Functions	65.7%	1961/2985
🟡	Lines	79.66% (-0.03% 🔻)	11210/14072

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟡	`...` / transactions.ts	75.32% (+0.15% 🔼)	42.86% (-0.69% 🔻)	52.17%	76.43% (+0.13% 🔼)
🟡	`...` / transaction-guards.ts	66.82% (-1.26% 🔻)	32.53% (-0.8% 🔻)	68.25%	66.88% (-1.79% 🔻)
🟢	`...` / useSafeCreationData.ts	98.57%	95.45% (-0.2% 🔻)	100%	100%

Test suite run success

1548 tests passing in 206 suites.

Report generated by 🧪jest coverage report action from e996e74

schmanu · 2024-09-27T10:57:01Z

src/components/new-safe/create/logic/index.test.ts

@@ -44,6 +43,9 @@ const latestSafeVersion = getLatestSafeVersion(
    .build(),
 )

+const SAFE_TO_L2_SETUP_ADDRESS = '0xBD89A1CE4DDe368FFAB0eC35506eEcE0b1fFdc54'


We can fetch this from the safe-deployments too here.

schmanu · 2024-09-27T10:57:23Z

src/components/new-safe/create/logic/index.test.ts

@@ -44,6 +43,9 @@ const latestSafeVersion = getLatestSafeVersion(
    .build(),
 )

+const SAFE_TO_L2_SETUP_ADDRESS = '0xBD89A1CE4DDe368FFAB0eC35506eEcE0b1fFdc54'
+const SAFE_TO_L2_SETUP_INTERFACE = new Interface(['function setupToL2(address l2Singleton)'])


We should try to get the lib contracts also added to our typechain.

schmanu · 2024-09-27T10:57:50Z

src/components/new-safe/create/logic/index.ts

@@ -226,6 +225,10 @@ export const createNewUndeployedSafeWithoutSalt = (
    throw new Error('No Safe deployment found')
  }

+  const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: safeVersion, network: chain.chainId })
+  const safeToL2SetupAddress = safeToL2SetupDeployment?.defaultAddress
+  const safeToL2SetupInterface = safeToL2SetupDeployment && new Interface(safeToL2SetupDeployment?.abi)


Ideally we try to use typechain for this.

schmanu · 2024-09-27T10:58:26Z

src/components/new-safe/create/logic/index.ts

@@ -226,6 +225,10 @@ export const createNewUndeployedSafeWithoutSalt = (
    throw new Error('No Safe deployment found')
  }

+  const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: safeVersion, network: chain.chainId })


This contract only exists for 1.4.1 Safes. So I think we need to always fetch it for those:

Suggested change

const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: safeVersion, network: chain.chainId })

const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: "1.4.1", network: chain.chainId })

schmanu · 2024-09-27T10:59:01Z

src/components/new-safe/create/logic/index.ts

@@ -226,6 +225,10 @@ export const createNewUndeployedSafeWithoutSalt = (
    throw new Error('No Safe deployment found')
  }

+  const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: safeVersion, network: chain.chainId })
+  const safeToL2SetupAddress = safeToL2SetupDeployment?.defaultAddress


I think its better to use this:

Suggested change

const safeToL2SetupAddress = safeToL2SetupDeployment?.defaultAddress

const safeToL2SetupAddress = safeToL2SetupDeployment?.networkAddresses[chain.chainId]

schmanu · 2024-09-27T11:00:06Z

src/components/transactions/TxDetails/TxData/DecodedData/SingleTxDecoded/index.tsx

@@ -26,14 +26,17 @@ export const SingleTxDecoded = ({ tx, txData, actionTitle, variant, expanded, on
  const addressInfo = txData.addressInfoIndex?.[tx.to]
  const name = addressInfo?.name

+  const safeToL2MigrationDeployment = getSafeToL2MigrationDeployment()
+  const safeToL2MigrationAddress = safeToL2MigrationDeployment?.defaultAddress


We should also use and check the networkAddresses here.
Not because they could be different but because the deployment could be missing on a chain.

schmanu · 2024-09-27T12:00:02Z

src/features/counterfactual/ActivateAccountFlow.tsx

@@ -96,6 +96,10 @@ const ActivateAccountFlow = () => {

  const { owners, threshold, safeVersion } = undeployedSafeSetup

+  const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: safeVersion, network: chain?.chainId })


Same here. We also gotta use 1.4.1:

Suggested change

const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: safeVersion, network: chain?.chainId })

const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: "1.4.1", network: chain?.chainId })

schmanu · 2024-09-27T12:00:20Z

src/utils/__tests__/transactions.test.ts


 jest.mock('@/services/tx/tx-sender/sdk')

+export const SAFE_TO_L2_MIGRATION_ADDRESS = '0xfF83F6335d8930cBad1c0D439A841f01888D9f69'


Lets also fetch this one from the deployments.

feat: use actual contract addresses from safe-deployments

e996e74

jmealy marked this pull request as ready for review September 27, 2024 09:11

jmealy requested a review from schmanu September 27, 2024 09:11

schmanu reviewed Sep 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feat(Multichain): use contract addresses from safe-deployments [SW-169] #4262

Feat(Multichain): use contract addresses from safe-deployments [SW-169] #4262

jmealy commented Sep 26, 2024 •

edited

Loading

notion-workspace bot commented Sep 26, 2024

github-actions bot commented Sep 26, 2024 •

edited

Loading

github-actions bot commented Sep 26, 2024

github-actions bot commented Sep 26, 2024

schmanu Sep 27, 2024

schmanu Sep 27, 2024

schmanu Sep 27, 2024

schmanu Sep 27, 2024

schmanu Sep 27, 2024

schmanu Sep 27, 2024

schmanu Sep 27, 2024

schmanu Sep 27, 2024

	const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: safeVersion, network: chain.chainId })
	const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: "1.4.1", network: chain.chainId })

	const safeToL2SetupAddress = safeToL2SetupDeployment?.defaultAddress
	const safeToL2SetupAddress = safeToL2SetupDeployment?.networkAddresses[chain.chainId]

		@@ -96,6 +96,10 @@ const ActivateAccountFlow = () => {

		const { owners, threshold, safeVersion } = undeployedSafeSetup

		const safeToL2SetupDeployment = getSafeToL2SetupDeployment({ version: safeVersion, network: chain?.chainId })


		jest.mock('@/services/tx/tx-sender/sdk')

		export const SAFE_TO_L2_MIGRATION_ADDRESS = '0xfF83F6335d8930cBad1c0D439A841f01888D9f69'

Feat(Multichain): use contract addresses from safe-deployments [SW-169] #4262

Are you sure you want to change the base?

Feat(Multichain): use contract addresses from safe-deployments [SW-169] #4262

Conversation

jmealy commented Sep 26, 2024 • edited Loading

What it solves

How this PR fixes it

How to test it

Checklist

notion-workspace bot commented Sep 26, 2024

github-actions bot commented Sep 26, 2024 • edited Loading

Branch preview

github-actions bot commented Sep 26, 2024

📦 Next.js Bundle Analysis for safe-wallet-web

⚠️ Global Bundle Size Increased

Sixteen Pages Changed Size

github-actions bot commented Sep 26, 2024

Coverage report

Test suite run success

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmealy commented Sep 26, 2024 •

edited

Loading

github-actions bot commented Sep 26, 2024 •

edited

Loading