Skip to content
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: extend signer to sign arbitrary message hashes #67

Closed

Conversation

taureau75
Copy link

@taureau75 taureau75 commented Nov 23, 2021

This is a pull request to allow wallets to sign messages without broadcasting them.

Ethereum had a lot of issues with this, so it will likely require some debate and the establishment of a standard on how messages are signed. See EIP 712 and EIP 191.

https://eips.ethereum.org/EIPS/eip-191
https://eips.ethereum.org/EIPS/eip-712

I know there's a lot of security issues around it, so I don't expect it to get approved without a lot of discussion and additional code, but I wanted to get the ball rolling by putting up some code to start.

@taureau75
Copy link
Author

For Zigzag, we sign orders off-chain and send them on-chain when they are matched.

Without the ability to sign messages through an external Signer, we can't support external wallets like Argent browser extension.

@taureau75
Copy link
Author

taureau75 commented Nov 24, 2021

The security issue is that if you allow arbitrary messages to be signed, a malicious app can present a user with a valid Starknet transaction to sign.

Since the garbled message won't look like a valid transaction, a user might sign it and sign their funds away.

@0xs34n
Copy link
Collaborator

0xs34n commented Nov 24, 2021

Thanks for starting this PR, let's continue the discussion here:

argentlabs/argent-x#14

@0xs34n 0xs34n marked this pull request as draft November 24, 2021 13:27
@0xs34n
Copy link
Collaborator

0xs34n commented Nov 24, 2021

Converting to draft as we get more clarity from the OZ and Argent team on standards. @vixidev99 feel free to open this PR for review as we make progress on the specifications. Thanks!

@janek26
Copy link
Member

janek26 commented Dec 9, 2021

moved to #87

@vixidev99 feedback appreciated !

@janek26 janek26 closed this Dec 9, 2021
YohanTz added a commit to YohanTz/starknet.js that referenced this pull request Apr 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants