-
Notifications
You must be signed in to change notification settings - Fork 246
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.
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
Rifeljm/#622 walletconnect bypass readonly commands #2055
base: main
Are you sure you want to change the base?
Changes from 2 commits
7fe7391
7e8243e
4e33099
32cabb7
cb438b6
4f5ab53
d50221a
6963175
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,3 +1,4 @@ | ||||||
import { useLocalStorage, useGetLoggedInFingerprintQuery } from '@chia-network/api-react'; | ||||||
import { Flex, MenuItem, SettingsHR, SettingsSection, SettingsTitle, SettingsText } from '@chia-network/core'; | ||||||
import { t, Trans } from '@lingui/macro'; | ||||||
import { | ||||||
|
@@ -45,6 +46,9 @@ export default function SettingsIntegration() { | |||||
useWalletConnectPairs(); | ||||||
|
||||||
const pairs = get(); | ||||||
const { data: fingerprint } = useGetLoggedInFingerprintQuery(); | ||||||
|
||||||
const [bypassReadonlyCommands, setBypassReadonlyCommands] = useLocalStorage<any>('bypass-readonly-commands', {}); | ||||||
|
||||||
const refreshBypassCommands = React.useCallback(() => { | ||||||
if (selectedPair.current) { | ||||||
|
@@ -131,6 +135,9 @@ export default function SettingsIntegration() { | |||||
} | ||||||
}, [topic, pairs, refreshBypassCommands]); | ||||||
|
||||||
const readonlySkipValue = | ||||||
bypassReadonlyCommands && topic && fingerprint ? bypassReadonlyCommands[topic][fingerprint] : false; | ||||||
paninaro marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
|
||||||
return ( | ||||||
<Grid container style={{ maxWidth: '624px' }} gap={3}> | ||||||
<Grid item> | ||||||
|
@@ -334,6 +341,31 @@ export default function SettingsIntegration() { | |||||
</Box> | ||||||
)} | ||||||
</Flex> | ||||||
<Box {...borderStyle}> | ||||||
<Typography variant="h6"> | ||||||
<Trans>Skip Confirmation For All Readonly Commands</Trans> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||||||
<Grid item container xs marginTop="20px" marginRight="8px"> | ||||||
<FormControl size="small"> | ||||||
<Select | ||||||
value={readonlySkipValue} | ||||||
onChange={(evt: PointerEvent) => { | ||||||
setBypassReadonlyCommands((localBypassReadonlyCommands) => ({ | ||||||
...localBypassReadonlyCommands, | ||||||
[topic]: { [fingerprint]: evt.target.value }, | ||||||
})); | ||||||
}} | ||||||
> | ||||||
<MenuItem value> | ||||||
<Trans>Always Skip</Trans> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What do you think about using "Always Allow" instead of "Always Skip"?
Suggested change
|
||||||
</MenuItem> | ||||||
<MenuItem value={false} onClick={() => {}}> | ||||||
<Trans>Require Confirmation</Trans> | ||||||
</MenuItem> | ||||||
</Select> | ||||||
</FormControl> | ||||||
</Grid> | ||||||
</Typography> | ||||||
</Box> | ||||||
|
||||||
{topic && commands.length > 0 && ( | ||||||
<> | ||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,4 +1,4 @@ | ||||||||||||||||||||||||||||||||||||
import { useGetKeysQuery, useGetLoggedInFingerprintQuery, usePrefs } from '@chia-network/api-react'; | ||||||||||||||||||||||||||||||||||||
import { useGetKeysQuery, useGetLoggedInFingerprintQuery, usePrefs, useLocalStorage } from '@chia-network/api-react'; | ||||||||||||||||||||||||||||||||||||
import { | ||||||||||||||||||||||||||||||||||||
ButtonLoading, | ||||||||||||||||||||||||||||||||||||
DialogActions, | ||||||||||||||||||||||||||||||||||||
|
@@ -51,6 +51,8 @@ export default function WalletConnectAddConnectionDialog(props: WalletConnectAdd | |||||||||||||||||||||||||||||||||||
const { onClose = () => {}, open = false } = props; | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const [step, setStep] = useState<Step>(Step.CONNECT); | ||||||||||||||||||||||||||||||||||||
const [bypassReadonlyCommands, setBypassReadonlyCommands] = useLocalStorage<any>('bypass-readonly-commands', {}); | ||||||||||||||||||||||||||||||||||||
const [bypassCheckbox, toggleBypassCheckbox] = useState(false); | ||||||||||||||||||||||||||||||||||||
const { pair, isLoading: isLoadingWallet } = useWalletConnectContext(); | ||||||||||||||||||||||||||||||||||||
const { data: keys, isLoading: isLoadingPublicKeys } = useGetKeysQuery({}); | ||||||||||||||||||||||||||||||||||||
const [sortedWallets] = usePrefs('sortedWallets', []); | ||||||||||||||||||||||||||||||||||||
|
@@ -118,6 +120,13 @@ export default function WalletConnectAddConnectionDialog(props: WalletConnectAdd | |||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
const topic = await pair(uri, selectedFingerprints, mainnet); | ||||||||||||||||||||||||||||||||||||
if (bypassCheckbox) { | ||||||||||||||||||||||||||||||||||||
let skipReadOnlyObject = {}; | ||||||||||||||||||||||||||||||||||||
selectedFingerprints.forEach((f: number) => { | ||||||||||||||||||||||||||||||||||||
skipReadOnlyObject = { ...skipReadOnlyObject, [f]: true }; | ||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||
setBypassReadonlyCommands({ ...bypassReadonlyCommands, [topic.toString()]: skipReadOnlyObject }); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems odd to create a nested data structure to hold a boolean. The data structure could be simplified to:
If the fingerprint is in the list, bypass confirmation is enabled. If it's not, then confirmation is required. |
||||||||||||||||||||||||||||||||||||
onClose(topic); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
|
@@ -212,6 +221,23 @@ export default function WalletConnectAddConnectionDialog(props: WalletConnectAdd | |||||||||||||||||||||||||||||||||||
return null; | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
function renderQuietModeOption() { | ||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||
<Flex | ||||||||||||||||||||||||||||||||||||
sx={{ cursor: 'pointer' }} | ||||||||||||||||||||||||||||||||||||
alignItems="center" | ||||||||||||||||||||||||||||||||||||
onClick={() => { | ||||||||||||||||||||||||||||||||||||
toggleBypassCheckbox(!bypassCheckbox); | ||||||||||||||||||||||||||||||||||||
}} | ||||||||||||||||||||||||||||||||||||
> | ||||||||||||||||||||||||||||||||||||
<Checkbox checked={bypassCheckbox} disableRipple sx={{ paddingLeft: 0 }} /> | ||||||||||||||||||||||||||||||||||||
<Typography variant="body2"> | ||||||||||||||||||||||||||||||||||||
<Trans>Bypass confirmation for all read-only commands</Trans> | ||||||||||||||||||||||||||||||||||||
</Typography> | ||||||||||||||||||||||||||||||||||||
</Flex> | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tweak wording and add an informative tooltip on hover.
Suggested change
|
||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||
<Dialog onClose={handleClose} maxWidth="xs" open={open} fullWidth> | ||||||||||||||||||||||||||||||||||||
<DialogTitle> | ||||||||||||||||||||||||||||||||||||
|
@@ -259,6 +285,7 @@ export default function WalletConnectAddConnectionDialog(props: WalletConnectAdd | |||||||||||||||||||||||||||||||||||
{renderSelectedAsPills()} | ||||||||||||||||||||||||||||||||||||
</Flex> | ||||||||||||||||||||||||||||||||||||
{renderKeysMultiSelect()} | ||||||||||||||||||||||||||||||||||||
{renderQuietModeOption()} | ||||||||||||||||||||||||||||||||||||
</Flex> | ||||||||||||||||||||||||||||||||||||
)} | ||||||||||||||||||||||||||||||||||||
</Flex> | ||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import api, { store, useGetLoggedInFingerprintQuery } from '@chia-network/api-react'; | ||
import api, { store, useGetLoggedInFingerprintQuery, useLocalStorage } from '@chia-network/api-react'; | ||
import { useOpenDialog, useAuth } from '@chia-network/core'; | ||
import { Trans } from '@lingui/macro'; | ||
import debug from 'debug'; | ||
|
@@ -80,6 +80,8 @@ export default function useWalletConnectCommand(options: UseWalletConnectCommand | |
|
||
const isLoading = isLoadingLoggedInFingerprint; | ||
|
||
const [bypassReadonlyCommands] = useLocalStorage<any>('bypass-readonly-commands', {}); | ||
|
||
async function confirm(props: { | ||
topic: string; | ||
message: ReactNode; | ||
|
@@ -139,8 +141,9 @@ export default function useWalletConnectCommand(options: UseWalletConnectCommand | |
|
||
const { fingerprint } = requestedParams; | ||
|
||
const pair = getPairBySession(topic); | ||
const pairedTopic = pair?.topic!; | ||
if (command === 'showNotification') { | ||
const pair = getPairBySession(topic); | ||
if (!pair) { | ||
throw new Error('Invalid session topic'); | ||
} | ||
|
@@ -172,24 +175,28 @@ export default function useWalletConnectCommand(options: UseWalletConnectCommand | |
values = newValues; | ||
} | ||
|
||
const confirmed = await confirm({ | ||
topic, | ||
message: | ||
!allFingerprints && isDifferentFingerprint ? ( | ||
<Trans> | ||
Do you want to log in to {fingerprint} and execute command {command}? | ||
</Trans> | ||
) : ( | ||
<Trans>Do you want to execute command {command}?</Trans> | ||
), | ||
params: definitionParams, | ||
values, | ||
fingerprint, | ||
isDifferentFingerprint, | ||
command, | ||
bypassConfirm, | ||
onChange: handleChangeParam, | ||
}); | ||
const isReadOnlyEnabled = bypassReadonlyCommands?.[pairedTopic]?.[fingerprint]; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we rename this to something like |
||
const confirmed = | ||
(isReadOnlyEnabled && definition.bypassConfirm) || | ||
(await confirm({ | ||
topic, | ||
message: | ||
!allFingerprints && isDifferentFingerprint ? ( | ||
<Trans> | ||
Do you want to log in to {fingerprint} and execute command {command}? | ||
</Trans> | ||
) : ( | ||
<Trans>Do you want to execute command {command}?</Trans> | ||
), | ||
params: definitionParams, | ||
values, | ||
fingerprint, | ||
isDifferentFingerprint, | ||
command, | ||
bypassConfirm, | ||
onChange: handleChangeParam, | ||
})); | ||
|
||
if (!confirmed) { | ||
throw new Error(`User cancelled command ${requestedCommand}`); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason not to use the
useWalletConnectPairs
oruseWalletConnectPreferences
hooks? Creating a new localStorage item is going to make maintenance more complex. At the very least we should make the naming of these localStorage items consistent -- maybewalletConnectBypassReadonlyCommands
. But I would prefer using the existing hooks if possible.