Skip to content

Commit

Permalink
Merge pull request #4800 from LedgerHQ/bugfix/onclose-LIVE-9487
Browse files Browse the repository at this point in the history
fix(LLM): platform-sdk & wallet-api onClose response in `account.request` & `message.sign` [LIVE-9487]
  • Loading branch information
Justkant authored Sep 22, 2023
2 parents bfb181f + e63d3bb commit acdb1d1
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 74 deletions.
7 changes: 7 additions & 0 deletions .changeset/thick-icons-pull.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"ledger-live-desktop": patch
"live-mobile": patch
"@ledgerhq/live-common": patch
---

fix: platform-sdk & wallet-api onClose response in `account.request` & `message.sign`
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ export const PlatformAPIWebview = forwardRef<WebviewAPI, WebviewProps>(
},
onClose: () => {
tracking.platformSignMessageUserRefused(manifest);
reject(UserRefusedOnDevice());
reject(new UserRefusedOnDevice());
},
}),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ function useUiHook(manifest: AppManifest): Partial<UiHook> {

return useMemo(
() => ({
"account.request": ({ accounts$, currencies, onSuccess, onError }) => {
"account.request": ({ accounts$, currencies, onSuccess, onCancel }) => {
setDrawer(
SelectAccountAndCurrencyDrawer,
{
Expand All @@ -55,7 +55,7 @@ function useUiHook(manifest: AppManifest): Partial<UiHook> {
{
onRequestClose: () => {
setDrawer();
onError();
onCancel();
},
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,14 @@ export default function BaseNavigator() {
name={NavigatorName.SignMessage}
component={SignMessageNavigator}
options={{ headerShown: false }}
listeners={({ route }) => ({
beforeRemove: () => {
const onClose = route.params?.onClose;
if (onClose && typeof onClose === "function") {
onClose();
}
},
})}
/>
<Stack.Screen
name={NavigatorName.SignTransaction}
Expand Down Expand Up @@ -255,17 +263,10 @@ export default function BaseNavigator() {
}}
listeners={({ route }) => ({
beforeRemove: () => {
/**
react-navigation workaround try to fetch params from current route params
or fallback to child navigator route params
since this listener is on top of another navigator
*/
const onError =
route.params?.onError || (route.params as unknown as typeof route)?.params?.onError;
// @TODO This route.params.error mechanism is deprecated, no part of the code is currently using it
// WalletAPI are suggesting they will rework that at some point
if (onError && typeof onError === "function" && route.params.error)
onError(route.params.error);
const onClose = route.params?.onClose;
if (onClose && typeof onClose === "function") {
onClose();
}
},
})}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,13 @@ export type AddAccountsNavigatorParamList = {
returnToSwap?: boolean;
analyticsPropertyFlow?: string;
onSuccess?: () => void;
onError?: (_: Error) => void;
};
[ScreenName.AddAccountsAccounts]: {
currency: CryptoOrTokenCurrency;
device: Device;
inline?: boolean;
returnToSwap?: boolean;
onSuccess?: (_?: unknown) => void;
onError?: (_: Error) => void;
};
[ScreenName.AddAccountsSuccess]?: {
currency: CryptoOrTokenCurrency;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,7 @@ export type BaseNavigatorStackParamList = {
})
| undefined;
[NavigatorName.RequestAccount]: NavigatorScreenParams<RequestAccountNavigatorParamList> & {
onError?: (_: Error) => void;
error?: Error;
onClose?: () => void;
};
[NavigatorName.Exchange]: NavigatorScreenParams<ExchangeLiveAppNavigatorParamList> | undefined;
[NavigatorName.ExchangeStack]: NavigatorScreenParams<ExchangeStackNavigatorParamList> & {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,13 @@ export type RequestAccountNavigatorParamList = {
currencies: CryptoOrTokenCurrency[];
allowAddAccount?: boolean;
onSuccess?: (account: AccountLike, parentAccount?: Account) => void;
onError?: (_: Error) => void;
};
[ScreenName.RequestAccountsSelectAccount]: {
accounts$?: Observable<WalletAPIAccount[]>;
currencies?: CryptoOrTokenCurrency[];
currency: CryptoOrTokenCurrency;
allowAddAccount?: boolean;
onSuccess?: (account: AccountLike, parentAccount?: Account) => void;
onError?: (_: Error) => void;
};
[NavigatorName.RequestAccountsAddAccounts]: NavigatorScreenParams<AddAccountsNavigatorParamList> &
Partial<{
Expand All @@ -31,6 +29,5 @@ export type RequestAccountNavigatorParamList = {
returnToSwap?: boolean;
analyticsPropertyFlow?: string;
onSuccess?: (account: AccountLike, parentAccount?: Account) => void;
onError?: (_: Error) => void;
}>;
};
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,9 @@ export const PlatformAPIWebview = forwardRef<WebviewAPI, WebviewProps>(
resolve(serializePlatformAccount(accountToPlatformAccount(account, parentAccount)));
};

const onError = (error: Error) => {
const onClose = () => {
tracking.platformRequestAccountFail(manifest);
reject(error);
reject(new Error("User cancelled"));
};

// if single currency available redirect to select account directly
Expand All @@ -157,8 +157,8 @@ export const PlatformAPIWebview = forwardRef<WebviewAPI, WebviewProps>(
currency,
allowAddAccount,
onSuccess,
onError,
},
onClose,
});
} else {
navigation.navigate(NavigatorName.RequestAccount, {
Expand All @@ -167,8 +167,8 @@ export const PlatformAPIWebview = forwardRef<WebviewAPI, WebviewProps>(
currencies: allCurrencies,
allowAddAccount,
onSuccess,
onError,
},
onClose,
});
}
}),
Expand Down Expand Up @@ -233,44 +233,41 @@ export const PlatformAPIWebview = forwardRef<WebviewAPI, WebviewProps>(
);

return new Promise((resolve, reject) => {
(navigation as StackNavigatorNavigation<BaseNavigatorStackParamList>).navigate(
NavigatorName.SignTransaction,
{
screen: ScreenName.SignTransactionSummary,
params: {
currentNavigation: ScreenName.SignTransactionSummary,
nextNavigation: ScreenName.SignTransactionSelectDevice,
transaction: tx as Transaction,
accountId,
parentId: parentAccount?.id,
appName: params?.useApp,
onSuccess: ({
signedOperation,
transactionSignError,
}: {
signedOperation: SignedOperation;
transactionSignError: Error;
}) => {
if (transactionSignError) {
tracking.platformSignTransactionFail(manifest);
reject(transactionSignError);
} else {
tracking.platformSignTransactionSuccess(manifest);
resolve(serializePlatformSignedTransaction(signedOperation));
const n =
navigation.getParent<
StackNavigatorNavigation<BaseNavigatorStackParamList>
>() || navigation;
n.pop();
}
},
onError: (error: Error) => {
navigation.navigate(NavigatorName.SignTransaction, {
screen: ScreenName.SignTransactionSummary,
params: {
currentNavigation: ScreenName.SignTransactionSummary,
nextNavigation: ScreenName.SignTransactionSelectDevice,
transaction: tx as Transaction,
accountId,
parentId: parentAccount?.id,
appName: params?.useApp,
onSuccess: ({
signedOperation,
transactionSignError,
}: {
signedOperation: SignedOperation;
transactionSignError: Error;
}) => {
if (transactionSignError) {
tracking.platformSignTransactionFail(manifest);
reject(error);
},
reject(transactionSignError);
} else {
tracking.platformSignTransactionSuccess(manifest);
resolve(serializePlatformSignedTransaction(signedOperation));
const n =
navigation.getParent<
StackNavigatorNavigation<BaseNavigatorStackParamList>
>() || navigation;
n.pop();
}
},
onError: (error: Error) => {
tracking.platformSignTransactionFail(manifest);
reject(error);
},
},
);
});
});
},
),
Expand Down Expand Up @@ -435,7 +432,7 @@ export const PlatformAPIWebview = forwardRef<WebviewAPI, WebviewProps>(
},
onClose: () => {
tracking.platformSignMessageUserRefused(manifest);
reject(UserRefusedOnDevice());
reject(new UserRefusedOnDevice());
},
});
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ function useUiHook(): Partial<UiHook> {

return useMemo(
() => ({
"account.request": ({ accounts$, currencies, onSuccess, onError }) => {
"account.request": ({ accounts$, currencies, onSuccess, onCancel }) => {
if (currencies.length === 1) {
navigation.navigate(NavigatorName.RequestAccount, {
screen: ScreenName.RequestAccountsSelectAccount,
Expand All @@ -260,8 +260,8 @@ function useUiHook(): Partial<UiHook> {
currency: currencies[0],
allowAddAccount: true,
onSuccess,
onError,
},
onClose: onCancel,
});
} else {
navigation.navigate(NavigatorName.RequestAccount, {
Expand All @@ -271,8 +271,8 @@ function useUiHook(): Partial<UiHook> {
currencies,
allowAddAccount: true,
onSuccess,
onError,
},
onClose: onCancel,
});
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ const List = ({

function SelectAccount({ navigation, route }: Props) {
const { colors } = useTheme();
const { accounts$, currency, allowAddAccount, onSuccess, onError } = route.params;
const { accounts$, currency, allowAddAccount, onSuccess } = route.params;
const accountIds = useGetAccountIds(accounts$);
const accounts = useSelector(accountsByCryptoCurrencyScreenSelector(currency, accountIds)) as {
account: AccountLike;
Expand All @@ -122,15 +122,10 @@ function SelectAccount({ navigation, route }: Props) {
screen: ScreenName.AddAccountsSelectDevice,
params: {
currency: currency as CryptoOrTokenCurrency,
onSuccess: () =>
navigation.navigate(NavigatorName.RequestAccount, {
screen: ScreenName.RequestAccountsSelectAccount,
params: route.params,
}),
onError,
onSuccess: () => navigation.navigate(ScreenName.RequestAccountsSelectAccount, route.params),
},
});
}, [currency, navigation, onError, route.params]);
}, [currency, navigation, route.params]);

const renderFooter = useCallback(
() =>
Expand Down
4 changes: 2 additions & 2 deletions libs/ledger-live-common/src/wallet-api/react.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ export interface UiHook {
accounts$: Observable<WalletAPIAccount[]>;
currencies: CryptoOrTokenCurrency[];
onSuccess: (account: AccountLike, parentAccount: Account | undefined) => void;
onError: () => void;
onCancel: () => void;
}) => void;
"account.receive": (params: {
account: AccountLike;
Expand Down Expand Up @@ -354,7 +354,7 @@ export function useWalletAPIServer({
tracking.requestAccountSuccess(manifest);
resolve(accountToWalletAPIAccount(account, parentAccount));
},
onError: () => {
onCancel: () => {
tracking.requestAccountFail(manifest);
reject(new Error("Canceled by user"));
},
Expand Down

1 comment on commit acdb1d1

@vercel
Copy link

@vercel vercel bot commented on acdb1d1 Sep 22, 2023

Choose a reason for hiding this comment

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

Please sign in to comment.