Questions about the fallback function of BaseWallet

[m0t0k1ch1]

I am an engineer developing a contract wallet. The upgradability based on the Argent's modular structure is very advanced and smart compared to other contract wallets, so I'm using it as a reference for development. Thank you for making such a great product public.

...However, there are some codes that I cannot fully understand with my own knowledge. If possible, could you tell me about the design intents of them?

Specifically, I have the following questions about the fallback function of BaseWallet.

• Q1. Why isn't it reverted even if the enabled module doesn't exist?
  • For a normal contract without fallback, it is reverted if the specified function selector does not exist.
• Q2. Why are only static calls delegated?
  • Is there any significant risk if call is used instead of staticcall?
  • I understand that there is a risk of storage slot clash and function selector clash when delegatecall is used.

fallback() external payable {
    address module = enabled(msg.sig);
    if (module == address(0)) {
        emit Received(msg.value, msg.sender, msg.data);
    } else {
        require(authorised[module], "BW: unauthorised module");

        // solhint-disable-next-line no-inline-assembly
        assembly {
            calldatacopy(0, 0, calldatasize())
            let result := staticcall(gas(), module, 0, calldatasize(), 0, 0)
            returndatacopy(0, 0, returndatasize())
            switch result
            case 0 {revert(0, returndatasize())}
            default {return (0, returndatasize())}
        }
    }
}

[olivdb]

Hi @m0t0k1ch1!

For context, the mission of the BaseWallet's fallback is twofold: 1) To log an event any time the wallet is called. 2) To return an appropriate value when the wallet is called via specific static calls as defined by certain EIPs, such as EIP1271's isValidSignature(bytes32,bytes) or EIP721's onERC721Received(address,address,uint256,bytes).

To perform a call to a third party contract, we do NOT use the BaseWallet's fallback but we instead use the flow: [relayer or owner] -> module -> wallet -> contract.

Q1. Why isn't it reverted even if the enabled module doesn't exist? For a normal contract without fallback, it is reverted if the specified function selector does not exist.

This is because we want to log an event containing the msg.data in this case. This handles the case where someone attaches a "communication" hex string to their transaction to the wallet (e.g. "here is my contribution to last night's drinks").

Q2. Why are only static calls delegated? Is there any significant risk if call is used instead of staticcall?

Yes, if call was allowed, anyone could call a wallet's fallback to call the init() method of a module (which is protected by an onlyWallet modifier), and break the assumption that the init() method is only called once at the time a module is enabled for a wallet.

I understand that there is a risk of storage slot clash and function selector clash when delegatecall is used.

Likewise, if delegatecall was allowed, a wallet could call its own fallback to call init(). I don't think you'd be able to break anything doing that with the current ArgentModule module (since init() would be run in the context of the wallet and not in the context of the module) but that could break future module implementations.

[m0t0k1ch1]

@olivdb Thank you so much! Your answer is very helpful. 🙏🙏🙏

If you don't mind, can I ask you one more question related to Q2?

According to ERC721 and ERC1155, the following interfaces don't force the implementation to be view function.

function onERC721Received(address _operator, address _from, uint256 _tokenId, bytes _data) external returns(bytes4);
function onERC1155Received(address _operator, address _from, uint256 _id, uint256 _value, bytes calldata _data) external returns(bytes4);
function onERC1155BatchReceived(address _operator, address _from, uint256[] calldata _ids, uint256[] calldata _values, bytes calldata _data) external returns(bytes4);   

So, developers can implement these interfaces without view. For example, it is possible to transfer the received token to another account in onERCXXXReceived. However, since the fallback function of BaseWallet can delegate only static calls, it is impossible to implement the above interfaces without view. And such cases cannot be handled by the relayer or user -> module -> wallet -> contract flow.

• Q. If the problems with init (or any other function with onlyWallet modifier) can be ignored, do you think it's okay to support delegating other than static calls as above by using call (with some whitelist)? Are there any risks to consider?

(Of course, the benefits of supporting it may not be significant at this time.)

[olivdb]

I suppose you could but at the cost of increased complexity. If that requirement is key to your model, module-based wallet may not be the best architecture for you and you may want to consider other patterns (e.g. EIP2535).

[m0t0k1ch1]

Thank you for your advice. 🙇

I will also refer to EIP2535.