Skip to content

Commit 0b20391

Browse files
authored
fix: Fix stuck pending withdraw (MetaMask#24214)
<!-- Please submit this PR as a draft initially. Do not mark it as "Ready for review" until the template has been completely filled out, and PR status checks have passed at least once. --> ## **Description** Withdrawal indicators in Perps could get stuck in "pending" or "bridging" state indefinitely, even after the withdrawal had successfully completed on-chain. This created a poor user experience where users saw stale pending indicators that never resolved. **Solution** Enhanced the 1useWithdrawalRequests1 hook to automatically reconcile pending withdrawal states with completed withdrawals from the HyperLiquid API: 1. Auto-reconciliation: When a completed withdrawal from the API matches a pending one in controller state, the controller is automatically updated to reflect the completion 2. Active polling: Polls the HyperLiquid ledger API every 10 seconds when there are active (pending/bridging) withdrawals to detect completions promptly 3. Relaxed flexible matching: Matches withdrawals by amount (±$0.01 tolerance) and asset type; removed timestamp constraint since bridging operations can sometimes take hours 4. Deduplication tracking: Uses a ref to track which withdrawals have already been updated, preventing duplicate updateWithdrawalStatus calls Account isolation: Filters withdrawals by the current selected account to prevent showing stale indicators from other accounts ## **Changelog** CHANGELOG entry: Fixed an issue where Perps withdrawal indicators could remain stuck in "pending" state after the withdrawal completed ## **Related issues** Fixes: https://consensyssoftware.atlassian.net/browse/TAT-2128 ## **Manual testing steps** ```gherkin Feature: Perps Withdrawal Status Resolution Scenario: Pending withdrawal resolves to completed Given user has an active Perps account with USDC balance And user initiates a withdrawal from Perps When the withdrawal completes on HyperLiquid (wait up to 10 seconds for poll) Then the withdrawal status updates from "pending" or "bridging" to "completed" And the withdrawal displays the transaction hash Scenario: Pending indicators clear on account switch Given user has a pending withdrawal on Account A And user switches to Account B which has no withdrawals When the withdrawal list renders Then no pending withdrawal indicators are shown for Account B Scenario: Completed withdrawals persist after app restart Given user had a withdrawal complete while the app was closed When user opens the app and navigates to Perps withdrawals Then the withdrawal shows as "completed" (fetched from HyperLiquid API) And no stale "pending" indicator is displayed ``` ## **Screenshots/Recordings** <!-- If applicable, add screenshots and/or recordings to visualize the before and after of your change. --> ### **Before** <!-- [screenshots/recordings] --> ### **After** <!-- [screenshots/recordings] --> ## **Pre-merge author checklist** - [x] I’ve followed [MetaMask Contributor Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile Coding Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md). - [x] I've completed the PR template to the best of my ability - [x] I’ve included tests if applicable - [x] I’ve documented my code using [JSDoc](https://jsdoc.app/) format if applicable - [x] I’ve applied the right labels on the PR (see [labeling guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)). Not required for external contributors. ## **Pre-merge reviewer checklist** - [ ] I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed). - [ ] I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Resolves stuck pending/bridging withdrawal indicators by reconciling controller state with HyperLiquid ledger completions. > > - Reworks `useWithdrawalRequests` to merge pending with completed withdrawals, matching by `amount` (±0.01) and `asset` and removing timestamp constraints; requires `txHash` for completed matches > - Moves side-effectful controller updates into `useEffect` and adds `updatedWithdrawalIdsRef` to prevent duplicate `updateWithdrawalStatus` calls > - Adds polling every 10s when active withdrawals exist; maintains sorting by timestamp (newest first) and filters by selected account > - Updates tests to cover long-bridge timestamp gaps, small amount tolerances, txHash requirement, polling behavior, and error handling > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 836ad08. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent dc5cb66 commit 0b20391

2 files changed

Lines changed: 154 additions & 79 deletions

File tree

app/components/UI/Perps/hooks/useWithdrawalRequests.test.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -675,10 +675,12 @@ describe('useWithdrawalRequests', () => {
675675
expect(mockController.updateWithdrawalStatus).not.toHaveBeenCalled();
676676
});
677677

678-
it('does not match withdrawals with different timestamps', async () => {
678+
it('matches withdrawals even with very different timestamps (bridging can take hours)', async () => {
679+
// This test verifies that withdrawals match regardless of timestamp differences
680+
// because HyperLiquid -> Arbitrum bridging can take hours, not just minutes
679681
const pendingWithdrawal = {
680-
id: 'withdrawal-no-match',
681-
timestamp: 1640995200000,
682+
id: 'withdrawal-long-bridge',
683+
timestamp: 1640995200000, // When withdrawal was initiated
682684
amount: '100',
683685
asset: 'USDC',
684686
accountAddress: mockAddress,
@@ -701,7 +703,7 @@ describe('useWithdrawalRequests', () => {
701703
nonce: 456,
702704
},
703705
hash: '0xledger1',
704-
time: 1640995200000 + 1000000, // 16+ minutes later
706+
time: 1640995200000 + 3600000, // 1 hour later (bridging took time)
705707
},
706708
] as unknown as unknown[]);
707709

@@ -714,12 +716,18 @@ describe('useWithdrawalRequests', () => {
714716
});
715717

716718
const withdrawalRequests = result.current.withdrawalRequests;
717-
const pendingWithdrawalResult = withdrawalRequests.find(
718-
(w) => w.id === 'withdrawal-no-match',
719+
const matchedWithdrawal = withdrawalRequests.find(
720+
(w) => w.id === 'withdrawal-long-bridge',
719721
);
720722

721-
expect(pendingWithdrawalResult?.status).toBe('pending');
722-
expect(mockController.updateWithdrawalStatus).not.toHaveBeenCalled();
723+
// Should match and be marked completed even though timestamps differ significantly
724+
expect(matchedWithdrawal?.status).toBe('completed');
725+
expect(matchedWithdrawal?.txHash).toBe('0xledger1');
726+
expect(mockController.updateWithdrawalStatus).toHaveBeenCalledWith(
727+
'withdrawal-long-bridge',
728+
'completed',
729+
'0xledger1',
730+
);
723731
});
724732

725733
it('does not match withdrawals with different assets', async () => {

app/components/UI/Perps/hooks/useWithdrawalRequests.ts

Lines changed: 138 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useEffect, useState, useMemo } from 'react';
1+
import { useCallback, useEffect, useState, useMemo, useRef } from 'react';
22
import { useSelector } from 'react-redux';
33
import Engine from '../../../../core/Engine';
44
import { usePerpsSelector } from './usePerpsSelector';
@@ -200,87 +200,154 @@ export const useWithdrawalRequests = (
200200
}
201201
}, [startTime, selectedAddress]);
202202

203-
// Combine pending and completed withdrawals
204-
const allWithdrawals = useMemo(() => {
205-
// More nuanced approach: only mark as completed if we have a matching completed withdrawal
206-
// with a transaction hash (indicating it reached Arbitrum)
207-
const combined = [...pendingWithdrawals, ...completedWithdrawals];
208-
209-
// Remove duplicates by matching pending/bridging withdrawals with completed ones
210-
const uniqueWithdrawals = combined.reduce((acc, withdrawal) => {
211-
// For pending and bridging withdrawals, keep them as-is
212-
if (withdrawal.status === 'pending' || withdrawal.status === 'bridging') {
213-
acc.push(withdrawal);
214-
return acc;
215-
}
203+
// Track which withdrawals have already been updated in the controller
204+
// to prevent duplicate updateWithdrawalStatus calls
205+
const updatedWithdrawalIdsRef = useRef<Set<string>>(new Set());
216206

217-
// For completed withdrawals, try to match with a pending or bridging one
218-
if (withdrawal.status === 'completed') {
219-
// Only match if the completed withdrawal has a transaction hash (reached Arbitrum)
220-
if (withdrawal.txHash) {
221-
// Look for a pending or bridging withdrawal with similar timestamp and amount
222-
const pendingMatch = acc.findIndex((w) => {
223-
if (w.status !== 'pending' && w.status !== 'bridging') {
224-
return false;
225-
}
226-
227-
// More flexible amount matching - allow for small differences
228-
const amountDiff = Math.abs(
229-
parseFloat(w.amount) - parseFloat(withdrawal.amount),
230-
);
231-
const isAmountMatch = amountDiff < 0.01; // Allow up to 1 cent difference
232-
233-
// More flexible timestamp matching - allow up to 15 minutes
234-
const timeDiff = Math.abs(w.timestamp - withdrawal.timestamp);
235-
const isTimeMatch = timeDiff < 900000; // 15 minutes
236-
237-
// Asset must match
238-
const isAssetMatch = w.asset === withdrawal.asset;
239-
240-
return isAmountMatch && isTimeMatch && isAssetMatch;
207+
// Combine pending and completed withdrawals (pure data transformation, no side effects)
208+
const allWithdrawals = useMemo(() => {
209+
// Build a list that merges pending withdrawals with their completed counterparts
210+
const result: WithdrawalRequest[] = [];
211+
212+
// Track which completed withdrawals have been matched to prevent
213+
// multiple same-amount pending withdrawals from matching the same completed one
214+
const matchedCompletedIds = new Set<string>();
215+
216+
// First, add all pending/bridging withdrawals
217+
for (const pending of pendingWithdrawals) {
218+
if (pending.status === 'pending' || pending.status === 'bridging') {
219+
// Check if this pending withdrawal has a matching UNMATCHED completed one
220+
const matchingCompleted = completedWithdrawals.find((completed) => {
221+
if (!completed.txHash) return false;
222+
// Skip completed withdrawals that have already been matched
223+
if (matchedCompletedIds.has(completed.id)) return false;
224+
225+
const amountDiff = Math.abs(
226+
parseFloat(pending.amount) - parseFloat(completed.amount),
227+
);
228+
const isAmountMatch = amountDiff < 0.01;
229+
const isAssetMatch = pending.asset === completed.asset;
230+
231+
return isAmountMatch && isAssetMatch;
232+
});
233+
234+
if (matchingCompleted) {
235+
// Mark this completed withdrawal as matched
236+
matchedCompletedIds.add(matchingCompleted.id);
237+
// Merge: use pending's ID but update with completed data
238+
result.push({
239+
...pending,
240+
status: 'completed',
241+
txHash: matchingCompleted.txHash,
242+
withdrawalId: matchingCompleted.withdrawalId,
241243
});
242-
243-
if (pendingMatch >= 0) {
244-
// Update the pending/bridging withdrawal with completed data
245-
const matchedWithdrawal = acc[pendingMatch];
246-
acc[pendingMatch] = {
247-
...matchedWithdrawal,
248-
status: 'completed',
249-
txHash: withdrawal.txHash,
250-
withdrawalId: withdrawal.withdrawalId,
251-
};
252-
253-
// Update the controller state to reflect the completion
254-
const controller = Engine.context.PerpsController;
255-
if (controller) {
256-
controller.updateWithdrawalStatus(
257-
matchedWithdrawal.id,
258-
'completed',
259-
withdrawal.txHash,
260-
);
261-
}
262-
} else {
263-
// No pending/bridging match found, add as new completed withdrawal
264-
acc.push(withdrawal);
265-
}
266244
} else {
267-
// Completed withdrawal without txHash - don't match, keep as separate
268-
acc.push(withdrawal);
245+
// No match found, keep as pending/bridging
246+
result.push(pending);
269247
}
270248
} else {
271-
// For failed withdrawals, add as-is
272-
acc.push(withdrawal);
249+
// Already completed or failed in controller state
250+
// Check if this matches an API completed withdrawal to prevent duplicates
251+
// This handles the case where controller state was updated in a previous render
252+
if (pending.status === 'completed' && pending.txHash) {
253+
const matchingApiCompleted = completedWithdrawals.find(
254+
(completed) => completed.txHash === pending.txHash,
255+
);
256+
if (matchingApiCompleted) {
257+
matchedCompletedIds.add(matchingApiCompleted.id);
258+
}
259+
}
260+
result.push(pending);
261+
}
262+
}
263+
264+
// Add completed withdrawals that weren't matched to any pending ones
265+
// (historical withdrawals that were completed before the app started tracking)
266+
for (const completed of completedWithdrawals) {
267+
// Skip if already matched to a pending withdrawal
268+
if (matchedCompletedIds.has(completed.id)) {
269+
continue;
273270
}
274271

275-
return acc;
276-
}, [] as WithdrawalRequest[]);
272+
if (!completed.txHash) {
273+
result.push(completed);
274+
continue;
275+
}
277276

278-
// Sort by timestamp (newest first)
279-
const sorted = uniqueWithdrawals.sort((a, b) => b.timestamp - a.timestamp);
277+
// This completed withdrawal wasn't matched, add it to results
278+
result.push(completed);
279+
}
280280

281-
return sorted;
281+
// Sort by timestamp (newest first)
282+
return result.sort((a, b) => b.timestamp - a.timestamp);
282283
}, [pendingWithdrawals, completedWithdrawals]);
283284

285+
// Update controller state when we detect completed withdrawals that match pending ones
286+
// This is a side effect that should be in useEffect, not useMemo
287+
useEffect(() => {
288+
const controller = Engine.context.PerpsController;
289+
if (!controller) return;
290+
291+
// Track which pending withdrawals have been matched in THIS render cycle
292+
// to prevent multiple completed withdrawals from matching the same pending one
293+
const matchedPendingIdsThisCycle = new Set<string>();
294+
295+
for (const completed of completedWithdrawals) {
296+
// Skip if no txHash (not confirmed on chain)
297+
if (!completed.txHash) continue;
298+
299+
// Find first UNMATCHED pending withdrawal that matches this completed one
300+
const matchingPending = pendingWithdrawals.find((w) => {
301+
if (w.status !== 'pending' && w.status !== 'bridging') {
302+
return false;
303+
}
304+
// Skip if already matched in this render cycle
305+
if (matchedPendingIdsThisCycle.has(w.id)) {
306+
return false;
307+
}
308+
// Skip if already updated in a previous render cycle
309+
if (updatedWithdrawalIdsRef.current.has(w.id)) {
310+
return false;
311+
}
312+
313+
// Match by amount - allow for small differences (fees, rounding)
314+
const amountDiff = Math.abs(
315+
parseFloat(w.amount) - parseFloat(completed.amount),
316+
);
317+
const isAmountMatch = amountDiff < 0.01;
318+
319+
// Asset must match
320+
const isAssetMatch = w.asset === completed.asset;
321+
322+
return isAmountMatch && isAssetMatch;
323+
});
324+
325+
if (matchingPending) {
326+
// Mark as matched in this cycle to prevent another completed from matching it
327+
matchedPendingIdsThisCycle.add(matchingPending.id);
328+
329+
DevLogger.log(
330+
'useWithdrawalRequests: Updating withdrawal status to completed',
331+
{
332+
pendingId: matchingPending.id,
333+
txHash: completed.txHash,
334+
amount: completed.amount,
335+
},
336+
);
337+
338+
// Mark as updated to prevent duplicate calls across render cycles
339+
updatedWithdrawalIdsRef.current.add(matchingPending.id);
340+
341+
// Update the controller state to reflect the completion
342+
controller.updateWithdrawalStatus(
343+
matchingPending.id,
344+
'completed',
345+
completed.txHash,
346+
);
347+
}
348+
}
349+
}, [completedWithdrawals, pendingWithdrawals]);
350+
284351
// Initial fetch when component mounts
285352
useEffect(() => {
286353
if (!skipInitialFetch) {

0 commit comments

Comments
 (0)