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

[DDW-926] Improve error handling for incorrect passphrase / incorrect hardware wallet device #2860

Merged
merged 34 commits into from
May 12, 2022
Merged

[DDW-926] Improve error handling for incorrect passphrase / incorrect hardware wallet device #2860

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
a49b2b3
[DDW-926] Potential fix for incorrect phrase error handling
Feb 4, 2022
45cc72f
[DDW-926] Potential fix for incorrect phrase error handling
Feb 4, 2022
d2e782f
[DDW-926] Potential fix for incorrect phrase error handling
Feb 4, 2022
d9b2cdb
[DDW-926] Improve translations
Feb 8, 2022
d194e23
[DDW-926] Handle one more case when incorrect passphrase can be provided
Feb 8, 2022
eb8fea0
[DDW-926] Revert unrelated change
Feb 8, 2022
5f48a4f
[DDW-926] Fixes
Feb 8, 2022
d9880ad
Merge remote-tracking branch 'origin/develop' into fix/ddw-926-trezor…
Feb 8, 2022
25938be
[DDW-926] Fill in changelog
Feb 8, 2022
d6883af
Merge branch 'develop' into fix/ddw-926-trezor-error-handling
danielmain Feb 9, 2022
ea549b7
Merge remote-tracking branch 'origin/develop' into fix/ddw-926-trezor…
Feb 10, 2022
22e2834
[DDW-926] Update translations
Feb 11, 2022
f68c61d
[DDW-926] Update translations
Feb 11, 2022
d3027aa
[DDW-926] Update translations
Feb 11, 2022
64db538
Merge remote-tracking branch 'origin/develop' into fix/ddw-926-trezor…
Mar 1, 2022
4e0cc5c
[DDW-926] Add a CLI param to stop reloading Electron on changes in code
Mar 3, 2022
1a30326
[DDW-926] Manage translations
Mar 3, 2022
f39df36
[DDW-926] Clean up getHardwareWalletChannel.ts and add logging for Tr…
Mar 3, 2022
e47cdd8
Merge remote-tracking branch 'origin/develop' into fix/ddw-926-trezor…
Mar 7, 2022
4a562e6
[DDW-926] Manage translations
Mar 7, 2022
4952375
[DDW-926] Remove passing null path param to fix missing Trezor valida…
Mar 10, 2022
bfda600
Merge remote-tracking branch 'origin/develop' into fix/ddw-926-trezor…
Mar 10, 2022
de38049
Merge remote-tracking branch 'origin/develop' into fix/ddw-926-trezor…
Mar 22, 2022
889080d
Revert "[DDW-926] Remove passing null path param to fix missing Trezo…
Mar 22, 2022
a662ac0
[DDW-926] Ensure null is not passed as device path
Mar 22, 2022
5e4659d
[DDW-926] Add logging for changing paths to debug Ledger issues
Mar 24, 2022
2f86d73
[DDW-926] Improve translations
Mar 24, 2022
68fa148
[DDW-926] Apply UNRECOGNIZED_WALLET status for all cases where the cu…
Mar 24, 2022
b65c17b
Merge remote-tracking branch 'origin/develop' into fix/ddw-926-trezor…
Apr 14, 2022
07a541a
[DDW-926] Reinitialize Trezor Connect session in order to let user sw…
Apr 20, 2022
3bafb81
Merge remote-tracking branch 'origin/develop' into fix/ddw-926-trezor…
Apr 21, 2022
e17610d
[DDW-926] Fix translations
Apr 27, 2022
4182c72
Trigger build
Apr 29, 2022
e466617
Merge remote-tracking branch 'origin/develop' into fix/ddw-926-trezor…
May 10, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

### Fixes

