(Fix) - #1561 Outgoing instead of custom tx for sending collectibles (#1567)

* Refactor getTxData

* Add SAFE_TRANSFER_FROM in SAFE_METHODS_NAMES

* Adds check on isSendERC721Transaction for erc721 send

* Adds TOKEN_TRANSFER_METHODS_NAMES types

* Replace type SAFE_TRANSFER_FROM

* Fix import

* Adds nftAssetsListAddressSelector

* Remove txCode and knownTokens from isSendERC721Transaction
Now it directly checks agains the list of nftAssets on the store

* Refactor ENS_TOKEN_CONTRACT usage check

* Add TODO

* Add return for ENS symbol

Co-authored-by: Daniel Sanchez <daniel.sanchez@gnosis.pm>
Co-authored-by: Fernando <fernando.greco@gmail.com>

src/logic/collectibles/store/selectors/index.ts

@@ -16,6 +16,10 @@ export const nftAssetsListSelector = createSelector(nftAssets, (assets): NFTAsse
   return assets ? Object.values(assets) : []
 })
 
+export const nftAssetsListAddressesSelector = createSelector(nftAssetsListSelector, (assets): string[] => {
+  return Array.from(new Set(assets.map((nftAsset) => nftAsset.address)))
+})
+
 export const availableNftAssetsAddresses = createSelector(nftTokensSelector, (userNftTokens): string[] => {
   return Array.from(new Set(userNftTokens.map((nftToken) => nftToken.assetAddress)))
 })

src/logic/safe/store/actions/__tests__/transactionHelpers.test.ts

