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

feat: keystore import #8442

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

feat: keystore import #8442

wants to merge 9 commits into from

Conversation

gomesalexandre
Copy link
Contributor

@gomesalexandre gomesalexandre commented Dec 24, 2024

Description

Does what it says on the box - web wire-up following shapeshift/hdwallet#703

⚠️ Note for reviewers: do the usual verdaccio dance when reviewing this, as hdwallet PR is not merged/published yet. In other words this is already reviewable, but not yet mergeable.

TODO:

- [x] make it work
- [x] make it clean
- [x] make it pretty
- [x] error-handling
- [x] translations

Issue (if applicable)

Risk

High Risk PRs Require 2 approvals

What protocols, transaction types, wallets or contract interactions might be affected by this PR?

Low - isolated to keystore imports

Testing

  • Create a keystore from THORSwap
  • Confirm you're able to import a keystore as a ShapeShift wallet e2e
  • Error-handling re: wrong password is working

Engineering

  • ☝🏽

Operations

  • 🏁 My feature is behind a flag and doesn't require operations testing (yet)
  • ☝🏽

Screenshots (if applicable)

image image image image

https://jam.dev/c/a8c50a0f-b6f3-44ca-b38d-415b71588991

@gomesalexandre gomesalexandre changed the title wip: keystore import feat: keystore import Jan 3, 2025

const hoverSx = { borderColor: 'blue.500' }

// TODO(gomes): use https://www.chakra-ui.com/docs/components/file-upload if/when we migrate to chakra@3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Logic very much stolen from there

Comment on lines +148 to +150
if (!keystoreFile) {
throw new Error('No keystore uploaded')
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a.k.a "this should never happen, but..."

Comment on lines +165 to +168
setError('mnemonic', {
type: 'manual',
message: 'walletProvider.shapeShift.import.invalidKeystore',
})
Copy link
Contributor Author

@gomesalexandre gomesalexandre Jan 3, 2025

Choose a reason for hiding this comment

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

Probably fine to ditch this one as we never end up there - but prefer to keep it in case we manage to detect the imported file as being a keystore or not, in which case we could move this catch and reach this error-handling

<VStack spacing={6}>
<FileUpload onFileSelect={handleFileSelect} />

{keystoreFile && (
Copy link
Contributor Author

@gomesalexandre gomesalexandre Jan 3, 2025

Choose a reason for hiding this comment

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

Hide password field until selected

@gomesalexandre gomesalexandre marked this pull request as ready for review January 3, 2025 12:41
@gomesalexandre gomesalexandre requested a review from a team as a code owner January 3, 2025 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Keystore wire up
1 participant