- Improved error handling for incorrect passphrase and incorrect hardware wallet error ([PR 2860](https://github.com/input-output-hk/daedalus/pull/2860))
- Fixed behaviour of wallet settings option of the app menu ([PR 2838](https://github.com/input-output-hk/daedalus/pull/2838))
- Fixed styling of ITN rewards feature ([PR 2861](https://github.com/input-output-hk/daedalus/pull/2861))
- Fixed available disk space takes a long time to show ([PR 2849](https://github.com/input-output-hk/daedalus/pull/2849))
Expand Down
1 change: 1 addition & 0 deletions source/renderer/app/components/hardware-wallet/HardwareWalletStatus.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
}

.clearIcon {
flex-shrink: 0;
height: 14px;
margin-right: 6px;
object-fit: contain;
Expand Down
9 changes: 8 additions & 1 deletion source/renderer/app/components/hardware-wallet/HardwareWalletStatus.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ const messages = defineMessages({
defaultMessage: '!!!Exporting the public key failed',
description: '"Exporting public key failed" device state',
},
unrecognized_wallet: {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be camel case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here, I followed the existing pattern where we map the HwDeviceStatus enum values directly to translations. The enum uses snake_case therefore we have this mapping which doesn't follow JavaScript naming conventions. Would you like me to refactor this? I would have to introduce an additional map to map statuses to translation keys.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok, sounds good.

Copy link
Contributor

Choose a reason for hiding this comment

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

How about using lodash's camelCase for mapping enum values to translations? This way we could kill two birds with one stone. (btw it is a funny sentence)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would work, however I'm afraid that it will make it harder to refactor and track which translations are actually in use. Currently you can just find translations by a string, with camelCase function it's not going to be that easy anymore.

id: 'wallet.hardware.deviceStatus.unrecognized_wallet',
defaultMessage:
'!!!Unrecognized hardware wallet. Make sure you are not using a different device or a wrong passphrase.',
description: '"Unrecognized wallet" device state',
},
exportingPublicKeyError: {
id: 'wallet.hardware.deviceStatus.exportingPublicKeyError',
defaultMessage:
Expand Down Expand Up @@ -230,7 +236,8 @@ class HardwareWalletStatus extends Component<Props, State> {
hwDeviceStatus === HwDeviceStatuses.UNSUPPORTED_DEVICE ||
hwDeviceStatus === HwDeviceStatuses.VERIFYING_TRANSACTION_FAILED ||
hwDeviceStatus === HwDeviceStatuses.VERIFYING_ADDRESS_FAILED ||
hwDeviceStatus === HwDeviceStatuses.VERIFYING_ADDRESS_ABORTED;
hwDeviceStatus === HwDeviceStatuses.VERIFYING_ADDRESS_ABORTED ||
hwDeviceStatus === HwDeviceStatuses.UNRECOGNIZED_WALLET;
const hasPassphraseLabel =
hwDeviceStatus === HwDeviceStatuses.EXPORTING_PUBLIC_KEY ||
hwDeviceStatus === HwDeviceStatuses.VERIFYING_TRANSACTION ||
Expand Down
5 changes: 4 additions & 1 deletion source/renderer/app/domains/Wallet.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ export type HwDeviceStatus =
| 'verifying_address_confirmation'
| 'verifying_address_failed'
| 'verifying_address_aborted'
| 'verifying_address_succeeded';
| 'verifying_address_succeeded'
| 'unrecognized_wallet';
export const HwDeviceStatuses: {
CONNECTING: HwDeviceStatus;
CONNECTING_FAILED: HwDeviceStatus;
Expand All @@ -75,6 +76,7 @@ export const HwDeviceStatuses: {
VERIFYING_ADDRESS_FAILED: HwDeviceStatus;
VERIFYING_ADDRESS_ABORTED: HwDeviceStatus;
VERIFYING_ADDRESS_SUCCEEDED: HwDeviceStatus;
UNRECOGNIZED_WALLET: HwDeviceStatus;
} = {
CONNECTING: 'connecting',
CONNECTING_FAILED: 'connecting_failed',
Expand All @@ -94,6 +96,7 @@ export const HwDeviceStatuses: {
VERIFYING_ADDRESS_FAILED: 'verifying_address_failed',
VERIFYING_ADDRESS_ABORTED: 'verifying_address_aborted',
VERIFYING_ADDRESS_SUCCEEDED: 'verifying_address_succeeded',
UNRECOGNIZED_WALLET: 'unrecognized_wallet',
};
export const WalletUnits: {
ADA: WalletUnit;
Expand Down