[LWDM] feat(perps): sign the deposit through the exchange swap flow - #20923
Draft
ooke-ledger wants to merge 5 commits into
Draft
[LWDM] feat(perps): sign the deposit through the exchange swap flow#20923ooke-ledger wants to merge 5 commits into
ooke-ledger wants to merge 5 commits into
Conversation
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio. |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a Perps deposit “signing” step on both mobile and desktop by reusing the swap exchange orchestration, which has been extracted into a shared executeSwap helper in live-common.
Changes:
- Extracts the
custom.exchange.swapend-to-end flow intolibs/ledger-live-common/src/wallet-api/Exchange/executeSwap.tsand moves UI request types intouiRequests.ts. - Introduces Perps deposit signing screens/dialogs + orchestration hooks on mobile and desktop (device steps: start → confirm → sign → broadcast).
- Updates Desktop
DeviceActionswap confirmation rendering to support a Perps-specific “continue on device” confirmation screen.
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 |
|---|---|
| libs/ledger-live-common/src/wallet-api/Exchange/uiRequests.ts | Centralizes wallet-api exchange UI request types used by handlers and perps flows. |
| libs/ledger-live-common/src/wallet-api/Exchange/server.ts | Refactors swap handler to delegate to shared executeSwap and re-exports request types. |
| libs/ledger-live-common/src/wallet-api/Exchange/executeSwap.ts | New shared swap execution pipeline (device nonce → payload → transaction → UI hooks). |
| libs/ledger-live-common/src/hw/actions/completeExchange.ts | Exposes Result type for consumers (perps deposit execution). |
| apps/ledger-live-mobile/src/mvvm/features/Perps/... | Adds Perps deposit signing screen + view model, routes to it from review, and adds execution hook + tests. |
| apps/ledger-live-mobile/src/const/navigation.ts | Registers PerpsDepositSign screen name. |
| apps/ledger-live-mobile/src/components/RootNavigator/... | Wires PerpsDepositSign into navigation types and stack navigator. |
| apps/ledger-live-mobile/src/components/DeviceAction/index.tsx | Exports Status type so perps hook can type device-action rendering bindings. |
| apps/ledger-live-desktop/src/renderer/components/DeviceAction/... | Adds perps confirmation override for swap confirmation UI and exports States type. |
| apps/ledger-live-desktop/src/mvvm/features/Perps/... | Adds Perps deposit signing dialog + components + execution hook + tests, and opens it from review. |
| apps/ledger-live-desktop/src/mvvm/features/GlobalDialogs/index.tsx | Mounts the new Perps deposit signing dialog globally. |
| .changeset/perps-deposit-signing.md | Declares release notes for the new perps deposit signing step + refactor. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+48
to
+54
| export type SwapUiRequest = CompleteExchangeUiRequest & { | ||
| provider?: string; | ||
| fromAccountId?: string; | ||
| toAccountId?: string; | ||
| tokenCurrency?: string; | ||
| correlationId?: string; | ||
| }; |
Comment on lines
+269
to
+274
| const magnitudeAwareRate = tx.amount && amountExpectedTo.dividedBy(tx.amount); | ||
| const refundAddress = decodePayload.refundAddress; | ||
| const payoutAddress = decodePayload.payoutAddress; | ||
|
|
||
| // tx.amount should be BigNumber | ||
| tx.amount = new BigNumber(tx.amount); |
| i18nKey="perpsDepositSign.terms" | ||
| components={{ | ||
| termsLink: ( | ||
| <span className="cursor-pointer underline" onClick={() => openURL(SWAPKIT_TERMS_URL)} /> |
Comment on lines
+520
to
+521
| // Perps deposit is a FUND, but keeps its own "continue on your device" screen | ||
| // regardless of exchange type — handle it before the swap/sell/fund switch. |
Comment on lines
+8
to
+13
| const mockNavigate = jest.fn(); | ||
|
|
||
| jest.mock("@react-navigation/native", () => ({ | ||
| ...jest.requireActual("@react-navigation/native"), | ||
| useNavigation: () => ({ navigate: mockNavigate }), | ||
| })); |
Contributor
|
Contributor
Web Tools Build Status
|
ooke-ledger
force-pushed
the
feat/LIVE-35327
branch
from
August 19, 2026 09:12
795b662 to
7eb5c2d
Compare
ooke-ledger
force-pushed
the
feat/LIVE-35328
branch
from
August 19, 2026 09:12
584f762 to
474975d
Compare
Contributor
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Suppressed comments (3)
libs/ledger-live-common/src/wallet-api/Exchange/uiRequests.ts:54
SwapUiRequestattempts to makeprovideroptional, but intersecting withCompleteExchangeUiRequest(whereprovideris required) will keepproviderrequired in TypeScript. If callers expectproviderto be optional (as suggested by usages likeexchangeParams.provider ?? ...), redefineSwapUiRequestasOmit<CompleteExchangeUiRequest, "provider"> & { provider?: string; ... }(or remove the optional override if it must be required everywhere) so the type matches runtime expectations.
export type SwapUiRequest = CompleteExchangeUiRequest & {
provider?: string;
fromAccountId?: string;
toAccountId?: string;
tokenCurrency?: string;
correlationId?: string;
};
libs/ledger-live-common/src/wallet-api/Exchange/executeSwap.ts:235
- Inside an
asyncfunction,return Promise.reject(new Error(...))is redundant and slightly harms readability/stack traces compared tothrow new Error(...). Prefer throwing the error directly here to keep control flow consistent with the rest of the function’s error handling.
if (transaction.family !== mainFromAccount.currency.family) {
return Promise.reject(
new Error(
`Account and transaction must be from the same family. Account family: ${mainFromAccount.currency.family}, Transaction family: ${transaction.family}`,
),
);
}
apps/ledger-live-desktop/src/mvvm/features/Perps/screens/PerpsDepositSign/components/PerpsDepositSignTerms.tsx:13
- This renders an interactive control as a
<span>withonClick, which is not keyboard-accessible by default and lacks link semantics for assistive tech. Prefer rendering an<a>(or<button>) with proper semantics (and keyboard support) so users can activate the terms link using keyboard navigation and screen readers (e.g., by using an<a href=...>withonClick/preventDefaultif needed).
components={{
termsLink: (
<span className="cursor-pointer underline" onClick={() => openURL(SWAPKIT_TERMS_URL)} />
),
}}
Comment on lines
+418
to
+429
| if (params.exchangeType === "SWAP" && params.toAccountId) { | ||
| const realToAccountId = getAccountIdFromWalletAccountId(params.toAccountId); | ||
| if (!realToAccountId) { | ||
| throw new ExchangeError(createAccounIdNotFound(params.toAccountId)); | ||
| } | ||
|
|
||
| toAccount = accounts.find(a => a.id === realToAccountId); | ||
|
|
||
| if (!toAccount) { | ||
| throw new ServerError(createAccountNotFound(params.toAccountId)); | ||
| } | ||
| } |
| const currency = params.tokenCurrency | ||
| ? await getCryptoAssetsStore().findTokenById(params.tokenCurrency) | ||
| : null; | ||
| const newTokenAccount = currency ? makeEmptyTokenAccount(toAccount, currency) : null; |
| fromCurrency: getCurrencyForAccount(fromAccount), | ||
| toAccount: newTokenAccount ? newTokenAccount : toAccount, | ||
| toParentAccount: toParentAccount, | ||
| toCurrency: getCurrencyForAccount(newTokenAccount ? newTokenAccount : toAccount), |
ooke-ledger
marked this pull request as draft
August 19, 2026 09:18
|
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
Adds the signing step of the Perps deposit flow - the screen reached from the
review, where the user confirms the deposit on device.
The swap orchestration that already backs the
custom.exchange.swaphandler isextracted out of
server.tsinto a sharedexecuteSwap, so the perps screensdrive the exact same device flow (partner key, payload, payout/refund address
checks, sign, broadcast) instead of a second implementation of it. The perps UI
stays perps: only the orchestration is shared.
The deposit executes against the quote the review screen priced against, so the
amount signed on device is the amount the user was shown.
🔗 Context
https://ledgerhq.atlassian.net/browse/LIVE-35328
Stacked on #20441 (LIVE-35327, the review step).
✅ Checklist
desktop and mobile.