Fix: multisend verification only worked for the latest contract versi…

…on (#2439) * Fix: pass safe version to multisend contract getters * Null check * Use trustedDelegateCallTarget instead of looking up contracts * Revert "Use trustedDelegateCallTarget instead of looking up contracts" This reverts commit a2ada2f. * Adjust comment * Rm isSupportedMultisend
safe-global · Aug 29, 2023 · 9134da3 · 9134da3
1 parent ad135b3
commit 9134da3
Show file tree

Hide file tree

Showing 6 changed files with 85 additions and 45 deletions.
diff --git a/src/components/transactions/TxDetails/TxData/index.tsx b/src/components/transactions/TxDetails/TxData/index.tsx
@@ -7,7 +7,6 @@ import {
   isMultisigDetailedExecutionInfo,
   isSettingsChangeTxInfo,
   isSpendingLimitMethod,
-  isSupportedMultiSendAddress,
   isSupportedSpendingLimitAddress,
   isTransferTxInfo,
 } from '@/utils/transaction-guards'
@@ -36,7 +35,7 @@ const TxData = ({ txDetails }: { txDetails: TransactionDetails }): ReactElement
     return <RejectionTxInfo nonce={txDetails.detailedExecutionInfo?.nonce} isTxExecuted={!!txDetails.executedAt} />
   }
 
