-
Notifications
You must be signed in to change notification settings - Fork 582
feat: support starknet multiple signers signature #1182
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
feat: support starknet multiple signers signature #1182
Conversation
f5bfaff
to
06ef71b
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.
Pull Request Overview
This PR adds support for verifying Starknet messages signed by multiple signers, enabling verification of Argent X smart account wallets with Argent Shield. It introduces a new signature parsing function and updates the verification logic to handle multi-signer scenarios while maintaining backward compatibility.
Key changes:
- Implements multi-signer signature verification for Starknet
- Adds new hash type for SNIP-12 format alias payload
- Maintains backward compatibility with existing single-signer verification
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
src/verify/starknet.ts | Adds getSignatureArray function and updates verify logic to handle multi-signer signatures |
src/verify/starknet.spec.ts | Adds test coverage for multi-signer verification scenarios |
test/fixtures/starknet/message-alias-multisigner.json | New test fixture for multi-signer signature verification |
src/sign/hashedTypes.json | Adds new hash type for SNIP-12 format alias payload |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
tests: fix test tests: fix tests refactor: code improvement
06ef71b
to
fde64e9
Compare
test('should return true when verifying on a different network', () => { | ||
expect( | ||
verify(message.address, message.sig, message.data, 'SN_SEPOLIA') | ||
).resolves.toBe(true); | ||
}); |
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.
why it will return true for different network 🤔
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 is the old behavior.
It's just highlighting that the behavior is different when there is a co-signer
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.
Is it because normal signatures doesn't depend on network?
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.
verification always occur onchain, by calling a contract method. it's weird that it works
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.
tAck
Toward snapshot-labs/sx-monorepo#1585
This PR adds support for verifying starknet messages signed by multiple signers.
This will enable verifying messages signed by argent x smart account wallets, with argent shield enabled (https://docs.ready.co/aa-use-cases/verifying-signatures-and-cosigners#verifying-multi-signatures)
Also adding a new hashtype for the new starknet alias payload, which is now following the SNIP-12 format (https://github.com/starknet-io/SNIPs/blob/main/SNIPS/snip-12.md)
How to test
verify
function (thesig
array first element should be0x2
)true
Note
We're only supporting SNIP-12 revision 0
Revision 1 require starknet.js v7, which require node >= 22