-
Notifications
You must be signed in to change notification settings - Fork 167
feat: splitting signing methods into prepare and sign #1629
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: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🌿 Documentation Preview
|
@@ -78,24 +138,18 @@ export const nativeSMASigner = ( | |||
|
|||
// we apply the expected 1271 packing here since the account contract will expect it | |||
async signMessage({ message }: { message: SignableMessage }): Promise<Hex> { |
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.
💬 try this with a this this is assuming we have we can infer the type from references to params sent down or
async signMessage(this: SigningMethods<'eth_signTypedData_v4'>, {message}: MessageTrait) {// do things}
or do an overloaded type definition based on the type
const signMessage: Type extends 'eth_signTypedData_v4' ? (value: MessageTrait) => RT : never
const signMessage = async ({message}) {// do things};
a33fb15
to
125311e
Compare
125311e
to
31073b5
Compare
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 need to update the ToSmartAccountParams
or w/e it's called as well to support passing in the extensions
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.
also looking at this, I don't think those prepare and format methods are actually exposed because of this. they need to be passed in threw params and then added to the resulting object from toSmartContractAccount
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.
drat.. will relook + add a test for this
account-kit/smart-contracts/src/ma-v2/account/nativeSMASigner.ts
Outdated
Show resolved
Hide resolved
account-kit/smart-contracts/src/ma-v2/modules/single-signer-validation/signer.ts
Outdated
Show resolved
Hide resolved
@@ -46,6 +46,21 @@ export enum DeploymentState { | |||
DEPLOYED = "0x2", | |||
} | |||
|
|||
export type SignatureRequest = |
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.
Can we use the type declared in @account-kit/wallet-client
? I'm not sure where it's exported, but I'm pretty sure this type exists somewhere else already.
31073b5
to
bed7f05
Compare
17c88be
to
3741625
Compare
Pull Request Checklist
yarn test
)site
folder, and guidelines for updating/adding docs can be found in the contribution guide)feat!: breaking change
)yarn lint:check
) and fix any issues? (yarn lint:write
)PR-Codex overview
This PR focuses on enhancing the signature handling in the
account-kit
smart contracts by introducing new types and methods for managing signature requests. It includes improvements for signing messages and typed data, ensuring better error handling and type safety.Detailed summary
SignatureRequest
type with specific request types.SigningMethods
type for signature preparation and formatting.assertNeverSignatureRequestType
for error handling.nativeSMASigner
andsingleSignerMessageSigner
to utilize new signing methods.