(feature) Adding origin for Apps Transactions (#576)

* Adding origin field when creates a TX

* refactor: replace list of arg by object in getApprovalTransaction and getExecutionTransaction function

* minor changes

* Allow execute if threshold is 1 for the first tx

- Related to issue #563
- `lastTx` is required due to #489

* - Normalizing logic between createTransaction and processTransaction
- Moving shared function to a new file

* Refactor `doesTxNeedApproval` back to the `isExecution`-related meaning

* Rename function and variable names

* Add tests for `getNewTxNonce` and `shouldExecuteTransaction` functions

* Pass `safeInstance` instead of `safeAddress` to `getNewTxNonce`

* Update Tests

- remove mocked `getGnosisSafeInstanceAt`
- pass `safeInstance` instead of `safeAddress` to `getNewTxNonce`

Co-authored-by: Fernando <fernando.greco@gmail.com>

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 (
+export const getExecutionTransaction = async ({
-  safeInstance: any,
+  safeInstance,
-  to: string,
+  to,
-  valueInWei: number | string,
+  valueInWei,
-  data: string,
+  data,
-  operation: Operation,
+  operation,
-  nonce: string | number,
+  safeTxGas,
-  safeTxGas: string | number,
+  baseGas,
-  baseGas: string | number,
+  gasPrice,
-  gasPrice: string | number,
+  gasToken,
-  gasToken: string,
+  refundReceiver,
-  refundReceiver: string,
+  sigs,
-  sender: string,
+}: Transaction & { sigs: string }) => {
-  sigs: string,
-) => {
   try {
     const web3 = getWeb3()
     const contract = new web3.eth.Contract(GnosisSafeSol.abi, safeInstance.address)

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)
 

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, {

src/routes/safe/components/Apps/sendTransactions.js

@@ -14,7 +14,7 @@ const multiSendAbi = [
   },
 ]
 
-const sendTransactions = (web3: any, createTransaction: any, safeAddress: String, txs: Array<any>) => {
+const sendTransactions = (web3: any, createTransaction: any, safeAddress: String, txs: Array<any>, 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

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()
+  })
+})

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'
+import { getLastTx, getNewTxNonce, shouldExecuteTransaction } from '~/routes/safe/store/actions/utils'
-
-const getLastTx = async (safeAddress: string): Promise<TransactionProps> => {
-  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<string> => {
-  // 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
-}
 
 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<GlobalState>, getState: GetState<GlobalState>) => {
   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 nonce = await getNewTxNonce(txNonce, lastTx, safeInstance)
-  const isExecution = (lastTx && lastTx.isExecuted && threshold.toNumber() === 1) || shouldExecute
+  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 = isExecution ? await getExecutionTransaction(txArgs) : await getApprovalTransaction(txArgs)
-      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,
-      )
-    }
 
     const sendParams = { from, value: 0 }
 
@@ -155,22 +112,12 @@ const createTransaction = ({
         pendingExecutionKey = showSnackbar(notificationsQueue.pendingExecution, enqueueSnackbar, closeSnackbar)
 
         try {
-          await saveTxToHistory(
+          await saveTxToHistory({
-            safeInstance,
+            ...txArgs,
-            to,
-            valueInWei,
-            txData,
-            operation,
-            nonce,
-            0,
-            0,
-            0,
-            ZERO_ADDRESS,
-            ZERO_ADDRESS,
             txHash,
-            from,
+            type: isExecution ? TX_TYPE_EXECUTION : TX_TYPE_CONFIRMATION,
-            isExecution ? TX_TYPE_EXECUTION : TX_TYPE_CONFIRMATION,
+            origin,
-          )
+          })
           dispatch(fetchTransactions(safeAddress))
         } catch (err) {
           console.error(err)

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<GlobalState>, getState: Function) => {
   const state: GlobalState = getState()
 
-  const safeInstance = await getGnosisSafeInstanceAt(safeAddress)
   const from = userAccountSelector(state)
-  const threshold = (await safeInstance.getThreshold()).toNumber()
+  const safeInstance = await getGnosisSafeInstanceAt(safeAddress)
-  const shouldExecute = threshold === tx.confirmations.size || approveAndExecute
+  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 = isExecution ? await getExecutionTransaction(txArgs) : await getApprovalTransaction(txArgs)
-      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,
-      )
-    }
 
     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(
+          await saveTxToHistory({
-            safeInstance,
+            ...txArgs,
-            tx.recipient,
-            tx.value,
-            tx.data,
-            tx.operation,
-            tx.nonce,
-            tx.safeTxGas,
-            tx.baseGas,
-            tx.gasPrice || '0',
-            tx.gasToken,
-            tx.refundReceiver,
             txHash,
-            from,
+            type: isExecution ? TX_TYPE_EXECUTION : TX_TYPE_CONFIRMATION,
-            shouldExecute ? 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))
         }
 

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<TransactionProps> => {
+  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
+}