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

fix: v0.8 review and polish #186

Merged
merged 2 commits into from
Oct 1, 2024
Merged

Conversation

jaypaik
Copy link
Collaborator

@jaypaik jaypaik commented Sep 30, 2024

Some updates to the spec after a review. This PR does not make any changes that would result in interface selectors changing. Will follow up on another PR for one change that would impact that for a more careful review.

Notable changes (non-exhaustive):

  1. Updates the title of the ERC from Modular Smart Contract Accounts and Modules to Modular Smart Contract Accounts. I think the latter implies that the standard would cover modules. Feel free to push back on this.
  2. Updates places where we refer to "validation functions" as "validations" to expand it to "validation functions" (for consistency with our Terms section).
  3. Within the execution function uninstallation section, replaces The account MUST remove all supported interfaces as specified in the manifest. to The account SHOULD remove all supported interfaces as specified in the manifest. This allows account implementations to choose custom behavior such as how to handle duplicate interface IDs.

@jaypaik jaypaik force-pushed the 09-30-fix_v0.8_review_and_polish branch from c2b2c5d to 2eccf6a Compare September 30, 2024 21:47
Copy link

octane-security-app-dev bot commented Sep 30, 2024

Summary by Octane

New Contracts

No new contracts were added in this PR.

Updated Contracts

  • IExecutionHookModule.sol: Updates require executeUserOp calls to pass full msg.data to hook modules in the data parameter.
  • IModularAccount.sol: Added uninstall functionality for modules, clarified data parameters, and structured execution validation processes.
  • IModularAccountView.sol: Clarifications in comments and return descriptions, improving documentation of function selectors and validation functions.
  • IValidationModule.sol: The signature validation function's return description is rephrased for consistent formatting without altering functionality.

🔗 Commit Hash: 2cc57dd

@jaypaik jaypaik marked this pull request as ready for review September 30, 2024 21:47
@jaypaik jaypaik requested a review from a team September 30, 2024 21:47
Copy link

octane-security-app-dev bot commented Sep 30, 2024

Overview

Octane AI analysis has finished. No vulnerabilities were found. Cheers! 🎉🎉🎉


🔗 Commit Hash: 2cc57dd

Copy link
Contributor

@huaweigu huaweigu left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@adamegyed adamegyed left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for the thorough review!

standard/ERCs/erc-6900.md Outdated Show resolved Hide resolved
@jaypaik jaypaik merged commit 64ddec2 into develop Oct 1, 2024
4 checks passed
@jaypaik jaypaik deleted the 09-30-fix_v0.8_review_and_polish branch October 1, 2024 20:18
jaypaik added a commit that referenced this pull request Oct 1, 2024
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.

3 participants