@@ -281,7 +281,6 @@ describe('isCustomTransaction', () => {
   it('It should return true if Is outgoing transaction, is not an erc20 transaction, not an upgrade transaction and not and erc721 transaction', async () => {
     // given
     const transaction = getMockedTxServiceModel({ to: safeAddress2, value: '0', data: 'test' })
-    const txCode = ''
     const knownTokens = Map<string, Record<TokenProps> & Readonly<TokenProps>>()
     const token = makeToken({
       address: '0x00Df91984582e6e96288307E9c2f20b38C8FeCE9',
@@ -299,7 +298,7 @@ describe('isCustomTransaction', () => {
     txHelpers.isSendERC721Transaction.mockImplementationOnce(() => false)
 
     // when
-    const result = await isCustomTransaction(transaction, txCode, safeAddress, knownTokens)
+    const result = await isCustomTransaction(transaction, safeAddress)
 
     // then
     expect(result).toBe(true)
@@ -309,7 +308,6 @@ describe('isCustomTransaction', () => {
   it('It should return true if is outgoing transaction, is not SendERC20Transaction, is not isUpgradeTransaction and not isSendERC721Transaction', async () => {
     // given
     const transaction = getMockedTxServiceModel({ to: safeAddress2, value: '0', data: 'test' })
-    const txCode = ''
     const knownTokens = Map<string, Record<TokenProps> & Readonly<TokenProps>>()
     const token = makeToken({
       address: '0x00Df91984582e6e96288307E9c2f20b38C8FeCE9',
@@ -327,7 +325,7 @@ describe('isCustomTransaction', () => {
     txHelpers.isSendERC721Transaction.mockImplementationOnce(() => false)
 
     // when
-    const result = await isCustomTransaction(transaction, txCode, safeAddress, knownTokens)
+    const result = await isCustomTransaction(transaction, safeAddress)
 
     // then
     expect(result).toBe(true)
@@ -338,7 +336,6 @@ describe('isCustomTransaction', () => {
     // given
     const upgradeTxData = `0x8d80ff0a000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000f200dfa693da0d16f5e7e78fdcbede8fc6ebea44f1cf000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000247de7edef000000000000000000000000d5d82b6addc9027b22dca772aa68d5d74cdbdf4400dfa693da0d16f5e7e78fdcbede8fc6ebea44f1cf00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000024f08a032300000000000000000000000034cfac646f301356faa8b21e94227e3583fe3f5f0000000000000000000000000000`
     const transaction = getMockedTxServiceModel({ to: safeAddress2, value: '0', data: upgradeTxData })
-    const txCode = ''
     const knownTokens = Map<string, Record<TokenProps> & Readonly<TokenProps>>()
     const token = makeToken({
       address: '0x00Df91984582e6e96288307E9c2f20b38C8FeCE9',
@@ -356,7 +353,7 @@ describe('isCustomTransaction', () => {
     txHelpers.isSendERC721Transaction.mockImplementationOnce(() => false)
 
     // when
-    const result = await isCustomTransaction(transaction, txCode, safeAddress, knownTokens)
+    const result = await isCustomTransaction(transaction, safeAddress)
 
     // then
     expect(result).toBe(false)
@@ -365,7 +362,6 @@ describe('isCustomTransaction', () => {
   it('It should return false if is outgoing transaction, is not SendERC20Transaction, not isUpgradeTransaction and isSendERC721Transaction', async () => {
     // given
     const transaction = getMockedTxServiceModel({ to: safeAddress2, value: '0', data: 'test' })
-    const txCode = ''
     const knownTokens = Map<string, Record<TokenProps> & Readonly<TokenProps>>()
     const token = makeToken({
       address: '0x00Df91984582e6e96288307E9c2f20b38C8FeCE9',
@@ -383,7 +379,7 @@ describe('isCustomTransaction', () => {
     txHelpers.isSendERC721Transaction.mockImplementationOnce(() => true)
 
     // when
-    const result = await isCustomTransaction(transaction, txCode, safeAddress, knownTokens)
+    const result = await isCustomTransaction(transaction, safeAddress)
 
     // then
     expect(result).toBe(false)
@@ -839,11 +835,9 @@ describe('buildTx', () => {
     const txResult = await buildTx({
       cancellationTxs,
       currentUser: userAddress,
-      knownTokens,
       outgoingTxs,
       safe: safeInstance,
       tx: transaction,
-      txCode: undefined,
     })
 
     // then

src/logic/safe/store/actions/transactions/fetchTransactions/loadOutgoingTransactions.ts

@@ -1,7 +1,6 @@
 import { fromJS, List, Map } from 'immutable'
 
 import generateBatchRequests from 'src/logic/contracts/generateBatchRequests'
-import { TOKEN_REDUCER_ID, TokenState } from 'src/logic/tokens/store/reducer/tokens'
 import { web3ReadOnly } from 'src/logic/wallets/getWeb3'
 import { PROVIDER_REDUCER_ID } from 'src/logic/wallets/store/reducer/provider'
 import { buildTx, isCancelTransaction } from 'src/logic/safe/store/actions/transactions/utils/transactionHelpers'
@@ -9,7 +8,6 @@ import { SAFE_REDUCER_ID } from 'src/logic/safe/store/reducer/safe'
 import { store } from 'src/store'
 import fetchTransactions from 'src/logic/safe/store/actions/transactions/fetchTransactions/fetchTransactions'
 import { Transaction, TransactionTypes } from 'src/logic/safe/store/models/types/transaction'
-import { Token } from 'src/logic/tokens/store/model/token'
 import { SafeRecord } from 'src/logic/safe/store/models/safe'
 import { DataDecoded } from 'src/logic/safe/store/models/types/transactions.d'
 
@@ -65,7 +63,6 @@ export type OutgoingTxs = {
 
 export type BatchProcessTxsProps = OutgoingTxs & {
   currentUser?: string
-  knownTokens: Map<string, Token>
   safe: SafeRecord
 }
 
@@ -138,7 +135,6 @@ const batchRequestContractCode = (transactions: TxServiceModel[]): Promise<Batch
 const batchProcessOutgoingTransactions = async ({
   cancellationTxs,
   currentUser,
-  knownTokens,
   outgoingTxs,
   safe,
 }: BatchProcessTxsProps): Promise<{
@@ -150,15 +146,13 @@ const batchProcessOutgoingTransactions = async ({
   const cancellationTxsWithData = cancelTxsValues.length ? await batchRequestContractCode(cancelTxsValues) : []
 
   const cancel = {}
-  for (const [tx, txCode] of cancellationTxsWithData) {
+  for (const [tx] of cancellationTxsWithData) {
     cancel[`${tx.nonce}`] = await buildTx({
       cancellationTxs,
       currentUser,
-      knownTokens,
       outgoingTxs,
       safe,
       tx,
-      txCode,
     })
   }
 
@@ -166,16 +160,14 @@ const batchProcessOutgoingTransactions = async ({
   const outgoingTxsWithData = outgoingTxs.length ? await batchRequestContractCode(outgoingTxs) : []
 
   const outgoing: Transaction[] = []
-  for (const [tx, txCode] of outgoingTxsWithData) {
+  for (const [tx] of outgoingTxsWithData) {
     outgoing.push(
       await buildTx({
         cancellationTxs,
         currentUser,
-        knownTokens,
         outgoingTxs,
         safe,
         tx,
-        txCode,
       }),
     )
   }
@@ -195,7 +187,6 @@ export const loadOutgoingTransactions = async (safeAddress: string): Promise<Saf
     return defaultResponse
   }
 
-  const knownTokens: TokenState = state[TOKEN_REDUCER_ID]
   const currentUser: string = state[PROVIDER_REDUCER_ID].get('account')
   const safe: SafeRecord = state[SAFE_REDUCER_ID].getIn(['safes', safeAddress])
 
@@ -215,7 +206,6 @@ export const loadOutgoingTransactions = async (safeAddress: string): Promise<Saf
   const { cancel, outgoing } = await batchProcessOutgoingTransactions({
     cancellationTxs,
     currentUser,
-    knownTokens,
     outgoingTxs,
     safe,
   })

src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts

@@ -1,6 +1,5 @@
-import { List, Map } from 'immutable'
+import { List } from 'immutable'
 import { getNetworkInfo } from 'src/config'
-import { TOKEN_REDUCER_ID, TokenState } from 'src/logic/tokens/store/reducer/tokens'
 import {
   getERC20DecimalsAndSymbol,
   getERC721Symbol,
@@ -33,7 +32,6 @@ import {
   TxServiceModel,
 } from 'src/logic/safe/store/actions/transactions/fetchTransactions/loadOutgoingTransactions'
 import { TypedDataUtils } from 'eth-sig-util'
-import { Token } from 'src/logic/tokens/store/model/token'
 import { ProviderRecord } from 'src/logic/wallets/store/model/provider'
 import { SafeRecord } from 'src/logic/safe/store/models/safe'
 import { DataDecoded, DecodedParams } from 'src/routes/safe/store/models/types/transactions.d'
@@ -83,16 +81,11 @@ export const isOutgoingTransaction = (tx: TxServiceModel, safeAddress?: string):
   return !sameAddress(tx.to, safeAddress) && !isEmptyData(tx.data)
 }
 
-export const isCustomTransaction = async (
+export const isCustomTransaction = async (tx: TxServiceModel, safeAddress?: string): Promise<boolean> => {
-  tx: TxServiceModel,
-  txCode?: string,
-  safeAddress?: string,
-  knownTokens?: TokenState,
-): Promise<boolean> => {
   const isOutgoing = isOutgoingTransaction(tx, safeAddress)
-  const isErc20 = await isSendERC20Transaction(tx, txCode, knownTokens)
+  const isErc20 = await isSendERC20Transaction(tx)
   const isUpgrade = isUpgradeTransaction(tx)
-  const isErc721 = isSendERC721Transaction(tx, txCode, knownTokens)
+  const isErc721 = isSendERC721Transaction(tx)
 
   return isOutgoing && !isErc20 && !isUpgrade && !isErc721
 }
@@ -232,27 +225,24 @@ export const calculateTransactionType = (tx: Transaction): TransactionTypeValues
 
 export type BuildTx = BatchProcessTxsProps & {
   tx: TxServiceModel
-  txCode?: string
 }
 
 export const buildTx = async ({
   cancellationTxs,
   currentUser,
-  knownTokens,
   outgoingTxs,
   safe,
   tx,
-  txCode,
 }: BuildTx): Promise<Transaction> => {
   const safeAddress = safe.address
   const { nativeCoin } = getNetworkInfo()
   const isModifySettingsTx = isModifySettingsTransaction(tx, safeAddress)
   const isTxCancelled = isTransactionCancelled(tx, outgoingTxs, cancellationTxs)
-  const isSendERC721Tx = isSendERC721Transaction(tx, txCode, knownTokens)
+  const isSendERC721Tx = isSendERC721Transaction(tx)
-  const isSendERC20Tx = await isSendERC20Transaction(tx, txCode, knownTokens)
+  const isSendERC20Tx = await isSendERC20Transaction(tx)
   const isMultiSendTx = isMultiSendTransaction(tx)
   const isUpgradeTx = isUpgradeTransaction(tx)
-  const isCustomTx = await isCustomTransaction(tx, txCode, safeAddress, knownTokens)
+  const isCustomTx = await isCustomTransaction(tx, safeAddress)
   const isCancellationTx = isCancelTransaction(tx, safeAddress)
   const refundParams = await getRefundParams(tx, getERC20DecimalsAndSymbol)
   const decodedParams = getDecodedParams(tx)
@@ -323,7 +313,6 @@ export type TxToMock = TxArgs & {
 }
 
 export const mockTransaction = (tx: TxToMock, safeAddress: string, state: AppReduxState): Promise<Transaction> => {
-  const knownTokens: Map<string, Token> = state[TOKEN_REDUCER_ID]
   const safe = safeSelector(state)
   const cancellationTxs = safeCancellationTransactionsSelector(state)
   const outgoingTxs = safeTransactionsSelector(state)
@@ -335,11 +324,9 @@ export const mockTransaction = (tx: TxToMock, safeAddress: string, state: AppRed
   return buildTx({
     cancellationTxs,
     currentUser: undefined,
-    knownTokens,
     outgoingTxs,
     safe,
     tx: (tx as unknown) as TxServiceModel,
-    txCode: EMPTY_DATA,
   })
 }
 

src/logic/safe/store/models/types/transactions.d.ts

@@ -241,7 +241,13 @@ export const METHOD_TO_ID = {
 
 export type SafeMethods = typeof SAFE_METHODS_NAMES[keyof typeof SAFE_METHODS_NAMES]
 
-type TokenMethods = 'transfer' | 'transferFrom' | 'safeTransferFrom'
+export const TOKEN_TRANSFER_METHODS_NAMES = {
+  TRANSFER: 'transfer',
+  TRANSFER_FROM: 'transferFrom',
+  SAFE_TRANSFER_FROM: 'safeTransferFrom',
+} as const
+
+type TokenMethods = typeof TOKEN_TRANSFER_METHODS_NAMES[keyof typeof TOKEN_TRANSFER_METHODS_NAMES]
 
 type SafeDecodedParams = {
   [key in SafeMethods]?: Record<string, string>

src/logic/tokens/utils/tokenHelpers.ts

@@ -8,11 +8,14 @@ import {
   getERC721TokenContract,
 } from 'src/logic/tokens/store/actions/fetchTokens'
 import { makeToken, Token } from 'src/logic/tokens/store/model/token'
-import { TokenState } from 'src/logic/tokens/store/reducer/tokens'
 import { ALTERNATIVE_TOKEN_ABI } from 'src/logic/tokens/utils/alternativeAbi'
 import { web3ReadOnly as web3 } from 'src/logic/wallets/getWeb3'
 import { isEmptyData } from 'src/logic/safe/store/actions/transactions/utils/transactionHelpers'
 import { TxServiceModel } from 'src/logic/safe/store/actions/transactions/fetchTransactions/loadOutgoingTransactions'
+import { sameString } from 'src/utils/strings'
+import { TOKEN_TRANSFER_METHODS_NAMES } from 'src/logic/safe/store/models/types/transactions.d'
+import { store } from 'src/store'
+import { nftAssetsListAddressesSelector } from 'src/logic/collectibles/store/selectors'
 
 export const SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH = '42842e0e'
 
@@ -42,14 +45,17 @@ export const isTokenTransfer = (tx: TxServiceModel): boolean => {
   return !isEmptyData(tx.data) && tx.data?.substring(0, 10) === '0xa9059cbb' && Number(tx.value) === 0
 }
 
-export const isSendERC721Transaction = (tx: TxServiceModel, txCode?: string, knownTokens?: TokenState): boolean => {
+export const isSendERC721Transaction = (tx: TxServiceModel): boolean => {
-  // "0x57f1887a8BF19b14fC0dF6Fd9B2acc9Af147eA85" - ens token contract, includes safeTransferFrom
+  let hasERC721Transfer = false
-  // but no proper ERC721 standard implemented
+
-  return (
+  if (tx.dataDecoded && sameString(tx.dataDecoded.method, TOKEN_TRANSFER_METHODS_NAMES.SAFE_TRANSFER_FROM)) {
-    (txCode?.includes(SAFE_TRANSFER_FROM_WITHOUT_DATA_HASH) &&
+    hasERC721Transfer = tx.dataDecoded.parameters.findIndex((param) => sameString(param.name, 'tokenId')) !== -1
-      tx.to !== '0x57f1887a8BF19b14fC0dF6Fd9B2acc9Af147eA85') ||
+  }
-    (isTokenTransfer(tx) && !knownTokens?.get(tx.to))
+
-  )
+  // Note: this is only valid with our current case (client rendering), if we move to server side rendering we need to refactor this
+  const state = store.getState()
+  const knownAssets = nftAssetsListAddressesSelector(state)
+  return knownAssets.includes(tx.to) || hasERC721Transfer
 }
 
 export const getERC721Symbol = async (contractAddress: string): Promise<string> => {
@@ -59,6 +65,12 @@ export const getERC721Symbol = async (contractAddress: string): Promise<string>
     const tokenInstance = await ERC721token.at(contractAddress)
     tokenSymbol = tokenInstance.symbol()
   } catch (err) {
+    // If the contract address is an ENS token contract, we know that the ERC721 standard is not proper implemented
+    // The method symbol() is missing
+    const ENS_TOKEN_CONTRACT = '0x57f1887a8BF19b14fC0dF6Fd9B2acc9Af147eA85'
+    if (sameString(contractAddress, ENS_TOKEN_CONTRACT)) {
+      return 'ENS'
+    }
     console.error(`Failed to retrieve token symbol for ERC721 token ${contractAddress}`)
   }
   return tokenSymbol
@@ -89,12 +101,8 @@ export const getERC20DecimalsAndSymbol = async (
   return tokenInfo
 }
 
-export const isSendERC20Transaction = async (
+export const isSendERC20Transaction = async (tx: TxServiceModel): Promise<boolean> => {
-  tx: TxServiceModel,
+  let isSendTokenTx = !isSendERC721Transaction(tx) && isTokenTransfer(tx)
-  txCode?: string,
-  knownTokens?: TokenState,
-): Promise<boolean> => {
-  let isSendTokenTx = !isSendERC721Transaction(tx, txCode, knownTokens) && isTokenTransfer(tx)
 
   if (isSendTokenTx) {
     const { decimals, symbol } = await getERC20DecimalsAndSymbol(tx.to)

src/routes/safe/components/Transactions/TxsTable/ExpandedTx/TxDescription/utils.ts

@@ -29,77 +29,142 @@ interface TxData {
   upgradeTx?: boolean
 }
 
-export const getTxData = (tx: Transaction): TxData => {
+const getTxDataForModifySettingsTxs = (tx: Transaction): TxData => {
   const txData: TxData = {}
 
-  if (tx.decodedParams) {
+  if (!tx.modifySettingsTx || !tx.decodedParams) {
-    if (tx.isTokenTransfer) {
+    return txData
-      const { to } = tx.decodedParams.transfer || {}
+  }
-      txData.recipient = to
-      txData.isTokenTransfer = true
-    } else if (tx.isCollectibleTransfer) {
-      const { safeTransferFrom, transfer, transferFrom } = tx.decodedParams
-      const { to, value } = safeTransferFrom || transferFrom || transfer || {}
-      txData.recipient = to
-      txData.tokenId = value
-      txData.isCollectibleTransfer = true
-    } else if (tx.modifySettingsTx) {
-      txData.recipient = tx.recipient
-      txData.modifySettingsTx = true
 
-      if (tx.decodedParams[SAFE_METHODS_NAMES.REMOVE_OWNER]) {
+  txData.recipient = tx.recipient
-        const { _threshold, owner } = tx.decodedParams[SAFE_METHODS_NAMES.REMOVE_OWNER]
+  txData.modifySettingsTx = true
-        txData.action = SAFE_METHODS_NAMES.REMOVE_OWNER
+
-        txData.removedOwner = owner
+  if (tx.decodedParams[SAFE_METHODS_NAMES.REMOVE_OWNER]) {
-        txData.newThreshold = _threshold
+    const { _threshold, owner } = tx.decodedParams[SAFE_METHODS_NAMES.REMOVE_OWNER]
-      } else if (tx.decodedParams[SAFE_METHODS_NAMES.CHANGE_THRESHOLD]) {
+    txData.action = SAFE_METHODS_NAMES.REMOVE_OWNER
-        const { _threshold } = tx.decodedParams[SAFE_METHODS_NAMES.CHANGE_THRESHOLD]
+    txData.removedOwner = owner
-        txData.action = SAFE_METHODS_NAMES.CHANGE_THRESHOLD
+    txData.newThreshold = _threshold
-        txData.newThreshold = _threshold
+
-      } else if (tx.decodedParams[SAFE_METHODS_NAMES.ADD_OWNER_WITH_THRESHOLD]) {
+    return txData
-        const { _threshold, owner } = tx.decodedParams[SAFE_METHODS_NAMES.ADD_OWNER_WITH_THRESHOLD]
+  }
-        txData.action = SAFE_METHODS_NAMES.ADD_OWNER_WITH_THRESHOLD
+  if (tx.decodedParams[SAFE_METHODS_NAMES.CHANGE_THRESHOLD]) {
-        txData.addedOwner = owner
+    const { _threshold } = tx.decodedParams[SAFE_METHODS_NAMES.CHANGE_THRESHOLD]
-        txData.newThreshold = _threshold
+    txData.action = SAFE_METHODS_NAMES.CHANGE_THRESHOLD
-      } else if (tx.decodedParams[SAFE_METHODS_NAMES.SWAP_OWNER]) {
+    txData.newThreshold = _threshold
-        const { newOwner, oldOwner } = tx.decodedParams[SAFE_METHODS_NAMES.SWAP_OWNER]
+    return txData
-        txData.action = SAFE_METHODS_NAMES.SWAP_OWNER
+  }
-        txData.removedOwner = oldOwner
+  if (tx.decodedParams[SAFE_METHODS_NAMES.ADD_OWNER_WITH_THRESHOLD]) {
-        txData.addedOwner = newOwner
+    const { _threshold, owner } = tx.decodedParams[SAFE_METHODS_NAMES.ADD_OWNER_WITH_THRESHOLD]
-      } else if (tx.decodedParams[SAFE_METHODS_NAMES.ENABLE_MODULE]) {
+    txData.action = SAFE_METHODS_NAMES.ADD_OWNER_WITH_THRESHOLD
-        const { module } = tx.decodedParams[SAFE_METHODS_NAMES.ENABLE_MODULE]
+    txData.addedOwner = owner
-        txData.action = SAFE_METHODS_NAMES.ENABLE_MODULE
+    txData.newThreshold = _threshold
-        txData.module = module
+    return txData
-      } else if (tx.decodedParams[SAFE_METHODS_NAMES.DISABLE_MODULE]) {
+  }
-        const { module } = tx.decodedParams[SAFE_METHODS_NAMES.DISABLE_MODULE]
+
-        txData.action = SAFE_METHODS_NAMES.DISABLE_MODULE
+  if (tx.decodedParams[SAFE_METHODS_NAMES.SWAP_OWNER]) {
-        txData.module = module
+    const { newOwner, oldOwner } = tx.decodedParams[SAFE_METHODS_NAMES.SWAP_OWNER]
-      }
+    txData.action = SAFE_METHODS_NAMES.SWAP_OWNER
-    } else if (tx.multiSendTx) {
+    txData.removedOwner = oldOwner
-      txData.recipient = tx.recipient
+    txData.addedOwner = newOwner
-      txData.data = tx.data
+    return txData
-      txData.customTx = true
+  }
-    } else {
+
-      txData.recipient = tx.recipient
+  if (tx.decodedParams[SAFE_METHODS_NAMES.ENABLE_MODULE]) {
-      txData.data = tx.data
+    const { module } = tx.decodedParams[SAFE_METHODS_NAMES.ENABLE_MODULE]
-      txData.customTx = true
+    txData.action = SAFE_METHODS_NAMES.ENABLE_MODULE
-    }
+    txData.module = module
-  } else if (tx.customTx) {
+    return txData
-    txData.recipient = tx.recipient
+  }
-    txData.data = tx.data
+
-    txData.customTx = true
+  if (tx.decodedParams[SAFE_METHODS_NAMES.DISABLE_MODULE]) {
-  } else if (Number(tx.value) > 0) {
+    const { module } = tx.decodedParams[SAFE_METHODS_NAMES.DISABLE_MODULE]
-    txData.recipient = tx.recipient
+    txData.action = SAFE_METHODS_NAMES.DISABLE_MODULE
-  } else if (tx.isCancellationTx) {
+    txData.module = module
-    txData.cancellationTx = true
+    return txData
-  } else if (tx.creationTx) {
-    txData.creationTx = true
-  } else if (tx.upgradeTx) {
-    txData.upgradeTx = true
-    txData.data = `The contract of this Safe is upgraded to Version ${getSafeVersion(tx.data)}`
-  } else {
-    txData.recipient = tx.recipient
   }
 
   return txData
 }
+
+const getTxDataForTxsWithDecodedParams = (tx: Transaction): TxData => {
+  const txData: TxData = {}
+
+  if (!tx.decodedParams) {
+    return txData
+  }
+
+  if (tx.isTokenTransfer) {
+    const { to } = tx.decodedParams.transfer || {}
+    txData.recipient = to
+    txData.isTokenTransfer = true
+    return txData
+  }
+
+  if (tx.isCollectibleTransfer) {
+    const { safeTransferFrom, transfer, transferFrom } = tx.decodedParams
+    const { to, value } = safeTransferFrom || transferFrom || transfer || {}
+    txData.recipient = to
+    txData.tokenId = value
+    txData.isCollectibleTransfer = true
+
+    return txData
+  }
+
+  if (tx.modifySettingsTx) {
+    return getTxDataForModifySettingsTxs(tx)
+  }
+
+  if (tx.multiSendTx) {
+    txData.recipient = tx.recipient
+    txData.data = tx.data
+    txData.customTx = true
+    return txData
+  }
+
+  txData.recipient = tx.recipient
+  txData.data = tx.data
+  txData.customTx = true
+
+  return txData
+}
+
+// @todo (agustin) this function does not makes much sense
+// it should be refactored to simplify unnecessary if's checks and re-asigning props to the txData object
+export const getTxData = (tx: Transaction): TxData => {
+  const txData: TxData = {}
+
+  if (tx.decodedParams) {
+    return getTxDataForTxsWithDecodedParams(tx)
+  }
+
+  if (tx.customTx) {
+    txData.recipient = tx.recipient
+    txData.data = tx.data
+    txData.customTx = true
+    return txData
+  }
+  if (Number(tx.value) > 0) {
+    txData.recipient = tx.recipient
+    return txData
+  }
+
+  if (tx.isCancellationTx) {
+    txData.cancellationTx = true
+    return txData
+  }
+
+  if (tx.creationTx) {
+    txData.creationTx = true
+    return txData
+  }
+
+  if (tx.upgradeTx) {
+    txData.upgradeTx = true
+    txData.data = `The contract of this Safe is upgraded to Version ${getSafeVersion(tx.data)}`
+
+    return txData
+  }
+  txData.recipient = tx.recipient
+
+  return txData
+}