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.

Sign up for GitHub

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

Jump to bottom

Stop using eip712_structs #394

Merged
merged 1 commit into from
Nov 9, 2022
Merged

Stop using eip712_structs #394

merged 1 commit into from
Nov 9, 2022

Conversation

Uxio0
Copy link
Member

@Uxio0 Uxio0 commented Nov 8, 2022 •
edited

  • Copy library from https://github.com/jvinet/eip712 and do some adjustments (removing dependencies, adding typing hints, rewriting some code, adding new methods)
  • EIP712 structs is not updated and it doesn't support Dictionary inputs (classes need to be declared so it's not easy to use with input data)

@Uxio0 Uxio0 requested a review from a team as a code owner November 8, 2022 16:23
@Uxio0 Uxio0 requested review from fmrsabino, hectorgomezv and diegopazosrego and removed request for a team November 8, 2022 16:23
fmrsabino
fmrsabino approved these changes Nov 9, 2022
View reviewed changes
Copy link
Contributor

@fmrsabino fmrsabino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I would just add a description on the changes either at the top of the file or PR.

For example eip712_encode_hash seems to be a new function and the usage of fast_keccak.

gnosis/eth/eip712/__init__.py
from eth_account import Account
from eth_typing import Hash32, HexStr

from ..utils import fast_keccak
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This changed right (from the original)?

I think we should mention this.

@Uxio0 Uxio0 merged commit 56ef616 into master Nov 9, 2022
@Uxio0 Uxio0 deleted the remove-eip712-structs branch November 9, 2022 11:10
@github-actions github-actions bot locked and limited conversation to collaborators Nov 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@fmrsabino fmrsabino fmrsabino approved these changes

@hectorgomezv hectorgomezv Awaiting requested review from hectorgomezv hectorgomezv was automatically assigned from safe-global/safe-services

@diegopazosrego diegopazosrego Awaiting requested review from diegopazosrego diegopazosrego was automatically assigned from safe-global/safe-services

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@Uxio0 @fmrsabino