-
Notifications
You must be signed in to change notification settings - Fork 2k
feat(forge): cheatcode eip712 #10570
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
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 run cargo cheats
twice to fix failing tests
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.
looks good, left couple of comments / nits!
So if I get this right the DX would be:
- generate bindings with
forge bind-json
(by default goes toutils/JsonBindings.sol
but could be configurable to differentpathToJsonBindings
path) - use
vm.eip712HashType(typeDefinition)
orvm.eip712HashType(pathToJsonBindings, typeDefinition)
to generate EIP-712 representation
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.
sorry, just realized params should be calldata and not memory
Getting the type string right is only half the battle, the other is also encoding/hashing. Would be nice if you had a json style chteatcode where you pass in the abi encoded bytes + the type name and you construct the EIP-712 bytes to be hashed to get the struct hash (including the typehash). As an auditor/dev I see people messing up the hashing of sub-collections / arrays in EIP-712 all the time would be nice to have a good way to verify/compare this. |
…solar (#10510) * wip * feat: eip712 type hash PoC * style: json * style: json * style: json * style: comments * wip * initial impl using solar * fix: untracked change * fix: optimize resolve_type * initial working impl * feat: eip712 solar resolver * style: docs + fmt + clippy * todo: cheatcode * docs: comments * fix: use HIR rather than AST * from build opts * docs * fix: rmv hashset * create utils for solar_pcx_from_build_opts * incorporate version logic into `solar_pcx_from_build_opts` * wip bind-json: eip712 resolver integration * forge(bind-json): integrate solar * fix: tests * style: clippy * undo cheatcode setup (will tackle it on its own PR) * rmv old test * style: fix typo * fix: win path * fix: merge conflicts * fix: dani's feedback * docs: explain bindings overriding * chore: patch solar * feat(forge): eip712 cheatcodes (#10570) * fix: bump solang parser * Remove unused from forge crate * Move tests to eip712 * Nit: comments --------- Co-authored-by: grandizzy <[email protected]>
Adds a new cheatcode to safely generate hashes following the EIP-712 spec.
implementation details
ref #4818
due to the lack of polymorphism, it is not possible for the cheatcode to accept a struct. Because of that, users must provide a string.
this impl allows users to either pass:
forge bind-json
(recommended)context
usually developers manually set up constants on their smart contracts to easily access the EIP-712 type hashes:
because of that, it is extremely important for these hashes to be canonical (properly computed following the EIP spec). However, as any manual approach this is error prone.
this new cheatcode aims to provide robustness to the codebases, and peace of mind for developers, giving them guarantees that no typos or sorting errors where made when manually generating the type definitions.
example usage
important note
when using
vm.eip712HashType
:amont
instead ofamount
in the string will still produce a valid EIP-712 hash, but for the incorrect type, leading to runtime signature verification errors.forge bind-json
generated bindings, ensuring the hash corresponds to your actual compiled contract structs. This provides a guarantee of alignment, catching discrepancies that the string method would miss.