Coinbase Smart Wallet - K42's results

Smart Wallet from Coinbase Wallet

General Information

Platform: Code4rena

Start Date: 14/03/2024

Pot Size: $49,000 USDC

Total HM: 3

Participants: 51

Period: 7 days

Judge: 3docSec

Id: 350

League: ETH

Coinbase

Findings Distribution

Researcher Performance

Rank: 11/51

Findings: 1

Award: $157.09

🌟 Selected for report: 1

🚀 Solo Findings: 0

Findings Information

🌟 Selected for report: K42

Also found by: 0x11singh99, 0xAnah, Hajime, SAQ, SM3_SS, albahaca, clara, dharma09, hunter_w3b, naman1778, shamsulhaq123, slvDev

Labels

bug
G (Gas Optimization)
grade-a
high quality report
selected for report
sponsor acknowledged
G-11

Awards

157.0894 USDC - $157.09

External Links

Gas Optimization Report for Coinbase-Wallet by K42

  • Note: I made sure these optimizations are unique in relation to the Bot Report and 4Analy3er Report.

Possible Optimization in MultiOwnable.sol

Possible Optimization =

  • In the _checkOwner() function, the contract checks if msg.sender is an owner or the contract itself. By rearranging the conditions to check the cheaper condition first (direct address comparison), we can save gas when the sender is the contract itself, avoiding the more expensive storage lookup.

Here is the optimized code:

function _checkOwner() internal view virtual {
    if (msg.sender == address(this) || isOwnerAddress(msg.sender)) {
        return;
    }
    revert Unauthorized();
}
  • Estimated gas saved = This optimization may save a small amount of gas in scenarios where msg.sender is the contract itself, as it avoids the potentially more expensive isOwnerAddress call. The exact savings depend on the EVM's current gas pricing for storage reads and condition checks but could be around 200 gas for each invocation that short-circuits.

Possible Optimizations in ERC1271.sol

Possible Optimization 1 =

  • The _hashStruct() function is a simple wrapper around a keccak256 computation. Inlining this operation in _eip712Hash() can save the overhead of an external function call.

Here is the optimized code snippet:

function _eip712Hash(bytes32 hash) internal view virtual returns (bytes32) {
    // Inlined _hashStruct operation
    bytes32 hashStruct = keccak256(abi.encode(_MESSAGE_TYPEHASH, hash));
    return keccak256(abi.encodePacked("\x19\x01", domainSeparator(), hashStruct));
}
  • Estimated gas saved = Inlining can save around 200 or more gas per call by avoiding the external function call overhead.

Possible Optimization 2 =

  • The domainSeparator() function computes the EIP-712 domain separator every time it's called, which involves multiple keccak256 hash computations and abi.encode. Since the domain separator only changes with contract deployment (due to its dependency on chainId and address(this)), it can be cached after the first computation to save gas on subsequent calls.

Here is the optimized code:

bytes32 private cachedDomainSeparator;
bool private isDomainSeparatorCached;

function domainSeparator() public view returns (bytes32) {
    if (isDomainSeparatorCached) {
        return cachedDomainSeparator;
    } else {
        (string memory name, string memory version) = _domainNameAndVersion();
        bytes32 separator = keccak256(
            abi.encode(
                keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"),
                keccak256(bytes(name)),
                keccak256(bytes(version)),
                block.chainid,
                address(this)
            )
        );
        return separator;
    }
}
  • Estimated gas saved = This optimization can save approximately 500 or more gas for each call after the first, depending on the complexity of _domainNameAndVersion() and the size of the strings involved.

Possible Optimizations in CoinbaseSmartWalletFactory.sol

Possible Optimization 1 =

  • The _getSalt() function is used to generate a unique salt for each account creation, based on the owners and nonce. Inlining this function within createAccount() and getAddress() can reduce the overhead of an external function call.

Here is the optimized code snippet:

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

    // Inline _getSalt operation
    bytes32 salt = keccak256(abi.encode(owners, nonce));

    (bool alreadyDeployed, address accountAddress) =
        LibClone.createDeterministicERC1967(msg.value, implementation, salt);

    account = CoinbaseSmartWallet(payable(accountAddress));

    if (!alreadyDeployed) {
        account.initialize(owners);
    }
}

function getAddress(bytes[] calldata owners, uint256 nonce) external view returns (address predicted) {
    // Inline _getSalt operation
    bytes32 salt = keccak256(abi.encode(owners, nonce));

    predicted = LibClone.predictDeterministicAddress(initCodeHash(), salt, address(this));
}
  • Estimated gas saved = Inlining can save around 200 gas per call by avoiding the external function call overhead.

Possible Optimization 2 =

  • The initCodeHash() function computes the hash of the initialization code for the ERC1967 proxy used in account creation. This value depends solely on the immutable implementation address and thus remains constant. Caching this value can save gas for each account creation after the first.

Here is the optimized code:

bytes32 private cachedInitCodeHash;
bool private isInitCodeHashCached;

