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: sign eip-712 message instead of hash #57

Merged
merged 3 commits into from
Jun 11, 2024

Conversation

banteg
Copy link
Contributor

@banteg banteg commented Jun 10, 2024

see also https://docs.safe.global/advanced/smart-account-signatures

What I did

sign eip-712 message instead of defunct message.

if you follow the naming in safe docs, i have switched eth_sign signature for ecdsa signature.

it has also allowed to remove the method to adjust v of the signature.

fixes: #54

How I did it

replace signing of safe_tx_hash with signing of safe_tx.

How to verify it

In [9]: yes.transfer(dev, '1 ether', nonce=1, sender=safe, submit=False)
Signing EIP712 Message
Domain
	Chain ID: 81457
	Contract: 0x...
Message
	to: 0x1a49351bdB4BE48C0009b661765D01ed58E8C2d8
	value: 0
	data: b'\xa9\x05\x9c\xbb\...'
	operation: 0
	safeTxGas: 0
	baseGas: 0
	gasPrice: 0
	gasToken: 0x0000000000000000000000000000000000000000
	refundReceiver: 0x0000000000000000000000000000000000000000
	nonce: 1


Sign:  [y/N]: y

Checklist

  • Passes all linting checks (pre-commit and CI jobs)
  • New test cases have been added and are passing
  • Documentation has been updated
  • PR title follows Conventional Commit standard (will be automatically included in the changelog)

@banteg
Copy link
Contributor Author

banteg commented Jun 10, 2024

some tests fail with GS026: Invalid owner provided
https://github.com/safe-global/safe-smart-account/blob/main/docs/error_codes.md

i tested in prod and it worked for me.

ape_safe/_cli/pending.py Show resolved Hide resolved
ape_safe/accounts.py Show resolved Hide resolved
@fubuloubu fubuloubu merged commit 70234dc into ApeWorX:main Jun 11, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

sign eip-712 safe messages instead of defunct messages
2 participants