-
Notifications
You must be signed in to change notification settings - Fork 375
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: make signing eip 712 compliant #916
Conversation
# Conflicts: # contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
Changes to gas cost
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
# Conflicts: # batcher/aligned-sdk/src/core/types.rs # contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json # contracts/src/core/BatcherPaymentService.sol
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.
Didn't finish the review but I leave you this request changes on the mean time
# Conflicts: # contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json
# Conflicts: # contracts/scripts/anvil/state/alignedlayer-deployed-anvil-state.json # docs/3_guides/4_generating_proofs.md
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.
Signed Fee should appear. Nonce would be better if it's not hashed, but I think this in inherited from some weird decision on the code
Important
Added
chain
param tosubmit
andsubmit_multiple
in the SDK. This is needed to get theBatcherPaymentService
contract address.Important
The version of the Aligned contracts was changed from =0.8.12 to ^0.8.12 to enable compatibility with open-zeppelin 5.0.0 (which is needed to use EIP712). We still use open-zeppelin from eigenlayer-middleware for all the imports but just for the EIP712 we use open-zeppelin 5.0.0.
Note
The GAP variable was lowered by 1 because of the added NONCED_VERIFICATION_DATA_TYPEHASH bytes32 constant.
Note
As stated in the code's comment, VerificationData was used as a bytes32 instead of a VerificationData struct because we don't have the necessary fields in the contract (BatcherPaymentService) to use the VerificationData struct. Also, chain_id is not part of the type hash because it is now part of the domain.
If you want to read more about the EIP-712.
Description
This PR makes the
NoncedVerificationData
signing EIP712 compliant. The EIP712Domain has:Deploying
Devnet:
make anvil_add_type_hash_to_batcher_payment_service
Testnet:
make upgrade_add_type_hash
Working with Metamask
Screen.Recording.2024-09-09.at.15.22.42.mov
To Test
make deps
.