Skip to content

Commit a28be83

Browse files
CP-13373: Avoid showing alert if user cancelled the tx (#3577)
1 parent a8a7978 commit a28be83

File tree

5 files changed

+252
-3
lines changed

5 files changed

+252
-3
lines changed
Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,169 @@
1+
/* eslint-disable sonarjs/no-element-overwrite */
2+
import { BoundedMap } from './boundedMap'
3+
4+
describe('BoundedMap', () => {
5+
describe('basic Map functionality', () => {
6+
it('should set and get values', () => {
7+
const map = new BoundedMap<string, number>(3)
8+
map.set('a', 1)
9+
map.set('b', 2)
10+
11+
expect(map.get('a')).toBe(1)
12+
expect(map.get('b')).toBe(2)
13+
expect(map.get('c')).toBeUndefined()
14+
})
15+
16+
it('should check if keys exist', () => {
17+
const map = new BoundedMap<string, number>(3)
18+
map.set('a', 1)
19+
20+
expect(map.has('a')).toBe(true)
21+
expect(map.has('b')).toBe(false)
22+
})
23+
24+
it('should track size correctly', () => {
25+
const map = new BoundedMap<string, number>(3)
26+
27+
expect(map.size).toBe(0)
28+
map.set('a', 1)
29+
expect(map.size).toBe(1)
30+
map.set('b', 2)
31+
expect(map.size).toBe(2)
32+
})
33+
34+
it('should delete entries', () => {
35+
const map = new BoundedMap<string, number>(3)
36+
map.set('a', 1)
37+
map.set('b', 2)
38+
39+
expect(map.delete('a')).toBe(true)
40+
expect(map.has('a')).toBe(false)
41+
expect(map.size).toBe(1)
42+
expect(map.delete('a')).toBe(false)
43+
})
44+
})
45+
46+
describe('FIFO eviction behavior', () => {
47+
it('should evict oldest entry when maxSize is reached', () => {
48+
const map = new BoundedMap<string, number>(3)
49+
50+
map.set('a', 1)
51+
map.set('b', 2)
52+
map.set('c', 3)
53+
expect(map.size).toBe(3)
54+
expect(map.has('a')).toBe(true)
55+
56+
// Adding 4th entry should evict 'a' (oldest)
57+
map.set('d', 4)
58+
expect(map.size).toBe(3)
59+
expect(map.has('a')).toBe(false)
60+
expect(map.has('b')).toBe(true)
61+
expect(map.has('c')).toBe(true)
62+
expect(map.has('d')).toBe(true)
63+
})
64+
65+
it('should continue evicting in FIFO order', () => {
66+
const map = new BoundedMap<string, number>(2)
67+
68+
map.set('a', 1)
69+
map.set('b', 2)
70+
map.set('c', 3) // evicts 'a'
71+
map.set('d', 4) // evicts 'b'
72+
73+
expect(map.has('a')).toBe(false)
74+
expect(map.has('b')).toBe(false)
75+
expect(map.has('c')).toBe(true)
76+
expect(map.has('d')).toBe(true)
77+
expect(map.size).toBe(2)
78+
})
79+
80+
it('should NOT refresh position when updating existing key', () => {
81+
const map = new BoundedMap<string, number>(3)
82+
83+
map.set('a', 1)
84+
map.set('b', 2)
85+
map.set('c', 3)
86+
87+
// Update 'a' (oldest) - does NOT move it to the end in FIFO
88+
map.set('a', 10)
89+
90+
// Adding 'd' should still evict 'a' (oldest by insertion order)
91+
map.set('d', 4)
92+
93+
expect(map.has('a')).toBe(false) // 'a' was evicted despite being updated
94+
expect(map.get('b')).toBe(2)
95+
expect(map.get('c')).toBe(3)
96+
expect(map.get('d')).toBe(4)
97+
})
98+
99+
it('should update value without changing eviction order', () => {
100+
const map = new BoundedMap<string, string>(2)
101+
102+
map.set('first', 'v1')
103+
map.set('second', 'v2')
104+
105+
// Update first entry's value
106+
map.set('first', 'updated')
107+
expect(map.get('first')).toBe('updated')
108+
109+
// Adding third entry should evict 'first' (still oldest)
110+
map.set('third', 'v3')
111+
112+
expect(map.has('first')).toBe(false)
113+
expect(map.has('second')).toBe(true)
114+
expect(map.has('third')).toBe(true)
115+
})
116+
})
117+
118+
describe('edge cases', () => {
119+
it('should handle maxSize of 1', () => {
120+
const map = new BoundedMap<string, number>(1)
121+
122+
map.set('a', 1)
123+
expect(map.size).toBe(1)
124+
125+
map.set('b', 2)
126+
expect(map.size).toBe(1)
127+
expect(map.has('a')).toBe(false)
128+
expect(map.has('b')).toBe(true)
129+
})
130+
131+
it('should handle updating the only entry when maxSize is 1', () => {
132+
const map = new BoundedMap<string, number>(1)
133+
134+
map.set('a', 1)
135+
map.set('a', 2)
136+
137+
expect(map.size).toBe(1)
138+
expect(map.get('a')).toBe(2)
139+
})
140+
141+
it('should handle different key types', () => {
142+
const map = new BoundedMap<number, string>(2)
143+
144+
map.set(1, 'one')
145+
map.set(2, 'two')
146+
map.set(3, 'three')
147+
148+
expect(map.has(1)).toBe(false)
149+
expect(map.get(2)).toBe('two')
150+
expect(map.get(3)).toBe('three')
151+
})
152+
153+
it('should maintain correct iteration order', () => {
154+
const map = new BoundedMap<string, number>(3)
155+
156+
map.set('a', 1)
157+
map.set('b', 2)
158+
map.set('c', 3)
159+
160+
const keys = Array.from(map.keys())
161+
expect(keys).toEqual(['a', 'b', 'c'])
162+
163+
// After eviction, order should be preserved
164+
map.set('d', 4)
165+
const keysAfterEviction = Array.from(map.keys())
166+
expect(keysAfterEviction).toEqual(['b', 'c', 'd'])
167+
})
168+
})
169+
})
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
export class BoundedMap<K, V> extends Map<K, V> {
2+
private readonly maxSize: number
3+
constructor(maxSize: number) {
4+
super()
5+
this.maxSize = Math.max(1, maxSize)
6+
}
7+
// Enforce a maximum size with FIFO eviction: oldest entries are removed first.
8+
// Note: updating an existing key does NOT refresh its position.
9+
override set(key: K, value: V): this {
10+
if (!this.has(key) && this.size >= this.maxSize) {
11+
const oldestKey = this.keys().next().value as K | undefined
12+
if (oldestKey !== undefined) {
13+
this.delete(oldestKey)
14+
}
15+
}
16+
return super.set(key, value)
17+
}
18+
}

packages/core-mobile/app/new/features/ledger/screens/LedgerReviewTransactionScreen.tsx

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@ import { AnimatedIconWithText } from 'new/features/ledger/components/AnimatedIco
1313
import { useSelector } from 'react-redux'
1414
import { selectActiveWalletId } from 'store/wallet/slice'
1515
import LedgerService from 'services/ledger/LedgerService'
16+
import { BackHandler } from 'react-native'
17+
import { useNavigation } from 'expo-router'
18+
import { TRANSACTION_CANCELLED_BY_USER } from 'vmModule/ApprovalController/utils'
1619
import { LedgerReviewTransactionParams } from '../services/ledgerParamsCache'
1720
import { useLedgerWalletMap } from '../store'
1821
import { getLedgerAppName } from '../utils'
@@ -23,7 +26,9 @@ const LedgerReviewTransactionScreen = ({
2326
}: {
2427
params: LedgerReviewTransactionParams
2528
}): JSX.Element => {
29+
const navigation = useNavigation()
2630
const approvalTriggeredRef = useRef(false)
31+
const dismissInProgressRef = useRef(false)
2732
const walletId = useSelector(selectActiveWalletId)
2833
const { ledgerWalletMap } = useLedgerWalletMap()
2934
const [isConnected, setIsConnected] = useState(false)
@@ -96,12 +101,47 @@ const LedgerReviewTransactionScreen = ({
96101
return () => clearTimeout(timer)
97102
}, [])
98103

104+
// Handle Android hardware back button
105+
useEffect(() => {
106+
const onBackPress = (): boolean => {
107+
if (isCancelEnabled && !dismissInProgressRef.current) {
108+
dismissInProgressRef.current = true
109+
onReject(TRANSACTION_CANCELLED_BY_USER)
110+
return true // Prevent default back behavior, onReject handles navigation
111+
}
112+
return false
113+
}
114+
115+
const backHandler = BackHandler.addEventListener(
116+
'hardwareBackPress',
117+
onBackPress
118+
)
119+
120+
return () => backHandler.remove()
121+
}, [onReject, isCancelEnabled])
122+
123+
// Handle gesture dismissal (swipe down)
124+
useEffect(() => {
125+
return navigation.addListener('beforeRemove', e => {
126+
if (
127+
e.data.action.type === 'POP' && // gesture dismissed
128+
isCancelEnabled &&
129+
!dismissInProgressRef.current
130+
) {
131+
e.preventDefault()
132+
dismissInProgressRef.current = true
133+
// Modal is being dismissed via gesture
134+
onReject(TRANSACTION_CANCELLED_BY_USER)
135+
}
136+
})
137+
}, [navigation, onReject, isCancelEnabled])
138+
99139
const renderFooter = useCallback(() => {
100140
return (
101141
<Button
102142
type="secondary"
103143
size="large"
104-
onPress={onReject}
144+
onPress={() => onReject(TRANSACTION_CANCELLED_BY_USER)}
105145
disabled={!isCancelEnabled}>
106146
Cancel
107147
</Button>

packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,14 @@ import { showLedgerReviewTransaction } from 'features/ledger/utils'
2222
import { promptForAppReviewAfterSuccessfulTransaction } from 'features/appReview/utils/promptForAppReviewAfterSuccessfulTransaction'
2323
import { CONFETTI_DURATION_MS } from 'common/consts'
2424
import { currentRouteStore } from 'new/routes/store'
25+
import { BoundedMap } from 'common/utils/boundedMap'
2526
import { onApprove } from './onApprove'
2627
import { onReject } from './onReject'
2728
import { handleLedgerErrorAndShowAlert } from './utils'
2829

2930
class ApprovalController implements VmModuleApprovalController {
31+
private userCancelledMap = new BoundedMap<string, boolean>(10)
32+
3033
async requestPublicKey({
3134
secretId,
3235
derivationPath,
@@ -142,6 +145,10 @@ class ApprovalController implements VmModuleApprovalController {
142145
displayData,
143146
signingData
144147
}: ApprovalParams): Promise<ApprovalResponse> {
148+
const requestId = request.requestId
149+
// Clear any previous cancellation state for this request
150+
this.userCancelledMap.delete(requestId)
151+
145152
return new Promise<ApprovalResponse>(resolve => {
146153
walletConnectCache.approvalParams.set({
147154
request,
@@ -156,6 +163,12 @@ class ApprovalController implements VmModuleApprovalController {
156163
value: ApprovalResponse | PromiseLike<ApprovalResponse>
157164
): void => {
158165
if ('error' in value) {
166+
// Don't show alert if user explicitly cancelled
167+
if (this.userCancelledMap.get(requestId)) {
168+
this.userCancelledMap.delete(requestId)
169+
return
170+
}
171+
159172
handleLedgerErrorAndShowAlert({
160173
error: value.error,
161174
network: params.network,
@@ -166,13 +179,15 @@ class ApprovalController implements VmModuleApprovalController {
166179
resolve: resolveWithRetry
167180
}),
168181
onCancel: () => {
182+
this.userCancelledMap.set(requestId, true)
169183
this.handleGoBackIfNeeded()
170184
this.handleLedgerOnReject({ resolve })
171185
}
172186
})
173187
} else {
174188
resolve(value)
175189
this.handleGoBackIfNeeded()
190+
this.userCancelledMap.delete(requestId)
176191
}
177192
}
178193

@@ -185,6 +200,7 @@ class ApprovalController implements VmModuleApprovalController {
185200
resolve: resolveWithRetry
186201
}),
187202
onReject: () => {
203+
this.userCancelledMap.set(requestId, true)
188204
this.handleLedgerOnReject({ resolve })
189205
this.handleGoBackIfNeeded()
190206
}

packages/core-mobile/app/vmModule/ApprovalController/utils.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import { RpcError } from '@avalabs/vm-module-types'
44
import { getLedgerAppName } from 'features/ledger/utils'
55
import { LEDGER_ERROR_CODES } from 'services/ledger/types'
66

7+
export const TRANSACTION_CANCELLED_BY_USER = 'Transaction cancelled by user'
8+
79
export const handleLedgerErrorAndShowAlert = ({
810
error,
911
network,
@@ -16,7 +18,7 @@ export const handleLedgerErrorAndShowAlert = ({
1618
onCancel: () => void
1719
}): void => {
1820
// @ts-ignore
19-
const message = error.data?.cause?.message || ''
21+
const message = error.data?.cause?.message || error.message || ''
2022
const lowercasedMessage = message.toLowerCase()
2123

2224
const ledgerAppName = getLedgerAppName(network)
@@ -45,7 +47,11 @@ export const handleLedgerErrorAndShowAlert = ({
4547
} else if (lowercasedMessage.includes(LEDGER_ERROR_CODES.UPDATE_REQUIRED)) {
4648
title = 'Update required'
4749
description = `Update the ${ledgerAppName} app on your Ledger device to continue`
48-
} else if (lowercasedMessage.includes(LEDGER_ERROR_CODES.USER_CANCELLED)) {
50+
} else if (
51+
lowercasedMessage.includes(LEDGER_ERROR_CODES.USER_CANCELLED) ||
52+
lowercasedMessage.includes(TRANSACTION_CANCELLED_BY_USER.toLowerCase()) ||
53+
lowercasedMessage.includes('action cancelled by user')
54+
) {
4955
// User cancelled, no need to show alert
5056
return
5157
} else if (

0 commit comments

Comments
 (0)