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

Attempt to import private key to validate it #3614

Merged
merged 3 commits into from
Sep 3, 2023

Conversation

jagodarybacka
Copy link
Contributor

@jagodarybacka jagodarybacka commented Sep 1, 2023

Ref #3608

What

Let's validate private key by trying to import it

Testing

  • Try to import the private key 0c1cd323ff8fd5b34f260d580c3a843f3e06c3e1e31c357d38a4a22e8b26809b, which has a leading 0 (more than one leading 0 should produce the same result). It should work without failing.
  • Try to import the private key 1cd323ff8fd5b34f260d580c3a843f3e06c3e1e31c357d38a4a22e8b26809b (same as above but with first two characters missing entirely). It should fail.

Latest build: extension-builds-3614 (as of Sun, 03 Sep 2023 03:40:35 GMT).

@jagodarybacka jagodarybacka changed the title Show different private key import errors Attempt to import private key to validate it Sep 1, 2023
Copy link
Contributor

@Shadowfiend Shadowfiend left a comment

Choose a reason for hiding this comment

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

Code looks good, will do a spot check here soon.

type: SignerSourceTypes.privateKey,
privateKey: trimmedPrivateKey,
}),
)) as unknown as AsyncThunkFulfillmentType<typeof importSigner>
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: you should be able to drop the as chain here now :) The dependency bumps included a typing rework that properly types dispatch.

@Shadowfiend
Copy link
Contributor

Confirmed the bug fix here. I used the private key 0c1cd323ff8fd5b34f260d580c3a843f3e06c3e1e31c357d38a4a22e8b26809b which I generated some-random-where. It fails on main and works on this branch. I also tested a few other private keys to make sure they still worked 💪

@Shadowfiend Shadowfiend merged commit 14c9074 into main Sep 3, 2023
5 checks passed
@Shadowfiend Shadowfiend deleted the pk-errors-improvements branch September 3, 2023 03:31
This was referenced Sep 3, 2023
Shadowfiend added a commit that referenced this pull request Sep 8, 2023
## Highlights
- Fixed an issue where valid private keys would not be accepted. These
always had one or more `0`s at the beginning.
- Fixed an issue where certain types of errors were causing sites not to
load correctly.

## What's Changed
* v0.48.0 by @Shadowfiend in
#3610
* Attempt to import private key to validate it by @jagodarybacka in
#3614
* Group private keys when categorizing by @Shadowfiend in
#3618
* Assert URL of the scan website opened for unverified assets by
@michalinacienciala in #3602
* Handle invalid responses on batch rpc providers by @hyphenized in
#3615
* Joint Privacy: Rework analytics service event management and add
signing tracking by @Shadowfiend in
#3623


**Full Changelog**:
v0.48.0...v0.49.0

Latest build:
[extension-builds-3625](https://github.com/tahowallet/extension/suites/15924368205/artifacts/909765830)
(as of Thu, 07 Sep 2023 20:56:50 GMT).
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.

2 participants