From e441edc56be0328dbf2d2fe07ea1b7ed8761c50f Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Mon, 23 Dec 2024 14:37:06 +0300 Subject: [PATCH 1/4] Fix: do not show queue warning for 1.3.0+ upgrades --- .../tx/confirmation-views/UpdateSafe/index.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.tsx b/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.tsx index 6dec5f3949..9a8af7605a 100644 --- a/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.tsx +++ b/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.tsx @@ -1,5 +1,6 @@ import type { ReactNode } from 'react' import { Alert, AlertTitle, Box, Divider, Stack, Typography } from '@mui/material' +import semverSatisfies from 'semver/functions/satisfies' import { LATEST_SAFE_VERSION } from '@/config/constants' import { useCurrentChain } from '@/hooks/useChains' import useSafeInfo from '@/hooks/useSafeInfo' @@ -7,6 +8,8 @@ import { useQueuedTxsLength } from '@/hooks/useTxQueue' import ExternalLink from '@/components/common/ExternalLink' import { maybePlural } from '@/utils/formatters' +const QUEUE_WARNING_VERSION = '<1.3.0' + function BgBox({ children, light }: { children: ReactNode; light?: boolean }) { return ( - Current version: {safe.version} + Current version: {safeVersion} → New version: {latestSafeVersion} - + Read about the updates in the new Safe contracts version in the{' '} version {latestSafeVersion} changelog - {queueSize && ( + {showQueueWarning && ( This upgrade will invalidate all queued transactions! You have {queueSize} unexecuted transaction{maybePlural(parseInt(queueSize))}. Please make sure to execute or @@ -52,7 +57,7 @@ function UpdateSafe() { )} - + ) } From 66cd67b37c1586da4778dbb6898bf9a516e736b6 Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Mon, 23 Dec 2024 14:53:03 +0300 Subject: [PATCH 2/4] Add tests --- .../transactions/TxSummary/styles.module.css | 6 ++--- .../UpdateSafe/index.test.tsx | 26 +++++++++++++++++++ .../confirmation-views/UpdateSafe/index.tsx | 26 +++++++++++++++---- 3 files changed, 50 insertions(+), 8 deletions(-) create mode 100644 apps/web/src/components/tx/confirmation-views/UpdateSafe/index.test.tsx diff --git a/apps/web/src/components/transactions/TxSummary/styles.module.css b/apps/web/src/components/transactions/TxSummary/styles.module.css index fc8ed69fac..bba12e11e3 100644 --- a/apps/web/src/components/transactions/TxSummary/styles.module.css +++ b/apps/web/src/components/transactions/TxSummary/styles.module.css @@ -29,9 +29,9 @@ } .gridContainer.conflictGroup { - grid-template-columns: var(--grid-type) var(--grid-info) var(--grid-date) var(--grid-confirmations) var(--grid-status) var( - --grid-actions - ); + grid-template-columns: + var(--grid-type) var(--grid-info) var(--grid-date) var(--grid-confirmations) var(--grid-status) + var(--grid-actions); grid-template-areas: 'type info date confirmations status actions'; } diff --git a/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.test.tsx b/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.test.tsx new file mode 100644 index 0000000000..b3989d8570 --- /dev/null +++ b/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.test.tsx @@ -0,0 +1,26 @@ +import type { ChainInfo } from '@safe-global/safe-gateway-typescript-sdk' +import { _UpdateSafe as UpdateSafe } from './index' +import { render } from '@/tests/test-utils' + +const chain = { + recommendedMasterCopyVersion: '1.4.1', +} as ChainInfo + +const warningText = 'This upgrade will invalidate all queued transactions!' + +describe('Container', () => { + it('renders correctly with a queue warning', async () => { + const container = render() + await expect(container.findByText(warningText)).resolves.not.toBeNull() + }) + + it('renders correctly without a queue warning because no queue', async () => { + const container = render() + await expect(container.findByText(warningText)).rejects.toThrowError(Error) + }) + + it('renders correctly without a queue warning because of compatible Safe version', async () => { + const container = render() + await expect(container.findByText(warningText)).rejects.toThrowError(Error) + }) +}) diff --git a/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.tsx b/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.tsx index 9a8af7605a..520b6be0fd 100644 --- a/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.tsx +++ b/apps/web/src/components/tx/confirmation-views/UpdateSafe/index.tsx @@ -7,6 +7,7 @@ import useSafeInfo from '@/hooks/useSafeInfo' import { useQueuedTxsLength } from '@/hooks/useTxQueue' import ExternalLink from '@/components/common/ExternalLink' import { maybePlural } from '@/utils/formatters' +import madProps from '@/utils/mad-props' const QUEUE_WARNING_VERSION = '<1.3.0' @@ -26,11 +27,15 @@ function BgBox({ children, light }: { children: ReactNode; light?: boolean }) { ) } -function UpdateSafe() { - const { safe } = useSafeInfo() - const chain = useCurrentChain() - const queueSize = useQueuedTxsLength() - const safeVersion = safe?.version || '' +export function _UpdateSafe({ + safeVersion, + queueSize, + chain, +}: { + safeVersion: string + queueSize: string + chain: ReturnType +}) { const showQueueWarning = queueSize && semverSatisfies(safeVersion, QUEUE_WARNING_VERSION) const latestSafeVersion = chain?.recommendedMasterCopyVersion || LATEST_SAFE_VERSION @@ -62,4 +67,15 @@ function UpdateSafe() { ) } +function useSafeVersion() { + const { safe } = useSafeInfo() + return safe?.version || '' +} + +const UpdateSafe = madProps(_UpdateSafe, { + chain: useCurrentChain, + safeVersion: useSafeVersion, + queueSize: useQueuedTxsLength, +}) + export default UpdateSafe From ecf6f5b06f03a707a314fb99263d34ee6d37c716 Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Tue, 24 Dec 2024 09:51:09 +0300 Subject: [PATCH 3/4] Preserve fallback handler --- apps/web/src/services/tx/safeUpdateParams.ts | 2 +- apps/web/src/utils/safe-migrations.ts | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/apps/web/src/services/tx/safeUpdateParams.ts b/apps/web/src/services/tx/safeUpdateParams.ts index 36fc3b7ef1..c76416d08c 100644 --- a/apps/web/src/services/tx/safeUpdateParams.ts +++ b/apps/web/src/services/tx/safeUpdateParams.ts @@ -37,7 +37,7 @@ export const createUpdateSafeTxs = async (safe: SafeInfo, chain: ChainInfo): Pro // 1.3.0 Safes are updated using a delegate call to a migration contract if (semverSatisfies(safe.version, '1.3.0')) { - return [createUpdateMigration(chain)] + return [createUpdateMigration(chain, safe.fallbackHandler?.value)] } // For older Safes, we need to create two transactions diff --git a/apps/web/src/utils/safe-migrations.ts b/apps/web/src/utils/safe-migrations.ts index beffec8694..45f08451ae 100644 --- a/apps/web/src/utils/safe-migrations.ts +++ b/apps/web/src/utils/safe-migrations.ts @@ -95,7 +95,7 @@ export const prependSafeToL2Migration = ( return __unsafe_createMultiSendTx(newTxs) } -export const createUpdateMigration = (chain: ChainInfo): MetaTransactionData => { +export const createUpdateMigration = (chain: ChainInfo, fallbackHandler?: string): MetaTransactionData => { const interfce = Safe_migration__factory.createInterface() const deployment = getSafeMigrationDeployment({ @@ -108,11 +108,19 @@ export const createUpdateMigration = (chain: ChainInfo): MetaTransactionData => throw new Error('Migration deployment not found') } + const method = ( + fallbackHandler + ? chain.l2 + ? 'migrateL2Singleton' + : 'migrateSingleton' + : chain.l2 + ? 'migrateL2WithFallbackHandler' + : 'migrateWithFallbackHandler' + ) as 'migrateSingleton' // apease typescript + const tx: MetaTransactionData = { - operation: OperationType.DelegateCall, // DELEGATE CALL REQUIRED - data: chain.l2 - ? interfce.encodeFunctionData('migrateL2WithFallbackHandler') - : interfce.encodeFunctionData('migrateWithFallbackHandler'), + operation: OperationType.DelegateCall, // delegate call required + data: interfce.encodeFunctionData(method), to: deployment.defaultAddress, value: '0', } From d41f64ddb44dd51be70e2a4af5fb9c69c23bbc9d Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Tue, 24 Dec 2024 12:06:27 +0300 Subject: [PATCH 4/4] Overwrite default fallback handler + add tests --- apps/web/.gitignore | 61 ------------------- apps/web/src/services/tx/safeUpdateParams.ts | 2 +- .../utils/__tests__/safe-migrations.test.ts | 42 +++++++++++-- apps/web/src/utils/safe-migrations.ts | 29 +++++++-- 4 files changed, 62 insertions(+), 72 deletions(-) delete mode 100644 apps/web/.gitignore diff --git a/apps/web/.gitignore b/apps/web/.gitignore deleted file mode 100644 index ade4b1731e..0000000000 --- a/apps/web/.gitignore +++ /dev/null @@ -1,61 +0,0 @@ -# See https://help.github.com/articles/ignoring-files/ for more about ignoring files. - -# dependencies -/node_modules -/.pnp -.pnp.js - -# testing -/coverage - -# types -/src/types/contracts/ - -# next.js -/.next/ -/out/ - -# production -/build - -# misc -.DS_Store -*.pem -.idea - -# debug -npm-debug.log* -yarn-debug.log* -yarn-error.log* -.pnpm-debug.log* - -# local env files -.env*.local - -# vercel -.vercel - -# typescript -*.tsbuildinfo - -# yalc -.yalc -yalc.lock -.env - -/cypress/videos -/cypress/screenshots -/cypress/downloads - -/public/sw.js -/public/sw.js.map -/public/worker-*.js -/public/workbox-*.js -/public/workbox-*.js.map -/public/fallback* -/public/*.js.LICENSE.txt -certificates -*storybook.log - -# Yarn v4 -.yarn/* diff --git a/apps/web/src/services/tx/safeUpdateParams.ts b/apps/web/src/services/tx/safeUpdateParams.ts index c76416d08c..d90c71b0d3 100644 --- a/apps/web/src/services/tx/safeUpdateParams.ts +++ b/apps/web/src/services/tx/safeUpdateParams.ts @@ -37,7 +37,7 @@ export const createUpdateSafeTxs = async (safe: SafeInfo, chain: ChainInfo): Pro // 1.3.0 Safes are updated using a delegate call to a migration contract if (semverSatisfies(safe.version, '1.3.0')) { - return [createUpdateMigration(chain, safe.fallbackHandler?.value)] + return [createUpdateMigration(chain, safe.version, safe.fallbackHandler?.value)] } // For older Safes, we need to create two transactions diff --git a/apps/web/src/utils/__tests__/safe-migrations.test.ts b/apps/web/src/utils/__tests__/safe-migrations.test.ts index 73a447d5aa..478b346ee3 100644 --- a/apps/web/src/utils/__tests__/safe-migrations.test.ts +++ b/apps/web/src/utils/__tests__/safe-migrations.test.ts @@ -325,7 +325,7 @@ describe('extractMigrationL2MasterCopyAddress', () => { } as unknown as ChainInfo it('should create a migration transaction for L1 chain', () => { - const result = createUpdateMigration(mockChain) + const result = createUpdateMigration(mockChain, '1.3.0') expect(result).toEqual({ operation: OperationType.DelegateCall, @@ -336,8 +336,8 @@ describe('extractMigrationL2MasterCopyAddress', () => { }) it('should create a migration transaction for L2 chain', () => { - const l2Chain = { ...mockChain, l2: true } - const result = createUpdateMigration(l2Chain) + const l2Chain = { ...mockChain, chainId: '137', l2: true } + const result = createUpdateMigration(l2Chain, '1.3.0+L2') expect(result).toEqual({ operation: OperationType.DelegateCall, @@ -348,7 +348,41 @@ describe('extractMigrationL2MasterCopyAddress', () => { }) it('should throw an error if deployment is not found', () => { - expect(() => createUpdateMigration(mockChainOld)).toThrow('Migration deployment not found') + expect(() => createUpdateMigration(mockChainOld, '1.1.1')).toThrow('Migration deployment not found') + }) + + it('should overwrite fallback handler if it is the default one', () => { + const result = createUpdateMigration(mockChain, '1.3.0', '0xf48f2B2d2a534e402487b3ee7C18c33Aec0Fe5e4') // 1.3.0 compatibility fallback handler + + expect(result).toEqual({ + operation: OperationType.DelegateCall, + data: '0xed007fc6', + to: '0x526643F69b81B008F46d95CD5ced5eC0edFFDaC6', + value: '0', + }) + }) + + it('should overwrite L2 fallback handler if it is the default one', () => { + const l2Chain = { ...mockChain, chainId: '137', l2: true } + const result = createUpdateMigration(l2Chain, '1.3.0+L2', '0xf48f2B2d2a534e402487b3ee7C18c33Aec0Fe5e4') // 1.3.0 compatibility fallback handler + + expect(result).toEqual({ + operation: OperationType.DelegateCall, + data: '0x68cb3d94', + to: '0x526643F69b81B008F46d95CD5ced5eC0edFFDaC6', + value: '0', + }) + }) + + it('should NOT overwrite a custom fallback handler', () => { + const result = createUpdateMigration(mockChain, '1.3.0', '0x526643F69b81B008F46d95CD5ced5eC0edFFDaC6') + + expect(result).toEqual({ + operation: OperationType.DelegateCall, + data: '0xf6682ab0', + to: '0x526643F69b81B008F46d95CD5ced5eC0edFFDaC6', + value: '0', + }) }) }) }) diff --git a/apps/web/src/utils/safe-migrations.ts b/apps/web/src/utils/safe-migrations.ts index 45f08451ae..f9c1b6c7d6 100644 --- a/apps/web/src/utils/safe-migrations.ts +++ b/apps/web/src/utils/safe-migrations.ts @@ -1,9 +1,15 @@ import { Safe_to_l2_migration__factory, Safe_migration__factory } from '@/types/contracts' +import { getCompatibilityFallbackHandlerDeployments } from '@safe-global/safe-deployments' import { type ExtendedSafeInfo } from '@/store/safeInfoSlice' -import { getSafeContractDeployment } from '@/services/contracts/deployments' +import { getSafeContractDeployment, hasMatchingDeployment } from '@/services/contracts/deployments' import { sameAddress } from './addresses' import { getSafeToL2MigrationDeployment, getSafeMigrationDeployment } from '@safe-global/safe-deployments' -import { type MetaTransactionData, OperationType, type SafeTransaction } from '@safe-global/safe-core-sdk-types' +import { + type MetaTransactionData, + OperationType, + type SafeTransaction, + type SafeVersion, +} from '@safe-global/safe-core-sdk-types' import type { ChainInfo } from '@safe-global/safe-gateway-typescript-sdk' import { isValidMasterCopy } from '@/services/contracts/safeContracts' import { isMultiSendCalldata } from './transaction-calldata' @@ -95,9 +101,11 @@ export const prependSafeToL2Migration = ( return __unsafe_createMultiSendTx(newTxs) } -export const createUpdateMigration = (chain: ChainInfo, fallbackHandler?: string): MetaTransactionData => { - const interfce = Safe_migration__factory.createInterface() - +export const createUpdateMigration = ( + chain: ChainInfo, + safeVersion: string, + fallbackHandler?: string, +): MetaTransactionData => { const deployment = getSafeMigrationDeployment({ version: chain.recommendedMasterCopyVersion || LATEST_SAFE_VERSION, released: true, @@ -108,8 +116,15 @@ export const createUpdateMigration = (chain: ChainInfo, fallbackHandler?: string throw new Error('Migration deployment not found') } + // Keep fallback handler if it's not a default one + const keepFallbackHandler = + !!fallbackHandler && + !hasMatchingDeployment(getCompatibilityFallbackHandlerDeployments, fallbackHandler, chain.chainId, [ + safeVersion as SafeVersion, + ]) + const method = ( - fallbackHandler + keepFallbackHandler ? chain.l2 ? 'migrateL2Singleton' : 'migrateSingleton' @@ -118,6 +133,8 @@ export const createUpdateMigration = (chain: ChainInfo, fallbackHandler?: string : 'migrateWithFallbackHandler' ) as 'migrateSingleton' // apease typescript + const interfce = Safe_migration__factory.createInterface() + const tx: MetaTransactionData = { operation: OperationType.DelegateCall, // delegate call required data: interfce.encodeFunctionData(method),