-
Notifications
You must be signed in to change notification settings - Fork 0
Feature/update contracts sdk with snickerdoodle wallet #1268
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
base: feature/new-contracts-package
Are you sure you want to change the base?
Feature/update contracts sdk with snickerdoodle wallet #1268
Conversation
…racts-sdk-with-snickerdoodle-wallet
…racts-sdk-with-snickerdoodle-wallet
…racts-sdk-with-snickerdoodle-wallet
…racts-sdk-with-snickerdoodle-wallet
…racts-sdk-with-snickerdoodle-wallet
…racts-sdk-with-snickerdoodle-wallet
…racts-sdk-with-snickerdoodle-wallet
…racts-sdk-with-snickerdoodle-wallet
@@ -32,6 +32,14 @@ import { | |||
SignerUnavailableError, | |||
NobleED25519KeyPair, | |||
ED25519PrivateKey, | |||
P256PublicKeyComponent, | |||
PasskeyId, | |||
PasskeyPublicKeyPointX, |
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.
Should this be P256PublicKeyPointX rather than passkey, keeping it consistent with the P256Signature stuff?
|
||
public parseRawP256PublicKey( | ||
id: PasskeyId, | ||
publicKeyArray: 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.
Right now we have a PublicKey defined and used as a binary hex string; it might be better to make this take that object. A new P256PublicKey primitive would be really good. Also, for PasskeyId, is that really required? It's part of P256PublicKeyComponent but it's not used by this method; I think we could exclude it safely.
// Third value tells you the type of the next value which MUST be an integer (0x02) if this is a signature array | ||
const metadataIndicatorByte = pubKeyView[2]; | ||
// Third byte MUST be equal to 48 if this is a legitimate public key array | ||
console.assert( |
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.
We should make this method return a Result<> so that this error could be returned. Doesn't need to be a ResultAsync<>, but this error should be properly handled
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.
Empty file
|
||
// returns a 64-byte ArrayBuffer containing r and s concatenated together | ||
public parseRawP256Signature( | ||
signatureArray: ArrayBuffer, |
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.
Let's change the name and type to signature: P256Signature
import { JSONString } from "@snickerdoodlelabs/objects"; | ||
import { BytesLike } from "ethers"; | ||
|
||
export class AuthenticatorData { |
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.
Need a method in CryptoUtils parseClientDataJson(clientDataJSON: JSONString): Result<AuthenticatorData, whatever>
@@ -0,0 +1,5 @@ | |||
import { P256SignatureR, P256SignatureS } from "@objects/primitives/index.js"; | |||
|
|||
export class P256SignatureComponent { |
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.
Maybe ParsedP256Signature, or just make it P256SignatureComponents
P256PublicKeyPointY, | ||
} from "@objects/primitives/index.js"; | ||
|
||
export class P256PublicKeyComponent { |
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.
Name -=> P256PublicKeyComponents, drop the keyId
parseRawP256Signature( | ||
signatureArray, | ||
msgPayload, | ||
): P256SignatureComponentArrayBuffer; |
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.
Make this return a P256SignatureComponents object, can be passed to the contract layer, make sure it takes strings not arrays
parseRawP256PublicKey( | ||
publicKey: P256PublicKey, | ||
): Result< | ||
{ x: P256PublicKeyPointX; y: P256PublicKeyPointY }, |
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.
Return P256PublicKeyComponents (minus the key ID)
>; | ||
|
||
addP256KeyWithP256Key( | ||
keyId: PasskeyId, |
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.
PasskeyId -> WebauthnCredentialId
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.
Notes after discussion
Updated package version for publishing
…racts-sdk-with-snickerdoodle-wallet
…racts-sdk-with-snickerdoodle-wallet
|
||
addP256KeyWithP256Key( | ||
keyId: WebauthnCredentialId, | ||
authenticatorData: string, |
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.
Primitive type string AuthenticatorData
@@ -281,4 +312,11 @@ export class SnickerdoodleWalletContract | |||
): SnickerdoodleWalletContractError { | |||
return new SnickerdoodleWalletContractError(msg, e, transaction); | |||
} | |||
|
|||
private isoBase64fromBuffer( |
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.
This method isn't needed now, should remove it, along with the import
): Result<string, InvalidParametersError> { | ||
return ok(evmAccountAddress); | ||
): Result<Uint8Array, InvalidParametersError> { | ||
// Remove 0x, convert to 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.
You should check this out, and make sure it works with both 0x123 and just 123. I think that .encode will handle both versions, you should test that.
Made Base and Base Sepolia control chains
…he control chain no longer makes sense. All chains may have the contract stack (indicated by contractStackDeployed). The control chain is configured already in the IP but doesn't need different data
…racts-sdk-with-snickerdoodle-wallet
… of https://github.com/SnickerdoodleLabs/protocol into feature/update-contracts-sdk-with-snickerdoodle-wallet
Release Notes
JIRA Link
Summary:
This PR updates the contracts-sdk to new functionalities of the SnickerdoodleWallet and Factory contracts.