From 357fc854033d93a3eaa8ce0ee57682046708aeb4 Mon Sep 17 00:00:00 2001 From: Manuel Gellfart Date: Mon, 23 Sep 2024 16:36:39 +0200 Subject: [PATCH] fix(Multichain): add more unsupported Safe cases (#4233) - do not support adding networks to Safes with unknown fallback handlers and Safe creations with reimbursements --- .../__tests__/useCompatibleNetworks.test.ts | 23 +-- .../__tests__/useSafeCreationData.test.ts | 132 ++++++++++++++++-- .../multichain/hooks/useCompatibleNetworks.ts | 40 ++---- .../multichain/hooks/useSafeCreationData.ts | 30 +++- src/services/contracts/deployments.ts | 26 ++++ 5 files changed, 198 insertions(+), 53 deletions(-) diff --git a/src/features/multichain/hooks/__tests__/useCompatibleNetworks.test.ts b/src/features/multichain/hooks/__tests__/useCompatibleNetworks.test.ts index e03a8f392e..9aa0f7a208 100644 --- a/src/features/multichain/hooks/__tests__/useCompatibleNetworks.test.ts +++ b/src/features/multichain/hooks/__tests__/useCompatibleNetworks.test.ts @@ -2,7 +2,6 @@ import { renderHook } from '@/tests/test-utils' import { useCompatibleNetworks } from '../useCompatibleNetworks' import { type ReplayedSafeProps } from '@/store/slices' import { faker } from '@faker-js/faker' -import { Safe__factory } from '@/types/contracts' import { EMPTY_DATA, ZERO_ADDRESS } from '@safe-global/protocol-kit/dist/src/utils/constants' import { ECOSYSTEM_ID_ADDRESS } from '@/config/constants' import { chainBuilder } from '@/tests/builders/chains' @@ -10,11 +9,10 @@ import { getSafeSingletonDeployments, getSafeL2SingletonDeployments, getProxyFactoryDeployments, + getCompatibilityFallbackHandlerDeployments, } from '@safe-global/safe-deployments' import * as useChains from '@/hooks/useChains' -const safeInterface = Safe__factory.createInterface() - const L1_111_MASTERCOPY_DEPLOYMENTS = getSafeSingletonDeployments({ version: '1.1.1' })?.deployments const L1_130_MASTERCOPY_DEPLOYMENTS = getSafeSingletonDeployments({ version: '1.3.0' })?.deployments const L1_141_MASTERCOPY_DEPLOYMENTS = getSafeSingletonDeployments({ version: '1.4.1' })?.deployments @@ -26,6 +24,9 @@ const PROXY_FACTORY_111_DEPLOYMENTS = getProxyFactoryDeployments({ version: '1.1 const PROXY_FACTORY_130_DEPLOYMENTS = getProxyFactoryDeployments({ version: '1.3.0' })?.deployments const PROXY_FACTORY_141_DEPLOYMENTS = getProxyFactoryDeployments({ version: '1.4.1' })?.deployments +const FALLBACK_HANDLER_130_DEPLOYMENTS = getCompatibilityFallbackHandlerDeployments({ version: '1.3.0' })?.deployments +const FALLBACK_HANDLER_141_DEPLOYMENTS = getCompatibilityFallbackHandlerDeployments({ version: '1.4.1' })?.deployments + describe('useCompatibleNetworks', () => { beforeAll(() => { jest.spyOn(useChains, 'default').mockReturnValue({ @@ -44,7 +45,7 @@ describe('useCompatibleNetworks', () => { expect(result.current).toHaveLength(0) }) - it('should set available to false for unknown masterCopies', () => { + it('should set available to false for unknown contracts', () => { const callData = { owners: [faker.finance.ethereumAddress()], threshold: 1, @@ -73,7 +74,7 @@ describe('useCompatibleNetworks', () => { threshold: 1, to: ZERO_ADDRESS, data: EMPTY_DATA, - fallbackHandler: faker.finance.ethereumAddress(), + fallbackHandler: FALLBACK_HANDLER_141_DEPLOYMENTS?.canonical?.address!, paymentToken: ZERO_ADDRESS, payment: 0, paymentReceiver: ECOSYSTEM_ID_ADDRESS, @@ -107,13 +108,13 @@ describe('useCompatibleNetworks', () => { } }) - it('should mark already deployed chains as not available', () => { + it('should mark compatible chains as available', () => { const callData = { owners: [faker.finance.ethereumAddress()], threshold: 1, to: ZERO_ADDRESS, data: EMPTY_DATA, - fallbackHandler: faker.finance.ethereumAddress(), + fallbackHandler: ZERO_ADDRESS, paymentToken: ZERO_ADDRESS, payment: 0, paymentReceiver: ECOSYSTEM_ID_ADDRESS, @@ -125,7 +126,7 @@ describe('useCompatibleNetworks', () => { factoryAddress: PROXY_FACTORY_130_DEPLOYMENTS?.canonical?.address!, masterCopy: L1_130_MASTERCOPY_DEPLOYMENTS?.canonical?.address!, saltNonce: '0', - safeAccountConfig: callData, + safeAccountConfig: { ...callData, fallbackHandler: FALLBACK_HANDLER_130_DEPLOYMENTS?.canonical?.address! }, safeVersion: '1.3.0', } const { result } = renderHook(() => useCompatibleNetworks(creationData)) @@ -140,7 +141,7 @@ describe('useCompatibleNetworks', () => { factoryAddress: PROXY_FACTORY_130_DEPLOYMENTS?.canonical?.address!, masterCopy: L2_130_MASTERCOPY_DEPLOYMENTS?.canonical?.address!, saltNonce: '0', - safeAccountConfig: callData, + safeAccountConfig: { ...callData, fallbackHandler: FALLBACK_HANDLER_130_DEPLOYMENTS?.canonical?.address! }, safeVersion: '1.3.0', } const { result } = renderHook(() => useCompatibleNetworks(creationData)) @@ -155,7 +156,7 @@ describe('useCompatibleNetworks', () => { factoryAddress: PROXY_FACTORY_130_DEPLOYMENTS?.eip155?.address!, masterCopy: L1_130_MASTERCOPY_DEPLOYMENTS?.eip155?.address!, saltNonce: '0', - safeAccountConfig: callData, + safeAccountConfig: { ...callData, fallbackHandler: FALLBACK_HANDLER_130_DEPLOYMENTS?.eip155?.address! }, safeVersion: '1.3.0', } const { result } = renderHook(() => useCompatibleNetworks(creationData)) @@ -170,7 +171,7 @@ describe('useCompatibleNetworks', () => { factoryAddress: PROXY_FACTORY_130_DEPLOYMENTS?.eip155?.address!, masterCopy: L2_130_MASTERCOPY_DEPLOYMENTS?.eip155?.address!, saltNonce: '0', - safeAccountConfig: callData, + safeAccountConfig: { ...callData, fallbackHandler: FALLBACK_HANDLER_130_DEPLOYMENTS?.eip155?.address! }, safeVersion: '1.3.0', } const { result } = renderHook(() => useCompatibleNetworks(creationData)) diff --git a/src/features/multichain/hooks/__tests__/useSafeCreationData.test.ts b/src/features/multichain/hooks/__tests__/useSafeCreationData.test.ts index 8eec258755..e26704e627 100644 --- a/src/features/multichain/hooks/__tests__/useSafeCreationData.test.ts +++ b/src/features/multichain/hooks/__tests__/useSafeCreationData.test.ts @@ -151,7 +151,46 @@ describe('useSafeCreationData', () => { }) }) - it('should throw error if RPC could not be created', async () => { + it('should throw error if outdated masterCopy is being used', async () => { + const setupData = Safe__factory.createInterface().encodeFunctionData('setup', [ + [faker.finance.ethereumAddress(), faker.finance.ethereumAddress()], + 1, + faker.finance.ethereumAddress(), + faker.string.hexadecimal({ length: 64 }), + faker.finance.ethereumAddress(), + faker.finance.ethereumAddress(), + 0, + faker.finance.ethereumAddress(), + ]) + + jest.spyOn(cgwSdk, 'getCreationTransaction').mockResolvedValue({ + data: { + created: new Date(Date.now()).toISOString(), + creator: faker.finance.ethereumAddress(), + factoryAddress: faker.finance.ethereumAddress(), + transactionHash: faker.string.hexadecimal({ length: 64 }), + masterCopy: getSafeSingletonDeployment({ version: '1.1.1' })?.defaultAddress, + setupData, + }, + response: new Response(), + }) + + const safeAddress = faker.finance.ethereumAddress() + const chainInfos = [chainBuilder().with({ chainId: '1', l2: false }).build()] + + // Run hook + const { result } = renderHook(() => useSafeCreationData(safeAddress, chainInfos)) + + await waitFor(() => { + expect(result.current).toEqual([ + undefined, + new Error(SAFE_CREATION_DATA_ERRORS.UNSUPPORTED_IMPLEMENTATION), + false, + ]) + }) + }) + + it('should throw error if unknown masterCopy is being used', async () => { const setupData = Safe__factory.createInterface().encodeFunctionData('setup', [ [faker.finance.ethereumAddress(), faker.finance.ethereumAddress()], 1, @@ -175,6 +214,80 @@ describe('useSafeCreationData', () => { response: new Response(), }) + const safeAddress = faker.finance.ethereumAddress() + const chainInfos = [chainBuilder().with({ chainId: '1', l2: false }).build()] + + // Run hook + const { result } = renderHook(() => useSafeCreationData(safeAddress, chainInfos)) + + await waitFor(() => { + expect(result.current).toEqual([ + undefined, + new Error(SAFE_CREATION_DATA_ERRORS.UNSUPPORTED_IMPLEMENTATION), + false, + ]) + }) + }) + + it('should throw error if the Safe creation uses reimbursement', async () => { + const setupData = Safe__factory.createInterface().encodeFunctionData('setup', [ + [faker.finance.ethereumAddress(), faker.finance.ethereumAddress()], + 1, + faker.finance.ethereumAddress(), + faker.string.hexadecimal({ length: 64 }), + faker.finance.ethereumAddress(), + faker.finance.ethereumAddress(), + 420, + faker.finance.ethereumAddress(), + ]) + + jest.spyOn(cgwSdk, 'getCreationTransaction').mockResolvedValue({ + data: { + created: new Date(Date.now()).toISOString(), + creator: faker.finance.ethereumAddress(), + factoryAddress: faker.finance.ethereumAddress(), + transactionHash: faker.string.hexadecimal({ length: 64 }), + masterCopy: getSafeSingletonDeployment({ version: '1.3.0' })?.defaultAddress, + setupData, + }, + response: new Response(), + }) + + const safeAddress = faker.finance.ethereumAddress() + const chainInfos = [chainBuilder().with({ chainId: '1', l2: false }).build()] + + // Run hook + const { result } = renderHook(() => useSafeCreationData(safeAddress, chainInfos)) + + await waitFor(() => { + expect(result.current).toEqual([undefined, new Error(SAFE_CREATION_DATA_ERRORS.PAYMENT_SAFE), false]) + }) + }) + + it('should throw error if RPC could not be created', async () => { + const setupData = Safe__factory.createInterface().encodeFunctionData('setup', [ + [faker.finance.ethereumAddress(), faker.finance.ethereumAddress()], + 1, + faker.finance.ethereumAddress(), + faker.string.hexadecimal({ length: 64 }), + faker.finance.ethereumAddress(), + ZERO_ADDRESS, + 0, + faker.finance.ethereumAddress(), + ]) + + jest.spyOn(cgwSdk, 'getCreationTransaction').mockResolvedValue({ + data: { + created: new Date(Date.now()).toISOString(), + creator: faker.finance.ethereumAddress(), + factoryAddress: faker.finance.ethereumAddress(), + transactionHash: faker.string.hexadecimal({ length: 64 }), + masterCopy: getSafeSingletonDeployment({ version: '1.3.0' })?.defaultAddress, + setupData, + }, + response: new Response(), + }) + jest.spyOn(web3, 'createWeb3ReadOnly').mockReturnValue(undefined) const safeAddress = faker.finance.ethereumAddress() @@ -195,14 +308,15 @@ describe('useSafeCreationData', () => { faker.finance.ethereumAddress(), faker.string.hexadecimal({ length: 64 }), faker.finance.ethereumAddress(), - faker.finance.ethereumAddress(), + ZERO_ADDRESS, 0, faker.finance.ethereumAddress(), ]) const mockTxHash = faker.string.hexadecimal({ length: 64 }) const mockFactoryAddress = faker.finance.ethereumAddress() - const mockMasterCopyAddress = faker.finance.ethereumAddress() + const mockMasterCopyAddress = getSafeSingletonDeployment({ version: '1.3.0' })?.defaultAddress + jest.spyOn(cgwSdk, 'getCreationTransaction').mockResolvedValue({ data: { created: new Date(Date.now()).toISOString(), @@ -237,14 +351,14 @@ describe('useSafeCreationData', () => { faker.finance.ethereumAddress(), faker.string.hexadecimal({ length: 64 }), faker.finance.ethereumAddress(), - faker.finance.ethereumAddress(), + ZERO_ADDRESS, 0, faker.finance.ethereumAddress(), ]) const mockTxHash = faker.string.hexadecimal({ length: 64 }) const mockFactoryAddress = faker.finance.ethereumAddress() - const mockMasterCopyAddress = faker.finance.ethereumAddress() + const mockMasterCopyAddress = getSafeSingletonDeployment({ version: '1.3.0' })?.defaultAddress! jest.spyOn(cgwSdk, 'getCreationTransaction').mockResolvedValue({ data: { created: new Date(Date.now()).toISOString(), @@ -291,7 +405,7 @@ describe('useSafeCreationData', () => { faker.finance.ethereumAddress(), faker.string.hexadecimal({ length: 64 }), faker.finance.ethereumAddress(), - faker.finance.ethereumAddress(), + ZERO_ADDRESS, 0, faker.finance.ethereumAddress(), ]) @@ -309,7 +423,7 @@ describe('useSafeCreationData', () => { const mockTxHash = faker.string.hexadecimal({ length: 64 }) const mockFactoryAddress = faker.finance.ethereumAddress() - const mockMasterCopyAddress = faker.finance.ethereumAddress() + const mockMasterCopyAddress = getSafeSingletonDeployment({ version: '1.3.0' })?.defaultAddress! jest.spyOn(cgwSdk, 'getCreationTransaction').mockResolvedValue({ data: { created: new Date(Date.now()).toISOString(), @@ -356,14 +470,14 @@ describe('useSafeCreationData', () => { faker.finance.ethereumAddress(), faker.string.hexadecimal({ length: 64, casing: 'lower' }), faker.finance.ethereumAddress(), - faker.finance.ethereumAddress(), + ZERO_ADDRESS, 0, faker.finance.ethereumAddress(), ]) const mockTxHash = faker.string.hexadecimal({ length: 64 }) const mockFactoryAddress = faker.finance.ethereumAddress() - const mockMasterCopyAddress = faker.finance.ethereumAddress() + const mockMasterCopyAddress = getSafeSingletonDeployment({ version: '1.3.0' })?.defaultAddress! jest.spyOn(cgwSdk, 'getCreationTransaction').mockResolvedValue({ data: { created: new Date(Date.now()).toISOString(), diff --git a/src/features/multichain/hooks/useCompatibleNetworks.ts b/src/features/multichain/hooks/useCompatibleNetworks.ts index 6e59a72fc0..ded3c949c4 100644 --- a/src/features/multichain/hooks/useCompatibleNetworks.ts +++ b/src/features/multichain/hooks/useCompatibleNetworks.ts @@ -1,9 +1,9 @@ import { type ReplayedSafeProps } from '@/features/counterfactual/store/undeployedSafesSlice' import useChains from '@/hooks/useChains' -import { sameAddress } from '@/utils/addresses' +import { hasMatchingDeployment } from '@/services/contracts/deployments' import { type SafeVersion } from '@safe-global/safe-core-sdk-types' import { - type SingletonDeploymentV2, + getCompatibilityFallbackHandlerDeployments, getProxyFactoryDeployments, getSafeL2SingletonDeployments, getSafeSingletonDeployments, @@ -12,16 +12,6 @@ import type { ChainInfo } from '@safe-global/safe-gateway-typescript-sdk' const SUPPORTED_VERSIONS: SafeVersion[] = ['1.4.1', '1.3.0'] -const hasDeployment = (chainId: string, contractAddress: string, deployments: SingletonDeploymentV2[]) => { - return deployments.some((deployment) => { - // Check that deployment contains the contract Address on given chain - const networkDeployments = deployment.networkAddresses[chainId] - return Array.isArray(networkDeployments) - ? networkDeployments.some((networkDeployment) => sameAddress(networkDeployment, contractAddress)) - : sameAddress(networkDeployments, contractAddress) - }) -} - /** * Returns all chains where the creations's masterCopy and factory are deployed. * @param creation @@ -35,27 +25,23 @@ export const useCompatibleNetworks = ( return [] } - const { masterCopy, factoryAddress } = creation - - const allL1SingletonDeployments = SUPPORTED_VERSIONS.map((version) => - getSafeSingletonDeployments({ version }), - ).filter(Boolean) as SingletonDeploymentV2[] - - const allL2SingletonDeployments = SUPPORTED_VERSIONS.map((version) => - getSafeL2SingletonDeployments({ version }), - ).filter(Boolean) as SingletonDeploymentV2[] + const { masterCopy, factoryAddress, safeAccountConfig } = creation - const allProxyFactoryDeployments = SUPPORTED_VERSIONS.map((version) => - getProxyFactoryDeployments({ version }), - ).filter(Boolean) as SingletonDeploymentV2[] + const { fallbackHandler } = safeAccountConfig return configs.map((config) => { return { ...config, available: - (hasDeployment(config.chainId, masterCopy, allL1SingletonDeployments) || - hasDeployment(config.chainId, masterCopy, allL2SingletonDeployments)) && - hasDeployment(config.chainId, factoryAddress, allProxyFactoryDeployments), + (hasMatchingDeployment(getSafeSingletonDeployments, masterCopy, config.chainId, SUPPORTED_VERSIONS) || + hasMatchingDeployment(getSafeL2SingletonDeployments, masterCopy, config.chainId, SUPPORTED_VERSIONS)) && + hasMatchingDeployment(getProxyFactoryDeployments, factoryAddress, config.chainId, SUPPORTED_VERSIONS) && + hasMatchingDeployment( + getCompatibilityFallbackHandlerDeployments, + fallbackHandler, + config.chainId, + SUPPORTED_VERSIONS, + ), } }) } diff --git a/src/features/multichain/hooks/useSafeCreationData.ts b/src/features/multichain/hooks/useSafeCreationData.ts index a2df2ad8b2..c2a4797052 100644 --- a/src/features/multichain/hooks/useSafeCreationData.ts +++ b/src/features/multichain/hooks/useSafeCreationData.ts @@ -10,6 +10,8 @@ import { determineMasterCopyVersion, isPredictedSafeProps } from '@/features/cou import { logError } from '@/services/exceptions' import ErrorCodes from '@/services/exceptions/ErrorCodes' import { asError } from '@/services/exceptions/utils' +import semverSatisfies from 'semver/functions/satisfies' +import { ZERO_ADDRESS } from '@safe-global/protocol-kit/dist/src/utils/constants' export const SAFE_CREATION_DATA_ERRORS = { TX_NOT_FOUND: 'The Safe creation transaction could not be found. Please retry later.', @@ -17,6 +19,9 @@ export const SAFE_CREATION_DATA_ERRORS = { UNSUPPORTED_SAFE_CREATION: 'The method this Safe was created with is not supported.', NO_PROVIDER: 'The RPC provider for the origin network is not available.', LEGACY_COUNTERFATUAL: 'This undeployed Safe cannot be replayed. Please activate the Safe first.', + PAYMENT_SAFE: 'The Safe creation used reimbursement. Adding networks to such Safes is not supported.', + UNSUPPORTED_IMPLEMENTATION: + 'The Safe was created using an unsupported or outdated implementation. Adding networks to this Safe is not possible.', } export const decodeSetupData = (setupData: string): ReplayedSafeProps['safeAccountConfig'] => { @@ -47,6 +52,12 @@ const getUndeployedSafeCreationData = async (undeployedSafe: UndeployedSafe): Pr const proxyFactoryInterface = Safe_proxy_factory__factory.createInterface() const createProxySelector = proxyFactoryInterface.getFunction('createProxyWithNonce').selector +/** + * Loads the creation data from the CGW or infers it from an undeployed Safe. + * + * Throws errors for the reasons in {@link SAFE_CREATION_DATA_ERRORS}. + * Checking the cheap cases not requiring RPC calls first. + */ const getCreationDataForChain = async ( chain: ChainInfo, undeployedSafe: UndeployedSafe, @@ -69,6 +80,19 @@ const getCreationDataForChain = async ( throw new Error(SAFE_CREATION_DATA_ERRORS.NO_CREATION_DATA) } + // Safes that were deployed with an unknown mastercopy or < 1.3.0 are not supported. + const safeVersion = determineMasterCopyVersion(creation.masterCopy, chain.chainId) + if (!safeVersion || semverSatisfies(safeVersion, '<1.3.0')) { + throw new Error(SAFE_CREATION_DATA_ERRORS.UNSUPPORTED_IMPLEMENTATION) + } + + const safeAccountConfig = decodeSetupData(creation.setupData) + + // Safes that used the reimbursement logic are not supported + if ((safeAccountConfig.payment && safeAccountConfig.payment > 0) || safeAccountConfig.paymentToken !== ZERO_ADDRESS) { + throw new Error(SAFE_CREATION_DATA_ERRORS.PAYMENT_SAFE) + } + // We need to create a readOnly provider of the deployed chain const customRpcUrl = chain ? customRpc?.[chain.chainId] : undefined const provider = createWeb3ReadOnly(chain, customRpcUrl) @@ -102,12 +126,6 @@ const getCreationDataForChain = async ( // We found the wrong tx. This tx seems to deploy multiple Safes at once. This is not supported yet. throw new Error(SAFE_CREATION_DATA_ERRORS.UNSUPPORTED_SAFE_CREATION) } - const safeAccountConfig = decodeSetupData(creation.setupData) - const safeVersion = determineMasterCopyVersion(creation.masterCopy, chain.chainId) - - if (!safeVersion) { - throw new Error('Could not determine Safe version of used master copy') - } return { factoryAddress: creation.factoryAddress, diff --git a/src/services/contracts/deployments.ts b/src/services/contracts/deployments.ts index 6995cf6317..ad454b084a 100644 --- a/src/services/contracts/deployments.ts +++ b/src/services/contracts/deployments.ts @@ -13,6 +13,7 @@ import type { SingletonDeployment, DeploymentFilter, SingletonDeploymentV2 } fro import type { ChainInfo, SafeInfo } from '@safe-global/safe-gateway-typescript-sdk' import { getLatestSafeVersion } from '@/utils/chains' import { sameAddress } from '@/utils/addresses' +import { type SafeVersion } from '@safe-global/safe-core-sdk-types' const toNetworkAddressList = (addresses: string | string[]) => (Array.isArray(addresses) ? addresses : [addresses]) @@ -28,6 +29,31 @@ export const hasCanonicalDeployment = (deployment: SingletonDeploymentV2 | undef return networkAddresses.some((networkAddress) => sameAddress(canonicalAddress, networkAddress)) } +/** + * Checks if any of the deployments returned by the `getDeployments` function for the given `network` and `versions` contain a deployment for the `contractAddress` + * + * @param getDeployments function to get the contract deployments + * @param contractAddress address that should be included in the deployments + * @param network chainId that is getting checked + * @param versions supported Safe versions + * @returns true if a matching deployment was found + */ +export const hasMatchingDeployment = ( + getDeployments: (filter?: DeploymentFilter) => SingletonDeploymentV2 | undefined, + contractAddress: string, + network: string, + versions: SafeVersion[], +): boolean => { + return versions.some((version) => { + const deployments = getDeployments({ version, network }) + if (!deployments) { + return false + } + const deployedAddresses = toNetworkAddressList(deployments.networkAddresses[network] ?? []) + return deployedAddresses.some((deployedAddress) => sameAddress(deployedAddress, contractAddress)) + }) +} + export const _tryDeploymentVersions = ( getDeployment: (filter?: DeploymentFilter) => SingletonDeployment | undefined, network: ChainInfo,