feat: adding validator recovery flow

[zeroknots]

untested, more meant as a conversation piece

[JohnGuilding]

Nice thanks for raising this PR. Here are a few thoughts.

This is definitely a valid way you could use email templates and would make the recovery module generic enough to be used across different validators

Thinking out loud about this approach:

• The good thing about this way of doing things is you can have a generic recovery module that can be used across different 7579 validators.
• Reduces work needed from an account developer to build their own module - which would be ideal
• An important constraint for this approach is that the EmailAuth circuit has a maximum length for the subject bytes passed in. This is 605 bytes. So if the calldata passed in the subject combined with the other values in the subject was more than this value, you wouldn’t be able to verify the email.
• One advantage of custom templates without using calldata is that the subjects are functional and somewhat human readable. If using calldata, we would want to make this as easy as possible for a guardian to understand.
  • Another thing that can be done in the email is that while the functional info is in the subject, the body of the email can break that info down into a more readable format. So if the subject was long and difficult to understand, that info could be repeated in the body but formatted in a nice way. With calldata bytes I think this would be particularly important so guardians don’t have to confirm an email with information that is difficult to understand.

The ideal solution would make integrating email recovery as easy as possible for developers, so having a generic recovery module like this for validators would be great. Given the constraint on the maximum length for the subject, and aiming to make the emails as human readable as possible, I’m wondering what other ideas could be explored to have a generic module that can take many different email subjects without passing a large calldata argument.

[JohnGuilding]

Was thinking about ways you could have a generic recovery module but keep custom templates.Perhaps you could define a function that returned the correct calldata for the validator function call. Maybe something like this:

    function getRecoveryCalldata(bytes[] calldata subjectParams) external override returns (bytes memory) {
        string memory functionSignature = "changeOwner(address,address,address)";

        address account = abi.decode(subjectParams[0], (address));
        address newOwner = abi.decode(subjectParams[1], (address));
        address recoveryModule = abi.decode(subjectParams[2], (address));

        return abi.encodeWithSignature(functionSignature, account, recoveryModule, newOwner);
    }

The logic that needs to be defined for a specific account implementation, if put in a file might be something like this. This contract contains a function for getting the subject templates, how to validate the subject templates, and getting the calldata for the recovery function call:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.25;

import { ISafe } from "./interfaces/ISafe.sol";

contract EmailRecoverySubjectHandler {

    error InvalidTemplateIndex();
    error InvalidSubjectParams();
    error InvalidOldOwner();
    error InvalidNewOwner();
    error InvalidRecoveryModule();

    constructor() {}

    function getRecoveryCalldata(bytes[] calldata subjectParams) external {
        string memory functionSignature = "swapOwner(address,address,address)";

        address account = abi.decode(subjectParams[0], (address));
        address oldOwner = abi.decode(subjectParams[1], (address));
        address newOwner = abi.decode(subjectParams[2], (address));

        address previousOwnerInLinkedList = getPreviousOwnerInLinkedList(account, oldOwner);

        bytes memory recoveryCallData =
            abi.encodeWithSignature(functionSignature, previousOwnerInLinkedList, oldOwner, newOwner);
    }

    function recoverySubjectTemplates() public pure returns (string[][] memory) {
        string[][] memory templates = new string[][](1);
        templates[0] = new string[](15);
        templates[0][0] = "Recover";
        templates[0][1] = "account";
        templates[0][2] = "{ethAddr}";
        templates[0][3] = "from";
        templates[0][4] = "old";
        templates[0][5] = "owner";
        templates[0][6] = "{ethAddr}";
        templates[0][7] = "to";
        templates[0][8] = "new";
        templates[0][9] = "owner";
        templates[0][10] = "{ethAddr}";
        templates[0][11] = "using";
        templates[0][12] = "recovery";
        templates[0][13] = "module";
        templates[0][14] = "{ethAddr}";
        return templates;
    }

    function validateRecoverySubjectTemplates(
        uint256 templateIdx,
        bytes[] memory subjectParams
    )
        internal
        view
        override
        returns (address)
    {
        if (templateIdx != 0) {
            revert InvalidTemplateIndex();
        }

        if (subjectParams.length != 4) {
            revert InvalidSubjectParams();
        }

        address accountInEmail = abi.decode(subjectParams[0], (address));
        address oldOwnerInEmail = abi.decode(subjectParams[1], (address));
        address newOwnerInEmail = abi.decode(subjectParams[2], (address));
        address recoveryModuleInEmail = abi.decode(subjectParams[3], (address));

        bool isOwner = ISafe(accountInEmail).isOwner(oldOwnerInEmail);
        if (!isOwner) {
            revert InvalidOldOwner();
        }
        if (newOwnerInEmail == address(0)) {
            revert InvalidNewOwner();
        }
        if (recoveryModuleInEmail == address(0)) {
            revert InvalidRecoveryModule();
        }

        return accountInEmail;
    }

    function getPreviousOwnerInLinkedList(
        address safe,
        address oldOwner
    )
        internal
        view
        returns (address)
    {
        address[] memory owners = ISafe(safe).getOwners();
        uint256 length = owners.length;

        uint256 oldOwnerIndex;
        for (uint256 i; i < length; i++) {
            if (owners[i] == oldOwner) {
                oldOwnerIndex = i;
                break;
            }
        }
        address sentinelOwner = address(0x1);
        return oldOwnerIndex == 0 ? sentinelOwner : owners[oldOwnerIndex - 1];
    }
}

I quite like the idea of letting the account developer define something like this subject handler contract - EmailRecoverySubjectHandler.sol. And then ValidatorRecoveryModule.sol & ZkEmailRecovery.sol can remain implementation agnostic. We could also do away with the inheritance needed to extend the ZkEmailRecovery, and have the handler defined separately. The issue with this approach is that we are constrained by the zkemail contract that ZkEmailRecovery inherits from - EmailAccountRecovery.sol. Which defines the subject templates there, and since those functions don't take any arguments, can't pass something like an address value to get the subject templates for a specific account.

[JohnGuilding]

Some additional thoughts on this approach:

• We could store reference to the calldata in a hash, and then validate that hash when recovery is executed
• looking at different approaches to recovery out there. I’ve seen cases where the calldata values (e.g. new owner) are validated when they get passed to the validator (e.g. zero address check), also seen an example where this validation wasn’t done and it was expected that the validation was done by the recovery module (this example was Clave but they are not technically 7579). So do we assume that all validators should validate inputs to the recover function? If not, then validation would be important to add in case an incorrect new owner or other value was passed to the validator
• could this flow work with Safe recovery? Or is the fact that safe owners are rotated on the account itself a constraint? I guess in this case you would have to pass the safe account address as the validator which might be confusing.

[JohnGuilding]

Resolved by #2