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

ERC-4337 Factory Support for setupModules() #568

Closed
dancoombs opened this issue May 18, 2023 · 2 comments · Fixed by #572
Closed

ERC-4337 Factory Support for setupModules() #568

dancoombs opened this issue May 18, 2023 · 2 comments · Fixed by #572
Assignees
Milestone

Comments

@dancoombs
Copy link

Context / issue

setupModules makes a call to gasLeft() that is invalid in the ERC-4337 protocol during validation. To make use of the ERC-4337 factory pattern, the factory must adhere to the validation rules. Thus, the factory cannot call setupModules with a to address to setup its modules.

https://github.com/safe-global/safe-contracts/blob/ad9b3190d4889abeeaa02c5c05138d9c327f2460/contracts/base/ModuleManager.sol#L38

https://eips.ethereum.org/EIPS/eip-4337#specification-1

An example factory that uses this pattern and is currently invalid:

https://github.com/eth-infinitism/account-abstraction/blob/abff2aca61a8f0934e533d0d352978055fddbd96/contracts/samples/gnosis/GnosisAccountFactory.sol#L42

Proposed solution

It seems like the gasLeft() could either be replaced with type(uint256).max (like is done in the same file) or, gasLeft() can be pushed down such that the GAS opcode occurs before CALL or DELEGATECALL which is valid in ERC-4337.

Could push the call down here: https://github.com/safe-global/safe-contracts/blob/ad9b3190d4889abeeaa02c5c05138d9c327f2460/contracts/base/Executor.sol#L31

@rmeissner
Copy link
Member

Related to #459

@KK68HK67
Copy link

KK68HK67 commented Sep 4, 2024

Jag har ingen aning av vad sommar hänt om jag har gjort fel så ber jag så mycket om ursäkt mvh Kai det här är till den artikeln nedan för.

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 a pull request may close this issue.

4 participants