function initCodeHash() public view returns (bytes32 result) {
    if (isInitCodeHashCached) {
        return cachedInitCodeHash;
    } else {
        cachedInitCodeHash = LibClone.initCodeHashERC1967(implementation);
        isInitCodeHashCached = true;
        return cachedInitCodeHash;
    }
}
  • Estimated gas saved = This optimization can save approximately 800 or so gas for each call after the first, depending on the EVM's current gas pricing for storage reads and the SLOAD operation.

Possible Optimizations in CoinbaseSmartWallet.sol

Possible Optimization 1 =

  • The _validateSignature() function decodes the signature parameter multiple times to extract SignatureWrapper and then again for WebAuthn.WebAuthnAuth or address. This redundancy can be minimized by decoding once and passing the necessary components to other functions as needed.

Here is the optimized code snippet:

function _validateSignature(bytes32 message, bytes calldata signature)
    internal
    view
    override
    returns (bool)
{
    SignatureWrapper memory sigWrapper = abi.decode(signature, (SignatureWrapper));
    bytes memory ownerBytes = ownerAtIndex(sigWrapper.ownerIndex);

    // Directly pass decoded values to reduce redundant operations
    return _processSignature(message, ownerBytes, sigWrapper.signatureData);
}

function _processSignature(bytes32 message, bytes memory ownerBytes, bytes memory signatureData)
    internal
    view
    returns (bool)
{
    if (ownerBytes.length == 32) {
        address owner = abi.decode(ownerBytes, (address));
        return SignatureCheckerLib.isValidSignatureNow(owner, message, signatureData);
    } else if (ownerBytes.length == 64) {
        (uint256 x, uint256 y) = abi.decode(ownerBytes, (uint256, uint256));
        WebAuthn.WebAuthnAuth memory auth = abi.decode(signatureData, (WebAuthn.WebAuthnAuth));
        return WebAuthn.verify({challenge: abi.encode(message), requireUV: false, webAuthnAuth: auth, x: x, y: y});
    }
    revert InvalidOwnerBytesLength(ownerBytes);
}
  • Estimated gas saved = This optimization can save around 200 or so gas per validation by reducing the overhead of multiple abi.decode calls.

Possible Optimization 2 =

  • The onlyEntryPointOrOwner() modifier checks if msg.sender is the entry point and then calls _checkOwner which again checks if msg.sender is an owner. This can be optimized by consolidating the checks to avoid redundant msg.sender comparisons and storage reads.

Here is the optimized code:

modifier onlyEntryPointOrOwner() {
    if (msg.sender != entryPoint() && !isOwnerAddress(msg.sender) && msg.sender != address(this)) {
        revert Unauthorized();
    }
    _;
}
  • Estimated gas saved = This change can save approximately 100 or so gas per transaction by reducing the number of conditional checks and potentially avoiding a storage read in _checkOwner.

Possible Optimizations in WebAuthn.sol

Possible Optimization 1 =

  • The clientDataJSONHash is computed every time the verify() function is called. This hash computation involves the entire clientDataJSON string, which can be relatively large. By computing this hash once and passing it as a parameter to functions that require it, we can save gas.

Here is the optimized code snippet:

