Skip to content

Commit

Permalink
Fix: correct filtering of history for txs marked trusted and imitatio…
Browse files Browse the repository at this point in the history
…n [SW-100] (#3907)

* fix: filter imitation txs with trusted tokens

* update gateway sdk

* fix: create separate params for hiding trusted and imitation transactions

* fix: separate untrusted and imitation filters in getTxHistory and getFilteredTxHistory

* tests: update unit tests for  fetchFilteredTxHistory

* fix: remove unecessary default value
  • Loading branch information
jmealy authored Aug 9, 2024
1 parent 02b67fc commit 15b70d1
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 25 deletions.
6 changes: 3 additions & 3 deletions src/components/transactions/TrustedToggle/index.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { useHasFeature } from '@/hooks/useChains'
import { useAppDispatch, useAppSelector } from '@/store'
import { selectSettings, setshowOnlyTrustedTransactions } from '@/store/settingsSlice'
import { selectSettings, hideSuspiciousTransactions } from '@/store/settingsSlice'
import { FEATURES } from '@/utils/chains'
import madProps from '@/utils/mad-props'
import _TrustedToggleButton from './TrustedToggleButton'

const useOnlyTrusted = () => {
const userSettings = useAppSelector(selectSettings)
return userSettings.showOnlyTrustedTransactions || false
return userSettings.hideSuspiciousTransactions || false
}

const useHasDefaultTokenList = () => {
Expand All @@ -17,7 +17,7 @@ const useHasDefaultTokenList = () => {
const useSetOnlyTrusted = () => {
const dispatch = useAppDispatch()
return (isOn: boolean) => {
dispatch(setshowOnlyTrustedTransactions(isOn))
dispatch(hideSuspiciousTransactions(isOn))
}
}

Expand Down
10 changes: 6 additions & 4 deletions src/hooks/loadables/useLoadTxHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,21 @@ import { FEATURES } from '@/utils/chains'
export const useLoadTxHistory = (): AsyncResult<TransactionListPage> => {
const { safe, safeAddress, safeLoaded } = useSafeInfo()
const { chainId, txHistoryTag } = safe
const { showOnlyTrustedTransactions } = useAppSelector(selectSettings)
const { hideSuspiciousTransactions } = useAppSelector(selectSettings)
const hasDefaultTokenlist = useHasFeature(FEATURES.DEFAULT_TOKENLIST)
const hideUntrustedTxs = (hasDefaultTokenlist && hideSuspiciousTransactions) || false
const hideImitationTxs = hideSuspiciousTransactions || false

// Re-fetch when chainId, address, showOnlyTrustedTransactions, or txHistoryTag changes
// Re-fetch when chainId, address, hideSuspiciousTransactions, or txHistoryTag changes
const [data, error, loading] = useAsync<TransactionListPage>(
() => {
if (!safeLoaded) return
if (!safe.deployed) return Promise.resolve({ results: [] })

return getTxHistory(chainId, safeAddress, hasDefaultTokenlist && showOnlyTrustedTransactions)
return getTxHistory(chainId, safeAddress, hideUntrustedTxs, hideImitationTxs)
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[safeLoaded, chainId, safeAddress, showOnlyTrustedTransactions, hasDefaultTokenlist, txHistoryTag, safe.deployed],
[safeLoaded, chainId, safeAddress, hideSuspiciousTransactions, hasDefaultTokenlist, txHistoryTag, safe.deployed],
false,
)

Expand Down
11 changes: 6 additions & 5 deletions src/hooks/useTxHistory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ const useTxHistory = (
// The latest page of the history is always in the store
const historyState = useAppSelector(selectTxHistory)
const [filter] = useTxFilter()
const { showOnlyTrustedTransactions } = useAppSelector(selectSettings)
const { hideSuspiciousTransactions } = useAppSelector(selectSettings)
const hasDefaultTokenlist = useHasFeature(FEATURES.DEFAULT_TOKENLIST)
const onlyTrusted = (hasDefaultTokenlist && showOnlyTrustedTransactions) || false
const hideUntrustedTxs = (hasDefaultTokenlist && hideSuspiciousTransactions) || false
const hideImitationTxs = hideSuspiciousTransactions || false

const {
safe: { chainId },
Expand All @@ -35,10 +36,10 @@ const useTxHistory = (
if (!(filter || pageUrl)) return

return filter
? fetchFilteredTxHistory(chainId, safeAddress, filter, onlyTrusted, pageUrl)
: getTxHistory(chainId, safeAddress, onlyTrusted, pageUrl)
? fetchFilteredTxHistory(chainId, safeAddress, filter, hideUntrustedTxs, hideImitationTxs, pageUrl)
: getTxHistory(chainId, safeAddress, hideUntrustedTxs, hideImitationTxs, pageUrl)
},
[chainId, safeAddress, pageUrl, filter, onlyTrusted],
[filter, pageUrl, chainId, safeAddress, hideUntrustedTxs, hideImitationTxs],
false,
)

Expand Down
12 changes: 10 additions & 2 deletions src/services/transactions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,21 @@ export const getTxDetails = memoize(
(id: string, chainId: string) => `${chainId}-${id}`,
)

export const getTxHistory = (chainId: string, safeAddress: string, trusted = false, pageUrl?: string) => {
export const getTxHistory = (
chainId: string,
safeAddress: string,
hideUntrustedTxs: boolean,
hideImitationTxs: boolean,
pageUrl?: string,
) => {
return getTransactionHistory(
chainId,
safeAddress,
{
timezone_offset: getTimezoneOffset(), // used for grouping txs by date
trusted, // if false, load all transactions, mark untrusted in the UI
// Untrusted and imitation txs are filtered together in the UI
trusted: hideUntrustedTxs, // if false, include transactions marked untrusted in the UI
imitation: !hideImitationTxs, // If true, include transactions marked imitation in the UI
},
pageUrl,
)
Expand Down
10 changes: 5 additions & 5 deletions src/store/settingsSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export type SettingsState = {

tokenList: TOKEN_LISTS

showOnlyTrustedTransactions?: boolean
hideSuspiciousTransactions?: boolean

shortName: {
copy: boolean
Expand All @@ -53,7 +53,7 @@ export const initialState: SettingsState = {

hiddenTokens: {},

showOnlyTrustedTransactions: false,
hideSuspiciousTransactions: false,

shortName: {
copy: true,
Expand Down Expand Up @@ -100,8 +100,8 @@ export const settingsSlice = createSlice({
setTokenList: (state, { payload }: PayloadAction<SettingsState['tokenList']>) => {
state.tokenList = payload
},
setshowOnlyTrustedTransactions: (state, { payload }: PayloadAction<boolean>) => {
state.showOnlyTrustedTransactions = payload
hideSuspiciousTransactions: (state, { payload }: PayloadAction<boolean>) => {
state.hideSuspiciousTransactions = payload
},
setRpc: (state, { payload }: PayloadAction<{ chainId: string; rpc: string }>) => {
const { chainId, rpc } = payload
Expand Down Expand Up @@ -135,7 +135,7 @@ export const {
setDarkMode,
setHiddenTokensForChain,
setTokenList,
setshowOnlyTrustedTransactions,
hideSuspiciousTransactions,
setRpc,
setTenderly,
setOnChainSigning,
Expand Down
10 changes: 7 additions & 3 deletions src/utils/__tests__/tx-history-filter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -388,13 +388,14 @@ describe('tx-history-filter', () => {
'0x123',
{ type: 'Incoming' as TxFilterType, filter: { value: '123' } },
false,
false,
'pageUrl1',
)

expect(getIncomingTransfers).toHaveBeenCalledWith(
'4',
'0x123',
{ value: '123', executed: undefined, timezone_offset: 3600000, trusted: false, imitation: false },
{ value: '123', executed: undefined, timezone_offset: 3600000, trusted: false, imitation: true },
'pageUrl1',
)

Expand All @@ -411,6 +412,7 @@ describe('tx-history-filter', () => {
filter: { execution_date__gte: '1970-01-01T00:00:00.000Z', executed: 'true' },
},
false,
false,
'pageUrl2',
)

Expand All @@ -422,7 +424,7 @@ describe('tx-history-filter', () => {
executed: 'true',
timezone_offset: 3600000,
trusted: false,
imitation: false,
imitation: true,
},
'pageUrl2',
)
Expand All @@ -437,13 +439,14 @@ describe('tx-history-filter', () => {
'0x789',
{ type: 'Module-based' as TxFilterType, filter: { to: '0x123' } },
false,
false,
'pageUrl3',
)

expect(getModuleTransactions).toHaveBeenCalledWith(
'1',
'0x789',
{ to: '0x123', executed: undefined, timezone_offset: 3600000, trusted: false, imitation: false },
{ to: '0x123', executed: undefined, timezone_offset: 3600000, trusted: false, imitation: true },
'pageUrl3',
)

Expand All @@ -460,6 +463,7 @@ describe('tx-history-filter', () => {
filter: { token_address: '0x123' },
},
false,
false,
'pageUrl3',
)

Expand Down
7 changes: 4 additions & 3 deletions src/utils/tx-history-filter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,15 +119,16 @@ export const fetchFilteredTxHistory = async (
chainId: string,
safeAddress: string,
filterData: TxFilter,
onlyTrusted: boolean,
hideUntrustedTxs: boolean,
hideImitationTxs: boolean,
pageUrl?: string,
): Promise<TransactionListPage> => {
const fetchPage = () => {
const query = {
...filterData.filter,
timezone_offset: getTimezoneOffset(),
trusted: onlyTrusted ?? false,
imitation: onlyTrusted ?? false,
trusted: hideUntrustedTxs,
imitation: !hideImitationTxs,
executed: filterData.type === TxFilterType.MULTISIG ? 'true' : undefined,
}

Expand Down

0 comments on commit 15b70d1

Please sign in to comment.