Request: Overload the createAccount function in CoinbaseSmartWalletFactory.sol

[kamescg]

Context

The ERC7579 standard is great because it allows third-parties to add new features and functionality to a smart account without changing the core logic. I'm assuming the Coinbase Smart Wallet support this standard or something similar at mainnet launch.

Proposal

Currently in the CoinbaseSmartWalletFactory.sol smart contract doesn't include support for enabling modules during setup.

function createAccount(bytes[] calldata owners, uint256 nonce)
        public
        payable
        virtual
        returns (CoinbaseSmartWallet account)
    {
        if (owners.length == 0) {
            revert OwnerRequired();
        }

        (bool alreadyDeployed, address accountAddress) =
            LibClone.createDeterministicERC1967(msg.value, implementation, _getSalt(owners, nonce));

        account = CoinbaseSmartWallet(payable(accountAddress));

        if (alreadyDeployed == false) {
            account.initialize(owners);
        }
    }

From a developer perspective it would be great if this function was also overloaded with calldata instructions argument so modules could be enabled during the deployment.

function createAccount(bytes[] calldata owners, uint256 nonce, bytes instructions)
        public
        payable
        virtual
        returns (CoinbaseSmartWallet account)
    {
        if (owners.length == 0) {
            revert OwnerRequired();
        }

        (bool alreadyDeployed, address accountAddress) =
            LibClone.createDeterministicERC1967(msg.value, implementation, _getSalt(owners, nonce));

        // 1. INSERT LOGIC FOR VALIDATING DEPLOYMENT SIGNATURES
        // 2. INSERT LOGIC FOR ENABLING MODULES

        account = CoinbaseSmartWallet(payable(accountAddress));

        if (alreadyDeployed == false) {
            account.initialize(owners);
        }
    }

As far as I understand ERC-4337 transaction bundling this could also be achieved by bundling a createAccount function call and subsequently a enableModule function call on the smart wallet directly, but I do think there is something to be said about the "developer experience" in simplifying this process.

This pattern can also be observed in the Safe smart account factory smart contracts but suffers from poor documentation and confusing to data field that uses the delegatecall functionality, which has personally lead to 6+ hours wasted debugging.

function setup(
        address[] calldata _owners,
        uint256 _threshold,
        address to,
        bytes calldata data,
        address fallbackHandler,
        address paymentToken,
        uint256 payment,
        address payable paymentReceiver
    ) external override {
        setupOwners(_owners, _threshold);
        if (fallbackHandler != address(0)) internalSetFallbackHandler(fallbackHandler);

        setupModules(to, data); // <- LOGIC TO ENABLE SAFE MODULES

        if (payment > 0) {
            handlePayment(payment, 0, 1, paymentToken, paymentReceiver);
        }
        emit SafeSetup(msg.sender, _owners, _threshold, to, fallbackHandler);
    }

Considerations

Allowing modules to be enabled during smart account creation without an additional authorization step introduces a "griefing" attack by allowing a "bad actor" to create a smart account on behalf of a user with a "malicious" module already enabled.

In a world with MEV and public mempools it's easy to front-run a smart account setup process with callback data for enabling arbitrary logic if module authorization is absent from the calldata.

Because of this the first step is authorize the setup with a signature by the owner(s) before completing the deployment process, which is why the "INSERT LOGIC FOR VALIDATING DEPLOYMENT SIGNATURES" comment is included in example overloaded function.

tl;dr

The proposed changes would make available two function calls:

• createAccount(bytes[] calldata owners, uint256 nonce)
• createAccount(bytes[] calldata owners, uint256 nonce, bytes instructions)

One for a vanilla deployment and another for authorized deployment with pre-enabled modules adhering to the ERC-7579 standard.

[kamescg]

alsooo i can't resist a good meme lol