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

feat: Add 7702 Semi-Modular Account Support #213

Merged
merged 2 commits into from
Feb 3, 2025
Merged

Conversation

Zer0dot
Copy link
Contributor

@Zer0dot Zer0dot commented Jan 31, 2025

Motivation

With EIP-7702 around the corner, developers would benefit from having a simple example of a Semi-Modular Account with a fallback signer that defaults to the address of the account itself.

Solution

Implement a variant of the Semi-Modular Account which includes the address of the account itself as the fallback signer rather than reading from bytecode as is done in the standard Semi-Modular Account.

This PR also affects the base Semi-Modular Account contract by making the account ID and unchecked fallback signer getter functions virtual as they must be overriden in the 7702 variant.

Copy link

octane-security-app bot commented Jan 31, 2025

Summary by Octane

New Contracts

  • SemiModularAccount7702.sol: The smart contract prevents upgrades and manages fallback signers for a semi-modular account with 7702 support.

Updated Contracts

  • ReferenceModularAccount.sol: Added a virtual keyword to the upgradeToAndCall function, allowing further overrides.
  • SemiModularAccount.sol: Made functions accountId and _retrieveFallbackSignerUnchecked virtual, allowing further overrides.

🔗 Commit Hash: f9215cd

Copy link

octane-security-app bot commented Jan 31, 2025

Overview

Vulnerabilities found: 1                                                                                
Severity breakdown: 1 Medium

Detailed findings

src/account/ReferenceModularAccount.sol


🔗 Commit Hash: f9215cd
🛡️ Octane Dashboard: All vulnerabilities

Copy link
Collaborator

@howydev howydev left a comment

Choose a reason for hiding this comment

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

LGTM

1 small security callout is that the upgradeToAndCall path still exists to perform an arbitrary call on the account, but this should be protected in the default cases of selector validation since that's an allowlist model

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.

+1 to @howydev 's point, we should prevent calling upgradeToAndCall as a footgun-prevention mechanism

@Zer0dot Zer0dot merged commit 2b2c061 into develop Feb 3, 2025
4 checks passed
@Zer0dot Zer0dot deleted the zer0dot/7702-sma branch February 3, 2025 18:16
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