-
Notifications
You must be signed in to change notification settings - Fork 12.3k
fix(docs): add EIP712 to multisig example contracts’ inheritance #6127
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
fix(docs): add EIP712 to multisig example contracts’ inheritance #6127
Conversation
|
WalkthroughThree smart contracts (MyAccountERC7913, MyAccountMultiSigner, and MyAccountMultiSignerWeighted) have been updated to inherit from the EIP712 contract. EIP712 has been added as the second item in the inheritance list for each contract, positioned after the Account contract. The constructors continue to initialize EIP712 with the same domain name and version parameters as specified in the original implementations. Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
🔇 Additional comments (3)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ernestognw
left a comment
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.
The EIP712 contract is inherited already through ERC7739. Regardless, I think it's worth adding it so I also took some time to update references from community contracts that were moved over to this repository
The multisig examples in docs/modules/ROOT/pages/multisig.adoc called the EIP712 base constructor without inheriting EIP712, which causes a compile-time error in Solidity because base constructors can only be invoked for direct or indirect bases. The examples also mix in ERC7739, whose nested EIP-712 hashing pattern requires an EIP-712 domain. Aligning with OpenZeppelin patterns (e.g., Governor), I added EIP712 to the inheritance lists of MyAccountERC7913, MyAccountMultiSigner, and MyAccountMultiSignerWeighted so that the existing EIP712("…","1") constructor calls are correct and the examples compile as intended