From 212cce6fa1ce4b66af1ca848514c2389318112c2 Mon Sep 17 00:00:00 2001 From: fernandomg Date: Tue, 1 Sep 2020 19:11:10 -0300 Subject: [PATCH 1/9] fix: prevent runtime error when prev is `undefined` --- src/logic/currentSession/store/reducer/currentSession.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/logic/currentSession/store/reducer/currentSession.ts b/src/logic/currentSession/store/reducer/currentSession.ts index 228bfc9eef..8507d3d661 100644 --- a/src/logic/currentSession/store/reducer/currentSession.ts +++ b/src/logic/currentSession/store/reducer/currentSession.ts @@ -13,7 +13,7 @@ export default handleActions( [UPDATE_VIEWED_SAFES]: (state, action) => { const safeAddress = action.payload - const newState = state.updateIn(['viewedSafes'], (prev) => + const newState = state.updateIn(['viewedSafes'], (prev = []) => prev.includes(safeAddress) ? prev : [...prev, safeAddress], ) From 9d350dcadc22675ea41b5b6ff56299bbbfc183fb Mon Sep 17 00:00:00 2001 From: fernandomg Date: Tue, 1 Sep 2020 19:11:45 -0300 Subject: [PATCH 2/9] fix: prevent runtime error when prev is `undefined` --- src/logic/notifications/store/reducer/notifications.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/logic/notifications/store/reducer/notifications.ts b/src/logic/notifications/store/reducer/notifications.ts index 802ab9bd85..d758ab9100 100644 --- a/src/logic/notifications/store/reducer/notifications.ts +++ b/src/logic/notifications/store/reducer/notifications.ts @@ -20,7 +20,7 @@ export default handleActions( const { dismissAll, key } = action.payload if (key) { - return state.update(key, (prev) => prev.set('dismissed', true)) + return state.update(key, (prev) => prev?.set('dismissed', true) ?? prev) } if (dismissAll) { return state.withMutations((map) => { From 891edd3c052f7e6c023f7205692293c08c9f2769 Mon Sep 17 00:00:00 2001 From: fernandomg Date: Tue, 1 Sep 2020 19:12:48 -0300 Subject: [PATCH 3/9] fix: prevent runtime error when 'safes' is `undefined` --- src/logic/safe/store/middleware/notificationsMiddleware.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/logic/safe/store/middleware/notificationsMiddleware.ts b/src/logic/safe/store/middleware/notificationsMiddleware.ts index c30f5fa142..d46af6e29f 100644 --- a/src/logic/safe/store/middleware/notificationsMiddleware.ts +++ b/src/logic/safe/store/middleware/notificationsMiddleware.ts @@ -103,7 +103,7 @@ const notificationsMiddleware = (store) => (next) => async (action) => { } case ADD_INCOMING_TRANSACTIONS: { action.payload.forEach((incomingTransactions, safeAddress) => { - const { latestIncomingTxBlock } = state.safes.get('safes').get(safeAddress) + const { latestIncomingTxBlock = 0 } = state.safes.get('safes').get(safeAddress, {}) const viewedSafes = state.currentSession ? state.currentSession.get('viewedSafes') : [] const recurringUser = viewedSafes?.includes(safeAddress) From 02f67dab35b007e3eb2de9951a9d769ea7bbf0a7 Mon Sep 17 00:00:00 2001 From: fernandomg Date: Wed, 2 Sep 2020 12:35:33 -0300 Subject: [PATCH 4/9] fix: add `dataDecoded` to the mocked tx so information is properly displayed for known methods --- src/logic/safe/store/actions/createTransaction.ts | 2 ++ .../store/actions/transactions/utils/transactionHelpers.ts | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/logic/safe/store/actions/createTransaction.ts b/src/logic/safe/store/actions/createTransaction.ts index 6f0424a730..f21a5d349d 100644 --- a/src/logic/safe/store/actions/createTransaction.ts +++ b/src/logic/safe/store/actions/createTransaction.ts @@ -5,6 +5,7 @@ import semverSatisfies from 'semver/functions/satisfies' import { ThunkAction } from 'redux-thunk' import { onboardUser } from 'src/components/ConnectButton' +import { decodeMethods } from 'src/logic/contracts/methodIds' import { getGnosisSafeInstanceAt } from 'src/logic/contracts/safeContracts' import { getNotificationsFromTxType } from 'src/logic/notifications' import { @@ -205,6 +206,7 @@ const createTransaction = ( confirmations: [], // this is used to determine if a tx is pending or not. See `calculateTransactionStatus` helper value: txArgs.valueInWei, safeTxHash, + dataDecoded: decodeMethods(txArgs.data), submissionDate: new Date().toISOString(), } const mockedTx = await mockTransaction(txToMock, safeAddress, state) diff --git a/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts b/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts index 5349d7b669..7a2eeccf03 100644 --- a/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts +++ b/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts @@ -34,7 +34,7 @@ import { TypedDataUtils } from 'eth-sig-util' import { Token } from 'src/logic/tokens/store/model/token' import { ProviderRecord } from 'src/logic/wallets/store/model/provider' import { SafeRecord } from 'src/logic/safe/store/models/safe' -import { DecodedParams } from 'src/routes/safe/store/models/types/transactions.d' +import { DataDecoded, DecodedParams } from 'src/routes/safe/store/models/types/transactions.d' export const isEmptyData = (data?: string | null): boolean => { return !data || data === EMPTY_DATA @@ -315,6 +315,7 @@ export type TxToMock = TxArgs & { safeTxHash: string value: string submissionDate: string + dataDecoded: DataDecoded } export const mockTransaction = (tx: TxToMock, safeAddress: string, state: AppReduxState): Promise => { From d797c2db3dcc4ea88efa4dd30c761ca8bc4931d9 Mon Sep 17 00:00:00 2001 From: fernandomg Date: Wed, 2 Sep 2020 12:39:43 -0300 Subject: [PATCH 5/9] fix: set 'pending' status for tx being processed - given that the confirmations key is no longer an empty array, tx status must be explicitly set this way --- .../safe/store/actions/processTransaction.ts | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/logic/safe/store/actions/processTransaction.ts b/src/logic/safe/store/actions/processTransaction.ts index 0791584c75..c8a26780de 100644 --- a/src/logic/safe/store/actions/processTransaction.ts +++ b/src/logic/safe/store/actions/processTransaction.ts @@ -1,4 +1,3 @@ -import { fromJS } from 'immutable' import semverSatisfies from 'semver/functions/satisfies' import { getGnosisSafeInstanceAt } from 'src/logic/contracts/safeContracts' @@ -20,7 +19,6 @@ import { import { getLastTx, getNewTxNonce, shouldExecuteTransaction } from 'src/logic/safe/store/actions/utils' import { getErrorMessage } from 'src/test/utils/ethereumErrors' -import { makeConfirmation } from '../models/confirmation' import { storeTx } from './createTransaction' import { TransactionStatus } from '../models/types/transaction' @@ -85,6 +83,8 @@ const processTransaction = ({ approveAndExecute, notifiedTransaction, safeAddres dispatch(closeSnackbarAction(beforeExecutionKey)) await saveTxToHistory({ ...txArgs, signature }) + // TODO: while we wait for the tx to be stored in the service and later update the tx info + // we should update the tx status in the store to disable owners' action buttons dispatch(enqueueSnackbar(notificationsQueue.afterExecution.moreConfirmationsNeeded)) dispatch(fetchTransactions(safeAddress)) @@ -105,9 +105,7 @@ const processTransaction = ({ approveAndExecute, notifiedTransaction, safeAddres const txToMock: TxToMock = { ...txArgs, - confirmations: txArgs.confirmations, // this is used to determine if a tx is pending or not. See `calculateTransactionStatus` helper value: txArgs.valueInWei, - submissionDate: txArgs.submissionDate, } const mockedTx = await mockTransaction(txToMock, safeAddress, state) @@ -123,10 +121,14 @@ const processTransaction = ({ approveAndExecute, notifiedTransaction, safeAddres await Promise.all([ saveTxToHistory({ ...txArgs, txHash }), storeTx( - mockedTx.updateIn( - ['ownersWithPendingActions', mockedTx.isCancellationTx ? 'reject' : 'confirm'], - (previous) => previous.push(from), - ), + mockedTx.withMutations((record) => { + record + .updateIn( + ['ownersWithPendingActions', mockedTx.isCancellationTx ? 'reject' : 'confirm'], + (previous) => previous.push(from), + ) + .set('status', TransactionStatus.PENDING) + }), safeAddress, dispatch, state, From 6b86d98f2486b1270f2f0fac218b2b3efbcb5a26 Mon Sep 17 00:00:00 2001 From: fernandomg Date: Wed, 2 Sep 2020 13:13:52 -0300 Subject: [PATCH 6/9] fix: properly update mockedTx --- .../safe/store/actions/processTransaction.ts | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/logic/safe/store/actions/processTransaction.ts b/src/logic/safe/store/actions/processTransaction.ts index c8a26780de..9c016876c6 100644 --- a/src/logic/safe/store/actions/processTransaction.ts +++ b/src/logic/safe/store/actions/processTransaction.ts @@ -20,7 +20,8 @@ import { getLastTx, getNewTxNonce, shouldExecuteTransaction } from 'src/logic/sa import { getErrorMessage } from 'src/test/utils/ethereumErrors' import { storeTx } from './createTransaction' -import { TransactionStatus } from '../models/types/transaction' +import { TransactionStatus } from 'src/logic/safe/store/models/types/transaction' +import { makeConfirmation } from 'src/logic/safe/store/models/confirmation' const processTransaction = ({ approveAndExecute, notifiedTransaction, safeAddress, tx, userAddress }) => async ( dispatch, @@ -177,16 +178,20 @@ const processTransaction = ({ approveAndExecute, notifiedTransaction, safeAddres : TransactionStatus.FAILED, ) .updateIn(['ownersWithPendingActions', 'reject'], (prev) => prev.clear()) + .updateIn(['ownersWithPendingActions', 'confirm'], (prev) => prev.clear()) + }) + : mockedTx.withMutations((record) => { + record + .updateIn(['ownersWithPendingActions', toStoreTx.isCancellationTx ? 'reject' : 'confirm'], (previous) => + previous.pop(), + ) + .set('status', TransactionStatus.AWAITING_CONFIRMATIONS) }) - : mockedTx.set('status', TransactionStatus.AWAITING_CONFIRMATIONS) await storeTx( - toStoreTx.withMutations((record) => { - record - .set('confirmations', fromJS([...tx.confirmations, makeConfirmation({ owner: from })])) - .updateIn(['ownersWithPendingActions', toStoreTx.isCancellationTx ? 'reject' : 'confirm'], (previous) => - previous.pop(from), - ) + toStoreTx.update('confirmations', (confirmations) => { + const index = confirmations.findIndex(({ owner }) => owner === from) + return index === -1 ? confirmations.push(makeConfirmation({ owner: from })) : confirmations }), safeAddress, dispatch, From d53ebf71f88d22d8e9dd6b788ee14f8263c3022e Mon Sep 17 00:00:00 2001 From: fernandomg Date: Wed, 2 Sep 2020 13:14:21 -0300 Subject: [PATCH 7/9] fix: hide buttons when tx is pending --- .../TxsTable/ExpandedTx/OwnersColumn/index.tsx | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/OwnersColumn/index.tsx b/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/OwnersColumn/index.tsx index 0413ee4860..bf23228989 100644 --- a/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/OwnersColumn/index.tsx +++ b/src/routes/safe/components/Transactions/TxsTable/ExpandedTx/OwnersColumn/index.tsx @@ -142,6 +142,7 @@ const OwnersColumn = ({ displayButtonRow = false } + // TODO: simplify this whole logic around tx status, it's getting hard to maintain and follow const showConfirmBtn = !tx.isExecuted && tx.status !== 'pending' && @@ -151,7 +152,8 @@ const OwnersColumn = ({ !currentUserAlreadyConfirmed && !thresholdReached - const showExecuteBtn = canExecute && !tx.isExecuted && thresholdReached + const showExecuteBtn = + canExecute && !tx.isExecuted && thresholdReached && tx.status !== 'pending' && cancelTx.status !== 'pending' const showRejectBtn = !cancelTx.isExecuted && @@ -163,7 +165,13 @@ const OwnersColumn = ({ !cancelThresholdReached && displayButtonRow - const showExecuteRejectBtn = !cancelTx.isExecuted && !tx.isExecuted && canExecuteCancel && cancelThresholdReached + const showExecuteRejectBtn = + !cancelTx.isExecuted && + !tx.isExecuted && + canExecuteCancel && + cancelThresholdReached && + tx.status !== 'pending' && + cancelTx.status !== 'pending' const txThreshold = cancelTx.isExecuted ? tx.confirmations.size : threshold const cancelThreshold = tx.isExecuted ? cancelTx.confirmations.size : threshold From 45ccc86ee3ddfd45467122357cabcc768bd7874e Mon Sep 17 00:00:00 2001 From: fernandomg Date: Mon, 7 Sep 2020 14:13:24 -0300 Subject: [PATCH 8/9] fix: type error --- src/logic/currentSession/store/reducer/currentSession.ts | 2 +- .../safe/store/actions/transactions/utils/transactionHelpers.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/logic/currentSession/store/reducer/currentSession.ts b/src/logic/currentSession/store/reducer/currentSession.ts index 2ab0bb9362..7b8c6e3a2b 100644 --- a/src/logic/currentSession/store/reducer/currentSession.ts +++ b/src/logic/currentSession/store/reducer/currentSession.ts @@ -17,7 +17,7 @@ export default handleActions( [UPDATE_VIEWED_SAFES]: (state, action) => { const safeAddress = action.payload - const newState = state.updateIn(['viewedSafes'], (prev = []) => + const newState = state.updateIn(['viewedSafes'], (prev: string[] = []) => prev.includes(safeAddress) ? prev : [...prev, safeAddress], ) diff --git a/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts b/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts index 1918750a3b..cde5cc0a42 100644 --- a/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts +++ b/src/logic/safe/store/actions/transactions/utils/transactionHelpers.ts @@ -315,7 +315,7 @@ export type TxToMock = TxArgs & { safeTxHash: string value: string submissionDate: string - dataDecoded: DataDecoded + dataDecoded: DataDecoded | null } export const mockTransaction = (tx: TxToMock, safeAddress: string, state: AppReduxState): Promise => { From 7f58f4358898044d2734113b14e000e0f39fa3f6 Mon Sep 17 00:00:00 2001 From: Daniel Sanchez Date: Thu, 10 Sep 2020 12:19:39 +0200 Subject: [PATCH 9/9] Rollback patches trying to fix bug --- src/logic/currentSession/store/reducer/currentSession.ts | 2 +- src/logic/notifications/store/reducer/notifications.ts | 2 +- src/logic/safe/store/middleware/notificationsMiddleware.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/logic/currentSession/store/reducer/currentSession.ts b/src/logic/currentSession/store/reducer/currentSession.ts index daa34877af..228bfc9eef 100644 --- a/src/logic/currentSession/store/reducer/currentSession.ts +++ b/src/logic/currentSession/store/reducer/currentSession.ts @@ -13,7 +13,7 @@ export default handleActions( [UPDATE_VIEWED_SAFES]: (state, action) => { const safeAddress = action.payload - const newState = state.updateIn(['viewedSafes'], (prev: string[] = []) => + const newState = state.updateIn(['viewedSafes'], (prev) => prev.includes(safeAddress) ? prev : [...prev, safeAddress], ) diff --git a/src/logic/notifications/store/reducer/notifications.ts b/src/logic/notifications/store/reducer/notifications.ts index d758ab9100..802ab9bd85 100644 --- a/src/logic/notifications/store/reducer/notifications.ts +++ b/src/logic/notifications/store/reducer/notifications.ts @@ -20,7 +20,7 @@ export default handleActions( const { dismissAll, key } = action.payload if (key) { - return state.update(key, (prev) => prev?.set('dismissed', true) ?? prev) + return state.update(key, (prev) => prev.set('dismissed', true)) } if (dismissAll) { return state.withMutations((map) => { diff --git a/src/logic/safe/store/middleware/notificationsMiddleware.ts b/src/logic/safe/store/middleware/notificationsMiddleware.ts index d46af6e29f..c30f5fa142 100644 --- a/src/logic/safe/store/middleware/notificationsMiddleware.ts +++ b/src/logic/safe/store/middleware/notificationsMiddleware.ts @@ -103,7 +103,7 @@ const notificationsMiddleware = (store) => (next) => async (action) => { } case ADD_INCOMING_TRANSACTIONS: { action.payload.forEach((incomingTransactions, safeAddress) => { - const { latestIncomingTxBlock = 0 } = state.safes.get('safes').get(safeAddress, {}) + const { latestIncomingTxBlock } = state.safes.get('safes').get(safeAddress) const viewedSafes = state.currentSession ? state.currentSession.get('viewedSafes') : [] const recurringUser = viewedSafes?.includes(safeAddress)