Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

Refactor create/processTransaction #3239

Merged
merged 24 commits into from
Jan 13, 2022
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 21 additions & 2 deletions src/logic/contracts/safeContractErrors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ 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'
import { logError, Errors } from '../exceptions/CodedException'

export const decodeMessage = (message: string): string => {
const code = CONTRACT_ERROR_CODES.find((code) => {
Expand All @@ -12,7 +13,7 @@ export const decodeMessage = (message: string): string => {
return code ? `${code}: ${CONTRACT_ERRORS[code]}` : message
}

export const getContractErrorMessage = async ({
const getContractErrorMessage = async ({
safeInstance,
from,
data,
Expand All @@ -36,6 +37,24 @@ export const getContractErrorMessage = async ({
const contractOutput = abi.rawDecode(['string'], returnBuffer.slice(4))[0]
return decodeMessage(contractOutput)
} catch (e) {
return decodeMessage(e.message)
return null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just to double-check if we're hiding the error on purpose

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand correctly, this was causing the app to crash. It was being handled incorrectly and couldn't be decoded. @katspaugh, can you confirm?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we weren't handling this in any special way because it's not critical and there's no good recovery strategy. Other than logging, which is a good move.

}
}

export const fetchOnchainError = async (
data: string,
safeInstance: GnosisSafe,
from: string,
): Promise<string | null> => {
const contractErrorMessage = await getContractErrorMessage({
safeInstance,
from,
data,
})

if (contractErrorMessage) {
logError(Errors._803, contractErrorMessage)
}

return contractErrorMessage
}
2 changes: 2 additions & 0 deletions src/logic/exceptions/registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ enum ErrorCodes {
_614 = '614: Error fetching transaction by id',
_615 = '615: Failed to retrieve last transaction from server',
_616 = '616: Failed to retrieve recommended nonce',
_617 = '617: Error fetching safeTxGas',
_700 = '700: Failed to read from local/session storage',
_701 = '701: Failed to write to local/session storage',
_702 = '702: Failed to remove from local/session storage',
Expand All @@ -47,6 +48,7 @@ enum ErrorCodes {
_811 = '811: Error calculating ERC721 transfer data for adding a Safe owner',
_812 = '812: Error calculating ERC721 transfer data for removing a Safe owner',
_813 = '813: Error calculating ERC721 transfer data for replacing a Safe owner',
_814 = '814: Error performing an offline tx signature',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: it's not offline but off-chain

_900 = '900: Error loading Safe App',
_901 = '901: Error processing Safe Apps SDK request',
_902 = '902: Error loading Safe Apps list',
Expand Down
37 changes: 36 additions & 1 deletion src/logic/notifications/notificationBuilder.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@ import { IconButton } from '@material-ui/core'
import { Close as IconClose } from '@material-ui/icons'

import { Notification, NOTIFICATIONS } from './notificationTypes'

import { Dispatch } from 'src/logic/safe/store/actions/types'
import closeSnackbarAction from 'src/logic/notifications/store/actions/closeSnackbar'
import { TX_NOTIFICATION_TYPES } from 'src/logic/safe/transactions'
import { getAppInfoFromOrigin } from 'src/routes/safe/components/Apps/utils'
import { store } from 'src/store'
import { isTxPendingError } from '../wallets/getWeb3'
import enqueueSnackbar from './store/actions/enqueueSnackbar'

const setNotificationOrigin = (notification: Notification, origin: string): Notification => {
if (!origin) {
Expand Down Expand Up @@ -261,3 +263,36 @@ export const enhanceSnackbarForAction = (
),
},
})

export const createTxNotifications = (
notifiedTransaction: string,
origin: string | null,
dispatch: Dispatch,
): {
closePending: () => void
showOnError: (err: Error & { code: number }, contractErrorMessage: string | null) => void
} => {
// Notifications
// Each tx gets a slot in the global snackbar queue
// When multiple snackbars are shown, it will re-use the same slot for
// notifications about different states of the tx
const notificationSlot = getNotificationsFromTxType(notifiedTransaction, origin)
const beforeExecutionKey = dispatch(enqueueSnackbar(notificationSlot.beforeExecution))

return {
closePending: () => dispatch(closeSnackbarAction({ key: beforeExecutionKey })),

showOnError: (err: Error & { code: number }, customErrorMessage: string | null) => {
const msg = isTxPendingError(err)
? NOTIFICATIONS.TX_PENDING_MSG
: {
...notificationSlot.afterExecutionError,
...(customErrorMessage && {
message: `${notificationSlot.afterExecutionError.message} - ${customErrorMessage}`,
}),
}

dispatch(enqueueSnackbar({ key: err.code, ...msg }))
},
}
}
13 changes: 8 additions & 5 deletions src/logic/safe/safeTxSigner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@ export const checkIfOffChainSignatureIsPossible = (
isExecution: boolean,
isSmartContractWallet: boolean,
safeVersion?: string,
): boolean =>
!isExecution &&
!isSmartContractWallet &&
!!safeVersion &&
semverSatisfies(safeVersion, SAFE_VERSION_FOR_OFF_CHAIN_SIGNATURES)
): boolean => {
return (
!isExecution &&
!isSmartContractWallet &&
!!safeVersion &&
semverSatisfies(safeVersion, SAFE_VERSION_FOR_OFF_CHAIN_SIGNATURES)
)
}

// https://docs.gnosis.io/safe/docs/contracts_signatures/#pre-validated-signatures
export const getPreValidatedSignatures = (from: string, initialString: string = EMPTY_DATA): string => {
Expand Down