From 5d898f6bab4f17953d525a75da5151430fe321de Mon Sep 17 00:00:00 2001 From: iamacook Date: Thu, 28 Oct 2021 08:26:25 +0100 Subject: [PATCH 1/8] feat: Contract error map and extractor --- .../contracts/__tests__/safeErrors.test.ts | 25 +++++++++++ src/logic/contracts/contracts.d.ts | 43 +++++++++++++++++++ src/logic/contracts/safeErrors.ts | 10 +++++ .../safe/store/actions/createTransaction.ts | 1 + .../safe/store/actions/processTransaction.ts | 1 + 5 files changed, 80 insertions(+) create mode 100644 src/logic/contracts/__tests__/safeErrors.test.ts create mode 100644 src/logic/contracts/contracts.d.ts create mode 100644 src/logic/contracts/safeErrors.ts diff --git a/src/logic/contracts/__tests__/safeErrors.test.ts b/src/logic/contracts/__tests__/safeErrors.test.ts new file mode 100644 index 0000000000..8f11d66ab5 --- /dev/null +++ b/src/logic/contracts/__tests__/safeErrors.test.ts @@ -0,0 +1,25 @@ +import { getSafeError, isSafeError } from '../safeErrors' + +describe('isSafeError', () => { + it('returns true if a safe error exists in any error name', () => { + expect(isSafeError({ name: 'GS000', message: 'test' })).toBe(true) + }) + it('returns true if a safe error exists in any error message', () => { + expect(isSafeError({ name: 'test', message: 'Could not finish initialization' })).toBe(true) + }) + it('returns false if not safe error', () => { + expect(isSafeError({ name: 'test', message: 'test' })).toBe(false) + }) +}) + +describe('getSafeError', () => { + it('returns safe errors', () => { + const error = { name: 'GS000', message: 'Will not be returned' } + expect(getSafeError(error)).toBe('Could not finish initialization') + }) + + it('returns provided errors if not safe errors', () => { + const error = { name: 'Not a Safe error', message: 'Will be returned' } + expect(getSafeError(error)).toBe('Will be returned') + }) +}) diff --git a/src/logic/contracts/contracts.d.ts b/src/logic/contracts/contracts.d.ts new file mode 100644 index 0000000000..551e10538a --- /dev/null +++ b/src/logic/contracts/contracts.d.ts @@ -0,0 +1,43 @@ +// https://github.com/gnosis/safe-contracts/blob/main/docs/error_codes.md +export enum CONTRACT_ERROR_CODES { + // General init related + GS000 = 'Could not finish initialization', + GS001 = 'Threshold needs to be defined', + + // General gas/ execution related + GS010 = 'Not enough gas to execute Safe transaction', + GS011 = 'Could not pay gas costs with ether', + GS012 = 'Could not pay gas costs with token', + GS013 = 'Safe transaction failed when gasPrice and safeTxGas were 0', + + // General signature validation related + GS020 = 'Signatures data too short', + GS021 = 'Invalid contract signature location = inside static part', + GS022 = 'Invalid contract signature location = length not present', + GS023 = 'Invalid contract signature location = data not complete', + GS024 = 'Invalid contract signature provided', + GS025 = 'Hash has not been approved', + GS026 = 'Invalid owner provided', + + // General auth related + GS030 = 'Only owners can approve a hash', + GS031 = 'Method can only be called from this contract', + + // Module management related + GS100 = 'Modules have already been initialized', + GS101 = 'Invalid module address provided', + GS102 = 'Module has already been added', + GS103 = 'Invalid prevModule, module pair provided', + GS104 = 'Method can only be called from an enabled module', + + // Owner management related + GS200 = 'Owners have already been setup', + GS201 = 'Threshold cannot exceed owner count', + GS202 = 'Threshold needs to be greater than 0', + GS203 = 'Invalid owner address provided', + GS204 = 'Address is already an owner', + GS205 = 'Invalid prevOwner, owner pair provided', + + // Guard management related + GS300 = 'Guard does not implement IERC165', +} diff --git a/src/logic/contracts/safeErrors.ts b/src/logic/contracts/safeErrors.ts new file mode 100644 index 0000000000..7236520ed7 --- /dev/null +++ b/src/logic/contracts/safeErrors.ts @@ -0,0 +1,10 @@ +import { CONTRACT_ERROR_CODES } from 'src/logic/contracts/contracts.d' + +export const isSafeError = (error: Error): boolean => { + return Object.entries(CONTRACT_ERROR_CODES) + .flatMap((v) => v) + .some((v) => Object.values(error).includes(v)) +} + +export const getSafeError = (error: Error): string => + isSafeError(error) ? CONTRACT_ERROR_CODES[error.name] : error.message diff --git a/src/logic/safe/store/actions/createTransaction.ts b/src/logic/safe/store/actions/createTransaction.ts index ba70ff2edd..988aec8508 100644 --- a/src/logic/safe/store/actions/createTransaction.ts +++ b/src/logic/safe/store/actions/createTransaction.ts @@ -210,6 +210,7 @@ export const createTransaction = .encodeABI() try { const errMsg = await getErrorMessage(safeInstance.options.address, 0, executeDataUsedSignatures, from) + // TODO: errMsg seemingly contains error from contract logError(Errors._803, errMsg) } catch (e) { logError(Errors._803, e.message) diff --git a/src/logic/safe/store/actions/processTransaction.ts b/src/logic/safe/store/actions/processTransaction.ts index 2823bd3170..4bf49cbf10 100644 --- a/src/logic/safe/store/actions/processTransaction.ts +++ b/src/logic/safe/store/actions/processTransaction.ts @@ -229,6 +229,7 @@ export const processTransaction = const executeData = safeInstance.methods.approveHash(txHash).encodeABI() try { const errMsg = await getErrorMessage(safeInstance.options.address, 0, executeData, from) + // TODO: errMsg seemingly contains error from contract logError(Errors._804, errMsg) } catch (e) { logError(Errors._804, e.message) From 6a94c0c89cf4f2274f9ef07282783b83d234cad4 Mon Sep 17 00:00:00 2001 From: iamacook Date: Thu, 28 Oct 2021 11:56:13 +0100 Subject: [PATCH 2/8] feat: Show contract error notifications --- .../__tests__/safeContractErrors.test.ts | 19 +++++++++++ .../contracts/__tests__/safeErrors.test.ts | 25 -------------- src/logic/contracts/contracts.d.ts | 4 ++- src/logic/contracts/safeContractErrors.ts | 25 ++++++++++++++ src/logic/contracts/safeErrors.ts | 10 ------ .../safe/store/actions/createTransaction.ts | 32 +++++++++--------- .../safe/store/actions/processTransaction.ts | 33 ++++++++++--------- src/test/utils/ethereumErrors.ts | 16 --------- 8 files changed, 81 insertions(+), 83 deletions(-) create mode 100644 src/logic/contracts/__tests__/safeContractErrors.test.ts delete mode 100644 src/logic/contracts/__tests__/safeErrors.test.ts create mode 100644 src/logic/contracts/safeContractErrors.ts delete mode 100644 src/logic/contracts/safeErrors.ts delete mode 100644 src/test/utils/ethereumErrors.ts diff --git a/src/logic/contracts/__tests__/safeContractErrors.test.ts b/src/logic/contracts/__tests__/safeContractErrors.test.ts new file mode 100644 index 0000000000..6ec50cacce --- /dev/null +++ b/src/logic/contracts/__tests__/safeContractErrors.test.ts @@ -0,0 +1,19 @@ +import { decodeContractError } from '../safeContractErrors' + +describe('decodeContractError', () => { + it('returns safe errors', () => { + expect(decodeContractError('GS000: Could not finish initialization')).toBe('Could not finish initialization') + }) + + it('returns safe errors irregardless of place in error', () => { + expect(decodeContractError('testGS000test')).toBe('Could not finish initialization') + expect(decodeContractError('test GS000 test')).toBe('Could not finish initialization') + }) + it('returns safe errors irregardless of case', () => { + expect(decodeContractError('gs000: testing')).toBe('Could not finish initialization') + }) + + it('returns provided errors if not safe errors', () => { + expect(decodeContractError('Not a Safe error')).toBe('Not a Safe error') + }) +}) diff --git a/src/logic/contracts/__tests__/safeErrors.test.ts b/src/logic/contracts/__tests__/safeErrors.test.ts deleted file mode 100644 index 8f11d66ab5..0000000000 --- a/src/logic/contracts/__tests__/safeErrors.test.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { getSafeError, isSafeError } from '../safeErrors' - -describe('isSafeError', () => { - it('returns true if a safe error exists in any error name', () => { - expect(isSafeError({ name: 'GS000', message: 'test' })).toBe(true) - }) - it('returns true if a safe error exists in any error message', () => { - expect(isSafeError({ name: 'test', message: 'Could not finish initialization' })).toBe(true) - }) - it('returns false if not safe error', () => { - expect(isSafeError({ name: 'test', message: 'test' })).toBe(false) - }) -}) - -describe('getSafeError', () => { - it('returns safe errors', () => { - const error = { name: 'GS000', message: 'Will not be returned' } - expect(getSafeError(error)).toBe('Could not finish initialization') - }) - - it('returns provided errors if not safe errors', () => { - const error = { name: 'Not a Safe error', message: 'Will be returned' } - expect(getSafeError(error)).toBe('Will be returned') - }) -}) diff --git a/src/logic/contracts/contracts.d.ts b/src/logic/contracts/contracts.d.ts index 551e10538a..98310f575f 100644 --- a/src/logic/contracts/contracts.d.ts +++ b/src/logic/contracts/contracts.d.ts @@ -1,5 +1,5 @@ // https://github.com/gnosis/safe-contracts/blob/main/docs/error_codes.md -export enum CONTRACT_ERROR_CODES { +export enum CONTRACT_ERRORS { // General init related GS000 = 'Could not finish initialization', GS001 = 'Threshold needs to be defined', @@ -41,3 +41,5 @@ export enum CONTRACT_ERROR_CODES { // Guard management related GS300 = 'Guard does not implement IERC165', } + +export const CONTRACT_ERROR_CODES = Object.keys(CONTRACTERRORS) diff --git a/src/logic/contracts/safeContractErrors.ts b/src/logic/contracts/safeContractErrors.ts new file mode 100644 index 0000000000..966c2d6f61 --- /dev/null +++ b/src/logic/contracts/safeContractErrors.ts @@ -0,0 +1,25 @@ +import abi from 'ethereumjs-abi' + +import { CONTRACT_ERRORS, CONTRACT_ERROR_CODES } from 'src/logic/contracts/contracts.d' +import { getWeb3 } from 'src/logic/wallets/getWeb3' + +export const getContractError = async (to: string, value: number, data: string, from: string): Promise => { + const web3 = getWeb3() + const returnData = await web3.eth.call({ + to, + from, + value, + data, + }) + const returnBuffer = Buffer.from(returnData.slice(2), 'hex') + + return abi.rawDecode(['string'], returnBuffer.slice(4))[0] +} + +export const decodeContractError = (error: string): string => { + const code = CONTRACT_ERROR_CODES.find((code) => { + return error.toUpperCase().includes(code.toUpperCase()) + }) + + return code ? `${code}:${CONTRACT_ERRORS[code]}` : error +} diff --git a/src/logic/contracts/safeErrors.ts b/src/logic/contracts/safeErrors.ts deleted file mode 100644 index 7236520ed7..0000000000 --- a/src/logic/contracts/safeErrors.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { CONTRACT_ERROR_CODES } from 'src/logic/contracts/contracts.d' - -export const isSafeError = (error: Error): boolean => { - return Object.entries(CONTRACT_ERROR_CODES) - .flatMap((v) => v) - .some((v) => Object.values(error).includes(v)) -} - -export const getSafeError = (error: Error): string => - isSafeError(error) ? CONTRACT_ERROR_CODES[error.name] : error.message diff --git a/src/logic/safe/store/actions/createTransaction.ts b/src/logic/safe/store/actions/createTransaction.ts index 988aec8508..9ad9995716 100644 --- a/src/logic/safe/store/actions/createTransaction.ts +++ b/src/logic/safe/store/actions/createTransaction.ts @@ -21,7 +21,6 @@ import enqueueSnackbar from 'src/logic/notifications/store/actions/enqueueSnackb import closeSnackbarAction from 'src/logic/notifications/store/actions/closeSnackbar' import { generateSafeTxHash } from 'src/logic/safe/store/actions/transactions/utils/transactionHelpers' import { getLastTx, getNewTxNonce, shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils' -import { getErrorMessage } from 'src/test/utils/ethereumErrors' import fetchTransactions from './transactions/fetchTransactions' import { TxArgs } from 'src/logic/safe/store/models/types/transaction' import { PayableTx } from 'src/types/contracts/types.d' @@ -35,6 +34,7 @@ import { currentChainId } from 'src/logic/config/store/selectors' import { generateSafeRoute, history, SAFE_ROUTES } from 'src/routes/routes' import { getCurrentShortChainName, getNetworkId } from 'src/config' import { ETHEREUM_NETWORK } from 'src/config/networks/network.d' +import { decodeContractError, getContractError } from 'src/logic/contracts/safeContractErrors' export interface CreateTransactionArgs { navigateToTransactionsTab?: boolean @@ -191,31 +191,33 @@ export const createTransaction = }) } catch (err) { onError?.() - - const notification = isTxPendingError(err) - ? NOTIFICATIONS.TX_PENDING_MSG - : { - ...notificationsQueue.afterExecutionError, - message: `${notificationsQueue.afterExecutionError.message} - ${err.message}`, - } - - dispatch(closeSnackbarAction({ key: beforeExecutionKey })) - dispatch(enqueueSnackbar({ key: err.code, ...notification })) - logError(Errors._803, err.message) + let contractError + if (err.code !== METAMASK_REJECT_CONFIRM_TX_ERROR_CODE) { const executeDataUsedSignatures = safeInstance.methods .execTransaction(to, valueInWei, txData, operation, 0, 0, 0, ZERO_ADDRESS, ZERO_ADDRESS, sigs) .encodeABI() try { - const errMsg = await getErrorMessage(safeInstance.options.address, 0, executeDataUsedSignatures, from) - // TODO: errMsg seemingly contains error from contract - logError(Errors._803, errMsg) + contractError = await getContractError(safeInstance.options.address, 0, executeDataUsedSignatures, from) + logError(Errors._803, contractError) } catch (e) { logError(Errors._803, e.message) } } + + const notification = isTxPendingError(err) + ? NOTIFICATIONS.TX_PENDING_MSG + : { + ...notificationsQueue.afterExecutionError, + message: `${notificationsQueue.afterExecutionError.message} - ${ + contractError ? decodeContractError(contractError) : err.message + }`, + } + + dispatch(closeSnackbarAction({ key: beforeExecutionKey })) + dispatch(enqueueSnackbar({ key: err.code, ...notification })) } return txHash diff --git a/src/logic/safe/store/actions/processTransaction.ts b/src/logic/safe/store/actions/processTransaction.ts index 4bf49cbf10..a2f91a2bf3 100644 --- a/src/logic/safe/store/actions/processTransaction.ts +++ b/src/logic/safe/store/actions/processTransaction.ts @@ -22,12 +22,9 @@ import { fetchSafe } from 'src/logic/safe/store/actions/fetchSafe' import fetchTransactions from 'src/logic/safe/store/actions/transactions/fetchTransactions' import { getLastTx, getNewTxNonce, shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils' import { AppReduxState } from 'src/store' -import { getErrorMessage } from 'src/test/utils/ethereumErrors' import { TxParameters } from 'src/routes/safe/container/hooks/useTransactionParameters' - import { Dispatch, DispatchReturn } from './types' import { PayableTx } from 'src/types/contracts/types' - import { updateTransactionStatus } from 'src/logic/safe/store/actions/updateTransactionStatus' import { Confirmation } from 'src/logic/safe/store/models/types/confirmation' import { Operation, TransactionStatus } from '@gnosis.pm/safe-react-gateway-sdk' @@ -35,6 +32,7 @@ import { isTxPendingError } from 'src/logic/wallets/getWeb3' import { Errors, logError } from 'src/logic/exceptions/CodedException' import { getNetworkId } from 'src/config' import { ETHEREUM_NETWORK } from 'src/config/networks/network.d' +import { decodeContractError, getContractError } from 'src/logic/contracts/safeContractErrors' interface ProcessTransactionArgs { approveAndExecute: boolean @@ -203,15 +201,7 @@ export const processTransaction = return receipt.transactionHash }) } catch (err) { - const notification = isTxPendingError(err) - ? NOTIFICATIONS.TX_PENDING_MSG - : { - ...notificationsQueue.afterExecutionError, - message: `${notificationsQueue.afterExecutionError.message} - ${err.message}`, - } - - dispatch(closeSnackbarAction({ key: beforeExecutionKey })) - dispatch(enqueueSnackbar({ key: err.code, ...notification })) + logError(Errors._804, err.message) dispatch( updateTransactionStatus({ @@ -223,18 +213,29 @@ export const processTransaction = }), ) - logError(Errors._804, err.message) + let contractError if (txHash) { const executeData = safeInstance.methods.approveHash(txHash).encodeABI() try { - const errMsg = await getErrorMessage(safeInstance.options.address, 0, executeData, from) - // TODO: errMsg seemingly contains error from contract - logError(Errors._804, errMsg) + contractError = await getContractError(safeInstance.options.address, 0, executeData, from) + logError(Errors._804, contractError) } catch (e) { logError(Errors._804, e.message) } } + + const notification = isTxPendingError(err) + ? NOTIFICATIONS.TX_PENDING_MSG + : { + ...notificationsQueue.afterExecutionError, + message: `${notificationsQueue.afterExecutionError.message} - ${ + contractError ? decodeContractError(contractError) : err.message + }`, + } + + dispatch(closeSnackbarAction({ key: beforeExecutionKey })) + dispatch(enqueueSnackbar({ key: err.code, ...notification })) } return txHash diff --git a/src/test/utils/ethereumErrors.ts b/src/test/utils/ethereumErrors.ts deleted file mode 100644 index c141e836ac..0000000000 --- a/src/test/utils/ethereumErrors.ts +++ /dev/null @@ -1,16 +0,0 @@ -// -import abi from 'ethereumjs-abi' -import { getWeb3 } from 'src/logic/wallets/getWeb3' - -export const getErrorMessage = async (to, value, data, from) => { - const web3 = getWeb3() - const returnData: any = await web3.eth.call({ - to, - from, - value, - data, - }) - const returnBuffer = Buffer.from(returnData.slice(2), 'hex') - - return abi.rawDecode(['string'], returnBuffer.slice(4))[0] -} From 164246257fc95539468cd2969b391b5d42d5f7ed Mon Sep 17 00:00:00 2001 From: iamacook Date: Thu, 28 Oct 2021 12:03:06 +0100 Subject: [PATCH 3/8] fix: Tests --- src/logic/contracts/__tests__/safeContractErrors.test.ts | 8 ++++---- src/logic/contracts/contracts.d.ts | 2 +- src/logic/contracts/safeContractErrors.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/logic/contracts/__tests__/safeContractErrors.test.ts b/src/logic/contracts/__tests__/safeContractErrors.test.ts index 6ec50cacce..b03c3c5d4a 100644 --- a/src/logic/contracts/__tests__/safeContractErrors.test.ts +++ b/src/logic/contracts/__tests__/safeContractErrors.test.ts @@ -2,15 +2,15 @@ import { decodeContractError } from '../safeContractErrors' describe('decodeContractError', () => { it('returns safe errors', () => { - expect(decodeContractError('GS000: Could not finish initialization')).toBe('Could not finish initialization') + expect(decodeContractError('GS000: Could not finish initialization')).toBe('GS000: Could not finish initialization') }) it('returns safe errors irregardless of place in error', () => { - expect(decodeContractError('testGS000test')).toBe('Could not finish initialization') - expect(decodeContractError('test GS000 test')).toBe('Could not finish initialization') + expect(decodeContractError('testGS000test')).toBe('GS000: Could not finish initialization') + expect(decodeContractError('test GS000 test')).toBe('GS000: Could not finish initialization') }) it('returns safe errors irregardless of case', () => { - expect(decodeContractError('gs000: testing')).toBe('Could not finish initialization') + expect(decodeContractError('gs000: testing')).toBe('GS000: Could not finish initialization') }) it('returns provided errors if not safe errors', () => { diff --git a/src/logic/contracts/contracts.d.ts b/src/logic/contracts/contracts.d.ts index 98310f575f..15d685cf91 100644 --- a/src/logic/contracts/contracts.d.ts +++ b/src/logic/contracts/contracts.d.ts @@ -42,4 +42,4 @@ export enum CONTRACT_ERRORS { GS300 = 'Guard does not implement IERC165', } -export const CONTRACT_ERROR_CODES = Object.keys(CONTRACTERRORS) +export const CONTRACT_ERROR_CODES = Object.keys(CONTRACT_ERRORS) diff --git a/src/logic/contracts/safeContractErrors.ts b/src/logic/contracts/safeContractErrors.ts index 966c2d6f61..e0b4cb6bdf 100644 --- a/src/logic/contracts/safeContractErrors.ts +++ b/src/logic/contracts/safeContractErrors.ts @@ -21,5 +21,5 @@ export const decodeContractError = (error: string): string => { return error.toUpperCase().includes(code.toUpperCase()) }) - return code ? `${code}:${CONTRACT_ERRORS[code]}` : error + return code ? `${code}: ${CONTRACT_ERRORS[code]}` : error } From 7749fac2c8647a75094a5c349df81945bb12a0c1 Mon Sep 17 00:00:00 2001 From: iamacook Date: Thu, 28 Oct 2021 12:33:40 +0100 Subject: [PATCH 4/8] chore: Move dispatch + type contractError --- src/logic/safe/store/actions/createTransaction.ts | 5 +++-- src/logic/safe/store/actions/processTransaction.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/logic/safe/store/actions/createTransaction.ts b/src/logic/safe/store/actions/createTransaction.ts index 9ad9995716..2347e2e5c4 100644 --- a/src/logic/safe/store/actions/createTransaction.ts +++ b/src/logic/safe/store/actions/createTransaction.ts @@ -193,7 +193,9 @@ export const createTransaction = onError?.() logError(Errors._803, err.message) - let contractError + dispatch(closeSnackbarAction({ key: beforeExecutionKey })) + + let contractError: undefined | string if (err.code !== METAMASK_REJECT_CONFIRM_TX_ERROR_CODE) { const executeDataUsedSignatures = safeInstance.methods @@ -216,7 +218,6 @@ export const createTransaction = }`, } - dispatch(closeSnackbarAction({ key: beforeExecutionKey })) dispatch(enqueueSnackbar({ key: err.code, ...notification })) } diff --git a/src/logic/safe/store/actions/processTransaction.ts b/src/logic/safe/store/actions/processTransaction.ts index a2f91a2bf3..8fb160fdf0 100644 --- a/src/logic/safe/store/actions/processTransaction.ts +++ b/src/logic/safe/store/actions/processTransaction.ts @@ -203,6 +203,8 @@ export const processTransaction = } catch (err) { logError(Errors._804, err.message) + dispatch(closeSnackbarAction({ key: beforeExecutionKey })) + dispatch( updateTransactionStatus({ chainId, @@ -213,7 +215,7 @@ export const processTransaction = }), ) - let contractError + let contractError: undefined | string if (txHash) { const executeData = safeInstance.methods.approveHash(txHash).encodeABI() @@ -234,7 +236,6 @@ export const processTransaction = }`, } - dispatch(closeSnackbarAction({ key: beforeExecutionKey })) dispatch(enqueueSnackbar({ key: err.code, ...notification })) } From 66ab99d13ffb3bf0ed4c2ace923060460b3c1ff3 Mon Sep 17 00:00:00 2001 From: iamacook Date: Fri, 29 Oct 2021 09:04:00 +0100 Subject: [PATCH 5/8] fix: Set error in catch --- src/logic/safe/store/actions/createTransaction.ts | 5 +++-- src/logic/safe/store/actions/processTransaction.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/logic/safe/store/actions/createTransaction.ts b/src/logic/safe/store/actions/createTransaction.ts index 2347e2e5c4..90bb8f0ee8 100644 --- a/src/logic/safe/store/actions/createTransaction.ts +++ b/src/logic/safe/store/actions/createTransaction.ts @@ -203,12 +203,13 @@ export const createTransaction = .encodeABI() try { contractError = await getContractError(safeInstance.options.address, 0, executeDataUsedSignatures, from) - logError(Errors._803, contractError) } catch (e) { - logError(Errors._803, e.message) + contractError = e.message } } + logError(Errors._803, contractError) + const notification = isTxPendingError(err) ? NOTIFICATIONS.TX_PENDING_MSG : { diff --git a/src/logic/safe/store/actions/processTransaction.ts b/src/logic/safe/store/actions/processTransaction.ts index 8fb160fdf0..c2f2b4bcaa 100644 --- a/src/logic/safe/store/actions/processTransaction.ts +++ b/src/logic/safe/store/actions/processTransaction.ts @@ -221,12 +221,13 @@ export const processTransaction = const executeData = safeInstance.methods.approveHash(txHash).encodeABI() try { contractError = await getContractError(safeInstance.options.address, 0, executeData, from) - logError(Errors._804, contractError) } catch (e) { - logError(Errors._804, e.message) + contractError = e.message } } + logError(Errors._804, contractError) + const notification = isTxPendingError(err) ? NOTIFICATIONS.TX_PENDING_MSG : { From f4df41fbdb08eb9b62127d270810b7bfcc10c2bd Mon Sep 17 00:00:00 2001 From: iamacook Date: Wed, 3 Nov 2021 17:15:00 +0100 Subject: [PATCH 6/8] chore: Refactor + small fixes --- src/components/InfiniteScroll/index.tsx | 9 +++- src/logic/contracts/safeContractErrors.ts | 46 +++++++++++++------ .../safe/store/actions/createTransaction.ts | 29 ++++++------ .../safe/store/actions/processTransaction.ts | 26 +++++------ src/logic/safe/utils/aboutToExecuteTx.ts | 3 +- src/routes/index.tsx | 4 +- 6 files changed, 69 insertions(+), 48 deletions(-) diff --git a/src/components/InfiniteScroll/index.tsx b/src/components/InfiniteScroll/index.tsx index 1ae3eba5bd..75be255e7b 100644 --- a/src/components/InfiniteScroll/index.tsx +++ b/src/components/InfiniteScroll/index.tsx @@ -44,9 +44,16 @@ export const InfiniteScroll = ({ children, hasMore, next, config }: InfiniteScro }) useEffect(() => { - if (inView && hasMore) { + // Avoid memory leak - queue/history have separate InfiniteScroll wrappers + let isMounted = true + + if (isMounted && inView && hasMore) { next() } + + return () => { + isMounted = false + } }, [inView, hasMore, next]) return {children} diff --git a/src/logic/contracts/safeContractErrors.ts b/src/logic/contracts/safeContractErrors.ts index e0b4cb6bdf..e13c516b29 100644 --- a/src/logic/contracts/safeContractErrors.ts +++ b/src/logic/contracts/safeContractErrors.ts @@ -2,24 +2,42 @@ import abi from 'ethereumjs-abi' import { CONTRACT_ERRORS, CONTRACT_ERROR_CODES } from 'src/logic/contracts/contracts.d' import { getWeb3 } from 'src/logic/wallets/getWeb3' +import { GnosisSafe } from 'src/types/contracts/gnosis_safe.d' -export const getContractError = async (to: string, value: number, data: string, from: string): Promise => { - const web3 = getWeb3() - const returnData = await web3.eth.call({ - to, - from, - value, - data, +export const decodeContractError = (contractError: string): string => { + const code = CONTRACT_ERROR_CODES.find((code) => { + return contractError.toUpperCase().includes(code.toUpperCase()) }) - const returnBuffer = Buffer.from(returnData.slice(2), 'hex') - return abi.rawDecode(['string'], returnBuffer.slice(4))[0] + return code ? `${code}: ${CONTRACT_ERRORS[code]}` : contractError } -export const decodeContractError = (error: string): string => { - const code = CONTRACT_ERROR_CODES.find((code) => { - return error.toUpperCase().includes(code.toUpperCase()) - }) +export const getContractErrorMessage = async ({ + safeInstance, + from, + data, +}: { + safeInstance: GnosisSafe + from: string + data: string +}): Promise => { + const web3 = getWeb3() + + let contractError: string + + try { + const returnData = await web3.eth.call({ + to: safeInstance.options.address, + from, + value: 0, + data, + }) + + const returnBuffer = Buffer.from(returnData.slice(2), 'hex') + contractError = abi.rawDecode(['string'], returnBuffer.slice(4))[0] + } catch (e) { + contractError = e.message + } - return code ? `${code}: ${CONTRACT_ERRORS[code]}` : error + return decodeContractError(contractError) } diff --git a/src/logic/safe/store/actions/createTransaction.ts b/src/logic/safe/store/actions/createTransaction.ts index 90bb8f0ee8..8894d587c1 100644 --- a/src/logic/safe/store/actions/createTransaction.ts +++ b/src/logic/safe/store/actions/createTransaction.ts @@ -34,7 +34,7 @@ import { currentChainId } from 'src/logic/config/store/selectors' import { generateSafeRoute, history, SAFE_ROUTES } from 'src/routes/routes' import { getCurrentShortChainName, getNetworkId } from 'src/config' import { ETHEREUM_NETWORK } from 'src/config/networks/network.d' -import { decodeContractError, getContractError } from 'src/logic/contracts/safeContractErrors' +import { getContractErrorMessage } from 'src/logic/contracts/safeContractErrors' export interface CreateTransactionArgs { navigateToTransactionsTab?: boolean @@ -195,28 +195,25 @@ export const createTransaction = dispatch(closeSnackbarAction({ key: beforeExecutionKey })) - let contractError: undefined | string + const executeDataUsedSignatures = safeInstance.methods + .execTransaction(to, valueInWei, txData, operation, 0, 0, 0, ZERO_ADDRESS, ZERO_ADDRESS, sigs) + .encodeABI() - if (err.code !== METAMASK_REJECT_CONFIRM_TX_ERROR_CODE) { - const executeDataUsedSignatures = safeInstance.methods - .execTransaction(to, valueInWei, txData, operation, 0, 0, 0, ZERO_ADDRESS, ZERO_ADDRESS, sigs) - .encodeABI() - try { - contractError = await getContractError(safeInstance.options.address, 0, executeDataUsedSignatures, from) - } catch (e) { - contractError = e.message - } - } + const contractErrorMessage = await getContractErrorMessage({ + safeInstance, + from, + data: executeDataUsedSignatures, + }) - logError(Errors._803, contractError) + logError(Errors._803, contractErrorMessage) const notification = isTxPendingError(err) ? NOTIFICATIONS.TX_PENDING_MSG : { ...notificationsQueue.afterExecutionError, - message: `${notificationsQueue.afterExecutionError.message} - ${ - contractError ? decodeContractError(contractError) : err.message - }`, + ...(contractErrorMessage && { + message: `${notificationsQueue.afterExecutionError.message} - ${contractErrorMessage}`, + }), } dispatch(enqueueSnackbar({ key: err.code, ...notification })) diff --git a/src/logic/safe/store/actions/processTransaction.ts b/src/logic/safe/store/actions/processTransaction.ts index c2f2b4bcaa..79230f305a 100644 --- a/src/logic/safe/store/actions/processTransaction.ts +++ b/src/logic/safe/store/actions/processTransaction.ts @@ -32,7 +32,7 @@ import { isTxPendingError } from 'src/logic/wallets/getWeb3' import { Errors, logError } from 'src/logic/exceptions/CodedException' import { getNetworkId } from 'src/config' import { ETHEREUM_NETWORK } from 'src/config/networks/network.d' -import { decodeContractError, getContractError } from 'src/logic/contracts/safeContractErrors' +import { getContractErrorMessage } from 'src/logic/contracts/safeContractErrors' interface ProcessTransactionArgs { approveAndExecute: boolean @@ -215,26 +215,22 @@ export const processTransaction = }), ) - let contractError: undefined | string - - if (txHash) { - const executeData = safeInstance.methods.approveHash(txHash).encodeABI() - try { - contractError = await getContractError(safeInstance.options.address, 0, executeData, from) - } catch (e) { - contractError = e.message - } - } + const executeData = safeInstance.methods.approveHash(txHash).encodeABI() + const contractErrorMessage = await getContractErrorMessage({ + safeInstance, + from, + data: executeData, + }) - logError(Errors._804, contractError) + logError(Errors._804, contractErrorMessage) const notification = isTxPendingError(err) ? NOTIFICATIONS.TX_PENDING_MSG : { ...notificationsQueue.afterExecutionError, - message: `${notificationsQueue.afterExecutionError.message} - ${ - contractError ? decodeContractError(contractError) : err.message - }`, + ...(contractErrorMessage && { + message: `${notificationsQueue.afterExecutionError.message} - ${contractErrorMessage}`, + }), } dispatch(enqueueSnackbar({ key: err.code, ...notification })) diff --git a/src/logic/safe/utils/aboutToExecuteTx.ts b/src/logic/safe/utils/aboutToExecuteTx.ts index 239a2d5f4f..0c82efc067 100644 --- a/src/logic/safe/utils/aboutToExecuteTx.ts +++ b/src/logic/safe/utils/aboutToExecuteTx.ts @@ -6,6 +6,7 @@ import { HistoryPayload } from 'src/logic/safe/store/reducer/gatewayTransactions import { TX_NOTIFICATION_TYPES } from 'src/logic/safe/transactions' import { isUserAnOwner } from 'src/logic/wallets/ethAddresses' import { SafesMap } from 'src/logic/safe/store/reducer/types/safe' +import { Notification } from 'src/logic/notifications/notificationTypes' let nonce: number | undefined @@ -17,7 +18,7 @@ export const getNotification = ( { safeAddress, values }: HistoryPayload, userAddress: string, safes: SafesMap, -): undefined => { +): undefined | Notification => { const currentSafe = safes.get(safeAddress) // no notification if not in the current safe or if its not an owner diff --git a/src/routes/index.tsx b/src/routes/index.tsx index 7856ce85ff..2258768e6c 100644 --- a/src/routes/index.tsx +++ b/src/routes/index.tsx @@ -47,7 +47,9 @@ const Routes = (): React.ReactElement => { ? location.pathname.replace(getPrefixedSafeAddressSlug(), 'SAFE_ADDRESS') : location.pathname trackPage(pathname + location.search) - }, [location, trackPage]) + + // Track when pathname changes + }, [location.pathname, trackPage]) return ( From 8d1c2926f36f25ab487142d9dda3a498a72ddf96 Mon Sep 17 00:00:00 2001 From: iamacook Date: Mon, 8 Nov 2021 13:57:00 +0100 Subject: [PATCH 7/8] fix: Track page w/ search + only decode thrown err --- src/logic/contracts/safeContractErrors.ts | 12 ++++++------ src/logic/safe/store/actions/createTransaction.ts | 4 +++- src/logic/safe/store/actions/processTransaction.ts | 4 +++- src/routes/index.tsx | 2 +- 4 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/logic/contracts/safeContractErrors.ts b/src/logic/contracts/safeContractErrors.ts index e13c516b29..0b7cba5f59 100644 --- a/src/logic/contracts/safeContractErrors.ts +++ b/src/logic/contracts/safeContractErrors.ts @@ -20,11 +20,9 @@ export const getContractErrorMessage = async ({ safeInstance: GnosisSafe from: string data: string -}): Promise => { +}): Promise => { const web3 = getWeb3() - let contractError: string - try { const returnData = await web3.eth.call({ to: safeInstance.options.address, @@ -34,10 +32,12 @@ export const getContractErrorMessage = async ({ }) const returnBuffer = Buffer.from(returnData.slice(2), 'hex') - contractError = abi.rawDecode(['string'], returnBuffer.slice(4))[0] + + // Will throw if there was an error + abi.rawDecode(['string'], returnBuffer.slice(4))[0] } catch (e) { - contractError = e.message + return decodeContractError(e.message) } - return decodeContractError(contractError) + return null } diff --git a/src/logic/safe/store/actions/createTransaction.ts b/src/logic/safe/store/actions/createTransaction.ts index 8894d587c1..3aeee56475 100644 --- a/src/logic/safe/store/actions/createTransaction.ts +++ b/src/logic/safe/store/actions/createTransaction.ts @@ -205,7 +205,9 @@ export const createTransaction = data: executeDataUsedSignatures, }) - logError(Errors._803, contractErrorMessage) + if (contractErrorMessage) { + logError(Errors._803, contractErrorMessage) + } const notification = isTxPendingError(err) ? NOTIFICATIONS.TX_PENDING_MSG diff --git a/src/logic/safe/store/actions/processTransaction.ts b/src/logic/safe/store/actions/processTransaction.ts index 79230f305a..77272f60f7 100644 --- a/src/logic/safe/store/actions/processTransaction.ts +++ b/src/logic/safe/store/actions/processTransaction.ts @@ -222,7 +222,9 @@ export const processTransaction = data: executeData, }) - logError(Errors._804, contractErrorMessage) + if (contractErrorMessage) { + logError(Errors._804, contractErrorMessage) + } const notification = isTxPendingError(err) ? NOTIFICATIONS.TX_PENDING_MSG diff --git a/src/routes/index.tsx b/src/routes/index.tsx index 2258768e6c..f4cd5f4e68 100644 --- a/src/routes/index.tsx +++ b/src/routes/index.tsx @@ -49,7 +49,7 @@ const Routes = (): React.ReactElement => { trackPage(pathname + location.search) // Track when pathname changes - }, [location.pathname, trackPage]) + }, [location.pathname, location.search, trackPage]) return ( From a041f9f78e023cdf05120a1e67c003957d1d2783 Mon Sep 17 00:00:00 2001 From: iamacook Date: Tue, 16 Nov 2021 17:37:23 +0100 Subject: [PATCH 8/8] chore: Decode contract output --- .../contracts/__tests__/safeContractErrors.test.ts | 14 +++++++------- src/logic/contracts/safeContractErrors.ts | 14 ++++++-------- src/logic/safe/store/actions/createTransaction.ts | 2 +- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/src/logic/contracts/__tests__/safeContractErrors.test.ts b/src/logic/contracts/__tests__/safeContractErrors.test.ts index b03c3c5d4a..393c921607 100644 --- a/src/logic/contracts/__tests__/safeContractErrors.test.ts +++ b/src/logic/contracts/__tests__/safeContractErrors.test.ts @@ -1,19 +1,19 @@ -import { decodeContractError } from '../safeContractErrors' +import { decodeMessage } from '../safeContractErrors' -describe('decodeContractError', () => { +describe('decodeMessage', () => { it('returns safe errors', () => { - expect(decodeContractError('GS000: Could not finish initialization')).toBe('GS000: Could not finish initialization') + expect(decodeMessage('GS000: Could not finish initialization')).toBe('GS000: Could not finish initialization') }) it('returns safe errors irregardless of place in error', () => { - expect(decodeContractError('testGS000test')).toBe('GS000: Could not finish initialization') - expect(decodeContractError('test GS000 test')).toBe('GS000: Could not finish initialization') + expect(decodeMessage('testGS000test')).toBe('GS000: Could not finish initialization') + expect(decodeMessage('test GS000 test')).toBe('GS000: Could not finish initialization') }) it('returns safe errors irregardless of case', () => { - expect(decodeContractError('gs000: testing')).toBe('GS000: Could not finish initialization') + expect(decodeMessage('gs000: testing')).toBe('GS000: Could not finish initialization') }) it('returns provided errors if not safe errors', () => { - expect(decodeContractError('Not a Safe error')).toBe('Not a Safe error') + expect(decodeMessage('Not a Safe error')).toBe('Not a Safe error') }) }) diff --git a/src/logic/contracts/safeContractErrors.ts b/src/logic/contracts/safeContractErrors.ts index 0b7cba5f59..7057f8fa9c 100644 --- a/src/logic/contracts/safeContractErrors.ts +++ b/src/logic/contracts/safeContractErrors.ts @@ -4,12 +4,12 @@ import { CONTRACT_ERRORS, CONTRACT_ERROR_CODES } from 'src/logic/contracts/contr import { getWeb3 } from 'src/logic/wallets/getWeb3' import { GnosisSafe } from 'src/types/contracts/gnosis_safe.d' -export const decodeContractError = (contractError: string): string => { +export const decodeMessage = (message: string): string => { const code = CONTRACT_ERROR_CODES.find((code) => { - return contractError.toUpperCase().includes(code.toUpperCase()) + return message.toUpperCase().includes(code.toUpperCase()) }) - return code ? `${code}: ${CONTRACT_ERRORS[code]}` : contractError + return code ? `${code}: ${CONTRACT_ERRORS[code]}` : message } export const getContractErrorMessage = async ({ @@ -33,11 +33,9 @@ export const getContractErrorMessage = async ({ const returnBuffer = Buffer.from(returnData.slice(2), 'hex') - // Will throw if there was an error - abi.rawDecode(['string'], returnBuffer.slice(4))[0] + const contractOutput = abi.rawDecode(['string'], returnBuffer.slice(4))[0] + return decodeMessage(contractOutput) } catch (e) { - return decodeContractError(e.message) + return decodeMessage(e.message) } - - return null } diff --git a/src/logic/safe/store/actions/createTransaction.ts b/src/logic/safe/store/actions/createTransaction.ts index 3aeee56475..eb9cd35536 100644 --- a/src/logic/safe/store/actions/createTransaction.ts +++ b/src/logic/safe/store/actions/createTransaction.ts @@ -190,8 +190,8 @@ export const createTransaction = return receipt.transactionHash }) } catch (err) { - onError?.() logError(Errors._803, err.message) + onError?.() dispatch(closeSnackbarAction({ key: beforeExecutionKey }))