Skip to content

Add ERC7579 Executor modules #121

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

Merged
merged 97 commits into from
May 16, 2025
Merged

Add ERC7579 Executor modules #121

merged 97 commits into from
May 16, 2025

Conversation

ernestognw
Copy link
Member

@ernestognw ernestognw commented Apr 25, 2025

More generalized alternatives to #93 and #85

Co-authored-by: Gonzalo Othacehe <[email protected]>
@ernestognw
Copy link
Member Author

ernestognw commented May 15, 2025

I changed the can* functions to internal _validate* functions, it keeps an internal interface for validation and is more consistent with the delayed executor.

Build is failing since the MultisigMocks need to be adapted, but it's still unclear how:

abstract contract ERC7579MultisigExecutorMock is ERC7579Executor, ERC7579Multisig, EIP712 {
    bytes32 private constant EXECUTE_OPERATION =
        keccak256("ExecuteOperation(address account,bytes32 mode,bytes executionCalldata,bytes32 salt)");

    function _validateExecution(
        address account,
        Mode mode,
        bytes calldata executionCalldata,
        bytes32 salt
    ) internal view virtual returns (bool) {
        // We're missing the `signature` here
        return _validateMultisignature(account, _getExecuteTypeHash(account, mode, executionCalldata, salt), signature);
    }

    function _getExecuteTypeHash(
        address account,
        Mode mode,
        bytes calldata executionCalldata,
        bytes32 salt
    ) internal view virtual returns (bytes32) {
        return
            _hashTypedDataV4(
                keccak256(abi.encode(EXECUTE_OPERATION, account, Mode.unwrap(mode), executionCalldata, salt))
            );
    }
}

For the modules to be successful, _validateExecution and _validateMultisignature must combine seamlessly.

@ernestognw
Copy link
Member Author

I changed the can* functions to internal _validate* functions, it keeps an internal interface for validation and is more consistent with the delayed executor.

Build is failing since the MultisigMocks need to be adapted, but it's still unclear how:

// ...

For the modules to be successful, _validateExecution and _validateMultisignature must combine seamlessly.

Fixed by differentiating the data argument from the public schedule, execute and cancel functions and executeCalldata on their internal versions. The reasoning is that developers must override the public versions if they want to perform validation on data, slice it and extract the executeCalldata from there. This provides more flexibility and allows combining use cases as in the ERC7579MultisigMocks

@ernestognw
Copy link
Member Author

Happy with the current design and it's good enough for the community version. Given the schedule, I think we should move on and merge so I can start writing ERC-7579 guides that include the social recovery use case. Also I should start playing around with this on Wizard.

@ernestognw ernestognw merged commit c7bd22e into master May 16, 2025
11 checks passed
ernestognw added a commit that referenced this pull request Jun 4, 2025
Co-authored-by: Gonzalo Othacehe <[email protected]>
Co-authored-by: Gonzalo Othacehe <[email protected]>
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.

4 participants