diff --git a/src/logic/safe/transactions/send.js b/src/logic/safe/transactions/send.js index 379e100858..d3daabb252 100644 --- a/src/logic/safe/transactions/send.js +++ b/src/logic/safe/transactions/send.js @@ -8,20 +8,33 @@ export const DELEGATE_CALL = 1 export const TX_TYPE_EXECUTION = 'execution' export const TX_TYPE_CONFIRMATION = 'confirmation' -export const getApprovalTransaction = async ( +type Transaction = { safeInstance: any, to: string, valueInWei: number | string, data: string, operation: Operation, - nonce: number, safeTxGas: number, baseGas: number, gasPrice: number, gasToken: string, refundReceiver: string, - sender: string, -) => { +} + +export const getApprovalTransaction = async ({ + safeInstance, + to, + valueInWei, + data, + operation, + nonce, + safeTxGas, + baseGas, + gasPrice, + gasToken, + refundReceiver, + sender, +}: Transaction & { nonce: number | string, sender: string }) => { const txHash = await safeInstance.getTransactionHash( to, valueInWei, @@ -49,21 +62,19 @@ export const getApprovalTransaction = async ( } } -export const getExecutionTransaction = async ( - safeInstance: any, - to: string, - valueInWei: number | string, - data: string, - operation: Operation, - nonce: string | number, - safeTxGas: string | number, - baseGas: string | number, - gasPrice: string | number, - gasToken: string, - refundReceiver: string, - sender: string, - sigs: string, -) => { +export const getExecutionTransaction = async ({ + safeInstance, + to, + valueInWei, + data, + operation, + safeTxGas, + baseGas, + gasPrice, + gasToken, + refundReceiver, + sigs, +}: Transaction & { sigs: string }) => { try { const web3 = getWeb3() const contract = new web3.eth.Contract(GnosisSafeSol.abi, safeInstance.address) diff --git a/src/logic/safe/transactions/txHistory.js b/src/logic/safe/transactions/txHistory.js index 5e6c0a1d14..f35f9535aa 100644 --- a/src/logic/safe/transactions/txHistory.js +++ b/src/logic/safe/transactions/txHistory.js @@ -21,6 +21,7 @@ const calculateBodyFrom = async ( transactionHash: string, sender: string, confirmationType: TxServiceType, + origin: string | null, ) => { const contractTransactionHash = await safeInstance.getTransactionHash( to, @@ -50,6 +51,7 @@ const calculateBodyFrom = async ( transactionHash, sender: getWeb3().utils.toChecksumAddress(sender), confirmationType, + origin, } } @@ -60,7 +62,23 @@ export const buildTxServiceUrl = (safeAddress: string) => { return `${host}${base}` } -export const saveTxToHistory = async ( +export const saveTxToHistory = async ({ + safeInstance, + to, + valueInWei, + data, + operation, + nonce, + safeTxGas, + baseGas, + gasPrice, + gasToken, + refundReceiver, + txHash, + sender, + type, + origin, +}: { safeInstance: any, to: string, valueInWei: number | string, @@ -75,7 +93,8 @@ export const saveTxToHistory = async ( txHash: string, sender: string, type: TxServiceType, -) => { + origin: string | null, +}) => { const url = buildTxServiceUrl(safeInstance.address) const body = await calculateBodyFrom( safeInstance, @@ -92,6 +111,7 @@ export const saveTxToHistory = async ( txHash, sender, type, + origin || null, ) const response = await axios.post(url, body) diff --git a/src/routes/safe/components/Apps/index.jsx b/src/routes/safe/components/Apps/index.jsx index 49bd8fc1fc..3b27f40956 100644 --- a/src/routes/safe/components/Apps/index.jsx +++ b/src/routes/safe/components/Apps/index.jsx @@ -54,7 +54,7 @@ function Apps({ web3, safeAddress, safeName, ethBalance, network, createTransact const onConfirm = async () => { closeModal() - const txHash = await sendTransactions(web3, createTransaction, safeAddress, data.data) + const txHash = await sendTransactions(web3, createTransaction, safeAddress, data.data, getSelectedApp().name) if (txHash) { sendMessageToIframe(operations.ON_TX_UPDATE, { diff --git a/src/routes/safe/components/Apps/sendTransactions.js b/src/routes/safe/components/Apps/sendTransactions.js index 8be4b46962..b4ce89eb79 100644 --- a/src/routes/safe/components/Apps/sendTransactions.js +++ b/src/routes/safe/components/Apps/sendTransactions.js @@ -14,7 +14,7 @@ const multiSendAbi = [ }, ] -const sendTransactions = (web3: any, createTransaction: any, safeAddress: String, txs: Array) => { +const sendTransactions = (web3: any, createTransaction: any, safeAddress: String, txs: Array, origin: string) => { const multiSend = new web3.eth.Contract(multiSendAbi, multiSendAddress) const encodeMultiSendCalldata = multiSend.methods @@ -43,6 +43,7 @@ const sendTransactions = (web3: any, createTransaction: any, safeAddress: String closeSnackbar: () => {}, operation: DELEGATE_CALL, navigateToTransactionsTab: false, + origin, }) } export default sendTransactions diff --git a/src/routes/safe/store/actions/__tests__/utils.test.js b/src/routes/safe/store/actions/__tests__/utils.test.js new file mode 100644 index 0000000000..dfbc722116 --- /dev/null +++ b/src/routes/safe/store/actions/__tests__/utils.test.js @@ -0,0 +1,117 @@ +import { getNewTxNonce, shouldExecuteTransaction } from '~/routes/safe/store/actions/utils' + +describe('Store actions utils > getNewTxNonce', () => { + it(`should return txNonce if it's a valid value`, async () => { + // Given + const txNonce = '45' + const lastTx = { + nonce: 44 + } + const safeInstance = { + nonce: () => Promise.resolve({ + toString: () => Promise.resolve('45') + }) + } + + // When + const nonce = await getNewTxNonce(txNonce, lastTx, safeInstance) + + // Then + expect(nonce).toBe('45') + }) + + it(`should return lastTx.nonce + 1 if txNonce is not valid`, async () => { + // Given + const txNonce = '' + const lastTx = { + nonce: 44 + } + const safeInstance = { + nonce: () => Promise.resolve({ + toString: () => Promise.resolve('45') + }) + } + + // When + const nonce = await getNewTxNonce(txNonce, lastTx, safeInstance) + + // Then + expect(nonce).toBe('45') + }) + + it(`should retrieve contract's instance nonce value, if txNonce and lastTx are not valid`, async () => { + // Given + const txNonce = '' + const lastTx = null + const safeInstance = { + nonce: () => Promise.resolve({ + toString: () => Promise.resolve('45') + }) + } + + // When + const nonce = await getNewTxNonce(txNonce, lastTx, safeInstance) + + // Then + expect(nonce).toBe('45') + }) +}) + +describe('Store actions utils > shouldExecuteTransaction', () => { + it(`should return false if there's a previous tx pending to be executed`, async () => { + // Given + const safeInstance = { + getThreshold: () => Promise.resolve({ + toNumber: () => 1 + }) + } + const nonce = '1' + const lastTx = { + isExecuted: false + } + + // When + const isExecution = await shouldExecuteTransaction(safeInstance, nonce, lastTx) + + // Then + expect(isExecution).toBeFalsy() + }) + + it(`should return false if threshold is greater than 1`, async () => { + // Given + const safeInstance = { + getThreshold: () => Promise.resolve({ + toNumber: () => 2 + }) + } + const nonce = '1' + const lastTx = { + isExecuted: true + } + + // When + const isExecution = await shouldExecuteTransaction(safeInstance, nonce, lastTx) + + // Then + expect(isExecution).toBeFalsy() + }) + + it(`should return true is threshold is 1 and previous tx is executed`, async () => { + // Given + const safeInstance = { + getThreshold: () => Promise.resolve({ + toNumber: () => 1 + }) + } + const nonce = '1' + const lastTx = { + isExecuted: true + } + + // When + const isExecution = await shouldExecuteTransaction(safeInstance, nonce, lastTx) + + // Then + expect(isExecution).toBeTruthy() + }) +}) diff --git a/src/routes/safe/store/actions/createTransaction.js b/src/routes/safe/store/actions/createTransaction.js index b432629c7c..c149c4c5fe 100644 --- a/src/routes/safe/store/actions/createTransaction.js +++ b/src/routes/safe/store/actions/createTransaction.js @@ -1,12 +1,10 @@ // @flow -import axios from 'axios' import type { Dispatch as ReduxDispatch, GetState } from 'redux' import { push } from 'connected-react-router' import { EMPTY_DATA } from '~/logic/wallets/ethTransactions' import { userAccountSelector } from '~/logic/wallets/store/selectors' import fetchTransactions from '~/routes/safe/store/actions/fetchTransactions' import { type GlobalState } from '~/store' -import { buildTxServiceUrl } from '~/logic/safe/transactions/txHistory' import { getGnosisSafeInstanceAt } from '~/logic/contracts/safeContracts' import { getApprovalTransaction, @@ -21,32 +19,7 @@ import { type NotificationsQueue, getNotificationsFromTxType, showSnackbar } fro import { getErrorMessage } from '~/test/utils/ethereumErrors' import { ZERO_ADDRESS } from '~/logic/wallets/ethAddresses' import { SAFELIST_ADDRESS } from '~/routes/routes' -import type { TransactionProps } from '~/routes/safe/store/models/transaction' - -const getLastTx = async (safeAddress: string): Promise => { - try { - const url = buildTxServiceUrl(safeAddress) - const response = await axios.get(url, { params: { limit: 1 } }) - - return response.data.results[0] - } catch (e) { - console.error('failed to retrieve last Tx from server', e) - return null - } -} - -const getSafeNonce = async (safeAddress: string): Promise => { - // use current's safe nonce as fallback - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) - return (await safeInstance.nonce()).toString() -} - -const getNewTxNonce = async (txNonce, lastTx, safeAddress) => { - if (!Number.isInteger(Number.parseInt(txNonce, 10))) { - return lastTx === null ? getSafeNonce(safeAddress) : lastTx.nonce + 1 - } - return txNonce -} +import { getLastTx, getNewTxNonce, shouldExecuteTransaction } from '~/routes/safe/store/actions/utils' type CreateTransactionArgs = { safeAddress: string, @@ -56,9 +29,10 @@ type CreateTransactionArgs = { notifiedTransaction: NotifiedTransaction, enqueueSnackbar: Function, closeSnackbar: Function, - shouldExecute?: boolean, txNonce?: number, operation?: 0 | 1, + navigateToTransactionsTab?: boolean, + origin?: string | null, } const createTransaction = ({ @@ -69,10 +43,10 @@ const createTransaction = ({ notifiedTransaction, enqueueSnackbar, closeSnackbar, - shouldExecute = false, txNonce, operation = CALL, navigateToTransactionsTab = true, + origin = null, }: CreateTransactionArgs) => async (dispatch: ReduxDispatch, getState: GetState) => { const state: GlobalState = getState() @@ -82,10 +56,9 @@ const createTransaction = ({ const from = userAccountSelector(state) const safeInstance = await getGnosisSafeInstanceAt(safeAddress) - const threshold = await safeInstance.getThreshold() const lastTx = await getLastTx(safeAddress) - const nonce = await getNewTxNonce(txNonce, lastTx, safeAddress) - const isExecution = (lastTx && lastTx.isExecuted && threshold.toNumber() === 1) || shouldExecute + const nonce = await getNewTxNonce(txNonce, lastTx, safeInstance) + const isExecution = await shouldExecuteTransaction(safeInstance, nonce, lastTx) // https://gnosis-safe.readthedocs.io/en/latest/contracts/signatures.html#pre-validated-signatures const sigs = `0x000000000000000000000000${from.replace( @@ -99,40 +72,24 @@ const createTransaction = ({ let txHash let tx + const txArgs = { + safeInstance, + to, + valueInWei, + data: txData, + operation, + nonce, + safeTxGas: 0, + baseGas: 0, + gasPrice: 0, + gasToken: ZERO_ADDRESS, + refundReceiver: ZERO_ADDRESS, + sender: from, + sigs, + } + try { - if (isExecution) { - tx = await getExecutionTransaction( - safeInstance, - to, - valueInWei, - txData, - operation, - nonce, - 0, - 0, - 0, - ZERO_ADDRESS, - ZERO_ADDRESS, - from, - sigs, - ) - } else { - tx = await getApprovalTransaction( - safeInstance, - to, - valueInWei, - txData, - operation, - nonce, - 0, - 0, - 0, - ZERO_ADDRESS, - ZERO_ADDRESS, - from, - sigs, - ) - } + tx = isExecution ? await getExecutionTransaction(txArgs) : await getApprovalTransaction(txArgs) const sendParams = { from, value: 0 } @@ -155,22 +112,12 @@ const createTransaction = ({ pendingExecutionKey = showSnackbar(notificationsQueue.pendingExecution, enqueueSnackbar, closeSnackbar) try { - await saveTxToHistory( - safeInstance, - to, - valueInWei, - txData, - operation, - nonce, - 0, - 0, - 0, - ZERO_ADDRESS, - ZERO_ADDRESS, + await saveTxToHistory({ + ...txArgs, txHash, - from, - isExecution ? TX_TYPE_EXECUTION : TX_TYPE_CONFIRMATION, - ) + type: isExecution ? TX_TYPE_EXECUTION : TX_TYPE_CONFIRMATION, + origin, + }) dispatch(fetchTransactions(safeAddress)) } catch (err) { console.error(err) diff --git a/src/routes/safe/store/actions/processTransaction.js b/src/routes/safe/store/actions/processTransaction.js index e545bb7da5..7465273d94 100644 --- a/src/routes/safe/store/actions/processTransaction.js +++ b/src/routes/safe/store/actions/processTransaction.js @@ -1,5 +1,6 @@ // @flow import type { Dispatch as ReduxDispatch } from 'redux' + import { type Transaction } from '~/routes/safe/store/models/transaction' import { userAccountSelector } from '~/logic/wallets/store/selectors' import fetchSafe from '~/routes/safe/store/actions/fetchSafe' @@ -17,6 +18,7 @@ import { import { generateSignaturesFromTxConfirmations } from '~/logic/safe/safeTxSigner' import { type NotificationsQueue, getNotificationsFromTxType, showSnackbar } from '~/logic/notifications' import { getErrorMessage } from '~/test/utils/ethereumErrors' +import { getLastTx, getNewTxNonce, shouldExecuteTransaction } from '~/routes/safe/store/actions/utils' type ProcessTransactionArgs = { safeAddress: string, @@ -39,10 +41,11 @@ const processTransaction = ({ }: ProcessTransactionArgs) => async (dispatch: ReduxDispatch, getState: Function) => { const state: GlobalState = getState() - const safeInstance = await getGnosisSafeInstanceAt(safeAddress) const from = userAccountSelector(state) - const threshold = (await safeInstance.getThreshold()).toNumber() - const shouldExecute = threshold === tx.confirmations.size || approveAndExecute + const safeInstance = await getGnosisSafeInstanceAt(safeAddress) + const lastTx = await getLastTx(safeAddress) + const nonce = await getNewTxNonce(null, lastTx, safeInstance) + const isExecution = approveAndExecute || (await shouldExecuteTransaction(safeInstance, nonce, lastTx)) let sigs = generateSignaturesFromTxConfirmations(tx.confirmations, approveAndExecute && userAddress) // https://gnosis-safe.readthedocs.io/en/latest/contracts/signatures.html#pre-validated-signatures @@ -59,39 +62,24 @@ const processTransaction = ({ let txHash let transaction + const txArgs = { + safeInstance, + to: tx.recipient, + valueInWei: tx.value, + data: tx.data, + operation: tx.operation, + nonce: tx.nonce, + safeTxGas: tx.safeTxGas, + baseGas: tx.baseGas, + gasPrice: tx.gasPrice || '0', + gasToken: tx.gasToken, + refundReceiver: tx.refundReceiver, + sender: from, + sigs, + } + try { - if (shouldExecute) { - transaction = await getExecutionTransaction( - safeInstance, - tx.recipient, - tx.value, - tx.data, - tx.operation, - tx.nonce, - tx.safeTxGas, - tx.baseGas, - tx.gasPrice || '0', - tx.gasToken, - tx.refundReceiver, - from, - sigs, - ) - } else { - transaction = await getApprovalTransaction( - safeInstance, - tx.recipient, - tx.value, - tx.data, - tx.operation, - tx.nonce, - tx.safeTxGas, - tx.baseGas, - tx.gasPrice || '0', - tx.gasToken, - tx.refundReceiver, - from, - ) - } + transaction = isExecution ? await getExecutionTransaction(txArgs) : await getApprovalTransaction(txArgs) const sendParams = { from, value: 0 } // if not set owner management tests will fail on ganache @@ -108,22 +96,11 @@ const processTransaction = ({ pendingExecutionKey = showSnackbar(notificationsQueue.pendingExecution, enqueueSnackbar, closeSnackbar) try { - await saveTxToHistory( - safeInstance, - tx.recipient, - tx.value, - tx.data, - tx.operation, - tx.nonce, - tx.safeTxGas, - tx.baseGas, - tx.gasPrice || '0', - tx.gasToken, - tx.refundReceiver, + await saveTxToHistory({ + ...txArgs, txHash, - from, - shouldExecute ? TX_TYPE_EXECUTION : TX_TYPE_CONFIRMATION, - ) + type: isExecution ? TX_TYPE_EXECUTION : TX_TYPE_CONFIRMATION, + }) dispatch(fetchTransactions(safeAddress)) } catch (err) { console.error(err) @@ -136,7 +113,7 @@ const processTransaction = ({ closeSnackbar(pendingExecutionKey) showSnackbar( - shouldExecute + isExecution ? notificationsQueue.afterExecution.noMoreConfirmationsNeeded : notificationsQueue.afterExecution.moreConfirmationsNeeded, enqueueSnackbar, @@ -144,7 +121,7 @@ const processTransaction = ({ ) dispatch(fetchTransactions(safeAddress)) - if (shouldExecute) { + if (isExecution) { dispatch(fetchSafe(safeAddress)) } diff --git a/src/routes/safe/store/actions/utils.js b/src/routes/safe/store/actions/utils.js new file mode 100644 index 0000000000..e53356ef16 --- /dev/null +++ b/src/routes/safe/store/actions/utils.js @@ -0,0 +1,43 @@ +// @flow +import axios from 'axios' + +import { buildTxServiceUrl } from '~/logic/safe/transactions/txHistory' + +export const getLastTx = async (safeAddress: string): Promise => { + try { + const url = buildTxServiceUrl(safeAddress) + const response = await axios.get(url, { params: { limit: 1 } }) + + return response.data.results[0] + } catch (e) { + console.error('failed to retrieve last Tx from server', e) + return null + } +} + +export const getNewTxNonce = async (txNonce, lastTx, safeInstance) => { + if (!Number.isInteger(Number.parseInt(txNonce, 10))) { + return lastTx === null + ? // use current's safe nonce as fallback + (await safeInstance.nonce()).toString() + : `${lastTx.nonce + 1}` + } + return txNonce +} + +export const shouldExecuteTransaction = async (safeInstance, nonce, lastTx) => { + const threshold = await safeInstance.getThreshold() + + // Tx will automatically be executed if and only if the threshold is 1 + if (threshold.toNumber() === 1) { + const isFirstTransaction = Number.parseInt(nonce) === 0 + // if the previous tx is not executed, it's delayed using the approval mechanisms, + // once the previous tx is executed, the current tx will be available to be executed + // by the user using the exec button. + const canExecuteCurrentTransaction = lastTx && lastTx.isExecuted + + return isFirstTransaction || canExecuteCurrentTransaction + } + + return false +}