Fix: propose when creating or signing

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

@@ -1,7 +1,7 @@
 import { FEATURES } from '@gnosis.pm/safe-react-gateway-sdk'
 
 import { ChainId } from 'src/config/chain.d'
-import { buildSafeOwners, extractRemoteSafeInfo, shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils'
+import { buildSafeOwners, extractRemoteSafeInfo, canExecuteCreatedTx } from 'src/logic/safe/store/actions/utils'
 import { SafeRecordProps } from 'src/logic/safe/store/models/safe'
 import { getMockedSafeInstance, getMockedStoredTServiceModel } from 'src/test/utils/safeHelper'
 import { LocalTransactionStatus } from '../../models/types/gateway.d'
@@ -14,15 +14,15 @@ import {
 
 const lastTxFromStore = getMockedStoredTServiceModel()
 
-describe('shouldExecuteTransaction', () => {
+describe('canExecuteCreatedTx', () => {
   it('It should return false if given a safe with a threshold > 1', async () => {
     // given
     const nonce = '0'
     const threshold = '2'
     const safeInstance = getMockedSafeInstance({ threshold })
 
     // when
-    const result = await shouldExecuteTransaction(safeInstance, nonce, lastTxFromStore)
+    const result = await canExecuteCreatedTx(safeInstance, nonce, lastTxFromStore)
 
     // then
     expect(result).toBe(false)
@@ -35,7 +35,7 @@ describe('shouldExecuteTransaction', () => {
     const lastTxFromStoreExecuted = { ...lastTxFromStore, txStatus: LocalTransactionStatus.SUCCESS }
 
     // when
-    const result = await shouldExecuteTransaction(safeInstance, nonce, lastTxFromStoreExecuted)
+    const result = await canExecuteCreatedTx(safeInstance, nonce, lastTxFromStoreExecuted)
 
     // then
     expect(result).toBe(true)
@@ -48,7 +48,7 @@ describe('shouldExecuteTransaction', () => {
     const lastTxFromStoreExecuted = { ...lastTxFromStore, txStatus: LocalTransactionStatus.SUCCESS }
 
     // when
-    const result = await shouldExecuteTransaction(safeInstance, nonce, lastTxFromStoreExecuted)
+    const result = await canExecuteCreatedTx(safeInstance, nonce, lastTxFromStoreExecuted)
 
     // then
     expect(result).toBe(true)
@@ -61,7 +61,7 @@ describe('shouldExecuteTransaction', () => {
     const lastTxFromStoreExecuted = { ...lastTxFromStore, txStatus: LocalTransactionStatus.FAILED }
 
     // when
-    const result = await shouldExecuteTransaction(safeInstance, nonce, lastTxFromStoreExecuted)
+    const result = await canExecuteCreatedTx(safeInstance, nonce, lastTxFromStoreExecuted)
 
     // then
     expect(result).toBe(false)

src/logic/safe/store/actions/createTransaction.ts

@@ -17,7 +17,7 @@ import { ZERO_ADDRESS } from 'src/logic/wallets/ethAddresses'
 import { EMPTY_DATA } from 'src/logic/wallets/ethTransactions'
 import { providerSelector } from 'src/logic/wallets/store/selectors'
 import { generateSafeTxHash } from 'src/logic/safe/store/actions/transactions/utils/transactionHelpers'
-import { getNonce, shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils'
+import { getNonce, canExecuteCreatedTx } from 'src/logic/safe/store/actions/utils'
 import fetchTransactions from './transactions/fetchTransactions'
 import { AppReduxState } from 'src/store'
 import { Dispatch, DispatchReturn } from './types'
@@ -102,25 +102,17 @@ export class TxSender {
   dispatch: Dispatch
   safeInstance: GnosisSafe
   safeVersion: string
-  approveAndExecute: boolean
   txId: string
 
   // On transaction completion (either confirming or executing)
   async onComplete(signature?: string, confirmCallback?: ConfirmEventHandler): Promise<void> {
-    const { txArgs, safeTxHash, txProps, dispatch, notifications, isFinalization, approveAndExecute = false } = this
+    const { txArgs, safeTxHash, txProps, dispatch, notifications, isFinalization } = this
 
+    // Propose the tx to the backend
+    // 1) If signing
+    // 2) If creating a new tx (no txId yet)
     let txDetails: TransactionDetails | null = null
-
-    const isOffChainSigning = !isFinalization && signature
-    const isOnChainSigning = isFinalization && !signature
-
-    // If 1/? threshold and owner chooses to execute created tx immediately
-    const isImmediateExecution = isOnChainSigning && !approveAndExecute
-
-    // Propose the tx to the backend if an owner and
-    // 1) It's a confirmation w/o exection
-    // 2) It's a creation + execution w/o pre-approved signatures
-    if (isOffChainSigning || isImmediateExecution) {
+    if (!isFinalization || !this.txId) {
       try {
         txDetails = await saveTxToHistory({ ...txArgs, signature, origin })
       } catch (err) {
@@ -129,8 +121,8 @@ export class TxSender {
       }
     }
 
-    // If 1/? threshold and owner chooses to execute created tx immediately
-    // we retrieve txId of newly created tx from proposal response
+    // If threshold reached except for last sig, and owner chooses to execute the created tx immediately
+    // we retrieve txId of newly created tx from the proposal response
     if (isFinalization && txDetails) {
       dispatch(addPendingTransaction({ id: txDetails.txId }))
     }
@@ -298,8 +290,7 @@ export const createTransaction = (
 
     // Execute right away?
     sender.isFinalization =
-      !props.delayExecution &&
-      (await shouldExecuteTransaction(sender.safeInstance, sender.nonce, getLastTransaction(state)))
+      !props.delayExecution && (await canExecuteCreatedTx(sender.safeInstance, sender.nonce, getLastTransaction(state)))
 
     // Prepare a TxArgs object
     sender.txArgs = {

src/logic/safe/store/actions/processTransaction.ts

@@ -3,16 +3,14 @@ import { AnyAction } from 'redux'
 import { ThunkAction } from 'redux-thunk'
 import { Operation } from '@gnosis.pm/safe-react-gateway-sdk'
 
-import { generateSignaturesFromTxConfirmations, getPreValidatedSignatures } from 'src/logic/safe/safeTxSigner'
+import { generateSignaturesFromTxConfirmations } from 'src/logic/safe/safeTxSigner'
 import { EMPTY_DATA } from 'src/logic/wallets/ethTransactions'
 import { AppReduxState } from 'src/store'
 import { TxParameters } from 'src/routes/safe/container/hooks/useTransactionParameters'
 import { Dispatch, DispatchReturn } from './types'
 import { Confirmation } from 'src/logic/safe/store/models/types/confirmation'
 import { TxSender } from './createTransaction'
 import { logError, Errors } from 'src/logic/exceptions/CodedException'
-import { getLastTransaction } from '../selectors/gatewayTransactions'
-import { shouldExecuteTransaction } from './utils'
 
 interface ProcessTransactionArgs {
   approveAndExecute: boolean
@@ -34,8 +32,8 @@ interface ProcessTransactionArgs {
     gasToken: string
     refundReceiver: string
   }
-  userAddress: string
   ethParameters?: Pick<TxParameters, 'ethNonce' | 'ethGasLimit' | 'ethGasPriceInGWei' | 'ethMaxPrioFeeInGWei'>
+  preApprovingOwner?: string
   thresholdReached: boolean
 }
 
@@ -75,14 +73,7 @@ export const processTransaction = (props: ProcessTransactionArgs): ProcessTransa
       return
     }
 
-    // Execute right away?
-    sender.isFinalization =
-      approveAndExecute ||
-      (await shouldExecuteTransaction(sender.safeInstance, sender.nonce, getLastTransaction(state)))
-
-    sender.approveAndExecute = approveAndExecute
-
-    const preApprovingOwner = approveAndExecute && !props.thresholdReached ? props.userAddress : undefined
+    sender.isFinalization = approveAndExecute && !!(props.thresholdReached || props.preApprovingOwner)
 
     sender.txArgs = {
       ...tx, // Merge previous tx with new data
@@ -91,9 +82,10 @@ export const processTransaction = (props: ProcessTransactionArgs): ProcessTransa
       data: txProps.txData,
       gasPrice: tx.gasPrice || '0',
       sender: sender.from,
-      sigs:
-        generateSignaturesFromTxConfirmations(tx.confirmations, preApprovingOwner) ||
-        getPreValidatedSignatures(sender.from),
+      sigs: generateSignaturesFromTxConfirmations(
+        tx.confirmations,
+        approveAndExecute ? props.preApprovingOwner : undefined,
+      ),
     }
 
     sender.safeTxHash = tx.safeTxHash

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

@@ -17,7 +17,7 @@ import { getGnosisSafeInstanceAt } from 'src/logic/contracts/safeContracts'
 import { logError, Errors } from 'src/logic/exceptions/CodedException'
 import { getRecommendedNonce } from '../../api/fetchSafeTxGasEstimation'
 
-export const shouldExecuteTransaction = async (
+export const canExecuteCreatedTx = async (
   safeInstance: GnosisSafe,
   nonce: string,
   lastTx: Transaction | null,

src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions.ts

@@ -11,11 +11,6 @@ import { grantedSelector } from 'src/routes/safe/container/selector'
 import { AppReduxState } from 'src/store'
 import { TxLocationContext } from '../TxLocationProvider'
 
-export const isThresholdReached = (executionInfo: MultisigExecutionInfo): boolean => {
-  const { confirmationsSubmitted, confirmationsRequired } = executionInfo
-  return confirmationsSubmitted >= confirmationsRequired
-}
-
 export type TransactionActions = {
   canConfirm: boolean
   canConfirmThenExecute: boolean

src/routes/safe/components/Transactions/TxList/modals/ApproveTxModal.tsx

@@ -22,14 +22,19 @@ import { processTransaction } from 'src/logic/safe/store/actions/processTransact
 import { TxParameters } from 'src/routes/safe/container/hooks/useTransactionParameters'
 import { EMPTY_DATA } from 'src/logic/wallets/ethTransactions'
 import { userAccountSelector } from 'src/logic/wallets/store/selectors'
-import { isThresholdReached } from 'src/routes/safe/components/Transactions/TxList/hooks/useTransactionActions'
 import { ModalHeader } from 'src/routes/safe/components/Balances/SendModal/screens/ModalHeader'
 import { Overwrite } from 'src/types/helpers'
 import { ZERO_ADDRESS } from 'src/logic/wallets/ethAddresses'
 import { makeConfirmation } from 'src/logic/safe/store/models/confirmation'
-import { ExpandedTxDetails, isMultiSigExecutionDetails, Transaction } from 'src/logic/safe/store/models/types/gateway.d'
+import {
+  ExpandedTxDetails,
+  isMultiSigExecutionDetails,
+  isMultisigExecutionInfo,
+  Transaction,
+} from 'src/logic/safe/store/models/types/gateway.d'
 import { extractSafeAddress } from 'src/routes/routes'
 import { TxModalWrapper } from '../../helpers/TxModalWrapper'
+import { grantedSelector } from 'src/routes/safe/container/selector'
 
 export const REJECT_TX_MODAL_SUBMIT_BTN_TEST_ID = 'reject-tx-modal-submit-btn'
 
@@ -190,21 +195,29 @@ type Props = {
 export const ApproveTxModal = ({ onClose, isOpen, transaction }: Props): React.ReactElement => {
   const dispatch = useDispatch()
   const userAddress = useSelector(userAccountSelector)
+  const isOwner = useSelector(grantedSelector)
   const classes = useStyles()
   const safeAddress = extractSafeAddress()
-  const executionInfo = transaction.executionInfo as MultisigExecutionInfo
-  const thresholdReached = !!(executionInfo && isThresholdReached(executionInfo))
+  const txInfo = useTxInfo(transaction)
+
+  const { executionInfo } = transaction
+  const { confirmationsSubmitted = 0, confirmationsRequired = 0 } = isMultisigExecutionInfo(executionInfo)
+    ? executionInfo
+    : {}
+  const thresholdReached = confirmationsSubmitted >= confirmationsRequired
   const { description, title } = getModalTitleAndDescription(thresholdReached)
 
-  const txInfo = useTxInfo(transaction)
-  const { confirmations } = txInfo
+  let preApprovingOwner: string | undefined = undefined
+  if (!thresholdReached && isOwner && confirmationsSubmitted === confirmationsRequired - 1) {
+    preApprovingOwner = userAddress
+  }
 
   const approveTx = (txParameters: TxParameters, delayExecution: boolean) => {
     dispatch(
       processTransaction({
         safeAddress,
         tx: txInfo,
-        userAddress,
+        preApprovingOwner,
         notifiedTransaction: TX_NOTIFICATION_TYPES.CONFIRMATION_TX,
         approveAndExecute: !delayExecution,
         ethParameters: txParameters,
@@ -219,8 +232,8 @@ export const ApproveTxModal = ({ onClose, isOpen, transaction }: Props): React.R
       <TxModalWrapper
         operation={txInfo.operation}
         txNonce={txInfo.nonce.toString()}
-        txConfirmations={confirmations}
-        txThreshold={executionInfo.confirmationsRequired}
+        txConfirmations={txInfo.confirmations}
+        txThreshold={confirmationsRequired}
         txTo={txInfo.to}
         txData={txInfo.data}
         txValue={txInfo.value}