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

Add base58check decoding to recovery iframe #18

Merged
merged 4 commits into from
Nov 20, 2023
Merged

Add base58check decoding to recovery iframe #18

merged 4 commits into from
Nov 20, 2023

Conversation

r-n-o
Copy link
Collaborator

@r-n-o r-n-o commented Nov 20, 2023

This branch adds base58check decoding. For now I've left base64url decoding in place, but we should be able to get rid of it once we release base58check recovery codes.

The nice thing about recovery codes is that they auto-expire after 30mins. So as soon as we switch to base58check codes, clock starts ticking, and we're good to delete the awkward branching after just 30mins (although, practically we'll wait a few hours at least!)

@r-n-o r-n-o requested review from oliviathet and emostov November 20, 2023 16:09
.nvmrc Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

👏

recovery/index.html Outdated Show resolved Hide resolved
recovery/index.html Outdated Show resolved Hide resolved
// Assuming random bytes in our bundle and a bundle length of 33 (public key, compressed) + 48 (encrypted cred) = 81 bytes.
// The odds of a byte being in the overlap set between base58 and base64url is 58/64=0.90625.
// Which means the odds of a 81 bytes string being in the overlap character set for its entire length is...
// ... 0.90625^81 = 0.0003444209703
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice

Copy link
Contributor

@emostov emostov left a comment

Choose a reason for hiding this comment

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

Didn't completely wrap my brain around ss58 impl, but the test coverage looks good so I trust it!

@emostov
Copy link
Contributor

emostov commented Nov 20, 2023

Readability updates look great 👍

Copy link
Contributor

@oliviathet oliviathet left a comment

Choose a reason for hiding this comment

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

🚀

@r-n-o r-n-o merged commit 787b1fc into main Nov 20, 2023
5 checks passed
@james-callahan james-callahan deleted the rno/base58check branch November 20, 2023 23:26
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.

3 participants