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
Rank: 11/51
Findings: 1
Award: $157.09
🌟 Selected for report: 1
🚀 Solo Findings: 0
🌟 Selected for report: K42
Also found by: 0x11singh99, 0xAnah, Hajime, SAQ, SM3_SS, albahaca, clara, dharma09, hunter_w3b, naman1778, shamsulhaq123, slvDev
157.0894 USDC - $157.09
Possible Optimization =
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(); }
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 Optimization 1 =
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)); }
Possible Optimization 2 =
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; } }
Possible Optimization 1 =
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)); }
external
function call overhead.Possible Optimization 2 =
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; } }
SLOAD
operation.Possible Optimization 1 =
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); }
abi.decode
calls.Possible Optimization 2 =
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(); } _; }
_checkOwner
.Possible Optimization 1 =
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... }
. The exact savings depend on the input size and the current gas price for the
SHA256`` precompile.Possible Optimization 2 =
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); }
staticcall
and the efficiency of the precompile.Possible Optimization 1 =
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) } }
MODEXP
precompile.Possible Optimization 2 =
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 ... }
Possible Optimization 1 =
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; }
Possible Optimization 2 =
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); }
SLOAD
and SSTORE
operations.Possible Optimization 3 =
_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; } }
#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