-  if (isSupportedMultiSendAddress(txInfo, chainId) && isMultiSendTxInfo(txInfo)) {
+  if (isMultiSendTxInfo(txInfo)) {
     return <MultiSendTxInfo txInfo={txInfo} />
   }
 

diff --git a/src/components/transactions/TxDetails/index.tsx b/src/components/transactions/TxDetails/index.tsx
@@ -14,7 +14,6 @@ import {
   isMultiSendTxInfo,
   isMultisigDetailedExecutionInfo,
   isMultisigExecutionInfo,
-  isSupportedMultiSendAddress,
   isTxQueued,
 } from '@/utils/transaction-guards'
 import { InfoDetails } from '@/components/transactions/InfoDetails'
@@ -39,7 +38,6 @@ type TxDetailsProps = {
 }
 
 const TxDetailsBlock = ({ txSummary, txDetails }: TxDetailsProps): ReactElement => {
-  const chainId = useChainId()
   const isPending = useIsPending(txSummary.id)
   const isQueue = isTxQueued(txSummary.txStatus)
   const awaitingExecution = isAwaitingExecution(txSummary.txStatus)
@@ -89,7 +87,7 @@ const TxDetailsBlock = ({ txSummary, txDetails }: TxDetailsProps): ReactElement
           <Summary txDetails={txDetails} />
         </div>
 
-        {isSupportedMultiSendAddress(txDetails.txInfo, chainId) && isMultiSendTxInfo(txDetails.txInfo) && (
+        {isMultiSendTxInfo(txDetails.txInfo) && (
           <div className={`${css.multiSend}`}>
             <ErrorBoundary fallback={<div>Error parsing data</div>}>
               <Multisend txData={txDetails.txData} />

diff --git a/src/components/transactions/TxInfo/index.tsx b/src/components/transactions/TxInfo/index.tsx
@@ -17,12 +17,10 @@ import {
   isMultiSendTxInfo,
   isNativeTokenTransfer,
   isSettingsChangeTxInfo,
-  isSupportedMultiSendAddress,
   isTransferTxInfo,
 } from '@/utils/transaction-guards'
 import { ellipsis, shortenAddress } from '@/utils/formatters'
 import { useCurrentChain } from '@/hooks/useChains'
-import useChainId from '@/hooks/useChainId'
 
 export const TransferTx = ({
   info,
@@ -98,13 +96,11 @@ const SettingsChangeTx = ({ info }: { info: SettingsChange }): ReactElement => {
 }
 
 const TxInfo = ({ info, ...rest }: { info: TransactionInfo; omitSign?: boolean; withLogo?: boolean }): ReactElement => {
-  const chainId = useChainId()
-
   if (isSettingsChangeTxInfo(info)) {
     return <SettingsChangeTx info={info} />
   }
 
-  if (isSupportedMultiSendAddress(info, chainId) && isMultiSendTxInfo(info)) {
+  if (isMultiSendTxInfo(info)) {
     return <MultiSendTx info={info} />
   }
 

diff --git a/src/services/contracts/safeContracts.ts b/src/services/contracts/safeContracts.ts
@@ -1,7 +1,6 @@
 import {
   getFallbackHandlerDeployment,
   getMultiSendCallOnlyDeployment,
-  getMultiSendDeployment,
   getProxyFactoryDeployment,
   getSafeL2SingletonDeployment,
   getSafeSingletonDeployment,
@@ -102,28 +101,6 @@ export const getReadOnlyGnosisSafeContract = (chain: ChainInfo, safeVersion: str
 
 // MultiSend
 
-const getMultiSendContractDeployment = (chainId: string) => {
-  return getMultiSendDeployment({ network: chainId }) || getMultiSendDeployment()
-}
-
-export const getMultiSendContractAddress = (chainId: string): string | undefined => {
-  const deployment = getMultiSendContractDeployment(chainId)
-
-  return deployment?.networkAddresses[chainId]
-}
-
-// MultiSendCallOnly
-
-const getMultiSendCallOnlyContractDeployment = (chainId: string) => {
-  return getMultiSendCallOnlyDeployment({ network: chainId }) || getMultiSendCallOnlyDeployment()
-}
-
-export const getMultiSendCallOnlyContractAddress = (chainId: string): string | undefined => {
-  const deployment = getMultiSendCallOnlyContractDeployment(chainId)
-
-  return deployment?.networkAddresses[chainId]
-}
-
 export const getMultiSendCallOnlyContract = (
   chainId: string,
   safeVersion: SafeInfo['version'] = LATEST_SAFE_VERSION,
@@ -132,7 +109,7 @@ export const getMultiSendCallOnlyContract = (
   const ethAdapter = createEthersAdapter(provider)
 
   return ethAdapter.getMultiSendCallOnlyContract({
-    singletonDeployment: getMultiSendCallOnlyContractDeployment(chainId),
+    singletonDeployment: getMultiSendCallOnlyDeployment({ network: chainId, version: safeVersion || undefined }),
     ..._getValidatedGetContractProps(chainId, safeVersion),
   })
 }
@@ -144,7 +121,7 @@ export const getReadOnlyMultiSendCallOnlyContract = (
   const ethAdapter = createReadOnlyEthersAdapter()
 
   return ethAdapter.getMultiSendCallOnlyContract({
-    singletonDeployment: getMultiSendCallOnlyContractDeployment(chainId),
+    singletonDeployment: getMultiSendCallOnlyDeployment({ network: chainId, version: safeVersion || undefined }),
     ..._getValidatedGetContractProps(chainId, safeVersion),
   })
 }

diff --git a/src/utils/__tests__/transactions.test.ts b/src/utils/__tests__/transactions.test.ts
@@ -5,6 +5,8 @@ import type {
   SafeAppData,
   Transaction,
 } from '@safe-global/safe-gateway-typescript-sdk'
+import { TransactionInfoType } from '@safe-global/safe-gateway-typescript-sdk'
+import { isMultiSendTxInfo } from '../transaction-guards'
 import { getQueuedTransactionCount, getTxOrigin } from '../transactions'
 
 describe('transactions', () => {
@@ -110,4 +112,77 @@ describe('transactions', () => {
       )
     })
   })
+
+  describe('isMultiSendTxInfo', () => {
+    it('should return true for a multisend tx', () => {
+      expect(
+        isMultiSendTxInfo({
+          type: TransactionInfoType.CUSTOM,
+          to: {
+            value: '0x40A2aCCbd92BCA938b02010E17A5b8929b49130D',
+            name: 'Gnosis Safe: MultiSendCallOnly',
+            logoUri:
+              'https://safe-transaction-assets.safe.global/contracts/logos/0x40A2aCCbd92BCA938b02010E17A5b8929b49130D.png',
+          },
+          dataSize: '1188',
+          value: '0',
+          methodName: 'multiSend',
+          actionCount: 3,
+          isCancellation: false,
+        }),
+      ).toBe(true)
+    })
+
+    it('should return false for non-multisend txs', () => {
+      expect(
+        isMultiSendTxInfo({
+          type: TransactionInfoType.CUSTOM,
+          to: {
+            value: '0x40A2aCCbd92BCA938b02010E17A5b8929b49130D',
+            name: 'Gnosis Safe: MultiSendCallOnly',
+            logoUri:
+              'https://safe-transaction-assets.safe.global/contracts/logos/0x40A2aCCbd92BCA938b02010E17A5b8929b49130D.png',
+          },
+          dataSize: '1188',
+          value: '0',
+          methodName: 'multiSend',
+          //actionCount: 3, // missing actionCount
+          isCancellation: false,
+        }),
+      ).toBe(false)
+
+      expect(
+        isMultiSendTxInfo({
+          type: TransactionInfoType.CUSTOM,
+          to: {
+            value: '0x40A2aCCbd92BCA938b02010E17A5b8929b49130D',
+            name: 'Gnosis Safe: MultiSendCallOnly',
+            logoUri:
+              'https://safe-transaction-assets.safe.global/contracts/logos/0x40A2aCCbd92BCA938b02010E17A5b8929b49130D.png',
+          },
+          dataSize: '1188',
+          value: '0',
+          methodName: 'notMultiSend', // wrong method
+          actionCount: 3,
+          isCancellation: false,
+        }),
+      ).toBe(false)
+
+      expect(
+        isMultiSendTxInfo({
+          type: TransactionInfoType.SETTINGS_CHANGE, // wrong type
+          dataDecoded: {
+            method: 'changeThreshold',
+            parameters: [
+              {
+                name: '_threshold',
+                type: 'uint256',
+                value: '2',
+              },
+            ],
+          },
+        }),
+      ).toBe(false)
+    })
+  })
 })
diff --git a/src/utils/transaction-guards.ts b/src/utils/transaction-guards.ts
@@ -35,7 +35,6 @@ import {
 } from '@safe-global/safe-gateway-typescript-sdk'
 import { getSpendingLimitModuleAddress } from '@/services/contracts/spendingLimitContracts'
 import { sameAddress } from '@/utils/addresses'
-import { getMultiSendCallOnlyContractAddress, getMultiSendContractAddress } from '@/services/contracts/safeContracts'
 import type { NamedAddress } from '@/components/new-safe/create/types'
 
 export const isTxQueued = (value: TransactionStatus): boolean => {
@@ -78,16 +77,12 @@ export const isCustomTxInfo = (value: TransactionInfo): value is Custom => {
   return value.type === TransactionInfoType.CUSTOM
 }
 
-export const isSupportedMultiSendAddress = (txInfo: TransactionInfo, chainId: string): boolean => {
-  const toAddress = isCustomTxInfo(txInfo) ? txInfo.to.value : ''
-  const multiSendAddress = getMultiSendContractAddress(chainId)
-  const multiSendCallOnlyAddress = getMultiSendCallOnlyContractAddress(chainId)
-
-  return sameAddress(multiSendAddress, toAddress) || sameAddress(multiSendCallOnlyAddress, toAddress)
-}
-
 export const isMultiSendTxInfo = (value: TransactionInfo): value is MultiSend => {
-  return value.type === TransactionInfoType.CUSTOM && value.methodName === 'multiSend'
+  return (
+    value.type === TransactionInfoType.CUSTOM &&
+    value.methodName === 'multiSend' &&
+    typeof value.actionCount === 'number'
+  )
 }
 
 export const isCancellationTxInfo = (value: TransactionInfo): value is Cancellation => {