Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feat/LIVE-12193 track swap cancel accept #6799

Merged
merged 15 commits into from
May 13, 2024
Merged

Feat/LIVE-12193 track swap cancel accept #6799

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
cf2cca3
feat: send Swap Metadata information to swap live app
andreicovaciu Apr 25, 2024
349e1e3
feat: adds custom.device.get as CustomHandler
andreicovaciu May 1, 2024
9f45615
feat: removes unused props
andreicovaciu May 2, 2024
d16cad5
feat: fix lint
andreicovaciu May 2, 2024
2e26f0d
feat: updates mobile custom handlers
andreicovaciu May 2, 2024
b099b70
feat: add changeset
andreicovaciu May 3, 2024
3cbcf90
feat: updates based on review
andreicovaciu May 6, 2024
4e4e276
feat: updates based on review
andreicovaciu May 8, 2024
ffdc09b
feat: updates based on review
andreicovaciu May 8, 2024
6c8cd44
feat: fix types
andreicovaciu May 8, 2024
d68fbea
feat: fix mobile implementation
andreicovaciu May 8, 2024
b4543d1
feat: fix mobile onResult types
andreicovaciu May 8, 2024
32560e2
feat: fix PlatformAPIWebview
andreicovaciu May 8, 2024
8fca0f8
feat: fix lint
andreicovaciu May 8, 2024
11c839e
feat: infer types
andreicovaciu May 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions .changeset/swift-pigs-kneel.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"ledger-live-desktop": minor
"live-mobile": minor
"@ledgerhq/live-common": minor
"@ledgerhq/wallet-api-exchange-module": minor
---

Track swap cancel and accept with by returning device property from custom.exchange.start handler
8 changes: 5 additions & 3 deletions apps/ledger-live-desktop/src/renderer/components/WebPTXPlayer/CustomHandlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,13 @@ import { closePlatformAppDrawer, openExchangeDrawer } from "~/renderer/actions/U
import { WebviewProps } from "../Web3AppWebview/types";
import { context } from "~/renderer/drawers/Provider";
import WebviewErrorDrawer from "~/renderer/screens/exchange/Swap2/Form/WebviewErrorDrawer";
import { getCurrentDevice } from "~/renderer/reducers/devices";

export function usePTXCustomHandlers(manifest: WebviewProps["manifest"]) {
const dispatch = useDispatch();
const accounts = useSelector(flattenAccountsSelector);
const { setDrawer } = React.useContext(context);
const device = useSelector(getCurrentDevice);

const tracking = useMemo(
() =>
Expand Down Expand Up @@ -57,10 +59,10 @@ export function usePTXCustomHandlers(manifest: WebviewProps["manifest"]) {
...exchangeParams,
exchangeType: ExchangeType[exchangeParams.exchangeType],
onResult: (nonce: string) => {
onSuccess(nonce);
onSuccess(nonce, device);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how much it keeps the device in sync here because it will be hoisted when the custom.exchange.start is called it could be the old value

It would probably be better if onResult and onCancel could return the device as it's done for LLM
I checked the code and it seems pretty straightforward to add some data:

Another solution could be to simply use const device = useSelector(getCurrentDevice); in apps/ledger-live-desktop/src/renderer/components/LiveAppDrawer.tsx because if we do the changes above we might want to update LLM so result.device comes from the action and not a state.

You can also cleanup this types that are not needed anymore
Screenshot 2024-05-06 at 16 08 00

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel this is a very deep understanding of the code and product to produce such detailed comment. Do you mind sharing any piece of documentation for future use cases? Thank you, @Justkant! 🙌🏽
P.S. I'll update accordingly

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey not sure we have a lot of documentation on the device actions and exchange part to be honest
I'm pretty familiar with this because I worked on some issues around it

My main concern here is to avoid having to much differences between LLD and LLM and sharing as much as possible, especially because you have a lot of ways to pass the data in the end correctly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

},
onCancel: (error: Error) => {
onCancel(error);
onCancel(error, device);
},
}),
);
Expand Down Expand Up @@ -88,5 +90,5 @@ export function usePTXCustomHandlers(manifest: WebviewProps["manifest"]) {
},
}),
};
}, [accounts, dispatch, manifest, tracking, setDrawer]);
}, [accounts, tracking, manifest, device, dispatch, setDrawer]);
}
4 changes: 2 additions & 2 deletions apps/ledger-live-mobile/src/components/WebPTXPlayer/CustomHandlers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ export function usePTXCustomHandlers(manifest: WebviewProps["manifest"]) {
},
onResult: result => {
if (result.startExchangeError) {
onCancel(result.startExchangeError);
onCancel(result.startExchangeError, result.device);
}

if (result.startExchangeResult) {
setDevice(result.device);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to look at this result.device later on to see how we can remove this and instead use result.startExchangeResult.device

onSuccess(result.startExchangeResult);
onSuccess(result.startExchangeResult, result.device);
}

navigation.pop();
Expand Down
5 changes: 4 additions & 1 deletion libs/exchange-module/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@ export class ExchangeModule extends CustomModule {
},
);

return result.transactionId;
return {
transactionId: result.transactionId,
device: result.device,
};
}

/**
Expand Down
1 change: 1 addition & 0 deletions libs/exchange-module/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export type ExchangeStartSwapParams = {

export type ExchangeStartResult = {
transactionId: string;
device?: { deviceId?: string; modelId?: string };
};

export type ExchangeCompleteBaseParams = {
Expand Down
5 changes: 3 additions & 2 deletions libs/ledger-live-common/src/wallet-api/Exchange/server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {

export { ExchangeType };
import { BigNumber } from "bignumber.js";
import { Device } from "../../hw/actions/types";
andreicovaciu marked this conversation as resolved.
Show resolved Hide resolved

type Handlers = {
"custom.exchange.start": RPCHandler<
Expand Down Expand Up @@ -82,8 +83,8 @@ type ExchangeStartParamsUiRequest =
type ExchangeUiHooks = {
"custom.exchange.start": (params: {
exchangeParams: ExchangeStartParamsUiRequest;
onSuccess: (nonce: string) => void;
onCancel: (error: Error) => void;
onSuccess: (nonce: string, device?: Device | null) => void;
onCancel: (error: Error, device?: Device | null) => void;
andreicovaciu marked this conversation as resolved.
Show resolved Hide resolved
}) => void;
"custom.exchange.complete": (params: {
exchangeParams: CompleteExchangeUiRequest;
Expand Down