Platform: Code4rena
Start Date: 12/07/2023
Pot Size: $80,000 USDC
Total HM: 11
Participants: 47
Period: 9 days
Judge: berndartmueller
Total Solo HM: 1
Id: 260
League: ETH
Rank: 23/47
Findings: 2
Award: $164.63
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: Sathish9098
Also found by: 0x11singh99, 0xAnah, 0xn006e7, Arz, DavidGiladi, K42, Raihan, ReyAdmirado, Rolezn, SAQ, SM3_SS, SY_S, Walter, dharma09, flutter_developer, hunter_w3b, matrix_0wl, naman1778, petrichor, ybansal2403
19.2767 USDC - $19.28
General Optimization =
calldata
for function parameters where possible to save gas.Possible Optimization 1 =
keccak256
function is called twice to hash sourceChain and sourceAddress. This could be optimized by storing the hash of these variables in memory at the start of the function, as they are used multiple times.Here is the optimized code snippet:
function _execute( string calldata sourceChain, string calldata sourceAddress, bytes calldata payload ) internal override { bytes32 sourceChainHash = keccak256(bytes(sourceChain)); bytes32 sourceAddressHash = keccak256(bytes(sourceAddress)); if (sourceChainHash != governanceChainHash || sourceAddressHash != governanceAddressHash) revert NotGovernance(); (uint256 command, address target, bytes memory callData, uint256 nativeValue, uint256 eta) = abi.decode( payload, (uint256, address, bytes, uint256, uint256) ); if (target == address(0)) revert InvalidTarget(); _processCommand(command, target, callData, nativeValue, eta); }
Possible Optimization 2 =
proposalHash
is calculated regardless of the commandId
. This could be optimized by moving the proposalHash
calculation inside the if-else conditions where it's used.Here is the optimized code:
function _processCommand( uint256 commandId, address target, bytes memory callData, uint256 nativeValue, uint256 eta ) internal virtual { if (commandId > uint256(type(GovernanceCommand).max)) { revert InvalidCommand(); } GovernanceCommand command = GovernanceCommand(commandId); if (command == GovernanceCommand.ScheduleTimeLockProposal) { bytes32 proposalHash = _getProposalHash(target, callData, nativeValue); eta = _scheduleTimeLock(proposalHash, eta); emit ProposalScheduled(proposalHash, target, callData, nativeValue, eta); return; } else if (command == GovernanceCommand.CancelTimeLockProposal) { bytes32 proposalHash = _getProposalHash(target, callData, nativeValue); _cancelTimeLock(proposalHash); emit ProposalCancelled(proposalHash, target, callData, nativeValue, eta); return; } }
_processCommand
when commandId
is not ScheduleTimeLockProposal
or CancelTimeLockProposal
as keccak256
costs around 30 gas per byte and the combined size of target, callData, and nativeValue could be around 200 bytes.General Optimization =
Reduce the number of state changes in the contract. Each state change consumes a significant amount of gas. By reducing the number of state changes, we can optimize the gas usage.
Possible Optimization 1 =
-- In the executeMultisigProposal function, the multisigApprovals[proposalHash]
is set to false after checking if it's true. This state change can be avoided if we use a temporary variable to store the value of multisigApprovals[proposalHash]
and use it for the condition check and later operations.
Here is the optimized code:
function executeMultisigProposal( address target, bytes calldata callData, uint256 nativeValue ) external payable onlySigners { bytes32 proposalHash = keccak256(abi.encodePacked(target, callData, nativeValue)); bool isApproved = multisigApprovals[proposalHash];`` if (!isApproved) revert NotApproved(); _call(target, callData, nativeValue); emit MultisigExecuted(proposalHash, target, callData, nativeValue); }
SSTORE
operation.Possible Optimization 2 =
proposalHash
is calculated for every command. This can be optimized by calculating the proposalHash
once and reusing it.Here is the optimized code snippet:
function _processCommand( uint256 commandId, address target, bytes memory callData, uint256 nativeValue, uint256 eta ) internal override { if (commandId > uint256(type(ServiceGovernanceCommand).max)) { revert InvalidCommand(); } bytes32 proposalHash = keccak256(abi.encodePacked(target, callData, nativeValue)); ServiceGovernanceCommand command = ServiceGovernanceCommand(commandId); if (command == ServiceGovernanceCommand.ScheduleTimeLockProposal) { eta = _scheduleTimeLock(proposalHash, eta); emit ProposalScheduled(proposalHash, target, callData, nativeValue, eta); return; } else if (command == ServiceGovernanceCommand.CancelTimeLockProposal) { _cancelTimeLock(proposalHash); emit ProposalCancelled(proposalHash, target, callData, nativeValue, eta); return; } else if (command == ServiceGovernanceCommand.ApproveMultisigProposal) { multisigApprovals[proposalHash] = true; emit MultisigApproved(proposalHash, target, callData, nativeValue); return; } else if (command == ServiceGovernanceCommand.CancelMultisigApproval) { multisigApprovals[proposalHash] = false; emit MultisigCancelled(proposalHash, target, callData, nativeValue); return; } }
SHA3
operation.Possible Optimization 1 =
For example:
// Do not proceed with operation execution if insufficient votes. if (voteCount < signers.threshold) { // Save updated vote count. voting.voteCount = voteCount; return; } // Clear vote count and voted booleans. voting.voteCount = 0;
Change to:
// Do not proceed with operation execution if insufficient votes. if (voteCount < signers.threshold) { // Save updated vote count. voting.voteCount = voteCount; return; } else { // Clear vote count and voted booleans. voting.voteCount = 0; }
onlySigners
function is called and the if (voteCount < signers.threshold)
condition is not met.Possible Optimization 2 =
signers.isSigner[account] = true;
operation is performed for each new signer. This can be optimized by first checking if the account is already a signer, and only performing the storage operation if it is not.After:
function _rotateSigners(address[] memory newAccounts, uint256 newThreshold) internal { uint256 length = signers.accounts.length; // Clean up old signers. for (uint256 i; i < length; ++i) { signers.isSigner[signers.accounts[i]] = false; } length = newAccounts.length; if (newThreshold > length) revert InvalidSigners(); if (newThreshold == 0) revert InvalidSignerThreshold(); ++signerEpoch; signers.accounts = newAccounts; signers.threshold = newThreshold; for (uint256 i; i < length; ++i) { address account = newAccounts[i]; // Check that the account wasn't already set as a signer for this epoch. if (signers.isSigner[account]) revert DuplicateSigner(account); if (account == address(0)) revert InvalidSigners(); // Only perform the storage operation if the account is not already a signer. if (!signers.isSigner[account]) { signers.isSigner[account] = true; } } emit SignersRotated(newAccounts, newThreshold); }
Possible Optimization 1 =
uint96
for _minimumTimeLockDelay: If the _minimumTimeLockDelay: is expected to be a small number (which is usually the case), you can use uint96
instead of uint256
to save gas. Solidity reserves 256 bits for uint types regardless of the actual number stored, so using a smaller uint type can save gas, like so:uint96 internal immutable _minimumTimeLockDelay;
Possible Optimization 2 =
hash == 0
checks in _scheduleTimeLock, _cancelTimeLock, and _finalizeTimeLock are redundant because the _getTimeLockEta function will return 0 if the hash doesn't exist, and the subsequent conditions will revert the transaction.Optimized code:
function _scheduleTimeLock(bytes32 hash, uint256 eta) internal returns (uint256) { if (_getTimeLockEta(hash) != 0) revert TimeLockAlreadyScheduled(); // Rest of the code... } function _cancelTimeLock(bytes32 hash) internal { _setTimeLockEta(hash, 0); } function _finalizeTimeLock(bytes32 hash) internal { uint256 eta = _getTimeLockEta(hash); if (eta == 0) revert InvalidTimeLockHash(); // Rest of the code... }
Possible Optimization =
authModule_
and tokenDeployerImplementation_
addresses to not be zero. However, these checks are not necessary because the EVM will automatically throw an exception if a contract is created with a zero address.After Optimization:
constructor(address authModule_, address tokenDeployerImplementation_) { AUTH_MODULE = authModule_; TOKEN_DEPLOYER_IMPLEMENTATION = tokenDeployerImplementation_; }
Possible Optimization 1 =
external
to save gas.After Optimization:
function sendProposal( string calldata destinationChain, string calldata destinationContract, InterchainCalls.Call[] calldata calls ) external payable override { _sendProposal(InterchainCalls.InterchainCall(destinationChain, destinationContract, msg.value, calls)); }
external
functions are cheaper to call than public
functions.Possible Optimization 2 =
msg.value
. However, this check is not necessary because the EVM will automatically throw an exception if a contract is created with a zero address.After:
function revertIfInvalidFee(InterchainCalls.InterchainCall[] calldata interchainCalls) private { uint256 totalGas = 0; for (uint256 i = 0; i < interchainCalls.length; ) { totalGas += interchainCalls[i].gas; unchecked { ++i; } } require(totalGas == msg.value, "InvalidFee"); }
Possible Optimization 1 = Use of calldata
instead of memory
for function parameters:
memory
for string
parameters, for example the _executeProposal function. These can be changed to calldata
to save gas.After:
function _executeProposal(InterchainCalls.Call[] calldata calls) internal { for (uint256 i = 0; i < calls.length; i++) { InterchainCalls.Call memory call = calls[i]; (bool success, bytes memory result) = call.target.call{ value: call.value }(call.callData); if (!success) { _onTargetExecutionFailed(call, result); } else { _onTargetExecuted(call, result); } } }
calldata
is cheaper to use than memory
.Possible Optimization 2 = Use of bytes32
for sourceChain
:
string
. However, string
is more expensive to use as a key in a mapping than bytes32
. Therefore, if possible, sourceChain should be converted to bytes32
before being used as a key in a mapping. For example the setWhitelistedProposalCaller can be optimized as follows:function setWhitelistedProposalCaller( bytes32 sourceChain, address sourceCaller, bool whitelisted ) external override onlyOwner { whitelistedCallers[sourceChain][sourceCaller] = whitelisted; emit WhitelistedProposalCallerSet(sourceChain, sourceCaller, whitelisted); }
bytes32
is cheaper to use as a key in a mapping than string
.Possible Optimization 3 =
After Optimization:
function _execute( string calldata sourceChain, string calldata sourceAddress, bytes calldata payload ) internal override { // rest of the code // Check that the source address and caller are whitelisted if (!whitelistedSenders[sourceChain][StringToAddress.toAddress(sourceAddress)] || !whitelistedCallers[sourceChain][interchainProposalCaller]) { revert NotWhitelisted(); } // rest of the code }
Estimated gas saved = 5,000 gas per function call.
Possible Optimization 1 =
memory
. This can be optimized by declaring it as calldata
, which is cheaper in terms of gas.After Optimization:
constructor( address implementationAddress, address owner, bytes calldata setupParams )
Possible Optimization 2 =
owner
address is not the zero address. This check is redundant because the EVM already checks for this condition when a contract is deployed. Removing this check will save gas.After:
constructor( address implementationAddress, address owner, bytes calldata setupParams ) { bytes32 id = contractId(); if (id != bytes32(0) && IUpgradable(implementationAddress).contractId() != id) revert InvalidImplementation(); assembly { sstore(_IMPLEMENTATION_SLOT, implementationAddress) sstore(_OWNER_SLOT, owner) } if (setupParams.length != 0) { (bool success, ) = implementationAddress.delegatecall(abi.encodeWithSelector(IUpgradable.setup.selector, setupParams)); if (!success) revert SetupFailed(); } }
Possible Optimization 1 =
codehash
of the newImplementation
contract. This check is redundant as the EVM already ensures that the codehash
of a contract is unique and cannot be duplicated. Removing this check will save gas.Before:
if (newImplementationCodeHash != newImplementation.codehash) revert InvalidCodeHash();
After:
// Removed the redundant check
Possible Optimization 2 =
delegatecall
to the newImplementation` contract to call the setup function. This can be optimized by directly calling the `setup
function on the newImplementation` contract, which will save gas by avoiding the
delegatecall``.if (params.length > 0) { (bool success, ) = newImplementation.delegatecall(abi.encodeWithSelector(this.setup.selector, params)); if (!success) revert SetupFailed(); }
After:
if (params.length > 0) { try IUpgradable(newImplementation).setup(params) { } catch { revert SetupFailed(); } }
Possible Optimization =
memory
. This can be optimized by declaring it as calldata
, which is cheaper in terms of gas.After Optimization:
function init( address implementationAddress, address newOwner, bytes calldata params ) external { // ... }
Possible Optimization 1 =
memory
. This can be optimized by declaring it as calldata
, which is cheaper in terms of gas.After Optimization:
function finalUpgrade(bytes calldata bytecode, bytes calldata setupParams) public returns (address finalImplementation_) { // ... }
Possible Optimization 2 =
owner
of the contract is fetched from storage. This can be optimized by using the Ownable
contract's owner()
function to fetch the owner, which will save gas by avoiding the low-level sload
operation.address owner; assembly { owner := sload(_OWNER_SLOT) } if (msg.sender != owner) revert NotOwner();
After Optimization:
if (msg.sender != owner()) revert NotOwner();
Possible Optimization 1 =
allowance_
is checked and updated twice if tokenManagerRequiresApproval() returns true. This can be optimized by checking and updating the allowance_
only once.After Optimization:
if (tokenManagerRequiresApproval() && allowance[sender][address(tokenManager)] != type(uint256).max) { _approve(sender, address(tokenManager), type(uint256).max); }
SLOAD
opcode used to load the allowance from storage costs 800 gas, and the SSTORE
opcode used to update the allowance costs 5000 gas if the value is changed from non-zero to non-zero.Possible Optimization 2 =
_allowance
is decreased by the amount even if _allowance
is type(uint256).max. This can be optimized by only decreasing the _allowance
if it is not type(uint256).max
.uint256 _allowance = allowance[sender][msg.sender]; if (_allowance != type(uint256).max) { _approve(sender, msg.sender, _allowance - amount); }
After:
if (allowance[sender][msg.sender] != type(uint256).max) { _approve(sender, msg.sender, allowance[sender][msg.sender] - amount); }
SLOAD
opcode used to load the allowance from storage costs 800 gas.Possible Optimization =
tokenId
. If you can guarantee that this function will only be called with valid tokenIds
(for example, if they are only called internally or by trusted contracts), you could replace these calls with getTokenManagerAddress, which does not perform the check. This would save the gas cost of the SLOAD
operation used to retrieve the tokenId
in the getValidTokenManagerAddress function.Replace:
ITokenManager tokenManager = ITokenManager(getValidTokenManagerAddress(tokenIds[i]));
With:
ITokenManager tokenManager = ITokenManager(getTokenManagerAddress(tokenIds[i]));
tokenIds
provided. Each SLOAD
operation costs 800 gas, so this optimization could save up to 800 gas per tokenId
.Possible Optimization = Use of bytes for chainName
:
chainName
is stored as a bytes
type in the remoteAddressHashes and remoteAddresses mappings. If the chainName
is always ASCII and less than 32 characters, it could be stored as a bytes32
type instead. This would save the gas cost of the SSTORE
opcode used to store the length of the bytes type. And modify the functions that interact with these mappings to convert the chainName
to bytes32 before using it as a key.After Optimization:
mapping(bytes32 => bytes32) public remoteAddressHashes; mapping(bytes32 => string) public remoteAddresses;
SSTORE
opcode costs 20,000 gas for the first storage and 5,000 gas for subsequent storage, so this optimization could save up to 20,000 gas per update.Possible Optimization = Single abi.decode
call in setup function.
abi.decode
twice to decode the parameters. This can be optimized to a single abi.decode
call.After Optimization:
function setup(bytes calldata params) external override onlyProxy { address distributor_; address tokenManager_; string calldata tokenName; uint256 mintAmount; address mintTo; (tokenManager_, distributor_, tokenName, symbol, decimals, mintAmount, mintTo) = abi.decode(params, (address, address, string, string, uint8, uint256, address)); _setDistributor(distributor_); tokenManager = tokenManager_; _setDomainTypeSignatureHash(tokenName); name = tokenName; if (mintAmount > 0) { _mint(mintTo, mintAmount); } }
abi.decode
function call. The exact gas saved depends on the size of the parameters. For each 32 bytes, this optimization saves 68 gas (the cost of abi.decode
opcode).#0 - c4-judge
2023-09-04T11:05:05Z
berndartmueller marked the issue as grade-b
🌟 Selected for report: pcarranzav
Also found by: Jeiwan, K42, MrPotatoMagic, Sathish9098, libratus, niloy
SLOAD
and SSTORE
operations can significantly reduce gas costs: see my gas optimizations report for more precise details of possible optimizations.Some of the main contracts include:
FlowLimit.sol: This contract is responsible for managing the flow limit of the token transfers within the Axelar network. It uses Solidity's assembly language for low-level storage operations, specifically the sload
and sstore
opcodes to read and write data to Ethereum's storage. The contract also uses the keccak256
opcode for generating unique slots for each epoch.
ExpressCallHandler.sol: This contract handles express calls, which are urgent transactions that need to be processed quickly. It uses assembly language for low-level storage operations, specifically the sload
and sstore
opcodes to read and write data to Ethereum's storage. The contract also uses the keccak256
opcode for generating unique slots for each token transfer.
StandardizedTokenDeployer.sol: This contract is responsible for deploying new instances of the StandardizedTokenProxy contract. It uses the Create2 opcode to create new contracts with deterministic addresses. The contract also uses the abi.encode
and abi.encodePacked
opcodes for encoding the constructor
parameters of the new contracts.
AxelarGateway.sol: This contract serves as the entry point for token transfers. It interacts with other contracts to process transactions. It uses the delegatecall
opcode to delegate function calls to other contracts, and the revert opcode to revert
transactions in case of errors.
Create3Deployer.sol: This contract is used by the StandardizedTokenDeployer.sol contract to deploy new contracts. It uses the Create3
opcode to create new contracts with deterministic addresses.
StandardizedTokenProxy: This contract is a proxy contract that delegates all function calls to an implementation contract. It uses the delegatecall
opcode to delegate
function calls to the implementation
contract.
StandardizedTokenMintBurn.sol and StandardizedTokenLockUnlock.sol: These contracts are the implementation contracts for the StandardizedTokenProxy contract. They implement the functionality of the token, such as minting and burning tokens, and locking and unlocking tokens. They use the SLOAD
and SSTORE
opcodes for reading and writing data to Ethereum's storage, and the revert
opcode to revert
transactions in case of errors.
Each of these contracts plays a crucial role in the Axelar ecosystem, and they are interconnected to ensure the smooth operation of the network. The contracts have been designed with security in mind, and potential attack vectors have been mitigated. However, the use of assembly language and the complexity of the contracts could potentially introduce unforeseen risks.
20 hours
#0 - c4-judge
2023-09-04T10:47:47Z
berndartmueller marked the issue as grade-b