-
Notifications
You must be signed in to change notification settings - Fork 85
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
Account Abstraction - Custom Validation Signature Scheme #229
base: main
Are you sure you want to change the base?
Conversation
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.
Please review #200 for the basic of account abstraction already explained.
Try to keep a more neutral tone, do not use words such as thief
, newbies
...
@julio4 Pls check the adjustments I made, also the custom signature validation I used was the standard implementation for Secp256r1 and Secp256k1 used by Argent. Let me know what needs to be added or removed pls |
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.
It looks good to me, I requested some changes.
Lastly, it would be great to add a few notes on cryptography theory about "digital signature" before going into the implementation (not a deep dive, just a global overview)
ii. **Signers:** These are responsible for digitally signing transactions and provide the authorization needed to initiate transactions. | ||
Digital signatures are cryptographic proofs that transactions are authorized by corresponding accounts. | ||
|
||
In summary, Starknet accounts are normal blockchain accounts that hold assets and initiate transactions onchain, while signers provide the authorization required to ensure that transactions originating from these accounts are secure, valid and executed. |
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 put more emphasis on the fact that signers are offchain, and that the implementation of the validation functions of SNIP-6 dictate how to "link" signers onchain, by verify the signatures in account contracts. You can compare it with EOA in Ethereum, and explains that it adds more flexibility.
With great power comes great responsibility, so a small advisory on potential security issues with improper validation implementation would be great as well.
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 would still think that an emphasis on the offchain part of signers is missing
listings/advanced-concepts/custom_signature_validation/src/custom_signature.cairo
Outdated
Show resolved
Hide resolved
Made some adjustments based on your review @julio4 |
Reviews implemented and pushed @julio4 |
@@ -1,12 +1,15 @@ | |||
# Custom Signature Validation Scheme | |||
|
|||
Account Abstraction is a native feature on Starknet, this makes it possible for anyone to implement custom signature schemes. The implication is that signature schemes on Starknet are not limited to just one, any standard signature scheme can be validated, for example Starknet signature, Secp256k1, Secp256r1, Eip191 etc are some of the custom signatures that can be validated on Starknet currently. | |||
Digital signatures are a fundamental aspect of modern cryptography, used to verify the authenticity and integrity of digital messages or transactions. They are based on public-key cryptography, where a pair of keys (a public key and a private key) are used to create and verify signatures. | |||
Private keys are kept secret and secure by the owner, and are used to sign the message or transaction, while the public key can be used by anyone to verify the signature. |
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.
- secured
- to sign data such as messages or transactions
- and can be verified by anyone with the public key
Digital signatures are a fundamental aspect of modern cryptography, used to verify the authenticity and integrity of digital messages or transactions. They are based on public-key cryptography, where a pair of keys (a public key and a private key) are used to create and verify signatures. | ||
Private keys are kept secret and secure by the owner, and are used to sign the message or transaction, while the public key can be used by anyone to verify the signature. | ||
|
||
Account Abstraction is a native feature on Starknet, this makes it possible for anyone to implement custom signature schemes. The implication is that signature schemes on Starknet are not limited to just one, any standard signature scheme can be validated, for example Starknet signature, Secp256k1, Secp256r1, Eip191 etc are some of the custom signatures that can be validated on Starknet currently. |
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.
Little redundant, you can compact it like this:
Account Abstraction is a native feature on Starknet, this makes it possible for anyone to implement custom signature schemes and use it to validate transactions with the implementation of the signature validation logic.
ii. **Signers:** These are responsible for digitally signing transactions and provide the authorization needed to initiate transactions. | ||
Digital signatures are cryptographic proofs that transactions are authorized by corresponding accounts. | ||
|
||
In summary, Starknet accounts are normal blockchain accounts that hold assets and initiate transactions onchain, while signers provide the authorization required to ensure that transactions originating from these accounts are secure, valid and executed. |
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 would still think that an emphasis on the offchain part of signers is missing
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.
Thank you for applying the changes, it is better!
Overall you should try to keep the sentences as straightforward as possible. There's lots of sentences you can simplify and make it more digestible for readers. I added some comments with examples.
And lastly I would like to break the last part, the actual implementation example, in specific block with some comment on what part of the implementation it is.
@@ -17,18 +20,14 @@ All Account contracts on Starknet must implement the SNIP-6 standard as mentione | |||
|
|||
`is_valid_signature` returns true if the signature is valid, `__validate__` validates the signature and marks them as 'VALIDATED', while `__execute__` executes the validated transaction. Sample implementation of SNIP-6 standard: [Sample SNIP-6 Implementation](https://starknet-by-example.voyager.online/advanced-concepts/account_abstraction/account_contract.html#minimal-account-contract-executing-transactions) |
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.
Could you use relative path to account contract instead of absolute url
@@ -17,18 +20,14 @@ All Account contracts on Starknet must implement the SNIP-6 standard as mentione | |||
|
|||
`is_valid_signature` returns true if the signature is valid, `__validate__` validates the signature and marks them as 'VALIDATED', while `__execute__` executes the validated transaction. Sample implementation of SNIP-6 standard: [Sample SNIP-6 Implementation](https://starknet-by-example.voyager.online/advanced-concepts/account_abstraction/account_contract.html#minimal-account-contract-executing-transactions) | |||
|
|||
|
|||
On Ethereum, EOAs directly sign transactions onchain using their private keys. This onchain signing of transactions is more secure and straightforward but less flexible. Offchain signing employed on Starknet gives room for more flexibility aside the ability to implement custom signature schemes, however, care must be taken to validate all signatures meticulously to ensure that: | |||
On Ethereum, only **one** signature scheme is used for signing messages and transactions, and also for signature authentications: the Elliptic Curve Digital Signature Algorithm (ECDSA). That means that no other signature algorithms can be validated on Ethereum, making it more secure but less flexible. |
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.
On Ethereum, only one signature scheme is used: ECDSA. It makes Ethereum more secure but also less flexible.
In summary, Starknet accounts are normal blockchain accounts that hold assets and initiate transactions onchain, while signers provide the authorization required to ensure that transactions originating from these accounts are secure and valid. | ||
|
||
Digital signatures are a fundamental aspect of modern cryptography, used to verify the authenticity and integrity of digital messages or transactions. They are based on public-key cryptography, where a pair of keys (a public key and a private key) are used to create and verify signatures. | ||
Private keys are kept secret and secure by the owner, and are used to sign the message or transaction, while the public key can be used by anyone to verify the signature. | ||
|
||
### Custom signature validation sample | ||
|
||
The example below shows a sample implementation of `Secp256r1` and `Secp256k1` signature schemes: |
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.
As it can be a bit complex in one big block, try to break it in each steps of the implementation
@julio4 Please check the adjustments and let me know what else to do |
Issue(s): Close #191
Description
Please provide a brief description of the changes made in this pull request and how they address the related issue.
Checklist
./scripts/cairo_programs_verifier.sh
successfully