From 8e4d2a0822eea1c12dba352b48256c073208be06 Mon Sep 17 00:00:00 2001 From: katspaugh <381895+katspaugh@users.noreply.github.com> Date: Tue, 5 Apr 2022 17:46:26 +0200 Subject: [PATCH] Refactor: overwrite the queue instead of merging + use txId for deep links (#3751) * Refactor: overwrite the queue instead of merging * Fix: when only "next" txns are in queue * Update tests * PR comments (thanks Aaron) * Search for a specific Queued label instead of last label * More PR comments * Fix: never add singular txns to History * Display pending txns as history * Use tx id for deep links * Cache fetchTxDetails * Move fallback to hook --- package.json | 2 +- src/logic/safe/store/actions/utils.ts | 2 +- .../__tests__/gatewayTransactions.test.ts | 559 +----------------- .../safe/store/reducer/gatewayTransactions.ts | 121 +--- .../transactions/api/fetchSafeTransaction.ts | 21 +- src/routes/routes.ts | 4 +- .../Transactions/TxList/TxDetails.tsx | 8 +- .../Transactions/TxList/TxShareButton.tsx | 6 +- .../Transactions/TxList/TxSingularDetails.tsx | 139 +---- .../Transactions/TxList/TxSummary.tsx | 13 +- .../TxList/hooks/useTransactionDetails.ts | 10 +- src/utils/__tests__/googleTagManager.test.tsx | 4 +- yarn.lock | 9 +- 13 files changed, 133 insertions(+), 765 deletions(-) diff --git a/package.json b/package.json index 7ae26b14f1..002003fffe 100644 --- a/package.json +++ b/package.json @@ -89,7 +89,7 @@ "@gnosis.pm/safe-core-sdk": "^2.0.0", "@gnosis.pm/safe-deployments": "^1.8.0", "@gnosis.pm/safe-react-components": "^1.1.2", - "@gnosis.pm/safe-react-gateway-sdk": "^2.10.1", + "@gnosis.pm/safe-react-gateway-sdk": "^2.10.2", "@gnosis.pm/safe-web3-lib": "^1.0.0", "@material-ui/core": "^4.12.3", "@material-ui/icons": "^4.11.0", diff --git a/src/logic/safe/store/actions/utils.ts b/src/logic/safe/store/actions/utils.ts index f2983d4d36..5be9f0bf8e 100644 --- a/src/logic/safe/store/actions/utils.ts +++ b/src/logic/safe/store/actions/utils.ts @@ -139,7 +139,7 @@ export const navigateToTx = (safeAddress: string, txDetails: TransactionDetails) const prefixedSafeAddress = getPrefixedSafeAddressSlug({ shortName: extractShortChainName(), safeAddress }) const txRoute = generatePath(SAFE_ROUTES.TRANSACTIONS_SINGULAR, { [SAFE_ADDRESS_SLUG]: prefixedSafeAddress, - [TRANSACTION_ID_SLUG]: txDetails.detailedExecutionInfo.safeTxHash, + [TRANSACTION_ID_SLUG]: txDetails.txId, }) history.push(txRoute) diff --git a/src/logic/safe/store/reducer/__tests__/gatewayTransactions.test.ts b/src/logic/safe/store/reducer/__tests__/gatewayTransactions.test.ts index b0d675793a..329f7b5ac4 100644 --- a/src/logic/safe/store/reducer/__tests__/gatewayTransactions.test.ts +++ b/src/logic/safe/store/reducer/__tests__/gatewayTransactions.test.ts @@ -1,9 +1,9 @@ import { ConflictHeader, Label, + LabelValue, MultisigExecutionInfo, Transaction, - TransactionDetails, TransactionStatus, TransactionSummary, TransactionTokenType, @@ -37,12 +37,12 @@ const MOCK_TX_META: Omit = { describe('gatewayTransactionsReducer', () => { describe('ADD_QUEUED_TRANSACTIONS', () => { const NEXT_LABEL: Label = { - label: 'next', + label: LabelValue.Next, type: 'LABEL', } const QUEUED_LABEL: Label = { - label: 'queued', + label: LabelValue.Queued, type: 'LABEL', } @@ -71,95 +71,6 @@ describe('gatewayTransactionsReducer', () => { executionInfo: MOCK_MULTISIG_EXECUTION_INFO, } - const MOCK_TX_DETAILS: Omit = { - executedAt: null, - txInfo: { - type: 'Transfer', - sender: { value: '', name: null, logoUri: null }, - recipient: { value: '', name: null, logoUri: null }, - direction: TransferDirection.OUTGOING, - transferInfo: { - type: TransactionTokenType.NATIVE_COIN, - value: '', - }, - }, - txData: null, - detailedExecutionInfo: null, - txHash: null, - safeAppInfo: null, - } - - it('adds `queued.queued` transaction summaries', () => { - const mockTx: Transaction = { - transaction: { - id: 'sdifhgsiodugheiorjngnegerg', - ...MOCK_TX_SUMMARY, - }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [QUEUED_LABEL, mockTx], - } - - const newState = gatewayTransactionsReducer(EMPTY_STATE, addQueuedTransactions(mockPayload)) - - const expectedState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: {}, - queued: { - 0: [mockTx.transaction], - }, - }, - history: {}, - }, - }, - } - - expect(newState).toEqual(expectedState) - }) - it('adds `queued.next` transaction summaries', () => { - const mockTx: Transaction = { - transaction: { - id: 'sdfgsergrse5gsr5ghsxdfgh', - ...MOCK_TX_SUMMARY, - txStatus: TransactionStatus.AWAITING_CONFIRMATIONS, - executionInfo: { - ...MOCK_MULTISIG_EXECUTION_INFO, - confirmationsSubmitted: 0, - }, - }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [NEXT_LABEL, mockTx], - } - - const newState = gatewayTransactionsReducer(EMPTY_STATE, addQueuedTransactions(mockPayload)) - - const expectedState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: { - 0: [mockTx.transaction], - }, - queued: {}, - }, - history: {}, - }, - }, - } - - expect(newState).toEqual(expectedState) - }) it('handles payloads with multiple transactions of the same label/same nonce', () => { const mockTx: Transaction = { transaction: { id: 'sdfgdsfgdsfgdfgfd', ...MOCK_TX_SUMMARY }, @@ -177,7 +88,7 @@ describe('gatewayTransactionsReducer', () => { const mockPayload: QueuedPayload = { chainId: '4', safeAddress: ZERO_ADDRESS, - values: [QUEUED_LABEL, mockTx, mockTx2, mockTx3], + values: [NEXT_LABEL, mockTx, mockTx2, mockTx3], } const newState = gatewayTransactionsReducer(EMPTY_STATE, addQueuedTransactions(mockPayload)) @@ -186,10 +97,10 @@ describe('gatewayTransactionsReducer', () => { 4: { [ZERO_ADDRESS]: { queued: { - next: {}, - queued: { + next: { 0: [mockTx.transaction, mockTx2.transaction, mockTx3.transaction], }, + queued: {}, }, history: {}, }, @@ -198,6 +109,7 @@ describe('gatewayTransactionsReducer', () => { expect(newState).toEqual(expectedState) }) + it('handles payloads with multiple transactions with varying labels', () => { const mockNextTx: Transaction = { transaction: { id: 'btr6uy5r6rbfghhjg', ...MOCK_TX_SUMMARY }, @@ -214,6 +126,13 @@ describe('gatewayTransactionsReducer', () => { }, ...MOCK_TX_META, } + const mockQueuedTx2: Transaction = { + ...mockQueuedTx, + transaction: { + ...mockQueuedTx.transaction, + id: '123', + }, + } const mockPayload: QueuedPayload = { chainId: '4', @@ -223,6 +142,7 @@ describe('gatewayTransactionsReducer', () => { mockNextTx, QUEUED_LABEL, // Different label mockQueuedTx, + mockQueuedTx2, ], } @@ -236,7 +156,7 @@ describe('gatewayTransactionsReducer', () => { 0: [mockNextTx.transaction], }, queued: { - 1: [mockQueuedTx.transaction], + 1: [mockQueuedTx.transaction, mockQueuedTx2.transaction], }, }, history: {}, @@ -247,48 +167,7 @@ describe('gatewayTransactionsReducer', () => { expect(newState).toEqual(expectedState) }) - it('sets the label to `queued.next` if there is no label and the transaction is already loaded under `queued.next', () => { - const mockTx: Transaction = { - transaction: { id: 'dfgbh65rb5yrthhdft', ...MOCK_TX_SUMMARY }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [mockTx], - } - - const prevState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: { 0: [mockTx.transaction] }, - queued: {}, - }, - history: {}, - }, - }, - } - - const newState = gatewayTransactionsReducer(prevState, addQueuedTransactions(mockPayload)) - - const expectedState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: { 0: [mockTx.transaction] }, - queued: {}, - }, - history: {}, - }, - }, - } - - expect(newState).toEqual(expectedState) - }) - - it('defaults to a `queued.queued` if there is no label and previous state', () => { + it('defaults to a `queued.next` if there is no label and previous state', () => { const mockTx: Transaction = { transaction: { id: 'dfgbh65rb5yrthhdft', ...MOCK_TX_SUMMARY }, ...MOCK_TX_META, @@ -306,10 +185,10 @@ describe('gatewayTransactionsReducer', () => { 4: { [ZERO_ADDRESS]: { queued: { - next: {}, - queued: { + next: { 0: [mockTx.transaction], }, + queued: {}, }, history: {}, }, @@ -319,30 +198,6 @@ describe('gatewayTransactionsReducer', () => { expect(newState).toEqual(expectedState) }) - it('does not add transactions without a nonce to be added', () => { - const mockTx: Transaction = { - transaction: { - id: 'dfgbh65rb5yrthhdft', - ...MOCK_TX_SUMMARY, - executionInfo: { - ...MOCK_MULTISIG_EXECUTION_INFO, - nonce: undefined as unknown as number, // Remove nonce - }, - }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [mockTx], - } - - const newState = gatewayTransactionsReducer(EMPTY_STATE, addQueuedTransactions(mockPayload)) - - expect(newState).toEqual(EMPTY_STATE) - }) - it('removes conflict headers', () => { const mockConflicHeader: ConflictHeader = { nonce: 0, @@ -359,6 +214,7 @@ describe('gatewayTransactionsReducer', () => { expect(newState).toEqual(EMPTY_STATE) }) + it('adds `queued.queued` transactions to different nonce keys if the nonce differs between then', () => { const mockTx: Transaction = { transaction: { id: 'sdfgdsfgdsfgdfgfd', ...MOCK_TX_SUMMARY }, @@ -413,380 +269,5 @@ describe('gatewayTransactionsReducer', () => { expect(newState).toEqual(expectedState) }) - - it('merges existing `queued.queued` transactions', () => { - const mockTx: Transaction = { - transaction: { id: 'gsdfgsdfgfdgsf', ...MOCK_TX_SUMMARY }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [QUEUED_LABEL, mockTx], - } - - const prevState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: {}, - queued: { - 0: [ - { - ...mockTx.transaction, - // Would have been added via `UPDATE_TRANSACTION_DETAILS` - txDetails: { - txId: mockTx.transaction.id, - txStatus: mockTx.transaction.txStatus, - ...MOCK_TX_DETAILS, - }, - }, - ], - }, - }, - history: {}, - }, - }, - } - - const newState = gatewayTransactionsReducer(prevState, addQueuedTransactions(mockPayload)) - - // Should remain the same as it keeps the `txDetails` via merge - expect(newState).toEqual(prevState) - }) - - it('sorts `queued.queued` transactions by ascending timestamp when adding a new transaction', () => { - const mockOldestTx = { - transaction: { id: 'gsdfgsdfgfdgsf', ...MOCK_TX_SUMMARY }, // Timestamp of 0 - ...MOCK_TX_META, - } - const mockOldTx = { - transaction: { id: 'sdgbfghsdfhsd', ...MOCK_TX_SUMMARY, timestamp: 1 }, - ...MOCK_TX_META, - } - const mockNewTx = { - transaction: { id: 'ser5464e5s45', ...MOCK_TX_SUMMARY, timestamp: 2 }, - ...MOCK_TX_META, - } - const mockNewestTx = { - transaction: { id: 'sdegrsdh656', ...MOCK_TX_SUMMARY, timestamp: 3 }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [QUEUED_LABEL, mockNewTx, mockOldestTx, mockNewestTx, mockOldTx], - } - - const newState = gatewayTransactionsReducer(EMPTY_STATE, addQueuedTransactions(mockPayload)) - - const expectedState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: {}, - queued: { - 0: [mockOldestTx.transaction, mockOldTx.transaction, mockNewTx.transaction, mockNewestTx.transaction], - }, - }, - history: {}, - }, - }, - } - - expect(newState).toEqual(expectedState) - }) - - it('merges existing `queued.next` transactions', () => { - const mockTx: Transaction = { - transaction: { id: 'gsdfgsdfgfdgsf', ...MOCK_TX_SUMMARY }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [NEXT_LABEL, mockTx], - } - - const prevState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - queued: {}, - next: { - 0: [ - { - ...mockTx.transaction, - // Would have been added via `UPDATE_TRANSACTION_DETAILS` - txDetails: { - txId: mockTx.transaction.id, - txStatus: mockTx.transaction.txStatus, - ...MOCK_TX_DETAILS, - }, - }, - ], - }, - }, - history: {}, - }, - }, - } - - const newState = gatewayTransactionsReducer(prevState, addQueuedTransactions(mockPayload)) - - // Should remain the same as it keeps the `txDetails` via merge - expect(newState).toEqual(prevState) - }) - - it('sorts `queued.next` transactions by timestamp when adding a new transaction', () => { - const mockOldestTx = { - transaction: { id: 'gsdfgsdfgfdgsf', ...MOCK_TX_SUMMARY }, // Timestamp of 0 - ...MOCK_TX_META, - } - const mockOldTx = { - transaction: { id: 'sdgbfghsdfhsd', ...MOCK_TX_SUMMARY, timestamp: 1 }, - ...MOCK_TX_META, - } - const mockNewTx = { - transaction: { id: 'ser5464e5s45', ...MOCK_TX_SUMMARY, timestamp: 2 }, - ...MOCK_TX_META, - } - const mockNewestTx = { - transaction: { id: 'sdegrsdh656', ...MOCK_TX_SUMMARY, timestamp: 3 }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [NEXT_LABEL, mockNewTx, mockOldestTx, mockNewestTx, mockOldTx], - } - - const newState = gatewayTransactionsReducer(EMPTY_STATE, addQueuedTransactions(mockPayload)) - - const expectedState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: { - 0: [mockOldestTx.transaction, mockOldTx.transaction, mockNewTx.transaction, mockNewestTx.transaction], - }, - queued: {}, - }, - history: {}, - }, - }, - } - - expect(newState).toEqual(expectedState) - }) - - it('replaces `queued.queued` transactions if there are new confirmations', () => { - const mockTx: Transaction = { - transaction: { id: 'gsdfgsdfgfdgsf', ...MOCK_TX_SUMMARY }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [QUEUED_LABEL, mockTx], - } - - const prevState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: {}, - queued: { - 0: [ - { - ...mockTx.transaction, - executionInfo: { - ...MOCK_MULTISIG_EXECUTION_INFO, - confirmationsSubmitted: 0, - }, - // Would have been added via `UPDATE_TRANSACTION_DETAILS` - txDetails: { - txId: mockTx.transaction.id, - txStatus: mockTx.transaction.txStatus, - ...MOCK_TX_DETAILS, - }, - }, - ], - }, - }, - history: {}, - }, - }, - } - - const newState = gatewayTransactionsReducer(prevState, addQueuedTransactions(mockPayload)) - - const expectedState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: {}, - queued: { - // Removed `txDetails`, using the payload transaction instead - 0: [mockTx.transaction], - }, - }, - history: {}, - }, - }, - } - - expect(newState).toEqual(expectedState) - }) - it('replaces `queued.next` transactions if there are new confirmations', () => { - const mockTx: Transaction = { - transaction: { id: 'gsdfgsdfgfdgsf', ...MOCK_TX_SUMMARY }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [NEXT_LABEL, mockTx], - } - - const prevState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: { - 0: [ - { - ...mockTx.transaction, - executionInfo: { - ...MOCK_MULTISIG_EXECUTION_INFO, - confirmationsSubmitted: 0, - }, - // Would have been added via `UPDATE_TRANSACTION_DETAILS` - txDetails: { - txId: mockTx.transaction.id, - txStatus: mockTx.transaction.txStatus, - ...MOCK_TX_DETAILS, - }, - }, - ], - }, - queued: {}, - }, - history: {}, - }, - }, - } - - const newState = gatewayTransactionsReducer(prevState, addQueuedTransactions(mockPayload)) - - const expectedState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: { - // Removed `txDetails`, using the payload transaction instead - 0: [mockTx.transaction], - }, - queued: {}, - }, - history: {}, - }, - }, - } - - expect(newState).toEqual(expectedState) - }) - it('removes the transaction that moves from `queued.queued` to `queued.next`', () => { - const mockTx: Transaction = { - transaction: { id: 'sdfgsdfgdsfg', ...MOCK_TX_SUMMARY }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [NEXT_LABEL, mockTx], - } - - const prevState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: {}, - queued: { - 0: [mockTx.transaction], - }, - }, - history: {}, - }, - }, - } - - const newState = gatewayTransactionsReducer(prevState, addQueuedTransactions(mockPayload)) - - const expectedState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: { - 0: [mockTx.transaction], - }, - queued: {}, // Transaction removed - }, - history: {}, - }, - }, - } - - expect(newState).toEqual(expectedState) - }) - it('resets the `queued.next` if there are no `queued` transactions at all', () => { - const mockTx: Transaction = { - transaction: { id: 'sdfgsdfgdsfg', ...MOCK_TX_SUMMARY }, - ...MOCK_TX_META, - } - - const mockPayload: QueuedPayload = { - chainId: '4', - safeAddress: ZERO_ADDRESS, - values: [], - } - - const prevState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: { - 0: [mockTx.transaction], - }, - queued: {}, - }, - history: {}, - }, - }, - } - - const newState = gatewayTransactionsReducer(prevState, addQueuedTransactions(mockPayload)) - - const expectedState: GatewayTransactionsState = { - 4: { - [ZERO_ADDRESS]: { - queued: { - next: {}, // Next cleared - queued: {}, - }, - history: {}, - }, - }, - } - - expect(newState).toEqual(expectedState) - }) }) }) diff --git a/src/logic/safe/store/reducer/gatewayTransactions.ts b/src/logic/safe/store/reducer/gatewayTransactions.ts index b913b1da7f..369d8c4d8f 100644 --- a/src/logic/safe/store/reducer/gatewayTransactions.ts +++ b/src/logic/safe/store/reducer/gatewayTransactions.ts @@ -1,7 +1,7 @@ import get from 'lodash/get' import cloneDeep from 'lodash/cloneDeep' import { Action, handleActions } from 'redux-actions' -import merge from 'lodash/merge' +import { LabelValue } from '@gnosis.pm/safe-react-gateway-sdk' import { ADD_HISTORY_TRANSACTIONS, @@ -9,7 +9,6 @@ import { } from 'src/logic/safe/store/actions/transactions/gatewayTransactions' import { HistoryGatewayResponse, - isConflictHeader, isLabel, isMultisigExecutionInfo, isTransactionSummary, @@ -43,32 +42,20 @@ export type TransactionDetailsPayload = { type Payload = HistoryPayload | QueuedPayload | TransactionDetailsPayload -const getSingularUpdatedQueueTx = (storedTx: Transaction, newTx: Transaction) => { - const isUpdatedTx = - isMultisigExecutionInfo(storedTx.executionInfo) && - isMultisigExecutionInfo(newTx.executionInfo) && - storedTx.executionInfo.confirmationsSubmitted !== newTx.executionInfo.confirmationsSubmitted - - return isUpdatedTx - ? // Will remove txDetails as they will have changed because of new confirmations - newTx - : // Create new object, preserving txDetails - merge({}, storedTx, newTx) -} - -const getUpdatedQueueTxs = (txs: Transaction[], newTx: Transaction) => { - const newTxs = txs.slice() - const txIndex = txs.findIndex(({ id }) => sameString(id, newTx.id)) +/** + * Create a hash map of transactions by nonce. + * Each nonce maps to one or more transactions (e.g. original tx and a rejection tx). + */ +const makeQueueByNonce = (items: QueuedGatewayResponse['results']): Record => { + return items.reduce((result, item) => { + if (!(isTransactionSummary(item) && isMultisigExecutionInfo(item.transaction.executionInfo))) return result - if (txIndex >= 0) { - const storedTx = newTxs[txIndex] - newTxs[txIndex] = getSingularUpdatedQueueTx(storedTx, newTx) - } else { - newTxs.push(newTx) - newTxs.sort((a, b) => a.timestamp - b.timestamp) - } + const { nonce } = item.transaction.executionInfo + if (!result[nonce]) result[nonce] = [] + result[nonce].push(item.transaction) - return newTxs + return result + }, {}) } export const gatewayTransactionsReducer = handleActions( @@ -118,68 +105,24 @@ export const gatewayTransactionsReducer = handleActions) => { const { chainId, safeAddress, values } = action.payload - let newNext = state[chainId]?.[safeAddress]?.queued?.next || {} - // We must clone as we delete transactions that move from queue to next - let newQueued = { ...(state[chainId]?.[safeAddress]?.queued?.queued || {}) } - - let label: 'next' | 'queued' | undefined - - values.forEach((value) => { - if (isLabel(value)) { - // we're assuming that the first page will always provide `next` and `queued` labels - label = value.label.toLowerCase() as 'next' | 'queued' - return - } - - if ( - // Conflict headers are not needed in the current implementation - isConflictHeader(value) || - !isMultisigExecutionInfo(value.transaction.executionInfo) - ) { - return - } - - const txNonce = value.transaction.executionInfo?.nonce - - if (txNonce === undefined) { - console.warn('A transaction without nonce was provided by client-gateway:', JSON.stringify(value)) - return - } - - if (!label) { - const oldNext = state[chainId]?.[safeAddress]?.queued?.next - label = oldNext?.[txNonce] ? 'next' : 'queued' - } - - const newTx = value.transaction - - if (label === 'queued') { - if (newQueued?.[txNonce]) { - newQueued[txNonce] = getUpdatedQueueTxs(newQueued[txNonce], newTx) - } else { - newQueued = { ...newQueued, [txNonce]: [newTx] } - } - } else { - if (newNext?.[txNonce]) { - newNext[txNonce] = getUpdatedQueueTxs(newNext[txNonce], newTx) - } else { - newNext = { [txNonce]: [newTx] } - } - // Remove queued transaction from queue that moved to next - if (newQueued?.[txNonce]) { - delete newQueued[txNonce] - } - } - }) - // No new txs, empty queue list, cleanup - if (!values.length && !Object.keys(newQueued).length && Object.keys(newNext).length === 1) { - newNext = {} + // From the list of TransactionItems, create two sub-lists split by where the "Queued" label is. + // If there's no "Queued" label, put all the items into the "next" group. + let nextItems = values + let queuedItems = values.slice(0, 0) + const qLabelIndex = values.findIndex((item) => isLabel(item) && item.label === LabelValue.Queued) + if (qLabelIndex >= 0) { + nextItems = values.slice(0, qLabelIndex) + queuedItems = values.slice(qLabelIndex) } return { @@ -190,10 +133,10 @@ export const gatewayTransactionsReducer = handleActions sameString(id, transactionId)) - if (txIndex !== -1) { + if (txIndex >= 0) { txGroup[timestamp][txIndex]['txDetails'] = value break txLocationLoop } diff --git a/src/logic/safe/transactions/api/fetchSafeTransaction.ts b/src/logic/safe/transactions/api/fetchSafeTransaction.ts index d1888e80d9..820e6ef26d 100644 --- a/src/logic/safe/transactions/api/fetchSafeTransaction.ts +++ b/src/logic/safe/transactions/api/fetchSafeTransaction.ts @@ -3,9 +3,24 @@ import { getTransactionDetails, TransactionDetails } from '@gnosis.pm/safe-react import { _getChainId } from 'src/config' import { GATEWAY_URL } from 'src/utils/constants' +// Cache the request promise to avoid simulateneous requests +// It's cleared as soon as the promise is resolved +const cache = {} + /** - * @param {string} clientGatewayTxId safeTxHash or transaction id from client-gateway + * @param {string} txId safeTxHash or transaction id from client-gateway */ -export const fetchSafeTransaction = async (clientGatewayTxId: string): Promise => { - return getTransactionDetails(GATEWAY_URL, _getChainId(), clientGatewayTxId) +export const fetchSafeTransaction = async (txId: string): Promise => { + const chainId = _getChainId() + const cacheKey = `${chainId}_${txId}` + + const promise: Promise = cache[cacheKey] || getTransactionDetails(GATEWAY_URL, chainId, txId) + + // Save the promise into cache + cache[cacheKey] = promise + + // Clear cache when promise finishes + promise.catch(() => null).then(() => delete cache[cacheKey]) + + return promise } diff --git a/src/routes/routes.ts b/src/routes/routes.ts index b21c5353a6..20aa85a37f 100644 --- a/src/routes/routes.ts +++ b/src/routes/routes.ts @@ -27,7 +27,7 @@ export const SAFE_SECTION_ROUTE = `${ADDRESSED_ROUTE}/:${SAFE_SECTION_SLUG}` export const SAFE_SUBSECTION_SLUG = 'safeSubsection' export const SAFE_SUBSECTION_ROUTE = `${SAFE_SECTION_ROUTE}/:${SAFE_SUBSECTION_SLUG}` -export const TRANSACTION_ID_SLUG = `safeTxHash` +export const TRANSACTION_ID_SLUG = 'txId' // URL: gnosis-safe.io/app/:[SAFE_ADDRESS_SLUG]/:[SAFE_SECTION_SLUG]/:[SAFE_SUBSECTION_SLUG] export type SafeRouteSlugs = { @@ -52,7 +52,7 @@ export const SAFE_ROUTES = { TRANSACTIONS: `${ADDRESSED_ROUTE}/transactions`, TRANSACTIONS_HISTORY: `${ADDRESSED_ROUTE}/transactions/history`, TRANSACTIONS_QUEUE: `${ADDRESSED_ROUTE}/transactions/queue`, - TRANSACTIONS_SINGULAR: `${ADDRESSED_ROUTE}/transactions/:${TRANSACTION_ID_SLUG}(${hashRegExp}+)`, // [TRANSACTION_HASH_SLUG] === 'safeTxHash' + TRANSACTIONS_SINGULAR: `${ADDRESSED_ROUTE}/transactions/:${TRANSACTION_ID_SLUG}`, ADDRESS_BOOK: `${ADDRESSED_ROUTE}/address-book`, APPS: `${ADDRESSED_ROUTE}/apps`, SETTINGS: `${ADDRESSED_ROUTE}/settings`, diff --git a/src/routes/safe/components/Transactions/TxList/TxDetails.tsx b/src/routes/safe/components/Transactions/TxList/TxDetails.tsx index a8cc3c7b7b..2104bf93b3 100644 --- a/src/routes/safe/components/Transactions/TxList/TxDetails.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxDetails.tsx @@ -89,7 +89,7 @@ type TxDetailsProps = { export const TxDetails = ({ transaction }: TxDetailsProps): ReactElement => { const { txLocation } = useContext(TxLocationContext) - const { data, loading } = useTransactionDetails(transaction.id) + const { data, loading } = useTransactionDetails(transaction.id, transaction.txDetails) const txStatus = useTxStatus(transaction) const willBeReplaced = txStatus === LocalTransactionStatus.WILL_BE_REPLACED const isPending = txStatus === LocalTransactionStatus.PENDING @@ -99,18 +99,18 @@ export const TxDetails = ({ transaction }: TxDetailsProps): ReactElement => { // To avoid prop drilling into TxDataGroup, module details are positioned here accordingly const getModuleDetails = () => { - if (!transaction.txDetails || !isModuleExecutionInfo(transaction.txDetails.detailedExecutionInfo)) { + if (!data || !isModuleExecutionInfo(data.detailedExecutionInfo)) { return null } return (
- +
) } - if (loading) { + if (loading && !data) { return ( diff --git a/src/routes/safe/components/Transactions/TxList/TxShareButton.tsx b/src/routes/safe/components/Transactions/TxList/TxShareButton.tsx index dcc6248a79..3f3496a5fe 100644 --- a/src/routes/safe/components/Transactions/TxList/TxShareButton.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxShareButton.tsx @@ -6,13 +6,13 @@ import { getPrefixedSafeAddressSlug, SAFE_ADDRESS_SLUG, SAFE_ROUTES, TRANSACTION import { PUBLIC_URL } from 'src/utils/constants' type Props = { - safeTxHash: string + txId: string } -const TxShareButton = ({ safeTxHash }: Props): ReactElement => { +const TxShareButton = ({ txId }: Props): ReactElement => { const txDetailsPathname = generatePath(SAFE_ROUTES.TRANSACTIONS_SINGULAR, { [SAFE_ADDRESS_SLUG]: getPrefixedSafeAddressSlug(), - [TRANSACTION_ID_SLUG]: safeTxHash, + [TRANSACTION_ID_SLUG]: txId, }) const txDetailsLink = `${window.location.origin}${PUBLIC_URL}${txDetailsPathname}` diff --git a/src/routes/safe/components/Transactions/TxList/TxSingularDetails.tsx b/src/routes/safe/components/Transactions/TxList/TxSingularDetails.tsx index 79a2d26db7..9ae7ecc464 100644 --- a/src/routes/safe/components/Transactions/TxList/TxSingularDetails.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxSingularDetails.tsx @@ -1,133 +1,50 @@ -import { ReactElement, useEffect, useState } from 'react' -import { useHistory, useParams } from 'react-router-dom' +import { ReactElement, useCallback } from 'react' +import { useParams } from 'react-router-dom' import { Loader } from '@gnosis.pm/safe-react-components' -import { shallowEqual, useDispatch, useSelector } from 'react-redux' +import { shallowEqual, useSelector } from 'react-redux' import { TransactionDetails } from '@gnosis.pm/safe-react-gateway-sdk' -import { isTxQueued } from 'src/logic/safe/store/models/types/gateway.d' +import { isTxQueued, TxLocation, Transaction } from 'src/logic/safe/store/models/types/gateway.d' import { extractPrefixedSafeAddress, - extractSafeAddress, generateSafeRoute, SafeRouteSlugs, SAFE_ROUTES, TRANSACTION_ID_SLUG, } from 'src/routes/routes' import { Centered } from './styled' -import { getTransactionWithLocationByAttribute } from 'src/logic/safe/store/selectors/gatewayTransactions' import { TxLocationContext } from './TxLocationProvider' import { AppReduxState } from 'src/store' -import { logError, Errors } from 'src/logic/exceptions/CodedException' import { fetchSafeTransaction } from 'src/logic/safe/transactions/api/fetchSafeTransaction' import { makeTxFromDetails } from './utils' -import { - addQueuedTransactions, - addHistoryTransactions, -} from 'src/logic/safe/store/actions/transactions/gatewayTransactions' -import { HistoryPayload, QueuedPayload } from 'src/logic/safe/store/reducer/gatewayTransactions' -import { currentChainId } from 'src/logic/config/store/selectors' import { QueueTxList } from './QueueTxList' import { HistoryTxList } from './HistoryTxList' import FetchError from '../../FetchError' -import { useQueueTransactions } from './hooks/useQueueTransactions' - -const TxSingularDetails = (): ReactElement => { - const { [TRANSACTION_ID_SLUG]: safeTxHash = '' } = useParams() - const [fetchedTx, setFetchedTx] = useState() - const [error, setError] = useState() - const dispatch = useDispatch() - const history = useHistory() - const chainId = useSelector(currentChainId) - const transactions = useQueueTransactions() +import useAsync from 'src/logic/hooks/useAsync' +import { getTransactionWithLocationByAttribute } from 'src/logic/safe/store/selectors/gatewayTransactions' - const indexedTx = useSelector( - (state: AppReduxState) => - fetchedTx - ? getTransactionWithLocationByAttribute(state, { attributeName: 'id', attributeValue: fetchedTx.txId }) - : null, - shallowEqual, +const useStoredTx = (txId?: string): { txLocation: TxLocation; transaction?: Transaction } => { + return ( + useSelector( + (state: AppReduxState) => + txId ? getTransactionWithLocationByAttribute(state, { attributeName: 'id', attributeValue: txId }) : undefined, + shallowEqual, + ) || { txLocation: 'history' } ) +} - // When safeTxHash changes, we fetch tx for this hash to get the txId - useEffect(() => { - let isCurrent = true - - setFetchedTx(undefined) - - if (!safeTxHash) { - const txsRoute = generateSafeRoute(SAFE_ROUTES.TRANSACTIONS, extractPrefixedSafeAddress()) - history.replace(txsRoute) - return - } - - const getTransaction = async (): Promise => { - let txDetails: TransactionDetails - try { - txDetails = await fetchSafeTransaction(safeTxHash) - } catch (e) { - logError(Errors._614, e.message) - setError(e) - return - } - - if (isCurrent) { - setFetchedTx(txDetails) - } - } - - getTransaction() - - return () => { - isCurrent = false - } - }, [history, safeTxHash, setFetchedTx]) - - // Add the tx to the store - useEffect(() => { - if (!fetchedTx || indexedTx) { - return - } - - // Format the tx details into a History or Queue-like tx item - const listItemTx = makeTxFromDetails(fetchedTx) - const payload: HistoryPayload | QueuedPayload = { - chainId, - safeAddress: extractSafeAddress(), - values: [ - { - transaction: listItemTx, - type: 'TRANSACTION', // Other types are discarded in reducer - conflictType: 'None', // Not used in reducer - }, - ], - } - - // Add historical transaction - if (!isTxQueued(listItemTx.txStatus)) { - dispatch(addHistoryTransactions(payload)) - return - } - - // Don't add queued transaction until transaction store has initialised - if (!transactions) { - return - } +const TxSingularDetails = (): ReactElement => { + // Get a safeTxHash from the URL + const { [TRANSACTION_ID_SLUG]: txId = '' } = useParams() + const { transaction, txLocation } = useStoredTx(txId) - // Prepend label to queue transaction payload - const isNext = transactions.next.transactions.some(([, txs]) => txs.some(({ id }) => id === listItemTx.id)) - payload.values = [ - { - label: isNext ? 'next' : 'queued', - type: 'LABEL', - }, - ...payload.values, - ] + // When this callback changes, we refetch the tx details + const fetchTxDetails = useCallback(() => fetchSafeTransaction(txId), [txId]) - // Add queued transaction - dispatch(addQueuedTransactions(payload)) - }, [fetchedTx, chainId, dispatch, transactions, indexedTx]) + // Fetch tx details + const { result: fetchedTx, error } = useAsync(fetchTxDetails) - if (!indexedTx && error) { + if (error) { const safeParams = extractPrefixedSafeAddress() return ( { ) } - if (!indexedTx) { + const detailedTx = transaction || (fetchedTx ? makeTxFromDetails(fetchedTx) : null) + + if (!detailedTx) { return ( @@ -146,12 +65,12 @@ const TxSingularDetails = (): ReactElement => { ) } - const { transaction, txLocation } = indexedTx - const TxList = isTxQueued(transaction.txStatus) ? QueueTxList : HistoryTxList + const isQueue = isTxQueued(detailedTx.txStatus) + const TxList = isQueue ? QueueTxList : HistoryTxList return ( - + ) } diff --git a/src/routes/safe/components/Transactions/TxList/TxSummary.tsx b/src/routes/safe/components/Transactions/TxList/TxSummary.tsx index fea3de0751..b76a2d2e16 100644 --- a/src/routes/safe/components/Transactions/TxList/TxSummary.tsx +++ b/src/routes/safe/components/Transactions/TxList/TxSummary.tsx @@ -60,13 +60,12 @@ export const TxSummary = ({ txDetails }: Props): ReactElement => { return ( <> - {isMultiSigExecutionDetails(txDetails.detailedExecutionInfo) && ( -
- - - -
- )} +
+ + + +
+ {txData?.operation === Operation.DELEGATE && (
diff --git a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionDetails.ts b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionDetails.ts index 19eaebf916..a1a6ce6aad 100644 --- a/src/routes/safe/components/Transactions/TxList/hooks/useTransactionDetails.ts +++ b/src/routes/safe/components/Transactions/TxList/hooks/useTransactionDetails.ts @@ -11,12 +11,16 @@ export type LoadTransactionDetails = { loading: boolean } -export const useTransactionDetails = (transactionId: string): LoadTransactionDetails => { +export const useTransactionDetails = ( + transactionId: string, + transactionDetails?: ExpandedTxDetails, +): LoadTransactionDetails => { const dispatch = useRef(useDispatch()) const [txDetails, setTxDetails] = useState({ - loading: true, - data: undefined, + loading: transactionDetails ? false : true, + data: transactionDetails, }) + const data = useSelector((state: AppReduxState) => getTransactionByAttribute(state, { attributeValue: transactionId, attributeName: 'id' }), ) diff --git a/src/utils/__tests__/googleTagManager.test.tsx b/src/utils/__tests__/googleTagManager.test.tsx index 922617ee6b..a0f69ab3a9 100644 --- a/src/utils/__tests__/googleTagManager.test.tsx +++ b/src/utils/__tests__/googleTagManager.test.tsx @@ -43,13 +43,13 @@ describe('googleTagManager', () => { url: '', params: { prefixedSafeAddress: '0x0000000000000000000000000000000000000000', - safeTxHash: '0x73e9512853f394f4c3485752a56806f61a5a0a98d8c13877ee3e7ae5d2769d2b', + txId: 'multisig_0xb3b83bf204C458B461de9B0CD2739DB152b4fa5A_0x73e9512853f394f4c3485752a56806f61a5a0a98d8c13877ee3e7ae5d2769d2b', }, })) const anonymizedLocation = getAnonymizedLocation({ pathname: - '/rin/0x0000000000000000000000000000000000000000/transactions/0x73e9512853f394f4c3485752a56806f61a5a0a98d8c13877ee3e7ae5d2769d2b', + '/rin/0x0000000000000000000000000000000000000000/transactions/multisig_0xb3b83bf204C458B461de9B0CD2739DB152b4fa5A_0x73e9512853f394f4c3485752a56806f61a5a0a98d8c13877ee3e7ae5d2769d2b', search: '?test=true', hash: '#hash', state: null, diff --git a/yarn.lock b/yarn.lock index c7da579f61..e7d1e2ad88 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1880,13 +1880,20 @@ react-media "^1.10.0" web3-utils "^1.6.0" -"@gnosis.pm/safe-react-gateway-sdk@^2.10.0", "@gnosis.pm/safe-react-gateway-sdk@^2.10.1": +"@gnosis.pm/safe-react-gateway-sdk@^2.10.0": version "2.10.1" resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-react-gateway-sdk/-/safe-react-gateway-sdk-2.10.1.tgz#62f4abf733855e734aa1eab4be4778ccd08fe689" integrity sha512-uIosTEqmoxhCy7WS8sIzXde2nJQwzC+KfNoeDQVeLZtEpnRZQ7R+N/qDtORMUJfKeyc8cIwkKXmVc2DRgSRxOQ== dependencies: cross-fetch "^3.1.5" +"@gnosis.pm/safe-react-gateway-sdk@^2.10.2": + version "2.10.2" + resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-react-gateway-sdk/-/safe-react-gateway-sdk-2.10.2.tgz#25d01ea5c947bc2701535c6cdf059b380276e0f5" + integrity sha512-9o0JiA3zS5s1fVQa61DB1gBdFgyqA9QxW3y8lTYzX/lWNbfpm2psonyLjBJkETp7RoMFSp30pM4wgPXXT9bjzQ== + dependencies: + cross-fetch "^3.1.5" + "@gnosis.pm/safe-react-gateway-sdk@^2.5.6": version "2.7.4" resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-react-gateway-sdk/-/safe-react-gateway-sdk-2.7.4.tgz#3a1d96e30d10e4859579b558586fd25cd4ae3480"