[Multichain] Feat: disable adding new chains to safes created from old mastercopy versions [SW-167] #4161

jmealy · 2024-09-12T07:51:47Z

What it solves

Resolves: https://www.notion.so/safe-global/Do-not-offer-adding-networks-for-1-1-1-created-Safes-9608b9ee16a549e891b795fe18b5e831?pvs=4

How this PR fixes it

Gets the mastercopy/factory address used to create a given safe, and checks if there is a corresponding deployment within the list of supported versions.
If there are no matching deployments, display a message informing the user that this safe cannot be replayed.

How to test it

Open a safe that was created as a 1.1.1 safe.
Click the network selector in the top right corner. It should show a message saying that the safe cannot be replayed.
Click on the context menu of the safe in the sidebar or the accounts page. Click the button to add another network. It should show a message saying that the safe cannot be replayed.

Screenshots

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) 🧑‍💻

…tion versions

…ltichain

github-actions · 2024-09-12T10:54:55Z

Branch preview

✅ Deploy successful!

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

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

github-actions · 2024-09-12T10:57:30Z

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

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

github-actions · 2024-09-12T11:00:16Z

📦 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`	`978.79 KB` (🟡 +38.39 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!

Nineteen Pages Changed Size

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

Page	Size (compressed)	First Load
`/`	`507 B` (🟢 -24.36 KB)	979.29 KB
`/address-book`	`26.11 KB` (🟡 +3 B)	1004.91 KB
`/apps/open`	`54.74 KB` (🟡 +1 B)	1.01 MB
`/balances`	`30.64 KB` (-3 B)	1009.43 KB
`/home`	`60.28 KB` (🟡 +31 B)	1.01 MB
`/new-safe/advanced-create`	`36.51 KB` (🟡 +1.37 KB)	1015.31 KB
`/new-safe/create`	`35.76 KB` (🟡 +1.36 KB)	1014.56 KB
`/new-safe/load`	`16.38 KB` (🟢 -7 B)	995.18 KB
`/settings/modules`	`9.76 KB` (🟢 -1 B)	988.56 KB
`/settings/setup`	`35.96 KB` (🟡 +1 B)	1014.75 KB
`/share/safe-app`	`9.72 KB` (🟢 -2 B)	988.51 KB
`/terms`	`11.24 KB` (🟡 +60 B)	990.04 KB
`/transactions`	`73.47 KB` (🟡 +1 B)	1.03 MB
`/transactions/history`	`73.44 KB` (🟡 +1 B)	1.03 MB
`/transactions/messages`	`39.12 KB` (-1 B)	1017.91 KB
`/transactions/msg`	`29.5 KB` (-1 B)	1008.29 KB
`/transactions/queue`	`31.22 KB` (🟡 +1 B)	1010.02 KB
`/transactions/tx`	`21.05 KB` (🟡 +1 B)	999.85 KB
`/welcome`	`6.77 KB` (🟢 -1 B)	985.56 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-12T11:01:51Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	78.38% (+0.46% 🔼)	12158/15512
🔴	Branches	59.09% (+1.06% 🔼)	3141/5316
🟡	Functions	65.9% (+0.81% 🔼)	1927/2924
🟡	Lines	79.77% (+0.39% 🔼)	10973/13756

Show new covered files 🐣

St.^❔	File	Statements	Branches	Functions	Lines
🟢	`...` / index.tsx	100%	100%	100%	100%
🟢	`...` / useRedefine.ts	98.28%	95.24%	100%	100%
🟢	`...` / index.ts	91.67%	70%	100%	91.67%
🔴	`...` / index.tsx	50.82%	2.63%	8.33%	55.56%
🟡	`...` / RedefineHint.tsx	54.55%	0%	0%	60%
🟢	`...` / useCompatibleNetworks.ts	100%	100%	100%	100%
🟡	`...` / RedefineBalanceChange.tsx	71.79%	7.14%	11.11%	71.05%

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🟡	`...` / wallets.ts	68.89% (+4.44% 🔼)	25% (-15% 🔻)	80% (+10% 🔼)	72.22% (+2.78% 🔼)
🔴	`...` / web3.ts	59.09% (-15.91% 🔻)	8.33% (-25% 🔻)	16.67% (-50% 🔻)	52.94% (-20.59% 🔻)
🟡	`...` / transactions.ts	74.19% (-0.17% 🔻)	43.55%	50%	76.3% (-0.17% 🔻)
🟡	`...` / safeContracts.ts	77.27%	62.5% (-7.5% 🔻)	66.67%	72.73%
🟡	`...` / addedSafesSlice.ts	58.62% (-6.9% 🔻)	75%	22.22% (-11.11% 🔻)	63.64% (-4.55% 🔻)
🟢	`...` / hooks.ts	80.83% (-0.2% 🔻)	72.73%	70.59%	80.91% (-0.22% 🔻)
🟢	src/utils/url.ts	85.19% (+2.83% 🔼)	100%	75% (-5% 🔻)	95.65% (+6.37% 🔼)
🟢	`...` / SignMessage.tsx	91.43% (-0.08% 🔻)	81.63% (-0.72% 🔻)	69.23%	92.16% (-0.08% 🔻)
🟢	`...` / config.tsx	86.96% (-0.54% 🔻)	100%	100%	100%
🟢	`...` / index.tsx	100%	86.96% (-1.04% 🔻)	100%	100%
🟢	`...` / index.tsx	95.65% (-0.64% 🔻)	86.36% (-2.1% 🔻)	100%	100%
🔴	`...` / useAllOwnedSafes.ts	38.89% (-44.44% 🔻)	0% (-50% 🔻)	0% (-100% 🔻)	41.18% (-41.18% 🔻)
🟢	`...` / useTxPendingStatuses.ts	82.86% (-0.95% 🔻)	65% (-2.5% 🔻)	75%	92.22% (-1.11% 🔻)
🟡	`...` / index.tsx	78.38% (-7.67% 🔻)	40.63% (+3.48% 🔼)	42.86%	77.14% (-8.22% 🔻)
🟢	`...` / index.tsx	88.17% (-0.13% 🔻)	78.72% (+2.93% 🔼)	50%	87.64% (-0.14% 🔻)
🟡	`...` / CounterfactualForm.tsx	69.44% (-0.56% 🔻)	40% (-2.86% 🔻)	66.67%	69.57% (-0.58% 🔻)
🟢	`...` / tokenTransferParams.ts	96.15% (-0.14% 🔻)	80%	80%	95.65% (-0.18% 🔻)
🟡	`...` / index.tsx	71.7% (+0.54% 🔼)	11.11% (-0.32% 🔻)	66.67%	72.55% (+0.55% 🔼)
🟢	`...` / index.tsx	69.23% (-2.2% 🔻)	100%	0%	81.82% (-1.52% 🔻)
🟢	`...` / index.ts	95.45% (+0.72% 🔼)	66.67% (-8.33% 🔻)	100%	100%
🟡	`...` / index.tsx	66.07% (+4.48% 🔼)	42.86% (-4.31% 🔻)	61.54% (+2.71% 🔼)	66.04% (+4.02% 🔼)
🔴	`...` / index.ts	58.23% (-1.77% 🔻)	31.82%	18.18%	59.42% (-1.39% 🔻)
🟡	`...` / index.tsx	55.56% (-44.44% 🔻)	0% (-100% 🔻)	0% (-100% 🔻)	71.43% (-28.57% 🔻)
🟢	`...` / index.tsx	84.21% (+8.02% 🔼)	16.67% (-33.33% 🔻)	50%	87.5% (+9.72% 🔼)
🔴	`...` / SpeedUpModal.tsx	38.96% (-1.3% 🔻)	0%	0%	39.47% (-1.32% 🔻)
🟡	`...` / ReviewSignMessageOnChain.tsx	74.29% (-0.71% 🔻)	47.62%	83.33%	74.24% (-0.76% 🔻)
🟡	`...` / index.tsx	75.34% (+1.37% 🔼)	58.62% (-1.99% 🔻)	40%	75.36% (+1.45% 🔼)
🔴	`...` / ReviewSpendingLimitTx.tsx	41.1% (-0.57% 🔻)	0%	0%	42.86% (-0.62% 🔻)

Test suite run success

1522 tests passing in 206 suites.

Report generated by 🧪jest coverage report action from ef06e9a

jmealy · 2024-09-12T11:19:17Z

src/features/multichain/hooks/__tests__/useCompatibleNetworks.test.ts

-    }
-  })
-
-  it('should remove already deployed chains from result', () => {


I moved this filtering to outside the hook because I needed to distinguish between

there being no more chains left to deploy to (already deployed on all chains)

there being no chains at all (unsupported safe creation version)

notion-workspace · 2024-09-12T14:32:38Z

Do not offer adding networks for 1.1.1 created Safes

schmanu · 2024-09-12T14:17:51Z

src/features/multichain/components/CreateSafeOnNewChain/index.tsx

+              <Typography variant="body2">Loading Safe data</Typography>
+            </Stack>
+          ) : isUnsupportedSafeCreationVersion ? (
+            <Typography>


Lets wrap this in an ErrorMessage.

schmanu · 2024-09-12T14:40:16Z

src/features/multichain/components/CreateSafeOnNewChain/index.tsx

-  const replayableChains = useReplayableNetworks(safeCreationResult[0], deployedChainIds)
+  const allCompatibleChains = useCompatibleNetworks(safeCreationResult[0])
+  const isUnsupportedSafeCreationVersion = Boolean(!allCompatibleChains?.length)
+  const replayableChains = allCompatibleChains?.filter((config) => !deployedChainIds.includes(config.chainId)) || []


I think we need to memoize this with useMemo

schmanu · 2024-09-12T14:40:40Z

src/features/multichain/components/CreateSafeOnNewChain/index.tsx

@@ -175,18 +198,21 @@ export const CreateSafeOnNewChain = ({
  )

  const safeCreationResult = useSafeCreationData(safeAddress, deployedChains)
-  const replayableChains = useReplayableNetworks(safeCreationResult[0], deployedChainIds)
+  const allCompatibleChains = useCompatibleNetworks(safeCreationResult[0])
+  const isUnsupportedSafeCreationVersion = Boolean(!allCompatibleChains?.length)


This will also be true for zkSync Safes as they have no compatible networks.

For zkSync, allCompatibleChains will just contain zkSync (because it's before you filter out the chains it's already deployed on). I tested it out just now and it behaves the same as it does on the epic branch.
But I think this speaks to the fact that this block is not super readable, and it's unclear what allCompatibleChains means. Any suggestions?

Also, I created a ticket for when there are no available chains to deploy to (like with zkSync): https://www.notion.so/safe-global/76f265123c0a4ad680947ce489804342?v=f9f164524ec4464286fdc38f31589d0c&p=4ec720b334ea410bbe8abb083509bf7b&pm=s

Right now it just shows the network selector with no options

Okay we can keep it as is then.

liliya-soroka · 2024-09-13T11:09:26Z

@jmealy , could you re-check "add network" in side menu

I am getting red error instead of text from the ticket ( 1.1.1 safe on Gnosis - gno:0x0A2f9630369120dbb021516889c96cdf80F94844 )

In network selector , I am also getting wrong text time to time

Or I am getting the next error too ( gno:0x0d6B27AfAA6Bbdd44300a68F89970604Cbe479ca)

liliya-soroka · 2024-09-13T11:19:32Z

In the PR , I am also getting the error for 1.3.0L2 Gnosis safe (gno:0x62AB1CEF969F9459E8fFbb5E0E8289f4101ace1c) -

Screen.Recording.2024-09-13.at.13.17.45.mov

schmanu · 2024-09-16T08:26:25Z

@liliya-soroka All errors are expected.

The Gnosis RPC can be super flaky. Often it returns that the tx hash does not exist. Thats why the error says the transaction could not be found.
Same here.
The Safe you are trying to replay here was not created with create2 (createProxyWithNonce) it was created by calling the createProxy method. So the error correctly states that the Safe was created with an unsupported method.

github-actions · 2024-09-17T14:40:10Z

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

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

jmealy added 3 commits September 11, 2024 17:32

Feat: do not allow adding additional chains for unsupported safe crea…

dedab1e

…tion versions

fix: update useReplayableNetworks unit tests

8a8c246

Merge branch 'epic/multichain-safes' into feat/older-safes-disable-mu…

8806d0b

…ltichain

jmealy force-pushed the feat/older-safes-disable-multichain branch from fc5b23f to d74bfb4 Compare September 12, 2024 11:05

tests: update useCompatibleNetworks test

4ca5876

jmealy force-pushed the feat/older-safes-disable-multichain branch from d74bfb4 to 4ca5876 Compare September 12, 2024 11:16

jmealy commented Sep 12, 2024

View reviewed changes

jmealy requested a review from schmanu September 12, 2024 11:19

jmealy marked this pull request as ready for review September 12, 2024 11:19

jmealy changed the title ~~[Multichain] Feat: disable adding new chains to safes created from old mastercopy versions~~ [Multichain] Feat: disable adding new chains to safes created from old mastercopy versions [SW-167] Sep 12, 2024

schmanu reviewed Sep 12, 2024

View reviewed changes

fix: review issues

ef06e9a

schmanu self-requested a review September 17, 2024 14:47

schmanu approved these changes Sep 17, 2024

View reviewed changes

schmanu merged commit 4f5e9bf into epic/multichain-safes Sep 17, 2024
14 checks passed

schmanu deleted the feat/older-safes-disable-multichain branch September 17, 2024 14:47

github-actions bot locked and limited conversation to collaborators Sep 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Multichain] Feat: disable adding new chains to safes created from old mastercopy versions [SW-167] #4161

[Multichain] Feat: disable adding new chains to safes created from old mastercopy versions [SW-167] #4161

jmealy commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

jmealy Sep 12, 2024

notion-workspace bot commented Sep 12, 2024

schmanu Sep 12, 2024

schmanu Sep 12, 2024

schmanu Sep 12, 2024

jmealy Sep 12, 2024

jmealy Sep 12, 2024 •

edited

Loading

schmanu Sep 13, 2024

liliya-soroka commented Sep 13, 2024 •

edited

Loading

liliya-soroka commented Sep 13, 2024

schmanu commented Sep 16, 2024

github-actions bot commented Sep 17, 2024

[Multichain] Feat: disable adding new chains to safes created from old mastercopy versions [SW-167] #4161

[Multichain] Feat: disable adding new chains to safes created from old mastercopy versions [SW-167] #4161

Conversation

jmealy commented Sep 12, 2024 • edited Loading

What it solves

How this PR fixes it

How to test it

Screenshots

Checklist

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

Branch preview

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

ESLint Summary View Full Report

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

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

⚠️ Global Bundle Size Increased

Nineteen Pages Changed Size

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

Coverage report

Test suite run success

jmealy Sep 12, 2024

Choose a reason for hiding this comment

notion-workspace bot commented Sep 12, 2024

schmanu Sep 12, 2024

Choose a reason for hiding this comment

schmanu Sep 12, 2024

Choose a reason for hiding this comment

schmanu Sep 12, 2024

Choose a reason for hiding this comment

jmealy Sep 12, 2024

Choose a reason for hiding this comment

jmealy Sep 12, 2024 • edited Loading

Choose a reason for hiding this comment

schmanu Sep 13, 2024

Choose a reason for hiding this comment

liliya-soroka commented Sep 13, 2024 • edited Loading

liliya-soroka commented Sep 13, 2024

schmanu commented Sep 16, 2024

github-actions bot commented Sep 17, 2024

ESLint Summary View Full Report

jmealy commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

jmealy Sep 12, 2024 •

edited

Loading

liliya-soroka commented Sep 13, 2024 •

edited

Loading