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.

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

Change the EVM legacy discovery error on old firmware #14648

Merged
merged 1 commit into from
Oct 1, 2024

Conversation

enjojoy
Copy link
Contributor

@enjojoy enjojoy commented Oct 1, 2024

Description

Changed the error message for non-normal EVM account types, when discovery error occurs.

Related Issue

Resolve #14638

Screenshots:

image

@enjojoy enjojoy requested review from tomasklim and Hermez-cz October 1, 2024 10:02
@enjojoy enjojoy marked this pull request as ready for review October 1, 2024 10:03
return value.concat(
<div key={account.symbol}>
{network.name}: {getAccountError(account.error)}
{network.name} {accountTypeDisplay}:{' '}
Copy link
Member

Choose a reason for hiding this comment

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

This means, that if it is normal, there is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, let me remove the " "

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed, now the space will be only when the type is not normal

@enjojoy enjojoy force-pushed the feat/ethereum-legacy-discovery-error branch from 3abb6ba to f2399fa Compare October 1, 2024 10:10
@enjojoy enjojoy requested a review from tomasklim October 1, 2024 10:15
Copy link
Member

@tomasklim tomasklim left a comment

Choose a reason for hiding this comment

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

@enjojoy enjojoy force-pushed the feat/ethereum-legacy-discovery-error branch from f2399fa to 5b0ae32 Compare October 1, 2024 10:22
@enjojoy
Copy link
Contributor Author

enjojoy commented Oct 1, 2024

https://github.com/trezor/trezor-suite/blob/develop/COMMITS.md

sorry, no idea how I could forget this. reworded.

@Hermez-cz
Copy link

The copywriting is clear and concise, directly explaining the issue. However, the [Retry] CTA feels out of place. From the user's perspective, a CTA to update the firmware would be more appropriate, as retrying will lead to the same result without the update. Since we can't change the flow at this point, how about adding '(see the blue banner above)' at the end of the body text?

@evgenysl evgenysl added the build-desktop This will trigger the build of desktop apps for your PR label Oct 1, 2024
@enjojoy
Copy link
Contributor Author

enjojoy commented Oct 1, 2024

The copywriting is clear and concise, directly explaining the issue. However, the [Retry] CTA feels out of place. From the user's perspective, a CTA to update the firmware would be more appropriate, as retrying will lead to the same result without the update. Since we can't change the flow at this point, how about adding '(see the blue banner above)' at the end of the body text?

Sure, I will add this note

@enjojoy enjojoy force-pushed the feat/ethereum-legacy-discovery-error branch 2 times, most recently from 2fda6dc to 1a23979 Compare October 1, 2024 11:19
@enjojoy enjojoy enabled auto-merge (rebase) October 1, 2024 11:20
@enjojoy enjojoy force-pushed the feat/ethereum-legacy-discovery-error branch 2 times, most recently from 2b193f8 to 025e472 Compare October 1, 2024 11:55
@enjojoy enjojoy force-pushed the feat/ethereum-legacy-discovery-error branch from 025e472 to 62c4892 Compare October 1, 2024 11:57
@enjojoy enjojoy merged commit 5f6f057 into develop Oct 1, 2024
22 checks passed
@enjojoy enjojoy deleted the feat/ethereum-legacy-discovery-error branch October 1, 2024 12:14
@evgenysl
Copy link

evgenysl commented Oct 1, 2024

QA OK

  • FW 2.6.0 and newer - no issues with Legacy account discovery

  • FW 2.5.3

Screenshot 2024-10-01 at 15 45 03

Screenshot 2024-10-01 at 15 46 15

Message is small and is not present in Legacy account itself (second screenshot), but this is an edge case for firmware from November 2022 and older.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
build-desktop This will trigger the build of desktop apps for your PR
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Change "Forbidden key path" for EVM legacy accounts
4 participants