Merge pull request #1807 from gnosis/hotfix/refund-displayed-as-rejection

Mergeback hotfix v2.18.1 to development

package.json

@@ -1,6 +1,6 @@
 {
   "name": "safe-react",
-  "version": "2.18.0",
+  "version": "2.18.1",
   "description": "Allowing crypto users manage funds in a safer way",
   "website": "https://github.com/gnosis/safe-react#readme",
   "bugs": {

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

@@ -23,6 +23,7 @@ import {
   isUpgradeTransaction,
 } from 'src/logic/safe/store/actions/transactions/utils/transactionHelpers'
 import { getERC20DecimalsAndSymbol } from 'src/logic/tokens/utils/tokenHelpers'
+import { DELEGATE_CALL } from 'src/logic/safe/transactions'
 
 const safeAddress = '0xdfA693da0D16F5E7E78FdCBeDe8FC6eBEa44f1Cf'
 const safeAddress2 = '0x344B941b1aAE2e4Be73987212FC4741687Bf0503'
@@ -91,6 +92,7 @@ describe('isInnerTransaction', () => {
 
 describe('isCancelTransaction', () => {
   const safeAddress = '0xdfA693da0D16F5E7E78FdCBeDe8FC6eBEa44f1Cf'
+  const mockedETHAccount = '0xd76e0B566e218a80F4c96458FE09a322EBAa9aF2'
   it('It should return false if given a inner transaction with empty data', () => {
     // given
     const transaction = getMockedTxServiceModel({ to: safeAddress, value: '0', data: null })
@@ -111,6 +113,101 @@ describe('isCancelTransaction', () => {
     // then
     expect(result).toBe(false)
   })
+  it('It should return false if the transaction recipient is not the safeAddress', () => {
+    // given
+    const transaction = getMockedTxServiceModel({ to: mockedETHAccount, value: '0', data: null })
+
+    // when
+    const result = isCancelTransaction(transaction, safeAddress)
+
+    // then
+    expect(result).toBe(false)
+  })
+  it('It should return false if the transaction value is not empty', () => {
+    // given
+    const transaction = getMockedTxServiceModel({ to: safeAddress, value: '100', data: null })
+
+    // when
+    const result = isCancelTransaction(transaction, safeAddress)
+
+    // then
+    expect(result).toBe(false)
+  })
+  it('It should return false if the transaction data is not empty', () => {
+    // given
+    const transaction = getMockedTxServiceModel({ to: safeAddress, value: '0', data: mockedETHAccount })
+
+    // when
+    const result = isCancelTransaction(transaction, safeAddress)
+
+    // then
+    expect(result).toBe(false)
+  })
+  it('It should return false if the transaction operation is not call', () => {
+    // given
+    const transaction = getMockedTxServiceModel({ to: safeAddress, value: '0', data: null, operation: DELEGATE_CALL })
+
+    // when
+    const result = isCancelTransaction(transaction, safeAddress)
+
+    // then
+    expect(result).toBe(false)
+  })
+  it('It should return false if the transaction baseGas is not empty', () => {
+    // given
+    const transaction = getMockedTxServiceModel({ to: safeAddress, value: '0', data: null, baseGas: 10 })
+
+    // when
+    const result = isCancelTransaction(transaction, safeAddress)
+
+    // then
+    expect(result).toBe(false)
+  })
+  it('It should return false if the transaction gasPrice is not empty', () => {
+    // given
+    const transaction = getMockedTxServiceModel({ to: safeAddress, value: '0', data: null, gasPrice: '10' })
+
+    // when
+    const result = isCancelTransaction(transaction, safeAddress)
+
+    // then
+    expect(result).toBe(false)
+  })
+  it('It should return false if the transaction gasToken is not empty', () => {
+    // given
+    const transaction = getMockedTxServiceModel({ to: safeAddress, value: '0', data: null, gasToken: mockedETHAccount })
+
+    // when
+    const result = isCancelTransaction(transaction, safeAddress)
+
+    // then
+    expect(result).toBe(false)
+  })
+  it('It should return false if the refundReceiver is not empty', () => {
+    // given
+    const transaction = getMockedTxServiceModel({
+      to: safeAddress,
+      value: '0',
+      data: null,
+      refundReceiver: mockedETHAccount,
+    })
+
+    // when
+    const result = isCancelTransaction(transaction, safeAddress)
+
+    // then
+    expect(result).toBe(false)
+  })
+  it('It should return true for a transaction with everything empty except for to parameter equals to the safeAddress,', () => {
+    // given
+    const transaction = getMockedTxServiceModel({ to: safeAddress })
+
+    // when
+    const result = isCancelTransaction(transaction, safeAddress)
+
+    // then
+    expect(result).toBe(true)
+  })
 })
 
 describe('isPendingTransaction', () => {

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

@@ -2,7 +2,7 @@ import { List } from 'immutable'
 import { getNetworkInfo } from 'src/config'
 import { getERC20DecimalsAndSymbol, isSendERC20Transaction } from 'src/logic/tokens/utils/tokenHelpers'
 import { getERC721Symbol, isSendERC721Transaction } from 'src/logic/collectibles/utils'
-import { sameAddress, ZERO_ADDRESS } from 'src/logic/wallets/ethAddresses'
+import { isEmptyAddress, sameAddress, ZERO_ADDRESS } from 'src/logic/wallets/ethAddresses'
 import { EMPTY_DATA } from 'src/logic/wallets/ethTransactions'
 import { makeConfirmation } from 'src/logic/safe/store/models/confirmation'
 import { Confirmation } from 'src/logic/safe/store/models/types/confirmation'
@@ -31,6 +31,7 @@ import { TypedDataUtils } from 'eth-sig-util'
 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'
+import { CALL } from 'src/logic/safe/transactions'
 
 export const isEmptyData = (data?: string | null): boolean => {
   return !data || data === EMPTY_DATA
@@ -48,8 +49,40 @@ export const isInnerTransaction = (tx: TxServiceModel | Transaction, safeAddress
   return isSameAddress && Number(tx.value) === 0
 }
 
-export const isCancelTransaction = (tx: TxServiceModel | Transaction, safeAddress: string): boolean => {
-  return isInnerTransaction(tx, safeAddress) && isEmptyData(tx.data)
+export const isCancelTransaction = (tx: TxServiceModel, safeAddress: string): boolean => {
+  if (!sameAddress(tx.to, safeAddress)) {
+    return false
+  }
+
+  if (Number(tx.value)) {
+    return false
+  }
+
+  if (tx.data && !isEmptyData(tx.data)) {
+    return false
+  }
+
+  if (tx.operation !== CALL) {
+    return false
+  }
+
+  if (tx.baseGas) {
+    return false
+  }
+
+  if (Number(tx.gasPrice)) {
+    return false
+  }
+
+  if (tx.gasToken && !isEmptyAddress(tx.gasToken)) {
+    return false
+  }
+
+  if (tx.refundReceiver && !isEmptyAddress(tx.refundReceiver)) {
+    return false
+  }
+
+  return true
 }
 
 export const isPendingTransaction = (tx: Transaction, cancelTx: Transaction): boolean => {
@@ -73,11 +106,11 @@ export const isUpgradeTransaction = (tx: TxServiceModel): boolean => {
   )
 }
 
-export const isOutgoingTransaction = (tx: TxServiceModel, safeAddress?: string): boolean => {
+export const isOutgoingTransaction = (tx: TxServiceModel, safeAddress: string): boolean => {
   return !sameAddress(tx.to, safeAddress) && !isEmptyData(tx.data)
 }
 
-export const isCustomTransaction = async (tx: TxServiceModel, safeAddress?: string): Promise<boolean> => {
+export const isCustomTransaction = async (tx: TxServiceModel, safeAddress: string): Promise<boolean> => {
   const isOutgoing = isOutgoingTransaction(tx, safeAddress)
   const isErc20 = await isSendERC20Transaction(tx)
   const isUpgrade = isUpgradeTransaction(tx)

src/logic/wallets/ethAddresses.ts

@@ -1,12 +1,18 @@
 import { List } from 'immutable'
 import { SafeRecord } from 'src/logic/safe/store/models/safe'
 import { sameString } from 'src/utils/strings'
+import { EMPTY_DATA } from 'src/logic/wallets/ethTransactions'
 export const ZERO_ADDRESS = '0x0000000000000000000000000000000000000000'
 
 export const sameAddress = (firstAddress: string | undefined, secondAddress: string | undefined): boolean => {
   return sameString(firstAddress, secondAddress)
 }
 
+export const isEmptyAddress = (address: string | undefined): boolean => {
+  if (!address) return true
+  return sameAddress(address, EMPTY_DATA) || sameAddress(address, ZERO_ADDRESS)
+}
+
 export const shortVersionOf = (value: string, cut: number): string => {
   if (!value) {
     return 'Unknown'