-
Notifications
You must be signed in to change notification settings - Fork 11.2k
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
sdk: add passkey recovery option #20722
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
cc66e3f
to
e2f21d4
Compare
*/ | ||
constructor(publicKey: Uint8Array, provider: PasskeyProvider) { | ||
constructor(provider: PasskeyProvider, publicKey?: Uint8Array) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure who is using this so far, but this is a breaking change
* const possiblePks = await PasskeyKeypair.signAndRecover(provider, testMessage); | ||
* const testMessage2 = new TextEncoder().encode('Hello world 2!'); | ||
* const possiblePks2 = await PasskeyKeypair.signAndRecover(provider, testMessage2); | ||
* const uniquePk = findUniquePublicKey(possiblePks, possiblePks2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unique seems backwards here, you want the non-unique key right? We can probably add an actual implementation in the example like possiblePks.find(pk => !!possiblePks2.find(pk2 => pk.equals(pk2))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah, didn't see we actually implemented this below
* @param arr2 - The second pubkeys array. | ||
* @returns The only common pubkey in both arrays. | ||
*/ | ||
export function findUniquePublicKey(arr1: PublicKey[], arr2: PublicKey[]): PublicKey { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comment as above, unique seems like the wrong term here.
@@ -98,7 +99,7 @@ export class BrowserPasskeyProvider implements PasskeyProvider { | |||
* A passkey signer used for signing transactions. This is a client side implementation for [SIP-9](https://github.com/sui-foundation/sips/blob/main/sips/sip-9.md). | |||
*/ | |||
export class PasskeyKeypair extends Signer { | |||
private publicKey: Uint8Array; | |||
private publicKey?: Uint8Array; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't figure out why we want to change this. We are asserting that this is set everywhere its used, and I don't see any examples of constructing a PasskeyKeypair
without a publicKey
Description
after thinking about https://github.com/MystenLabs/passkey-example?tab=readme-ov-file#notes-on-wallet-storage i realize we should provide the recovery ability in our sdk since getPasskeyInstance is a bit unflexible and create a new credential every time when called. we should also allow user to find its existing passkey wallet without creating a new one. similar to "log in to existing" vs "create new". since there are more than one way to "recover/log in" to an wallet, i leave the flexibility to the wallet implementer (to sign once vs sign twice).
Test plan
How did you test the new or updated feature?
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.