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] 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),