function verify(
    bytes memory challenge,
    bool requireUV,
    WebAuthnAuth memory webAuthnAuth,
    uint256 x,
    uint256 y
) internal view returns (bool) {
    bytes32 clientDataJSONHash = sha256(bytes(webAuthnAuth.clientDataJSON));
    // Use clientDataJSONHash in subsequent operations...
}
  • Estimated gas saved = This optimization can save approximately 200 or so gas depending on the size of ```clientDataJSON. The exact savings depend on the input size and the current gas price for the SHA256`` precompile.

Possible Optimization 2 =

  • The current implementation attempts to use a precompiled contract for signature verification and falls back to FCL.ecdsa_verify if the precompile call fails. This fallback mechanism can be optimized by checking the success of the precompile call more efficiently and avoiding unnecessary operations if the precompile verification is successful.

Here is the optimized code:

function verify(
    bytes memory challenge,
    bool requireUV,
    WebAuthnAuth memory webAuthnAuth,
    uint256 x,
    uint256 y
) internal view returns (bool) {
    // Compute messageHash and prepare args for precompile call...
    (bool success, bytes memory ret) = VERIFIER.staticcall(args);
    if (success && ret.length > 0) {
        return abi.decode(ret, (bool));
    }
    // Fallback to FCL.ecdsa_verify only if precompile call was not successful
    return FCL.ecdsa_verify(messageHash, webAuthnAuth.r, webAuthnAuth.s, x, y);
}
  • Estimated gas saved = This optimization can save gas by avoiding unnecessary decoding and logic when the precompile call is successful. The savings could be around 500 or more gas, depending on the EVM's current gas pricing for staticcall and the efficiency of the precompile.

Possible Optimizations in FCL.sol

Possible Optimization 1 =

  • The FCL_nModInv() and FCL_pModInv() functions use the MODEXP precompile for modular inversion, which involves setting up memory for the call. This setup can be optimized by reducing memory operations and directly using assembly for the entire operation.

Here is the optimized code snippet:

function optimizedModInv(uint256 u, uint256 modulus) internal view returns (uint256 result) {
    assembly {
        // Setup for MODEXP precompile in a compact form
        let freemem_pointer := mload(0x40)
        mstore(freemem_pointer, 0x20) // Length of Base
        mstore(add(freemem_pointer, 0x20), 0x20) // Length of Exponent
        mstore(add(freemem_pointer, 0x40), 0x20) // Length of Modulus
        mstore(add(freemem_pointer, 0x60), u) // Base: u
        mstore(add(freemem_pointer, 0x80), minus_2modn) // Exponent: p-2 for nModInv, minus_2 for pModInv
        mstore(add(freemem_pointer, 0xA0), modulus) // Modulus: n or p
        // Call MODEXP precompile
        if iszero(staticcall(not(0), MODEXP_PRECOMPILE, freemem_pointer, 0xC0, freemem_pointer, 0x20)) {
            revert(0, 0)
        }
        result := mload(freemem_pointer)
    }
}
  • Estimated gas saved = This optimization could save around 200 or so gas per call by streamlining the memory setup and reducing the overhead associated with preparing the call data for the MODEXP precompile.

Possible Optimization 2 =

  • The ecdsa_verify() function performs several checks that could be streamlined. The check for r and s being within valid ranges and the point being on the curve could be optimized by reordering operations or combining conditions.

Here is the optimized code:

function optimizedEcdsaVerify(bytes32 message, uint256 r, uint256 s, uint256 Qx, uint256 Qy) internal view returns (bool) {
    // Combine checks for efficiency
    if ((r == 0 || r >= n || s == 0 || s >= n) || !ecAff_isOnCurve(Qx, Qy)) {
        return false;
    }

    // Continue with the rest of the verification as before
    ...
}
  • Estimated gas saved = This change could save a few hundred gas by avoiding unnecessary condition evaluations and leveraging short-circuiting behaviour in logical operations.

Possible Optimizations in MagicSpend.sol

Possible Optimization 1 =

  • The isValidWithdrawSignature() function is called multiple times for the same WithdrawRequest. Caching the result of the first call and reusing it can save gas.

Here is the optimized code snippet:

mapping(bytes32 => bool) private _signatureVerificationCache;

function isValidWithdrawSignature(address account, WithdrawRequest memory withdrawRequest) public returns (bool) {
    bytes32 requestHash = getHash(account, withdrawRequest);
    if (_signatureVerificationCache[requestHash]) {
        return true;
    }
    bool isValid = SignatureCheckerLib.isValidSignatureNow(owner(), requestHash, withdrawRequest.signature);
    if (isValid) {
        _signatureVerificationCache[requestHash] = true;
    }
    return isValid;
}
  • Estimated gas saved = This optimization could save around 5,000 or so gas, for each subsequent verification of the same request, depending on the complexity of the signature verification process.

Possible Optimization 2 =

  • The _validateRequest() function checks if a nonce is used and then marks it as used. This operation can be optimized by checking and setting the nonce in a single operation to reduce the gas cost associated with storage access.

Here is the optimized code:

function _validateRequest(address account, WithdrawRequest memory withdrawRequest) internal {
    bytes32 nonceKey = keccak256(abi.encodePacked(account, withdrawRequest.nonce));
    require(!_nonceUsed[nonceKey], "InvalidNonce");
    _nonceUsed[nonceKey] = true;
    emit MagicSpendWithdrawal(account, withdrawRequest.asset, withdrawRequest.amount, withdrawRequest.nonce);
}
  • Estimated gas saved = This change could save a couple thousand gas by optimizing the current storage access patterns, as it reduces the number of SLOAD and SSTORE operations.

Possible Optimization 3 =

  • In the validatePaymasterUserOp() function, the contract updates _withdrawableETH mapping. This operation can be optimized by deferring updates until necessary, potentially batching them to save gas on storage operations.

Here is the optimized code snippet:

// Assume existence of a batch processing system
function _batchUpdateWithdrawableETH(address account, uint256 amount, bool increase) internal {
    if (increase) {
        _withdrawableETH[account] += amount;
    } else {
        require(_withdrawableETH[account] >= amount, "InsufficientBalance");
        _withdrawableETH[account] -= amount;
    }
}
  • Estimated gas saved = While the exact savings depend on the frequency and pattern of updates, deferring and batching updates could save thousands of gas by reducing the number of storage operations.

#0 - raymondfam

2024-03-22T21:58:23Z

Well elaborate optimization suggested for each of the in scope contracts.

#1 - c4-pre-sort

2024-03-22T21:58:28Z

raymondfam marked the issue as high quality report

#2 - c4-sponsor

2024-03-26T15:27:01Z

wilsoncusack (sponsor) acknowledged

#3 - c4-judge

2024-03-27T13:20:31Z

3docSec marked the issue as grade-a

#4 - c4-judge

2024-03-27T13:36:04Z

3docSec marked the issue as selected for report

#5 - 3docSec

2024-03-27T13:36:50Z

Best value on top of the gas findings reported by the bot

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax © 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter