From 85f3a2d8d863bb7d02775856388b89906f2f5b76 Mon Sep 17 00:00:00 2001 From: Izumi Hoshino Date: Fri, 13 Sep 2024 00:57:40 +0900 Subject: [PATCH] Fixed GUI crash on invalid walletconnect request (#2476) * Fixed GUI crash on invalid walletconnect request * Updated test --- package-lock.json | 1 + packages/gui/package.json | 1 + .../WalletConnectConfirmDialog.tsx | 151 ++++++++++++++---- ...reateOfferForIdsToOfferBuilderData.test.ts | 15 +- .../createOfferForIdsToOfferBuilderData.ts | 5 +- 5 files changed, 136 insertions(+), 37 deletions(-) diff --git a/package-lock.json b/package-lock.json index c5e83b25b1..35d2be7448 100644 --- a/package-lock.json +++ b/package-lock.json @@ -28753,6 +28753,7 @@ "redux": "4.2.1", "regenerator-runtime": "0.14.0", "seedrandom": "3.0.5", + "stacktrace-js": "2.0.2", "stream-browserify": "3.0.0", "styled-components": "6.0.7", "unique-names-generator": "4.6.0", diff --git a/packages/gui/package.json b/packages/gui/package.json index f59e702b9a..e7e8bcdcbc 100644 --- a/packages/gui/package.json +++ b/packages/gui/package.json @@ -92,6 +92,7 @@ "redux": "4.2.1", "regenerator-runtime": "0.14.0", "seedrandom": "3.0.5", + "stacktrace-js": "2.0.2", "stream-browserify": "3.0.0", "styled-components": "6.0.7", "unique-names-generator": "4.6.0", diff --git a/packages/gui/src/components/walletConnect/WalletConnectConfirmDialog.tsx b/packages/gui/src/components/walletConnect/WalletConnectConfirmDialog.tsx index 92f4b4e1f9..3a5eea029d 100644 --- a/packages/gui/src/components/walletConnect/WalletConnectConfirmDialog.tsx +++ b/packages/gui/src/components/walletConnect/WalletConnectConfirmDialog.tsx @@ -1,14 +1,55 @@ import { useGetKeysQuery } from '@chia-network/api-react'; -import { ConfirmDialog, Flex, LoadingOverlay } from '@chia-network/core'; +import { AlertDialog, ConfirmDialog, Flex, LoadingOverlay } from '@chia-network/core'; import { Trans } from '@lingui/macro'; import { Typography, Divider, Switch } from '@mui/material'; -import React, { type ReactNode, useState, useMemo } from 'react'; +import { styled } from '@mui/styles'; +import React, { type ReactNode, useState, useMemo, useCallback } from 'react'; +import StackTrace, { type StackFrame } from 'stacktrace-js'; import type WalletConnectCommandParam from '../../@types/WalletConnectCommandParam'; import useWalletConnectPairs from '../../hooks/useWalletConnectPairs'; import WalletConnectMetadata from './WalletConnectMetadata'; +const StyledPre = styled(Typography)(() => ({ + whiteSpace: 'pre-wrap', +})); + +function formatStackTrace(stack: StackFrame[]) { + const stackTrace = stack.map( + ({ fileName, columnNumber, lineNumber, functionName }) => + `at ${fileName}:${lineNumber}:${columnNumber} ${functionName}`, + ); + return stackTrace.join('\n'); +} + +type LocalErrorBoundaryProps = React.PropsWithChildren<{ + fallback?: ReactNode; + onError: (error: Error, stacktrace: string) => void; +}>; +type LocalErrorBoundaryState = { error: false | Error }; +class LocalErrorBoundary extends React.Component { + constructor(props: LocalErrorBoundaryProps) { + super(props); + this.state = { error: false }; + } + + static getDerivedStateFromError(error: Error) { + return { error }; + } + + async componentDidCatch(error: Error) { + this.props.onError(error, formatStackTrace(await StackTrace.fromError(error))); + } + + render() { + if (this.state.error) { + return this.props.fallback; + } + return this.props.children; + } +} + export type WalletConnectConfirmDialogProps = { topic: string; command: string; @@ -39,6 +80,7 @@ export default function WalletConnectConfirmDialog(props: WalletConnectConfirmDi } = props; const [values, setValues] = useState(defaultValues); + const [error, setError] = useState<{ e: Error; stacktrace: string } | false>(false); const { getPairBySession, bypassCommand } = useWalletConnectPairs(); const { data: keys, isLoading: isLoadingPublicKeys } = useGetKeysQuery(); const key = keys?.find((item) => item.fingerprint === fingerprint); @@ -47,21 +89,80 @@ export default function WalletConnectConfirmDialog(props: WalletConnectConfirmDi const pair = useMemo(() => getPairBySession(topic), [topic, getPairBySession]); - function handleRememberChoiceChanged(_: any, checked: boolean) { + const handleRememberChoiceChanged = useCallback((_: any, checked: boolean) => { setRememberChoice(checked); - } + }, []); - function handleClose(confirmed: boolean) { - if (rememberChoice) { - bypassCommand(topic, command, confirmed); - } + const onCloseConfirmDialog = useCallback( + (confirmed: boolean) => { + if (rememberChoice) { + bypassCommand(topic, command, confirmed); + } - onClose?.(confirmed); - } + onClose?.(confirmed); + }, + [rememberChoice, bypassCommand, onClose, topic, command], + ); - function handleChangeValues(newValues: Record) { - setValues(newValues); - onChange?.(newValues); + const onCloseAlertDialog = useCallback(() => { + setError(false); + onClose?.(false); + }, [onClose]); + + const handleChangeValues = useCallback( + (newValues: Record) => { + setValues(newValues); + onChange?.(newValues); + }, + [onChange], + ); + + const onError = useCallback((err: Error, stacktrace: string) => { + setError({ e: err, stacktrace }); + }, []); + + const paramRows = useMemo(() => { + if (params.length === 0) { + return null; + } + return ( + + {params.map(({ label, name, hide, displayComponent }) => { + if (hide || !(name in values)) { + return null; + } + + const value = values[name]; + return ( + + + {label ?? name} + + {displayComponent + ? displayComponent(value, params, values, handleChangeValues) + : value?.toString() ?? Not Available} + + + + ); + })} + + ); + }, [params, values, handleChangeValues, onError]); + + if (error) { + return ( + Invalid WalletConnect data} onClose={onCloseAlertDialog} open={open}> + + + + Error: {error.e.message} + + {error.stacktrace} + + + + ); } return ( @@ -70,34 +171,14 @@ export default function WalletConnectConfirmDialog(props: WalletConnectConfirmDi confirmColor="primary" confirmTitle={Confirm} cancelTitle={Reject} - onClose={handleClose} + onClose={onCloseConfirmDialog} open={open} > {message} - {params.length > 0 && ( - - {params.map(({ label, name, hide, displayComponent }) => { - if (hide || !(name in values)) { - return null; - } - - const value = values[name]; - return ( - - {label ?? name} - - {displayComponent - ? displayComponent(value, params, values, handleChangeValues) - : value?.toString() ?? Not Available} - - - ); - })} - - )} + {paramRows} diff --git a/packages/gui/src/util/createOfferForIdsToOfferBuilderData.test.ts b/packages/gui/src/util/createOfferForIdsToOfferBuilderData.test.ts index 84c27b9a5a..aa8c7db509 100644 --- a/packages/gui/src/util/createOfferForIdsToOfferBuilderData.test.ts +++ b/packages/gui/src/util/createOfferForIdsToOfferBuilderData.test.ts @@ -207,7 +207,20 @@ describe('createOfferForIdsToOfferBuilderData', () => { // call to createOfferForIdsToOfferBuilderData should throw an error expect(() => createOfferForIdsToOfferBuilderData(walletIdsAndAmounts as any, (() => {}) as any)).toThrow( - /Invalid value for/, + /^Invalid amount /, + ); + }); + }); + describe('when the amount is null', () => { + it('should throw an error', () => { + const walletIdsAndAmounts = { + 1: null, + 2: -1234, + }; + + // call to createOfferForIdsToOfferBuilderData should throw an error + expect(() => createOfferForIdsToOfferBuilderData(walletIdsAndAmounts as any, (() => {}) as any)).toThrow( + /^Amount is not set for /, ); }); }); diff --git a/packages/gui/src/util/createOfferForIdsToOfferBuilderData.ts b/packages/gui/src/util/createOfferForIdsToOfferBuilderData.ts index 175aaee400..e8ef6e1229 100644 --- a/packages/gui/src/util/createOfferForIdsToOfferBuilderData.ts +++ b/packages/gui/src/util/createOfferForIdsToOfferBuilderData.ts @@ -16,7 +16,10 @@ export default function createOfferForIdsToOfferBuilderData( const numericValue = new BigNumber(amount); if (numericValue.isNaN()) { - throw new Error(`Invalid value for ${walletOrAssetId}: ${amount}`); + if (amount === null) { + throw new Error(`Amount is not set for walletId(assetId):${walletOrAssetId}`); + } + throw new Error(`Invalid amount '${amount}' for walletId(assetId):${walletOrAssetId}`); } const section = numericValue.isPositive() ? offerBuilderData.requested : offerBuilderData.offered;