Open
Conversation
ruijialin-avalabs
requested review from
B0Y3R-AVA,
alexnicolae-ava,
atn4z7,
bogdandobritoiu and
onghwan
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the Ledger transaction review UX to be inline (via a shared footer component + store-driven params) instead of using dedicated Ledger review modal routes, and adjusts the stake reward claim flow to support Ledger step/progress UI.
Changes:
- Replace
showLedgerReviewTransactionnavigation withledgerParamsStore.setReviewTransactionParams(...)and render an inline Ledger footer in the approval sheet. - Add reusable
LedgerReviewFooter+ new hooks for Ledger claim reward / approval flows; remove Ledger review modal routes/screens. - Update earn/claim plumbing (progress callbacks, mutation reset exposure) and fix a typo in
RecoveryEvents.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.ts | Switch Ledger review triggering to ledgerParamsStore and clear params on completion. |
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.test.ts | Update tests to assert store params are set instead of navigation helper usage. |
| packages/core-mobile/app/services/earn/types.ts | Fix typo in RecoveryEvents enum value. |
| packages/core-mobile/app/services/earn/importC.ts | Simplify send step (removes retry/wrapping) before status polling. |
| packages/core-mobile/app/services/earn/EarnService.ts | Add delegation/claim progress callbacks and wire to claim flow. |
| packages/core-mobile/app/new/routes/(signedIn)/_layout.tsx | Remove Ledger review modal screens from route stack. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/index.tsx | Remove Ledger review transaction modal route entrypoint. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/_layout.tsx | Remove Ledger review transaction modal stack layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/index.tsx | Remove Ledger review staking modal route entrypoint. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/_layout.tsx | Remove Ledger review staking modal stack layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/useLedgerStaking.tsx | Refactor staking footer rendering to use shared LedgerReviewFooter and add cancel handling. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/confirm.tsx | Add cancel/reset wiring via ref to break circular dependency with delegation mutation. |
| packages/core-mobile/app/new/features/stake/screens/ClaimStakeRewardScreen.tsx | Add Ledger-aware claim flow with inline Ledger footer and dismissal behavior updates. |
| packages/core-mobile/app/new/features/stake/hooks/useLedgerClaimReward.tsx | New hook to manage Ledger BLE connection + claim progress footer rendering. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Remove showLedgerReviewTransaction helper (navigation-based flow). |
| packages/core-mobile/app/new/features/ledger/store.ts | Remove staking-progress-specific params from Ledger review transaction params type. |
| packages/core-mobile/app/new/features/ledger/screens/LedgerReviewStakingScreen.tsx | Remove staking-specific Ledger review screen. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerBLEConnection.ts | Add configurable required Ledger app type and update status messages. |
| packages/core-mobile/app/new/features/ledger/components/index.ts | Export LedgerReviewFooter + getStepConfig. |
| packages/core-mobile/app/new/features/ledger/components/LedgerReviewFooter.tsx | New shared UI footer for Ledger connection/progress + cancel. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/utils.ts | Extract account selector logic to helper. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/ApprovalScreen.tsx | Inline Ledger footer into approval ActionSheet and adjust approve flow for Ledger. |
| packages/core-mobile/app/new/features/approval/hooks/useLedgerApproval.tsx | New hook to render Ledger footer based on ledgerParamsStore and BLE state. |
| packages/core-mobile/app/new/common/consts/screenOptions.tsx | Remove now-unused ledgerModalScreensOptions. |
| packages/core-mobile/app/new/common/components/ActionSheet.tsx | Add renderFooterOverride to support custom footers (Ledger). |
| packages/core-mobile/app/hooks/useUiSafeMutation.ts | Expose React Query mutation reset. |
| packages/core-mobile/app/hooks/earn/useIssueDelegation.ts | Plumb mutation reset out to callers. |
| packages/core-mobile/app/hooks/earn/useClaimRewards.ts | Add progress callback support + expose reset for claim mutation. |
Comments suppressed due to low confidence (1)
packages/core-mobile/app/hooks/earn/useClaimRewards.ts:44
useXPAddressesis defined to accept only theaccountargument, but this call passes a secondfalseparameter. In TS this will fail type-checking. Either remove the second argument or updateuseXPAddresses's signature to support the intended behavior (and update other call sites accordingly).
const { xpAddresses, xpAddressDictionary } = useXPAddresses(activeAccount)
const {
totalFees,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/utils.ts
Outdated
Show resolved
Hide resolved
packages/core-mobile/app/new/features/stake/hooks/useLedgerClaimReward.tsx
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the mobile Ledger transaction review experience to use an inline, store-driven footer (instead of dedicated Ledger modal routes) and extends staking reward claim/delegation flows to support Ledger progress/step UI.
Changes:
- Replaces Ledger review modal navigation with
ledgerParamsStore.setReviewTransactionParams(...)and renders an inlineLedgerReviewFooterin approval/staking/claim screens. - Adds Ledger progress-capable hooks for staking and claim rewards; removes Ledger review modal routes/screens.
- Extends earn claim plumbing to support progress callbacks and exposes mutation resets for cancellation flows; fixes
RecoveryEventsenum typo.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.ts | Switches Ledger review trigger from navigation to ledger params store; clears params on success. |
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.test.ts | Updates tests to assert ledger params store is set for Ledger wallet types. |
| packages/core-mobile/app/services/earn/types.ts | Fixes typo in RecoveryEvents enum member. |
| packages/core-mobile/app/services/earn/importC.ts | Removes stray whitespace line. |
| packages/core-mobile/app/services/earn/EarnService.ts | Adds claim-reward progress callback support and emits progress steps. |
| packages/core-mobile/app/new/routes/(signedIn)/_layout.tsx | Removes Ledger modal screens from the signed-in stack. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/index.tsx | Removes Ledger review transaction modal route entry. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/_layout.tsx | Removes Ledger review transaction modal layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/index.tsx | Removes Ledger review staking modal route entry. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/_layout.tsx | Removes Ledger review staking modal layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/useLedgerStaking.tsx | Refactors staking Ledger UI to use shared LedgerReviewFooter and adds cancel callback. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/confirm.tsx | Wires Ledger cancel flow to reset the delegation mutation. |
| packages/core-mobile/app/new/features/stake/screens/ClaimStakeRewardScreen.tsx | Updates claim flow to support Ledger step UI and cancel/reset behavior. |
| packages/core-mobile/app/new/features/stake/hooks/useLedgerClaimReward.tsx | Adds new hook to manage Ledger connection + progress UI for claim rewards. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Removes showLedgerReviewTransaction helper (no longer navigates to modal routes). |
| packages/core-mobile/app/new/features/ledger/store.ts | Simplifies ledger review params type (removes staking modal progress params). |
| packages/core-mobile/app/new/features/ledger/screens/LedgerReviewStakingScreen.tsx | Deletes staking review screen (replaced by inline footer). |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerBLEConnection.ts | Adds reconnect state + configurable required app type for connection polling. |
| packages/core-mobile/app/new/features/ledger/components/index.ts | Exports the new LedgerReviewFooter and getStepConfig. |
| packages/core-mobile/app/new/features/ledger/components/LedgerReviewFooter.tsx | Introduces shared inline footer UI for Ledger connect/progress phases. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/utils.ts | Extracts account selector resolution into a reusable helper. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/ApprovalScreen.tsx | Integrates inline Ledger footer into approval sheet; updates account selector usage. |
| packages/core-mobile/app/new/features/approval/hooks/useLedgerApproval.tsx | Adds hook to drive inline Ledger approval footer based on ledger params store. |
| packages/core-mobile/app/new/common/consts/screenOptions.tsx | Removes Ledger modal-specific screen options (modals removed). |
| packages/core-mobile/app/new/common/components/ActionSheet.tsx | Adds support for overriding the default action footer rendering. |
| packages/core-mobile/app/hooks/useUiSafeMutation.ts | Exposes safeReset and adjusts mutation error handling behavior. |
| packages/core-mobile/app/hooks/earn/useIssueDelegation.ts | Exposes mutation reset via useUiSafeMutation.safeReset. |
| packages/core-mobile/app/hooks/earn/useClaimRewards.ts | Adds optional progress callback + exposes reset for claim mutation. |
Comments suppressed due to low confidence (1)
packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.ts:193
reviewTransactionParamsis only cleared on the success path (resolve(value)), but not when the user cancels via the retry/cancel alert or whenonRejectis invoked. That can leave stale callbacks/params inledgerParamsStore, potentially keeping the inline footer active on future approval sheets and retaining references to old requests. Consider clearingledgerParamsStorein the cancel/reject paths as well (or centralize cleanup insidehandleLedgerOnReject).
onCancel: () => {
this.userCancelledMap.set(requestId, true)
this.handleGoBackIfNeeded()
this.handleLedgerOnReject({ resolve })
}
})
} else {
resolve(value)
ledgerParamsStore.getState().setReviewTransactionParams(null)
this.handleGoBackIfNeeded()
this.userCancelledMap.delete(requestId)
}
}
ledgerParamsStore.getState().setReviewTransactionParams({
rpcMethod: request.method,
network: params.network,
onApprove: () =>
onApprove({
...params,
signingData,
resolve: resolveWithRetry
}),
onReject: (_message?: string) => {
this.userCancelledMap.set(requestId, true)
this.handleLedgerOnReject({ resolve })
this.handleGoBackIfNeeded()
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
packages/core-mobile/app/new/features/approval/hooks/useLedgerApproval.tsx
Outdated
Show resolved
Hide resolved
packages/core-mobile/app/new/features/stake/hooks/useLedgerClaimReward.tsx
Outdated
Show resolved
Hide resolved
packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/useLedgerStaking.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the Ledger transaction review UX to render inline (via a shared footer + store-driven params) instead of navigating to dedicated Ledger review modal routes, and extends the stake reward claim flow to support Ledger step/progress UI.
Changes:
- Replace
showLedgerReviewTransactionnavigation withledgerParamsStore.setReviewTransactionParams(...)and render an inlineLedgerReviewFooterin the approval sheet. - Add Ledger-specific hooks for approval / staking / claim flows and remove Ledger review modal routes/screens.
- Plumb delegation/claim progress callbacks + mutation reset, and fix
RecoveryEventsenum typo.
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.ts | Sets Ledger review params in store instead of navigating to Ledger review modals. |
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.test.ts | Updates tests to assert store params are set for Ledger wallets. |
| packages/core-mobile/app/services/earn/types.ts | Fixes RecoveryEvents enum typo. |
| packages/core-mobile/app/services/earn/importC.ts | Removes an extra blank line. |
| packages/core-mobile/app/services/earn/EarnService.ts | Adds claim-reward progress callbacks to support Ledger step UI. |
| packages/core-mobile/app/new/routes/(signedIn)/_layout.tsx | Removes Ledger review modal screens from the signed-in stack. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/index.tsx | Removes Ledger review transaction modal route entrypoint. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/_layout.tsx | Removes Ledger review transaction modal stack layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/index.tsx | Removes Ledger staking review modal route entrypoint. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/_layout.tsx | Removes Ledger staking review modal stack layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/useLedgerStaking.tsx | Replaces inline UI with reusable LedgerReviewFooter and adds cancel hook. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/confirm.tsx | Wires Ledger cancel -> delegation mutation reset via ref to avoid circular deps. |
| packages/core-mobile/app/new/features/stake/screens/ClaimStakeRewardScreen.tsx | Adds Ledger-aware claim flow with step/progress UI and mutation reset on cancel. |
| packages/core-mobile/app/new/features/stake/hooks/useLedgerClaimReward.tsx | New hook that coordinates BLE connection + claim action progress UI. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Removes legacy navigation helper showLedgerReviewTransaction. |
| packages/core-mobile/app/new/features/ledger/store.ts | Simplifies review params type (removes staking modal params). |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerBLEConnection.ts | Adds reconnecting state + supports required app type for connection messaging. |
| packages/core-mobile/app/new/features/ledger/components/index.ts | Exports LedgerReviewFooter and getStepConfig. |
| packages/core-mobile/app/new/features/ledger/components/LedgerReviewFooter.tsx | New shared footer component for Ledger connecting/progress UX. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/utils.ts | Extracts account-selector logic into helper. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/ApprovalScreen.tsx | Renders inline Ledger footer in approval sheet and adjusts Ledger approve behavior. |
| packages/core-mobile/app/new/features/approval/hooks/useLedgerApproval.tsx | New hook to drive inline Ledger approval footer from store params. |
| packages/core-mobile/app/new/common/consts/screenOptions.tsx | Removes Ledger-specific modal screen options (routes removed). |
| packages/core-mobile/app/new/common/components/ActionSheet.tsx | Adds renderFooterOverride to support custom (Ledger) footer rendering. |
| packages/core-mobile/app/hooks/useUiSafeMutation.ts | Exposes safeReset to reset mutations safely from UI flows. |
| packages/core-mobile/app/hooks/earn/useIssueDelegation.ts | Exposes reset for delegation mutation so Ledger cancel can reset state. |
| packages/core-mobile/app/hooks/earn/useClaimRewards.ts | Adds optional progress callback + exposes mutation reset for Ledger cancel. |
Comments suppressed due to low confidence (1)
packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.ts:193
- On Ledger rejection paths,
reviewTransactionParamsis not cleared fromledgerParamsStore(it is only cleared on success). To avoid keeping stale callbacks/network references around, clear the store in thisonRejecthandler (and similarly in the alertonCancelpath above) before navigating back/disconnecting.
onReject: (_message?: string) => {
this.userCancelledMap.set(requestId, true)
this.handleLedgerOnReject({ resolve })
this.handleGoBackIfNeeded()
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const cancelLedger = useCallback((): void => { | ||
| resetLedgerState() | ||
| onCancel?.() | ||
| LedgerService.disconnect() |
| const cancelLedger = useCallback((): void => { | ||
| resetLedgerState() | ||
| onCancel?.() | ||
| LedgerService.disconnect() |
| resetLedgerState() | ||
| reviewTransactionParams?.onReject(TRANSACTION_CANCELLED_BY_USER) | ||
| ledgerParamsStore.getState().setReviewTransactionParams(null) | ||
| LedgerService.disconnect() |
| AddDelegatorTransactionProps, | ||
| RecoveryEvents | ||
| } from 'services/earn/types' | ||
| import { OnDelegationProgress } from 'contexts/DelegationContext' |
Comment on lines
+179
to
+184
| await new Promise<void>(resolve => { | ||
| setTimeout(() => { | ||
| onProgress?.(1, Operation.IMPORT_C) | ||
| resolve() | ||
| }, 10) | ||
| }) |
B0Y3R-AVA
reviewed
Mar 18, 2026
Contributor
B0Y3R-AVA
left a comment
B0Y3R-AVA
left a comment
There was a problem hiding this comment.
Looks Great! few small requests for enums or consts and some inline comments
packages/core-mobile/app/new/features/approval/hooks/useLedgerApproval.tsx
Outdated
Show resolved
Hide resolved
packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/ApprovalScreen.tsx
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the mobile Ledger transaction review UX to render inline (via a shared footer + store-driven params) instead of navigating to dedicated Ledger review modal routes, and updates the staking reward claim flow to support Ledger step/progress UI.
Changes:
- Replaces Ledger review modal navigation with
ledgerParamsStore.setReviewTransactionParams(...)and renders an inline Ledger footer in the approval sheet. - Introduces reusable
LedgerReviewFooterand new Ledger hooks for approval / staking / claim reward flows; removes Ledger review modal routes/screens. - Updates earn/stake plumbing for progress callbacks and exposes mutation resets; fixes a
RecoveryEventsenum typo.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.ts | Switches Ledger review flow to store-driven inline params and clears params on success. |
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.test.ts | Updates tests to mock ledger params store instead of navigation helper. |
| packages/core-mobile/app/store/wallet/slice.ts | Adds Ledger wallet selectors (selectIsActiveWalletLedger, selectIsWalletLedger). |
| packages/core-mobile/app/store/account/thunks.ts | Uses Ledger selector to gate Ledger-specific xpub/address behavior. |
| packages/core-mobile/app/store/account/listeners.ts | Uses Ledger selector during account initialization. |
| packages/core-mobile/app/services/earn/types.ts | Fixes RecoveryEvents enum typo. |
| packages/core-mobile/app/services/earn/importC.ts | Removes stray whitespace line. |
| packages/core-mobile/app/services/earn/EarnService.ts | Adds claim reward progress callbacks and aligns enum usage. |
| packages/core-mobile/app/new/routes/(signedIn)/_layout.tsx | Removes Ledger review modal screens from navigation stack. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/index.tsx | Deletes obsolete Ledger review transaction modal route entry. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/_layout.tsx | Deletes obsolete Ledger review transaction modal layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/index.tsx | Deletes obsolete Ledger staking review modal route entry. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/_layout.tsx | Deletes obsolete Ledger staking review modal layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/useLedgerStaking.tsx | Replaces bespoke footer UI with shared LedgerReviewFooter and adds cancel handling. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/confirm.tsx | Wires Ledger cancel to mutation reset and uses updated staking Ledger hook signature. |
| packages/core-mobile/app/new/features/stake/screens/ClaimStakeRewardScreen.tsx | Adds Ledger-aware claim flow with inline footer and progress steps. |
| packages/core-mobile/app/new/features/stake/hooks/useLedgerClaimReward.tsx | Adds Ledger BLE connection/progress hook for claim reward flow. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Removes showLedgerReviewTransaction helper (navigation-based). |
| packages/core-mobile/app/new/features/ledger/store.ts | Simplifies Ledger params types (removes staking modal progress params). |
| packages/core-mobile/app/new/features/ledger/screens/LedgerReviewStakingScreen.tsx | Deletes obsolete Ledger staking review screen. |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerBLEConnection.ts | Adds appType support + reconnect state for inline Ledger UX. |
| packages/core-mobile/app/new/features/ledger/components/index.ts | Exports new LedgerReviewFooter and getStepConfig. |
| packages/core-mobile/app/new/features/ledger/components/LedgerReviewFooter.tsx | New shared inline Ledger connection/progress footer component. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/utils.ts | Extracts account selector selection logic into a helper. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/ApprovalScreen.tsx | Adds inline Ledger footer override and refactors gasless pre-approve handling. |
| packages/core-mobile/app/new/features/approval/hooks/useLedgerApproval.tsx | New hook to drive inline Ledger approval footer flow. |
| packages/core-mobile/app/new/features/accountSettings/components/accountAddresses.tsx | Uses Ledger selector instead of local wallet-type checks. |
| packages/core-mobile/app/new/common/consts/screenOptions.tsx | Removes Ledger modal-specific screen options. |
| packages/core-mobile/app/new/common/components/WalletCard.tsx | Uses Ledger selector instead of local wallet-type checks. |
| packages/core-mobile/app/new/common/components/ActionSheet.tsx | Adds renderFooterOverride hook for inline Ledger footer rendering. |
| packages/core-mobile/app/hooks/useUiSafeMutation.ts | Exposes safeReset for mutation state resets and cleans up timers. |
| packages/core-mobile/app/hooks/earn/useIssueDelegation.ts | Exposes mutation reset for delegation flow. |
| packages/core-mobile/app/hooks/earn/useClaimRewards.ts | Adds optional progress callback + exposes mutation reset for claim flow. |
Comments suppressed due to low confidence (2)
packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/useLedgerStaking.tsx:93
- The
resetOnFailurelogic relies on the delegatedactionpromise rejecting/throwing; if the caller usesuseUiSafeMutation.safeMutate(which swallows errors), this catch path won't run and the hook can stay in PROGRESS withapprovalInProgress=trueafter failures. Consider requiringactionto reject on failure, or handling error/reset explicitly via callbacks instead of relying on promise rejection.
const resetOnFailure = (): void => {
if (!isMountedRef.current) {
return
}
setApprovalInProgress(false)
setLedgerPhase(LedgerReviewPhase.CONNECTING)
}
try {
const result = pendingActionRef.current?.(onProgress)
if (result instanceof Promise) {
result.catch(resetOnFailure)
}
} catch {
resetOnFailure()
}
packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.ts:193
onRejectprovided tosetReviewTransactionParamsaccepts a message but currently ignores it. This drops theTRANSACTION_CANCELLED_BY_USERreason passed from the inline Ledger footer, and prevents downstreamonRejectfrom producing a message-specific error. Consider threading_messagethrough tohandleLedgerOnReject/onReject({ resolve, message })(and optionally clearingreviewTransactionParamshere as well).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+61
to
+89
| // Execute signing once device is connected and correct app is open | ||
| useEffect(() => { | ||
| if ( | ||
| !isLedger || | ||
| ledgerPhase !== 'connecting' || | ||
| !isLedgerConnected || | ||
| !isRequiredAppOpen || | ||
| approvalInProgress | ||
| ) | ||
| return | ||
|
|
||
| setApprovalInProgress(true) | ||
| setLedgerPhase(LedgerReviewPhase.PROGRESS) | ||
|
|
||
| const resetOnFailure = (): void => { | ||
| if (!isMountedRef.current) return | ||
| setApprovalInProgress(false) | ||
| setLedgerPhase(LedgerReviewPhase.CONNECTING) | ||
| } | ||
|
|
||
| try { | ||
| const result = reviewTransactionParams?.onApprove() | ||
| if (result instanceof Promise) { | ||
| result.catch(resetOnFailure) | ||
| } | ||
| } catch { | ||
| resetOnFailure() | ||
| } | ||
| }, [ |
Comment on lines
+78
to
+92
| const resetOnFailure = (): void => { | ||
| if (!isMountedRef.current) { | ||
| return | ||
| } | ||
| setApprovalInProgress(false) | ||
| setLedgerPhase(LedgerReviewPhase.CONNECTING) | ||
| } | ||
|
|
||
| try { | ||
| const result = pendingActionRef.current?.(onProgress) | ||
| if (result instanceof Promise) { | ||
| result.catch(resetOnFailure) | ||
| } | ||
| } catch { | ||
| resetOnFailure() |
Comment on lines
+177
to
+179
| if (!isLedger) { | ||
| setSubmitting(false) | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the Ledger transaction review UX to render inline (via a shared footer and store-driven params) instead of navigating to dedicated Ledger review modal routes, and updates staking/claim reward flows to support Ledger progress/step UI.
Changes:
- Replace Ledger review modal navigation with
ledgerParamsStore.setReviewTransactionParams(...)and render inline Ledger UI in the approval sheet. - Add reusable
LedgerReviewFooterand new Ledger hooks for approval/staking/claim reward flows; remove Ledger review modal routes/screens. - Update earn/stake plumbing to support progress callbacks + mutation resets; fix
RecoveryEventstypo.
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.ts | Switches Ledger review trigger from navigation to ledger params store; clears params on success. |
| packages/core-mobile/app/vmModule/ApprovalController/ApprovalController.test.ts | Updates tests/mocks to validate ledger params store usage instead of navigation util. |
| packages/core-mobile/app/store/wallet/slice.ts | Adds centralized Ledger wallet selectors. |
| packages/core-mobile/app/store/account/thunks.ts | Refactors Ledger wallet checks to use the new selector. |
| packages/core-mobile/app/store/account/listeners.ts | Refactors Ledger wallet checks to use the new selector. |
| packages/core-mobile/app/services/earn/types.ts | Fixes typo in RecoveryEvents enum value. |
| packages/core-mobile/app/services/earn/importC.ts | Removes stray whitespace. |
| packages/core-mobile/app/services/earn/EarnService.ts | Updates recovery event name and adds claim reward progress callbacks/hooks integration. |
| packages/core-mobile/app/new/routes/(signedIn)/_layout.tsx | Removes Ledger review modal routes from navigation stack. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/index.tsx | Removes Ledger review transaction modal route entry point. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewTransaction/_layout.tsx | Removes Ledger review transaction modal layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/index.tsx | Removes Ledger review staking modal route entry point. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/ledgerReviewStaking/_layout.tsx | Removes Ledger review staking modal layout. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/useLedgerStaking.tsx | Migrates staking Ledger UI to shared LedgerReviewFooter and adds cancel/disconnect handling. |
| packages/core-mobile/app/new/routes/(signedIn)/(modals)/addStake/confirm.tsx | Wires staking cancellation to reset the delegation mutation via ref callback. |
| packages/core-mobile/app/new/features/stake/screens/ClaimStakeRewardScreen.tsx | Adds Ledger-aware claim reward flow + inline footer rendering; uses dismissAll on success. |
| packages/core-mobile/app/new/features/stake/hooks/useLedgerClaimReward.tsx | Introduces new hook to drive claim reward Ledger connection/progress footer. |
| packages/core-mobile/app/new/features/ledger/utils/index.ts | Removes showLedgerReviewTransaction navigation helper (store-only approach). |
| packages/core-mobile/app/new/features/ledger/store.ts | Removes stakingProgress params from ledger review transaction params type. |
| packages/core-mobile/app/new/features/ledger/screens/LedgerReviewStakingScreen.tsx | Deletes legacy staking review screen (now inline footer). |
| packages/core-mobile/app/new/features/ledger/hooks/useLedgerBLEConnection.ts | Adds appType support + reconnect state + improved connection status messaging. |
| packages/core-mobile/app/new/features/ledger/components/index.ts | Exports new LedgerReviewFooter and getStepConfig. |
| packages/core-mobile/app/new/features/ledger/components/LedgerReviewFooter.tsx | Adds reusable Ledger connection/progress footer component. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/utils.ts | Extracts account selector logic into a helper to support more signingData shapes correctly. |
| packages/core-mobile/app/new/features/approval/screens/ApprovalScreen/ApprovalScreen.tsx | Integrates inline Ledger footer + store-driven flow; refactors gasless pre-approve logic. |
| packages/core-mobile/app/new/features/approval/hooks/useLedgerApproval.tsx | Adds hook to drive inline Ledger footer for generic approvals. |
| packages/core-mobile/app/new/features/accountSettings/components/accountAddresses.tsx | Replaces local Ledger wallet checks with centralized selector. |
| packages/core-mobile/app/new/common/consts/screenOptions.tsx | Removes Ledger modal-specific screen options (modals removed). |
| packages/core-mobile/app/new/common/components/WalletCard.tsx | Replaces local Ledger wallet checks with centralized selector. |
| packages/core-mobile/app/new/common/components/ActionSheet.tsx | Adds renderFooterOverride to support inline Ledger footer rendering. |
| packages/core-mobile/app/hooks/useUiSafeMutation.ts | Exposes a safe mutation reset helper for UI-driven cancellation flows. |
| packages/core-mobile/app/hooks/earn/useIssueDelegation.ts | Exposes mutation reset to allow Ledger cancel to reset staking mutation state. |
| packages/core-mobile/app/hooks/earn/useClaimRewards.ts | Adds optional progress callback and exposes reset for claim reward flow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
+46
to
+50
| } = useLedgerBLEConnection({ | ||
| isLedger, | ||
| isConnecting: ledgerPhase === 'connecting', | ||
| appType | ||
| }) |
Comment on lines
+52
to
+59
| // Transition to connecting when new params arrive | ||
| useEffect(() => { | ||
| if (!isLedger || !reviewTransactionParams) return | ||
| if (reviewTransactionParams === prevParamsRef.current) return | ||
| prevParamsRef.current = reviewTransactionParams | ||
| setLedgerPhase(LedgerReviewPhase.CONNECTING) | ||
| setApprovalInProgress(false) | ||
| }, [reviewTransactionParams, isLedger]) |
Comment on lines
+177
to
+179
| if (!isLedger) { | ||
| setSubmitting(false) | ||
| } |
| // defer the completion progress update so the UI reflects the finished state after importC resolves | ||
| await new Promise<void>(resolve => { | ||
| setTimeout(() => { | ||
| onProgress?.(2, null) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
iOS: 7792
Android: 7793
Ticket: CP-13413 Ticket: CP-13647
Please provide:
Refactors the Ledger transaction review UX to be inline (via a shared footer component + store-driven params) instead of using dedicated Ledger review modal routes, and adjusts the stake reward claim flow to support Ledger step/progress UI.
Changes:
showLedgerReviewTransactionnavigation withledgerParamsStore.setReviewTransactionParams(...)and render an inline Ledger footer in the approval sheet.LedgerReviewFooter+ new hooks for Ledger claim reward / approval flows; remove Ledger review modal routes/screens.RecoveryEvents.Screenshots/Videos
RecordIt-1773844792.MOV
RecordIt-1773847297.MOV
RecordIt-1773847553.MOV
Testing
Dev Testing (if applicable)
QA Testing (if applicable)
Checklist
Please check all that apply (if applicable)