Platform: Code4rena
Start Date: 30/05/2023
Pot Size: $300,500 USDC
Total HM: 79
Participants: 101
Period: about 1 month
Judge: Trust
Total Solo HM: 36
Id: 242
League: ETH
Rank: 31/101
Findings: 2
Award: $1,413.76
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: 0xSmartContract
Also found by: 3kus-iosiro, Audit_Avengers, ByteBandits, IllIllI, Kamil-Chmielewski, Madalad, RED-LOTUS-REACH, Rolezn, Sathish9098, Stormreckson, Udsen, bin2chen, brgltd, ihtishamsudo, kaveyjoe, kodyvim, lukejohn, matrix_0wl, mgf15, nadin
803.432 USDC - $803.43
| Low Count | Issues | Instances |
|---|---|---|
| [L-1] | Lack of CEI (Check-Effects-Interactions) might be susceptible to a reentrancy attack | 3 |
| [L-2] | address(getUlyssesLP(routes[0].from).asset()).safeTransferFrom(msg.sender, address(this), amount) only transfer first value | 1 |
| [L-3] | Incorrect Handling of Negative Values in uniswapV3SwapCallback() Function | 2 |
| [L-4] | addPartner() and addVault() Functions Lack Address Existence Check in partners and vaults Arrays | 1 |
| [L-5] | Invalid getUserGaugeBoost[user][msg.sender] when totalSupply is Zero | 1 |
| [L-6] | Unchecked Subtraction of uint32 from uint in incrementFreezeWindow() Function May Lead to Unexpected Results | 1 |
| [L-7] | decimals() not part of ERC20 standard | 1 |
| [L-8] | Potential Array Index Out of Bounds Exception Due to Missing Length Check in assets and assetsAmounts Arrays | 1 |
| [L-9] | Missing Token Deposit Limit in the deposit() Function | 1 |
| [L-10] | distribute() Function Lacks Gauge Identification for Incentive Creation | 1 |
| [L-11] | Default Strategy in addStrategyForRewards() Function May Not Suit the Gauge | 1 |
| [L-12] | Inability to Remove Flywheel in createBribeFlywheel Function Poses Limitation | 1 |
| [L-13] | Tautology when checking msg.sender | 1 |
| [L-14] | Ensuring Proper Verification of Callee Address in Constructor to Avoid Expected Revert | 1 |
| [L-15] | Timelocks not implemented as per documentation | - |
| [L-16] | Unexpected Results when Converting Negative Values using uint256(int256) | 2 |
| [L-17] | Lack of Pool, token Removal Mechanism in UlyssesFactory Contract | 1 |
| [L-18] | Update depositNonce Value Incrementation in every Function Calls | 3 |
| [L-19] | Unused/empty fallback() function | 2 |
| [L-20] | withdraw() should use lock modifier to avoid reentrancy attack | 1 |
| [L-21] | Non-zero approval may be problems for some tokens like USDT | 1 |
| [L-22] | Contract existence of target[i] not checked before low level .call function | 1 |
| [L-23] | Gas griefing/theft is possible on unsafe external call | 1 |
| [L-24] | Use .call instead of .transfer to send ether | 1 |
| [L-25] | Return values of transfer()/transferFrom() not checked | 2 |
| NC Count | Issues | Instances |
|---|---|---|
| [NC-1] | Add an event for critical parameter changes | 4 |
| [NC-2] | Function does not followed the standard solidity naming | 1 |
| [NC-3] | No same value input control | 4 |
| [NC-4] | Status of approve() functions not checked | 2 |
| [NC-5] | Rectify all warnings to avoid unexpected behavior | - |
CEI (Check-Effects-Interactions) might be susceptible to a reentrancy attackToken withdrawal is performed by calling the withdraw function of the IPortStrategy contract with the specified parameters. Following that, the getPortStrategyTokenDebt and getStrategyTokenDebt mappings are updated by subtracting the amountToWithdraw from their respective values.it might be susceptible to a reentrancy attack. In a reentrancy attack, an external contract can maliciously call back into the contract during the execution of the withdrawal process, potentially leading to unexpected behavior and security vulnerabilities.
FILE: 2023-05-maia/src/ulysses-omnichain/BranchPort.sol 180: IPortStrategy(_strategy).withdraw(address(this), _token, amountToWithdraw); 181: 182: getPortStrategyTokenDebt[_strategy][_token] -= amountToWithdraw; 183: getStrategyTokenDebt[_token] -= amountToWithdraw;
CEI pattern could involve updating the necessary state variables, such as getPortStrategyTokenDebt and getStrategyTokenDebt, before invoking the withdraw function of the IPortStrategy contract
FILE: 2023-05-maia/src/ulysses-omnichain/BranchPort.sol + 182: getPortStrategyTokenDebt[_strategy][_token] -= amountToWithdraw; + 183: getStrategyTokenDebt[_token] -= amountToWithdraw; 180: IPortStrategy(_strategy).withdraw(address(this), _token, amountToWithdraw); 181: - 182: getPortStrategyTokenDebt[_strategy][_token] -= amountToWithdraw; - 183: getStrategyTokenDebt[_token] -= amountToWithdraw;
address(getUlyssesLP(routes[0].from).asset()).safeTransferFrom(msg.sender, address(this), amount) only transfer first valueThe swap function you provided has a transfer statement that only transfers the amount from msg.sender to the contract address for routes[0].from. This means that it transfers the amount for only the first element in the routes array, and subsequent elements are not accounted for in the transfer.
FILE: 2023-05-maia/src/ulysses-amm/UlyssesRouter.sol function swap(uint256 amount, uint256 minOutput, Route[] calldata routes) external returns (uint256) { address(getUlyssesLP(routes[0].from).asset()).safeTransferFrom(msg.sender, address(this), amount); uint256 length = routes.length; for (uint256 i = 0; i < length;) { amount = getUlyssesLP(routes[i].from).swapIn(amount, routes[i].to); unchecked { ++i; } } if (amount < minOutput) revert OutputTooLow(); unchecked { --length; } address(getUlyssesLP(routes[length].to).asset()).safeTransfer(msg.sender, amount); return amount; }
Swap function that incorporates the loop to transfer amounts for all elements in the routes array.
for (uint256 i = 0; i < routes.length; i++) { address(getUlyssesLP(routes[i].from).asset()).safeTransferFrom(msg.sender, address(this), amount); }
Incorrect Handling of Negative Values in uniswapV3SwapCallback() FunctionThe function uniswapV3SwapCallback() does not handle negative values in the amount0 and amount1 parameters in the right way.
The function checks if the value of amount0 or amount1 is equal to zero, and if it is, it reverts. However, this does not prevent the function from being called with negative values.
As per docs the function should check if the value of amount0 or amount1 is less than zero, and if it is, it should send the tokens to the pool. If the value is greater than zero, it should receive the tokens from the pool
FILE: 2023-05-maia/src/talos/base/TalosBaseStrategy.sol function uniswapV3SwapCallback(int256 amount0, int256 amount1, bytes calldata _data) external { if (msg.sender != address(pool)) revert CallerIsNotPool(); if (amount0 == 0 && amount1 == 0) revert AmountsAreZero(); SwapCallbackData memory data = abi.decode(_data, (SwapCallbackData)); bool zeroForOne = data.zeroForOne; if (zeroForOne) address(token0).safeTransfer(msg.sender, uint256(amount0)); else address(token1).safeTransfer(msg.sender, uint256(amount1)); }
Need to implement controls to handle negative values also
addPartner() and addVault() Functions Lack Address Existence Check in partners and vaults ArraysIn Solidity, arrays can accept duplicate values. There are no inherent restrictions or limitations on storing duplicate values in arrays.
Create confusion and uncertainty about which PartnerManager, IBaseVault accounts is the legitimate one.This could make it difficult for users to interact with the contract or for partners to coordinate their activities
Increase the risk of errors and inconsistencies. If multiple PartnerManager,IBaseVault accounts are able to approve or deny transactions, it could be difficult to track which account has approved or denied a particular transaction. This could lead to errors and inconsistencies in the contract's state.
FILE: 2023-05-maia/src/maia/factories/PartnerManagerFactory.sol function addPartner(PartnerManager newPartnerManager) external onlyOwner { uint256 id = partners.length; partners.push(newPartnerManager); partnerIds[newPartnerManager] == id; emit AddedPartner(newPartnerManager, id); } function addVault(IBaseVault newVault) external onlyOwner { uint256 id = vaults.length; vaults.push(newVault); vaultIds[newVault] == id; emit AddedVault(newVault, id); }
This code first checks if the newPartnerManager,newVault addresses already exists in the partners, vaults arrays
```solidity if (partners[partnerIds[partnerManager]] == newPartnerManager) revert InvalidPartnerManager(); if (vaults[vaultIds[vault]] == newVault) revert InvalidVault();
getUserGaugeBoost[user][msg.sender] when totalSupply is ZeroThe function getUserGaugeBoost[user][msg.sender] will always be invalid if the total supply of the gauge is zero. This is because the totalGaugeBoost field in the GaugeState struct is initialized with the total supply of the gauge. If the total supply is zero, then the totalGaugeBoost field will also be zero, which will make the getUserGaugeBoost[user][msg.sender] value invalid
FILE: Breadcrumbs2023-05-maia/src/erc-20/ERC20Boost.sol 131: getUserGaugeBoost[user][msg.sender] = 132: GaugeState({userGaugeBoost: userGaugeBoost, totalGaugeBoost: totalSupply.toUint128()});
The function could be modified to check if the total supply of the gauge is greater than zero before initializing the totalGaugeBoost field. This would prevent the getUserGaugeBoost[user][msg.sender] value from being invalid
if (totalSupply > 0) { getUserGaugeBoost[user][msg.sender] = GaugeState({userGaugeBoost: userGaugeBoost, totalGaugeBoost: totalSupply.toUint128()}); }
Unchecked Subtraction of uint32 from uint in incrementFreezeWindow() Function May Lead to Unexpected ResultsThe function unchecked { if (cycle - block.timestamp <= incrementFreezeWindow) revert IncrementFreezeError(); } in that unchecked keyword tells the compiler to not check for overflows. This means that if the subtraction of cycle and block.timestamp overflows, the function will not revert.
This may be not problem now but may cause problems in the future .
FILE: 2023-05-maia/src/erc-20/ERC20Gauges.sol 204: unchecked { 205: if (cycle - block.timestamp <= incrementFreezeWindow) revert IncrementFreezeError(); 206: }
Remove unchecked from cycle - block.timestamp subtraction operation. If any overflow/Underflows this will revert
decimals() not part of ERC20 standarddecimals() is not part of the official ERC20 standard and might fail for tokens that do not implement it. While in practice it is very unlikely, as usually most of the tokens implement it, this should still be considered as a potential issue.
FILE: Breadcrumbs2023-05-maia/src/erc-4626/ERC4626MultiToken.sol 51: require(ERC20(_assets[i]).decimals() == 18);
This code is more robust than the original code because it will not fail if the token does not implement the decimals() function.
if (erc20(_assets[i]).supportsInterface(IERC20Decimals(1))) { require(erc20(_assets[i]).decimals() == 18); } else { revert('Token does not implement IERC20Decimals'); }
This code will revert if the token does not support the IERC20Decimals interface or if the decimals property of the token is not equal to 18. This ensures that the token will always have the correct decimals property.
Array Index Out of Bounds Exception Due to Missing Length Check in assets and assetsAmounts ArraysThe issue is that the assets[i] variable is a state variable, while the assetsAmounts variable is a user-defined array. This means that there is a chance that the two variables could be mismatched. If the two variables are mismatched, then the safeTransferFrom function could revert.
If the two variables length are mismatched, then an array index out of bound exception may occur
FILE: 2023-05-maia/src/erc-4626/ERC4626MultiToken.sol 66: function receiveAssets(uint256[] memory assetsAmounts) private { 67: uint256 length = assetsAmounts.length; 68: for (uint256 i = 0; i < length;) { 69: assets[i].safeTransferFrom(msg.sender, address(this), assetsAmounts[i]); 70: 71: unchecked { 72: i++; 73: }
Add the length check
require(assets.length==assetsAmounts.length, "The length mismatched");
Token Deposit Limit in the deposit() FunctionThe deposit() function does not check to see if the total amount of tokens that the user is trying to deposit is greater than the total supply of tokens in the contract.
If an attacker were to deposit a large amount of tokens, it could cause the contract to run out of funds. This could also cause the contract to become unstable.
FILE: Breadcrumbs2023-05-maia/src/erc-4626/ERC4626MultiToken.sol function deposit(uint256[] calldata assetsAmounts, address receiver) public virtual nonReentrant returns (uint256 shares) { // Check for rounding error since we round down in previewDeposit. require((shares = previewDeposit(assetsAmounts)) != 0, "ZERO_SHARES"); // Need to transfer before minting or ERC777s could reenter. receiveAssets(assetsAmounts); _mint(receiver, shares); emit Deposit(msg.sender, receiver, assetsAmounts, shares); afterDeposit(assetsAmounts, shares); }
You could add a limit to the amount of tokens that can be deposited
require(shares <= maxSharesAllowed, "INSUFFICIENT_BALANCE");
distribute() Function Lacks Gauge Identification for Incentive CreationThe function distribute is not safe. The attacker could call the function and create an incentive from a gauge that they control. This could allow the attacker to steal the incentive funds.
FILE: 2023-05-maia/src/gauges/UniswapV3Gauge.sol 53: function distribute(uint256 amount) internal override { 54: IUniswapV3Staker(uniswapV3Staker).createIncentiveFromGauge(amount); 55: }
If the gauge parameter is not null, then the function will call the createIncentiveFromGauge function of the IUniswapV3Staker contract. The createIncentiveFromGauge function will create an incentive from the specified gauge and deposit the incentive funds into the contract.
This code will make the distribute function more secure and will prevent an attacker from stealing the incentive funds.
function distribute(uint256 amount, address gauge) internal override { IUniswapV3Staker(uniswapV3Staker).createIncentiveFromGauge(amount, gauge); }
addStrategyForRewards() Function May Not Suit the GaugeUsing a default strategy that is not appropriate for the gauge could be significant. For example, if the default strategy is designed for a different type of gauge, it could result in the incentive funds being distributed in an incorrect manner. This could lead to a number of problems,
FILE: 2023-05-maia/src/gauges/factories/BribesFactory.sol function addGaugetoFlywheel(address gauge, address bribeToken) external onlyGaugeFactory { if (address(flywheelTokens[bribeToken]) == address(0)) createBribeFlywheel(bribeToken); flywheelTokens[bribeToken].addStrategyForRewards(ERC20(gauge)); }
You could modify the function to allow the caller to specify the strategy that they want to add to the flywheel
Inability to Remove Flywheel in createBribeFlywheel Function Poses LimitationThis could be a problem if the flywheel is no longer needed or if it is no longer being used.
FILE: 2023-05-maia/src/gauges/factories/BribesFactory.sol function createBribeFlywheel(address bribeToken) public { if (address(flywheelTokens[bribeToken]) != address(0)) revert BribeFlywheelAlreadyExists(); FlywheelCore flywheel = new FlywheelCore( bribeToken, FlywheelBribeRewards(address(0)), flywheelGaugeWeightBooster, address(this) ); flywheelTokens[bribeToken] = flywheel; uint256 id = bribeFlywheels.length; bribeFlywheels.push(flywheel); bribeFlywheelIds[flywheel] = id; activeBribeFlywheels[flywheel] = true; flywheel.setFlywheelRewards(address(new FlywheelBribeRewards(flywheel, rewardsCycleLength))); emit BribeFlywheelCreated(bribeToken, flywheel); }
Implement deleteBribeFlywheel() function to remove flywheel is no longer needed or if it is no longer being used.
Tautology when checking msg.senderWhile taking into account the address(0) output of msg.sender is something that must always be done: in this particular case the check is a tautology
Indeed:
msg.sender == pendingAdmin is true, then msg.sender != address(0) will always be truemsg.sender == pendingAdmin is false, then msg.sender != address(0) will never be evaluatedFILE: 2023-05-maia/src/governance/GovernorBravoDelegateMaia.sol 507: require( 508: msg.sender == pendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending admin only" 509: );
Callee Address in Constructor to Avoid Expected RevertdelegateTo function is called in the constructor, it means that it is being invoked during the contract deployment process. In that case, it is important to ensure that the callee address passed to the function is indeed a valid contract address.
FILE: Breadcrumbs2023-05-maia/src/governance/GovernorBravoDelegator.sol constructor( address timelock_, address govToken_, address admin_, address implementation_, uint256 votingPeriod_, uint256 votingDelay_, uint256 proposalThreshold_ ) public { // Admin set to msg.sender for initialization admin = msg.sender; delegateTo( implementation_, abi.encodeWithSignature( "initialize(address,address,uint256,uint256,uint256)", timelock_, govToken_, votingPeriod_, votingDelay_, proposalThreshold_ ) ); _setImplementation(implementation_); admin = admin_; }
Include the validation before calling the delegateTo
require(Address.isContract(callee), "callee is not a contract"); // Check if callee is a contract
Timelocks not implemented as per documentation``Does it use a timelock function?: true``
The documentation for the contract states that there are timelocks in place to prevent users from withdrawing their collateral asset immediately after depositing it. However, the code for the contract does not actually implement any timelocks.
This is a potential security vulnerability, as it could allow a user to withdraw their collateral asset immediately after depositing it, even if the collateral asset has not yet reached the required level of liquidity.
Add the timelocks for critical setter functions as per documentation
Unexpected Results when Converting Negative Values using uint256(int256)When converting a negative int256 value to a uint256 using uint256(int256), the result will be unexpected. if you try to convert a negative int256 value to a uint256, the result will be interpreted as the two's complement representation of the negative value. This means that the resulting uint256 value will be a large positive number.
PROOF:
We attempt to convert the negative value -10 to a uint256. However, the resulting value will not be the expected value of -10 but rather a large positive number.
If you execute the function, the returned value will be 115792089237316195423570985008687907853269984665640564039457584007913129639934. This result corresponds to the two's complement representation of -10 when interpreted as an unsigned uint256.
FILE: 2023-05-maia/src/talos/base/TalosBaseStrategy.sol 339: if (zeroForOne) address(token0).safeTransfer(msg.sender, uint256(amount0)); 340: else address(token1).safeTransfer(msg.sender, uint256(amount1));
Carefully handle conversions between signed and unsigned integer types and ensure that the values are within the valid ranges of the target type.
Pool, token Removal Mechanism in UlyssesFactory ContractTypically, in decentralized exchange protocols, such as the Uniswap protocol, there is a mechanism to remove pools that are no longer desired or have become inactive. This removal mechanism is important for several reasons, including reducing clutter and maintaining an up-to-date list of active and relevant pools.
Without a pool removal mechanism in the UlyssesFactory contract, inactive or unwanted pools may persist indefinitely, leading to a cluttered pool list. This could potentially cause confusion for users and impact the overall efficiency of the protocol.
Consider implementing a mechanism within the UlyssesFactory contract that allows for the removal of pools,tokens
depositNonce Value Incrementation in every Function CallsThe function uses the depositNonce variable to encode the data for the cross-chain call. However, the depositNonce variable always returns the current value, not the incremented value. This means that if the function is called multiple times, the same nonce will be used for each call. This could lead to a replay attack
FILE: 2023-05-maia/src/ulysses-omnichain/BranchBridgeAgent.sol 226: bytes memory packedData = abi.encodePacked( 227: bytes1(0x04), msg.sender, depositNonce, _params, msg.value.toUint128(), _remoteExecutionGas 228: ); 248: depositNonce, bytes memory packedData = abi.encodePacked( bytes1(0x06), msg.sender, uint8(_dParams.hTokens.length), depositNonce, _dParams.hTokens, _dParams.tokens, _dParams.amounts, _deposits, _dParams.toChain, _params, msg.value.toUint128(), _remoteExecutionGas );
Ensures that a different nonce is used for each call
depositNonce++
If the intention is for the Ether to be used, the function should call another function, otherwise it should revert. Having no access control on the function means that someone may send Ether to the contract, and have no way to get anything back out, which is a loss of funds.
FILE: 2023-05-maia/src/ulysses-omnichain/BranchBridgeAgent.sol 1419: fallback() external payable {}
withdraw() should use lock modifier to avoid reentrancy attackThe function withdraw() should use the lock modifier to avoid reentrancy attacks. The lock modifier will prevent the contract from being called again while it is still executing. This will help to protect the contract from being exploited by an attacker.
FILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/BranchPort.sol function withdraw(address _recipient, address _underlyingAddress, uint256 _deposit) external virtual requiresBridgeAgent modifier lock() { require(_unlocked == 1); _unlocked = 2; _; _unlocked = 1; }
Use lock modifier for withdraw() function
problems for some tokens like USDT// Addresses of the output hTokens this tokens can be anything
There are some tokens like USDT did not allow to approve allowance from non-zero value to non-zero value.If you try to approve another non-zero value when it's already had allowance, TX will fail.
FILE: 2023-05-maia/src/ulysses-omnichain/BranchPort.sol 120: ERC20hTokenRoot(outputToken).approve(bridgeAgentAddress, amountOut); 148: ERC20hTokenRoot(outputTokens[i]).approve(bridgeAgentAddress, amountsOut[i]);
Add an approve(0) before approving
target[i] not checked before low level .call functionThe existence of target[i] is not checked before making the .call function. This could potentially lead to issues if target[i] is not a valid contract address. If11 target[i]11 does not point to a valid contract, the .call function may revert, resulting in a failed transaction.
FILE: 2023-05-maia/src/ulysses-omnichain/VirtualAccount.sol 49:(bool success, bytes memory data) = calls[i].target.call(calls[i].callData);
Check contract existence before .call
address targetAddress = calls[i].target; require(Address.isContract(targetAddress), "Invalid contract address");
griefing/theft is possible on unsafe external callhttps://twitter.com/pashovkrum/status/1607024043718316032?t=xs30iD6ORWtE2bTTYsCFIQ&s=19
If the external contract at calls[i].target is a malicious contract and specifically designed to consume excessive gas, it can manipulate the execution flow to consume more gas than necessary, resulting in a higher gas cost for your transaction. This gas cost cannot be fully recovered because the unused gas is not returned due to the malicious contract's behavior
FILE: 2023-05-maia/src/ulysses-omnichain/VirtualAccount.sol 49: (bool success, bytes memory data) = calls[i].target.call(calls[i].callData);
Usage like below, ‘out’ and ‘outsize’ values are given (0,0) . Thus, this storage disappears and may come from external contracts a possible Gas griefing/theft problem is avoided.
.call instead of .transfer to send ether.transfer will relay 2300 gas and .call will relay all the gas. If the receive/fallback function from the recipient proxy contract has complex logic, using .transfer will fail, causing integration issues.
FILE: 2023-05-maia/src/talos/base/TalosBaseStrategy.sol 409: if (amount0 > 0) _token0.transfer(msg.sender, amount0); 410: if (amount1 > 0) _token1.transfer(msg.sender, amount1);
Use .call instead of .transfer
Return values of transfer()/transferFrom() not checkedIf the return value of these functions is not checked, potential errors or failures during the token transfer can go unnoticed. This can lead to unexpected behavior and vulnerabilities in the contract
FILE: 2023-05-maia/src/talos/base/TalosBaseStrategy.sol 409: if (amount0 > 0) _token0.transfer(msg.sender, amount0); 410: if (amount1 > 0) _token1.transfer(msg.sender, amount1);
Check the return value of transfer() and transferFrom() and handle any failed transfers appropriately
Adding events for critical parameter changes will facilitate offchain monitoring and indexing.
The external function names not start with _
The setter functions does not have any checks to prevent the same value from being passed. This means that an onlyOwner could call the function with the same address, which would effectively do nothing
approve() functions not checkedWhen using the approve() function to grant an allowance for a specific address to spend tokens on behalf of the sender, it is crucial to verify the return value of the function
FILE: 2023-05-maia/src/ulysses-omnichain/BranchPort.sol 120: ERC20hTokenRoot(outputToken).approve(bridgeAgentAddress, amountOut); 148: ERC20hTokenRoot(outputTokens[i]).approve(bridgeAgentAddress, amountsOut[i]);
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> src/governance/GovernorBravoDelegator.sol:7:1: | 7 | contract GovernorBravoDelegator is GovernorBravoDelegatorStorage, GovernorBravoEvents { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> src/governance/GovernorBravoDelegator.sol:71:5: | 71 | fallback() external payable { | ^ (Relevant source part starts here and spans across multiple lines).
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> src/out-of-scope/governance/GovernorBravoDelegator.sol:6:1: | 6 | contract GovernorBravoDelegator is GovernorBravoDelegatorStorage, GovernorBravoEvents { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> src/out-of-scope/governance/GovernorBravoDelegator.sol:70:5: | 70 | fallback() external payable { | ^ (Relevant source part starts here and spans across multiple lines).
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> src/ulysses-omnichain/BranchBridgeAgent.sol:58:1: | 58 | contract BranchBridgeAgent is IBranchBridgeAgent { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> src/ulysses-omnichain/BranchBridgeAgent.sol:1419:5: | 1419 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> src/ulysses-omnichain/ArbitrumBranchBridgeAgent.sol:67:1: | 67 | contract ArbitrumBranchBridgeAgent is BranchBridgeAgent { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> src/ulysses-omnichain/BranchBridgeAgent.sol:1419:5: | 1419 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> src/ulysses-omnichain/CoreBranchRouter.sol:19:1: | 19 | contract CoreBranchRouter is BaseBranchRouter { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> src/ulysses-omnichain/CoreBranchRouter.sol:284:5: | 284 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> src/ulysses-omnichain/ArbitrumCoreBranchRouter.sol:37:1: | 37 | contract ArbitrumCoreBranchRouter is CoreBranchRouter { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> src/ulysses-omnichain/CoreBranchRouter.sol:284:5: | 284 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> src/ulysses-omnichain/RootBridgeAgent.sol:90:1: | 90 | contract RootBridgeAgent is IRootBridgeAgent { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> src/ulysses-omnichain/RootBridgeAgent.sol:1334:5: | 1334 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> test/ulysses-omnichain/mocks/MockRootBridgeAgent.t.sol:23:1: | 23 | contract MockRootBridgeAgent is RootBridgeAgent { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> src/ulysses-omnichain/RootBridgeAgent.sol:1334:5: | 1334 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> test/ulysses-omnichain/mocks/MockBranchBridgeAgent.t.sol:16:1: | 16 | contract MockBranchBridgeAgent is BranchBridgeAgent { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> src/ulysses-omnichain/BranchBridgeAgent.sol:1419:5: | 1419 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> test/ulysses-omnichain/ArbitrumBranchTest.t.sol:42:1: | 42 | contract ArbitrumBranchTest is DSTestPlus { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> test/ulysses-omnichain/ArbitrumBranchTest.t.sol:43:5: | 43 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> test/ulysses-omnichain/BranchBridgeAgentTest.t.sol:21:1: | 21 | contract BranchBridgeAgentTest is Test { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> test/ulysses-omnichain/BranchBridgeAgentTest.t.sol:87:5: | 87 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (3628): Warning: This contract has a payable fallback function, but no receive ether function. Consider adding a receive ether function. --> test/ulysses-omnichain/RootTest.t.sol:53:1: | 53 | contract RootTest is DSTestPlus { | ^ (Relevant source part starts here and spans across multiple lines). Note: The payable fallback function is defined here. --> test/ulysses-omnichain/RootTest.t.sol:547:5: | 547 | fallback() external payable {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (2462): Warning: Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient. --> src/governance/GovernorBravoDelegator.sol:8:5: | 8 | constructor( | ^ (Relevant source part starts here and spans across multiple lines).
Warning (2462): Warning: Visibility for constructor is ignored. If you want the contract to be non-deployable, making it "abstract" is sufficient. --> src/out-of-scope/governance/GovernorBravoDelegator.sol:7:5: | 7 | constructor( | ^ (Relevant source part starts here and spans across multiple lines).
Warning (8417): Warning: Since the VM version paris, "difficulty" was replaced by "prevrandao", which now returns a random number based on the beacon chain. --> test/ulysses-omnichain/mocks/Multicall2.sol:49:22: | 49 | difficulty = block.difficulty; | ^^^^^^^^^^^^^^^^
Warning (5740): Warning: Unreachable code. --> src/ulysses-omnichain/BranchBridgeAgent.sol:1037:13: | 1037 | return; | ^^^^^^^
Warning (5740): Warning: Unreachable code. --> src/ulysses-omnichain/BranchBridgeAgent.sol:1052:13: | 1052 | return; | ^^^^^^^
Warning (5740): Warning: Unreachable code. --> src/ulysses-omnichain/BranchBridgeAgent.sol:1071:13: | 1071 | return; | ^^^^^^^
Warning (5740): Warning: Unreachable code. --> src/ulysses-omnichain/BranchBridgeAgent.sol:1151:17: | 1151 | return (true, "already executed tx"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (5740): Warning: Unreachable code. --> src/ulysses-omnichain/BranchBridgeAgent.sol:1173:17: | 1173 | return (true, "already executed tx"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Warning (5740): Warning: Unreachable code. --> src/ulysses-omnichain/BranchBridgeAgent.sol:1197:17: | 1197 | return (true, "already executed tx"); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
#0 - c4-judge
2023-07-07T10:12:07Z
trust1995 marked the issue as grade-b
#1 - c4-judge
2023-07-25T14:57:26Z
trust1995 marked the issue as grade-a
#2 - 0xLightt
2023-09-07T11:12:06Z
#3 - c4-sponsor
2023-09-14T14:35:48Z
0xBugsy (sponsor) confirmed
🌟 Selected for report: Raihan
Also found by: 0x11singh99, 0xAnah, 0xSmartContract, 0xn006e7, Aymen0909, DavidGiladi, IllIllI, JCN, Jorgect, MohammedRizwan, Rageur, ReyAdmirado, Rickard, Rolezn, SAQ, SM3_SS, Sathish9098, TheSavageTeddy, hunter_w3b, kaveyjoe, lsaudit, matrix_0wl, naman1778, petrichor, shamsulhaq123, wahedtalash77
610.3258 USDC - $610.33
Pack structs by putting variables that can fit together next to each other- Saves 10000 gas, 5 SLOT
State variables can be packed to use fewer storage slots- Saves 16000 gas,8 SLOTS
Avoid emit state variable when stack variable available- Saves 900 gas, 9 SLOT
Multiple accesses of a mapping/array should use a local variable cache- Saves 1800 gas, `18 SLOD
Use nested require and if and, avoid multiple check combinations- Saves 360 GAS
65000 GASNOTE: Some functions have been truncated where necessary to just show affected parts of the code Through out the report some places might be denoted with audit tags to show the actual place affected.
10000 GAS, 5 SLOTAs the solidity EVM works with 32 bytes, variables less than 32 bytes should be packed inside a struct so that they can be stored in the same slot, this saves gas when writing to storage ~20000 gas
IBranchBridgeAgent.sol: hToken and toChain can be packed together: Gas saved: 1 * 2k = 2k , 1 SLOTFILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/interfaces/IBranchBridgeAgent.sol 26: struct DepositInput { 27: //Deposit Info 28: address hToken; //Input Local hTokens Address. + 32: uint24 toChain; //Destination chain for interaction. 29: address token; //Input Native / underlying Token Address. 30: uint256 amount; //Amount of Local hTokens deposited for interaction. 31: uint256 deposit; //Amount of native tokens deposited for interaction. - 32: uint24 toChain; //Destination chain for interaction. 33: }
IBranchBridgeAgent.sol: numberOfAssets,depositNonce,toChain,depositedGas can be packed together: Gas saved: 1 * 2k = 2k , 1 SLOTFILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/interfaces/IBranchBridgeAgent.sol 55: struct DepositMultipleParams { 56: //Deposit Info - 57: uint8 numberOfAssets; //Number of assets to deposit. - 58: uint32 depositNonce; //Deposit nonce. 59: address[] hTokens; //Input Local hTokens Address. 60: address[] tokens; //Input Native / underlying Token Address. 61: uint256[] amounts; //Amount of Local hTokens deposited for interaction. 62: uint256[] deposits; //Amount of native tokens deposited for interaction. 63: uint24 toChain; //Destination chain for interaction. 64: uint128 depositedGas; //BRanch chain gas token amount sent with request. + 57: uint8 numberOfAssets; //Number of assets to deposit. + 58: uint32 depositNonce; //Deposit nonce. 65: }
IRootBridgeAgent.sol: depositNonce,hToken,toChain can be packed together: Gas saved: 1 * 2k = 2k , 1 SLOTFILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/interfaces/IRootBridgeAgent.sol 63: struct DepositParams { 64: //Deposit Info 65: uint32 depositNonce; //Deposit nonce. + 70: uint24 toChain; //Destination chain for interaction. 66: address hToken; //Input Local hTokens Address. 67: address token; //Input Native / underlying Token Address. 68: uint256 amount; //Amount of Local hTokens deposited for interaction. 69: uint256 deposit; //Amount of native tokens deposited for interaction. - 70: uint24 toChain; //Destination chain for interaction. 71: }
IRootBridgeAgent.sol: numberOfAssets,depositNonce,toChain can be packed together: Gas saved: 1 * 2k = 2k , 1 SLOTFILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/interfaces/IRootBridgeAgent.sol 73: struct DepositMultipleParams { 74: //Deposit Info - 75: uint8 numberOfAssets; //Number of assets to deposit. - 76: uint32 depositNonce; //Deposit nonce. 77: address[] hTokens; //Input Local hTokens Address. 78: address[] tokens; //Input Native / underlying Token Address. 79: uint256[] amounts; //Amount of Local hTokens deposited for interaction. 80: uint256[] deposits; //Amount of native tokens deposited for interaction. + 75: uint8 numberOfAssets; //Number of assets to deposit. + 76: uint32 depositNonce; //Deposit nonce. 81: uint24 toChain; //Destination chain for interaction. 82: }
GovernorBravoInterfaces.sol#L105-L136: proposer,canceled,executed can be packed together: Gas saved: 1 * 2k = 2k , 1 SLOTFILE: Breadcrumbs2023-05-maia/src/governance/GovernorBravoInterfaces.sol 105: struct Proposal { 106: /// @notice Unique id for looking up a proposal 107: uint256 id; 108: /// @notice Creator of the proposal 109: address proposer; + 130: /// @notice Flag marking whether the proposal has been canceled + 131: bool canceled; + 132: /// @notice Flag marking whether the proposal has been executed + 133: bool executed; 110: /// @notice The timestamp that the proposal will be available for execution, set once the vote succeeds 111: uint256 eta; 112: /// @notice the ordered list of target addresses for calls to be made 113: address[] targets; 114: /// @notice The ordered list of values (i.e. msg.value) to be passed to the calls to be made 115: uint256[] values; 116: /// @notice The ordered list of function signatures to be called 117: string[] signatures; 118: /// @notice The ordered list of calldata to be passed to each call 119: bytes[] calldatas; 120: /// @notice The block at which voting begins: holders must delegate their votes prior to this block 121: uint256 startBlock; 122: /// @notice The block at which voting ends: votes must be cast prior to this block 123: uint256 endBlock; 124: /// @notice Current number of votes in favor of this proposal 125: uint256 forVotes; 126: /// @notice Current number of votes in opposition to this proposal 127: uint256 againstVotes; 128: /// @notice Current number of votes for abstaining for this proposal 129: uint256 abstainVotes; - 130: /// @notice Flag marking whether the proposal has been canceled - 131: bool canceled; - 132: /// @notice Flag marking whether the proposal has been executed - 133: bool executed; 134: /// @notice Receipts of ballots for the entire set of voters 135: mapping(address => Receipt) receipts; 136: }
16000 GAS, 8 SLOTThe EVM works with 32 byte words. Variables less than 32 bytes can be declared next to each other in storage and this will pack the values together into a single 32 byte storage slot (if the values combined are <= 32 bytes). If the variables packed together are retrieved together in functions we will effectively save ~2000 gas with every subsequent SLOAD for that storage slot. This is due to us incurring a Gwarmaccess (100 gas) versus a Gcoldsload (2100 gas)
RootBridgeAgent.sol: settlementNonce,accumulatedFees state variables can be packed together: Gas saved: 1 * 2k = 2k , 1 SLOT.accumulatedFees mostly saves the value 1 the other values also not exceeds the 20000. So uint128 is more than enough.
FILE: 2023-05-maia/src/ulysses-omnichain/RootBridgeAgent.sol - 160: uint32 public settlementNonce; /// @notice Mapping from Settlement nonce to Deposit Struct. mapping(uint32 => Settlement) public getSettlement; /*/////////////////////////////////////////////////////////////// EXECUTOR STATE //////////////////////////////////////////////////////////////*/ /// @notice If true, bridge agent has already served a request with this nonce from a given chain. Chain -> Nonce -> Bool mapping(uint256 => mapping(uint32 => bool)) public executionHistory; /*/////////////////////////////////////////////////////////////// GAS MANAGEMENT STATE //////////////////////////////////////////////////////////////*/ uint256 internal constant MIN_FALLBACK_RESERVE = 155_000; // 100_000 for anycall + 55_000 for fallback uint256 internal constant MIN_EXECUTION_OVERHEAD = 155_000; // 100_000 for anycall + 30_000 Pre 1st Gas Checkpoint Execution + 25_000 Post last Gas Checkpoint Execution uint256 public initialGas; UserFeeInfo public userFeeInfo; /*/////////////////////////////////////////////////////////////// DAO STATE //////////////////////////////////////////////////////////////*/ + 160: uint32 public settlementNonce; - 190: uint256 public accumulatedFees; + 190: uint128 public accumulatedFees;
RootBridgeAgent.sol: coreRootBridgeAgentAddress,bridgeAgentFactoriesLenght,_setup state variables can be packed together: Gas saved: 2 * 2k = 4k , 2 SLOTbridgeAgentFactoriesLenght the type can be changed to uint64 instead of uint256. bridgeAgentFactoriesLenght this value is incremented only once in initialize() function. the value is not changed any where inside the contract
FILE: 2023-05-maia/src/ulysses-omnichain/RootPort.sol - 40: address public coreRootBridgeAgentAddress; + 40: address public coreRootBridgeAgentAddress; - 83: uint256 public bridgeAgentFactoriesLenght; + 126: bool internal _setup; + 83: uint64 public bridgeAgentFactoriesLenght; - 126: bool internal _setup;
BaseV2Minter.sol: daoShare,dao and tailEmission,initializer state variables can be packed together: Gas saved: 2 * 2k = 4k , 2 SLOTdaoShare,tailEmission can be uint96 instead of uint256. Because _daoShare > max_dao_share,tail_emission > max_tail_emission checks. max_dao_share,max_tail_emission are constants and stores the value of max_dao_share=300,max_tail_emission=100. So daoShare,tailEmission can't store more than constants values.
FILE: 2023-05-maia/src/hermes/minters/BaseV2Minter.sol 39: address public override dao; + 42: uint96 public override daoShare = 100; 40: 41: /// @inheritdoc IBaseV2Minter - 42: uint256 public override daoShare = 100; - 43: uint256 public override tailEmission = 20; 44: /// @inheritdoc IBaseV2Minter 45: 46: /// @inheritdoc IBaseV2Minter 47: uint256 public override weekly; 48: /// @inheritdoc IBaseV2Minter 49: uint256 public override activePeriod; 50: + 43: uint96 public override tailEmission = 20; 51: address internal initializer;
ERC20hTokenRoot.sol: localChainId,rootPortAddress state variables can be packed together: Gas saved: 1 * 2k = 2k , 1 SLOTThe uint96 type is a 96-bit unsigned integer, which is more than enough for the vast majority of chain IDs. Also other contracts only using uint24 for chainId . it is perfectly safe to change the uint256 public localChainId; declaration to uint96 public localChainId;. This will save some gas, and it will make the contract more efficient.
FILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/token/ERC20hTokenRoot.sol 15: /// @inheritdoc IERC20hTokenRoot - 16: uint256 public localChainId; + 16: uint96 public localChainId; 17: 18: /// @inheritdoc IERC20hTokenRoot 19: address public rootPortAddress; 20: 21: /// @inheritdoc IERC20hTokenRoot 22: address public localBranchPortAddress; 23: 24: /// @inheritdoc IERC20hTokenRoot 25: address public factoryAddress; 26: 27: /// @inheritdoc IERC20hTokenRoot 28: mapping(uint256 => uint256) public getTokenBalance;
ERC20hTokenRootFactory.sol: coreRootRouterAddress,hTokensLenght state variables can be packed together: Gas saved: 1 * 2k = 2k , 1 SLOThTokensLenght uint96 is more than enough to htokens length. Because all new tokens pushed to hTokens array.The array exceeds 1024 elements EVM reverts. So maximum hTokensLenght may be 1024
FILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/factories/ERC20hTokenRootFactory.sol 22: address public coreRootRouterAddress; + 26: uint96 public hTokensLenght; 23: 24: ERC20hTokenRoot[] public hTokens; 25: - 26: uint256 public hTokensLenght;
vMaia.sol: currentMonth,unstakePeriodEnd state variables can be packed together: Gas saved: 1 * 2k = 2k , 1 SLOTcurrentMonth only stores month values (0-12) and unstakePeriodEnd this stores timestamp. For currentMonth,unstakePeriodEnd uint128 is more than enough. So we can save 1 SLOT and 2000 GAS
uint128 alone stores more than 10000 years timestamp
FILE: 2023-05-maia/src/maia/vMaia.sol - 34: uint256 private currentMonth; - 35: uint256 private unstakePeriodEnd; + 34: uint128 private currentMonth; + 35: uint128 private unstakePeriodEnd;
1900 GAS, 19 SLODThe instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
BaseV2Minter.sol: dao should be cached with local address stack variable : Saves 100 GAS,1 SLODFILE: 2023-05-maia/src/hermes/minters/BaseV2Minter.sol + address _dao=dao ; - 145: if (dao != address(0)) underlying.safeTransfer(dao, share); + 145: if (_dao != address(0)) underlying.safeTransfer(_dao, share);
ERC20Boost.sol: gaugeState.userGaugeBoost should be cached with local uint256 stack variable : Saves 400 GAS,4 SLODFILE: Breadcrumbs2023-05-maia/src/erc-20/ERC20Boost.sol + uint256 userGaugeBoost_=gaugeState.userGaugeBoost; - 177: if (boost >= gaugeState.userGaugeBoost) { + 177: if (boost >= userGaugeBoost_) { 178: _userGauges[msg.sender].remove(gauge); 179: delete getUserGaugeBoost[msg.sender][gauge]; 180: 181: emit Detach(msg.sender, gauge); 182: } else { 183: gaugeState.userGaugeBoost -= boost.toUint128(); 184: - 185: emit DecrementUserGaugeBoost(msg.sender, gauge, gaugeState.userGaugeBoost); + 185: emit DecrementUserGaugeBoost(msg.sender, gauge, userGaugeBoost_); 186: } 210: GaugeState storage gaugeState = getUserGaugeBoost[msg.sender][gauge]; 211: + uint256 userGaugeBoost_=gaugeState.userGaugeBoost; - 212: if (_deprecatedGauges.contains(gauge) || boost >= gaugeState.userGaugeBoost) { + 212: if (_deprecatedGauges.contains(gauge) || boost >= userGaugeBoost_) { 213: require(_userGauges[msg.sender].remove(gauge)); // Remove from set. Should never fail. 214: delete getUserGaugeBoost[msg.sender][gauge]; 215: 216: emit Detach(msg.sender, gauge); 217: } else { 218: gaugeState.userGaugeBoost -= boost.toUint128(); 219: - 220: emit DecrementUserGaugeBoost(msg.sender, gauge, gaugeState.userGaugeBoost); + 220: emit DecrementUserGaugeBoost(msg.sender, gauge, userGaugeBoost_); 221: }
RootBridgeAgent.sol: settlement.hTokens[i] ,settlement.toChain should be cached with local address and uint24 stack variable : Saves 200 GAS,2 SLOD per iterationsFILE: 2023-05-maia/src/ulysses-omnichain/RootBridgeAgent.sol 596:for (uint256 i = 0; i < settlement.hTokens.length;) { 597: //Check if asset + address hTokens_ = settlement.hTokens[i] ; + uint24 toChain_= settlement.toChain; - 598: if (settlement.hTokens[i] != address(0)) { //@audit settlement.hTokens[i] first SLOD + 598: if (hTokens_ != address(0)) { 599: //Move hTokens from Branch to Root + Mint Sufficient hTokens to match new port deposit 600: IPort(localPortAddress).bridgeToRoot( 601: msg.sender, - 602: IPort(localPortAddress).getGlobalTokenFromLocal(settlement.hTokens[i], settlement.toChain), //@audit settlement.hTokens[i] second SLOD , //@audit settlement.toChain first SLOD + 602: IPort(localPortAddress).getGlobalTokenFromLocal(hTokens_ , toChain_), 603: settlement.amounts[i], 604: settlement.deposits[i], - 605: settlement.toChain //@audit settlement.toChain second SLOD + 605: toChain_ 606: ); 607: }
RootBridgeAgent.sol: userFeeInfo.gasToBridgeOut should be cached with local uint128 stack variable : Saves 200 GAS,2 SLODFILE: 2023-05-maia/src/ulysses-omnichain/RootBridgeAgent.sol 746: uint256 _initialGas = initialGas; + uint128 _gasToBridgeOut = userFeeInfo.gasToBridgeOut ; 747: 748: if (_toChain == localChainId) { 749: //Transfer gasToBridgeOut Local Branch Bridge Agent if remote initiated call. 750: if (_initialGas > 0) { - 751: address(wrappedNativeToken).safeTransfer(getBranchBridgeAgent[localChainId], userFeeInfo.gasToBridgeOut); + 751: address(wrappedNativeToken).safeTransfer(getBranchBridgeAgent[localChainId],_gasToBridgeOut ); 752: } 753: - 754: return uint128(userFeeInfo.gasToBridgeOut); + 754: return uint128(_gasToBridgeOut); 755: } 756: 757: if (_initialGas > 0) { - 758: if (userFeeInfo.gasToBridgeOut <= MIN_FALLBACK_RESERVE * tx.gasprice) revert InsufficientGasForFees(); + 758: if (_gasToBridgeOut <= MIN_FALLBACK_RESERVE * tx.gasprice) revert InsufficientGasForFees(); - 759: (amountOut, gasToken) = _gasSwapOut(userFeeInfo.gasToBridgeOut, _toChain); + 759: (amountOut, gasToken) = _gasSwapOut(_gasToBridgeOut, _toChain);
UlyssesPool.sol: newTotalWeights should be used instead of totalWeights : Saves 100 GAS,1 SLODFILE: 2023-05-maia/src/ulysses-amm/UlyssesPool.sol 241: totalWeights = newTotalWeights; 242: - 243: if (totalWeights > MAX_TOTAL_WEIGHT || oldTotalWeights == newTotalWeights) { + 243: if (newTotalWeights> MAX_TOTAL_WEIGHT || oldTotalWeights == newTotalWeights) {
GovernorBravoDelegateMaia.sol: proposal.proposer should be cached with local address variable : Saves 200 GAS,2 SLODFILE: 2023-05-maia/src/governance/GovernorBravoDelegateMaia.sol + address _proposer= proposal.proposer; - if (msg.sender != proposal.proposer && msg.sender != admin) { + if (msg.sender != _proposer && msg.sender != admin) { // Whitelisted proposers can't be canceled for falling below proposal threshold if (isWhitelisted(proposal.proposer)) { require( - (govToken.getPriorVotes(proposal.proposer, sub256(block.number, 1)) < + (govToken.getPriorVotes(_proposer, sub256(block.number, 1)) < getProposalThresholdAmount()) && msg.sender == whitelistGuardian, "GovernorBravo::cancel: whitelisted proposer" ); } else { require( - (govToken.getPriorVotes(proposal.proposer, sub256(block.number, 1)) < + (govToken.getPriorVotes(_proposer, sub256(block.number, 1)) < getProposalThresholdAmount()), "GovernorBravo::cancel: proposer above threshold" ); }
GovernorBravoDelegateMaia.sol: pendingAdmin should be cached with local address variable : Saves 400 GAS,4 SLODFILE: 2023-05-maia/src/governance/GovernorBravoDelegateMaia.sol + address oldPendingAdmin = pendingAdmin; 507: require( - 508: msg.sender == pendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending + 508: msg.sender == oldPendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending admin only" 509: ); 510: 511: // Save current values for inclusion in log 512: address oldAdmin = admin; - 513: address oldPendingAdmin = pendingAdmin; 514: 515: // Store admin with value pendingAdmin - 516: admin = pendingAdmin; + 516: admin = oldPendingAdmin ; 517: 518: // Clear the pending value 519: pendingAdmin = address(0); 520: - 521: emit NewAdmin(oldAdmin, admin); + 521: emit NewAdmin(oldAdmin, oldPendingAdmin ); - 522: emit NewPendingAdmin(oldPendingAdmin, pendingAdmin); + 522: emit NewPendingAdmin(oldPendingAdmin, address(0));
ERC20Boost.sol: gaugeState.userGaugeBoost should be cached with local uint128 variable : Saves 100 GAS,1 SLODFILE: Breadcrumbs2023-05-maia/src/erc-20/ERC20Boost.sol 176: GaugeState storage gaugeState = getUserGaugeBoost[msg.sender][gauge]; + uint128 _userGaugeBoost = gaugeState.userGaugeBoost; - 177: if (boost >= gaugeState.userGaugeBoost) { + 177: if (boost >= _userGaugeBoost ) { 178: _userGauges[msg.sender].remove(gauge); 179: delete getUserGaugeBoost[msg.sender][gauge]; 180: 181: emit Detach(msg.sender, gauge); 182: } else { - 183: gaugeState.userGaugeBoost -= boost.toUint128(); + 183: gaugeState.userGaugeBoost = _userGaugeBoost - boost.toUint128(); 184: 185: emit DecrementUserGaugeBoost(msg.sender, gauge, gaugeState.userGaugeBoost);
TalosBaseStrategy.sol: liquidity should be cached with local uint128 variable : Saves 200 GAS,2 SLODFILE: 2023-05-maia/src/talos/base/TalosBaseStrategy.sol 214: uint256 supply = totalSupply; // Saves an extra SLOAD if totalSupply is non-zero. + uint128 _liquidity = liquidity; - 215: shares = supply == 0 ? liquidityDifference * MULTIPLIER : (liquidityDifference * supply) / liquidity; + 215: shares = supply == 0 ? liquidityDifference * MULTIPLIER : (liquidityDifference * supply) / _liquidity; - 216: liquidity += liquidityDifference; + 216: liquidity =_liquidity + liquidityDifference;
INonfungiblePositionManager _nonfungiblePositionManager = nonfungiblePositionManager; // Saves an extra SLOAD { + uint _liquidity= liquidity ; - uint128 liquidityToDecrease = uint128((_liquidity* shares) / totalSupply); (amount0, amount1) = _nonfungiblePositionManager.decreaseLiquidity( INonfungiblePositionManager.DecreaseLiquidityParams({ tokenId: _tokenId, liquidity: liquidityToDecrease, amount0Min: amount0Min, amount1Min: amount1Min, deadline: block.timestamp }) ); if (amount0 == 0 && amount1 == 0) revert AmountsAreZero(); _burn(_owner, shares); - liquidity -= liquidityToDecrease; + liquidity =_liquidity - liquidityToDecrease; }
900 GAS, 9 SLODThe gas cost for emitting a state variable is 100 gas, while the gas cost for emitting a stack variable is 8 gas. This means that emitting a stack variable instead of a state variable can save 92 gas
GovernorBravoDelegateMaia.sol: Emit and return stack variable newProposalID instead of state variable newProposal.id : Saves 200 GAS, 2 SLODFILE: Breadcrumbs2023-05-maia/src/governance/GovernorBravoDelegateMaia.sol 164: emit ProposalCreated( - 165: newProposal.id, msg.sender, targets, values, signatures, calldatas, startBlock, endBlock,description + 165: newProposalID, msg.sender, targets, values, signatures, calldatas, startBlock, endBlock,description 166: ); - 167: return newProposal.id; + 167: return newProposalID;
GovernorBravoDelegateMaia.sol: Emit stack variables newVotingDelay,newVotingPeriod,newProposalThreshold,account instead of votingDelay,votingPeriod,proposalThreshold , whitelistGuardian state variables : Saves 500 GAS, 5 SLODFILE: Breadcrumbs2023-05-maia/src/governance/GovernorBravoDelegateMaia.sol 404: votingDelay = newVotingDelay; 405: - 406: emit VotingDelaySet(oldVotingDelay, votingDelay); + 406: emit VotingDelaySet(oldVotingDelay, newVotingDelay); 420: votingPeriod = newVotingPeriod; 421: - 422: emit VotingPeriodSet(oldVotingPeriod, votingPeriod); + 422: emit VotingPeriodSet(oldVotingPeriod, newVotingPeriod); 437: proposalThreshold = newProposalThreshold; 438: - 439: emit ProposalThresholdSet(oldProposalThreshold, proposalThreshold); + 439: emit ProposalThresholdSet(oldProposalThreshold, newProposalThreshold); 464: whitelistGuardian = account; 465: - 466: emit WhitelistGuardianSet(oldGuardian, whitelistGuardian); + 466: emit WhitelistGuardianSet(oldGuardian, account);
GovernorBravoDelegator.sol: Emit stack variable implementation_ instead of implementation state variable : Saves 100 GAS, 1 SLODFILE: 2023-05-maia/src/governance/GovernorBravoDelegator.sol 48: implementation = implementation_; 49: - 50: emit NewImplementation(oldImplementation, implementation); + 50: emit NewImplementation(oldImplementation, implementation_);
TalosBaseStrategy.sol: Emit stack variable _tokenId instead of tokenId state variable : Saves 100 GAS, 1 SLODFILE: 2023-05-maia/src/talos/base/TalosBaseStrategy.sol 155: tokenId = _tokenId; 156: 157: _mint(receiver, shares); 158: if (totalSupply > optimizer.maxTotalSupply()) revert ExceedingMaxTotalSupply(); 159: - 160:` emit Initialize(tokenId, msg.sender, receiver, amount0, amount1, shares); + 160:` emit Initialize(_tokenId, msg.sender, receiver, amount0, amount1, shares);
require() or revert() statements that check input arguments should be at the top of the function (Also restructured some if’s)4300 GASFail early and cheaply
Checks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas) in a function that may ultimately revert in the unhappy case.
GovernorBravoDelegateMaia.sol: Cheaper to check the function parameters,constants before checking govToken.getPriorVotes(msg.sender, sub256(block.number, 1)) external calls. Saves 2100 gasFILE: Breadcrumbs2023-05-maia/src/governance/GovernorBravoDelegateMaia.sol 111: // Reject proposals before initiating as Governor 112: require(initialProposalId != 0, "GovernorBravo::propose: Governor Bravo not active"); + 123: require(targets.length != 0, "GovernorBravo::propose: must provide actions"); + 124: require(targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions"); 113: // Allow addresses above proposal threshold and whitelisted addresses to propose + 119: require( + 120: targets.length == values.length && targets.length == signatures.length && targets.length == calldatas.length, + 121: "GovernorBravo::propose: proposal function information arity mismatch" + 122: ); 114: require( 115: govToken.getPriorVotes(msg.sender, sub256(block.number, 1)) > getProposalThresholdAmount() 116: || isWhitelisted(msg.sender), 117: "GovernorBravo::propose: proposer votes below proposal threshold" 118: ); - 119: require( - 120: targets.length == values.length && targets.length == signatures.length && targets.length == calldatas.length, - 121: "GovernorBravo::propose: proposal function information arity mismatch" - 122: ); - 123: require(targets.length != 0, "GovernorBravo::propose: must provide actions"); - 124: require(targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions");
UlyssesPool.sol: Cheaper to check the function parameters, constants before checking factory.owner() external calls. Saves 2100 gasFILE: Breadcrumbs2023-05-maia/src/ulysses-amm/UlyssesPool.sol 323: function setProtocolFee(uint256 _protocolFee) external nonReentrant { - 324: if (msg.sender != factory.owner()) revert Unauthorized(); 325: 326: // Revert if the protocol fee is larger than 1% 327: if (_protocolFee > MAX_PROTOCOL_FEE) revert InvalidFee(); + 324: if (msg.sender != factory.owner()) revert Unauthorized(); 328: 329: protocolFee = _protocolFee; 330: }
RootPort.sol: Cheaper to check the _bridgeAgentFactory,_coreRootRouter function parameters before checking _setup state variable. Saves around 60-75 gas .FILE: 2023-05-maia/src/ulysses-omnichain/RootPort.sol 128: function initialize(address _bridgeAgentFactory, address _coreRootRouter) external onlyOwner { - 129: require(_setup, "Setup ended."); 130: require(_bridgeAgentFactory != address(0), "Bridge Agent Factory cannot be 0 address."); 131: require(_coreRootRouter != address(0), "Core Root Router cannot be 0 address."); + 129: require(_setup, "Setup ended."); 132: 133: isBridgeAgentFactory[_bridgeAgentFactory] = true; - 145: require(_setup, "Setup ended."); - 146: require(isBridgeAgent[_coreRootBridgeAgent], "Core Bridge Agent doesn't exist."); 147: require(_coreRootBridgeAgent != address(0), "Core Root Bridge Agent cannot be 0 address."); 148: require(_coreLocalBranchBridgeAgent != address(0), "Core Local Branch Bridge Agent cannot be 0 address."); 149: require(_localBranchPortAddress != address(0), "Local Branch Port Address cannot be 0 address."); + 145: require(_setup, "Setup ended."); + 146: require(isBridgeAgent[_coreRootBridgeAgent], "Core Bridge Agent doesn't exist.");
1800 GAS, 18 SLODCaching a mapping’s value in a local storage or calldata variable when the value is accessed multiple times saves ~42 gas per access due to not having to perform the same offset calculation every time. Help the Optimizer by saving a storage variable’s reference instead of repeatedly fetching it
To help the optimizer,declare a storage type variable and use it instead of repeatedly fetching the reference in a map or an array.
As an example, instead of repeatedly calling someMap[someIndex], save its reference like this: SomeStruct storage someStruct = someMap[someIndex] and use it
MultiRewardsDepot.sol: _assets[rewardsContract] mapping should be cached : Saves 100 GAS, 1 SLODFILE: 2023-05-maia/src/rewards/depots/MultiRewardsDepot.sol + address rewardsContract_=_assets[rewardsContract]; - 60: emit AssetRemoved(rewardsContract, _assets[rewardsContract]); + 60: emit AssetRemoved(rewardsContract, rewardsContract_); 61: - 62: delete _isAsset[_assets[rewardsContract]]; + 62: delete _isAsset[rewardsContract_];
MultiRewardsDepot.sol: _assets[rewardsContract] mapping should be cached : Saves 100 GAS, 1 SLODFILE: 2023-05-maia/src/maia/factories/PartnerManagerFactory.sol + uint256 _partnerManager = partners[partnerIds[partnerManager]]; - 81: if (partners[partnerIds[partnerManager]] != partnerManager) revert InvalidPartnerManager(); + 81: if (_partnerManager != partnerManager) revert InvalidPartnerManager(); - 82: delete partners[partnerIds[partnerManager]]; + 82: delete partners[_partnerManager];
BranchBridgeAgent.sol: uint8(getDeposit[_depositNonce].hTokens.length),getDeposit[_depositNonce].tokens[0],getDeposit[_depositNonce].hTokens[0],getDeposit[_depositNonce].amounts[0],getDeposit[_depositNonce].deposits[0],getDeposit[nonce].hTokens,getDeposit[nonce].tokens, getDeposit[nonce].amounts, getDeposit[nonce].deposits,getDeposit[_depositNonce].depositedGas mappings should be cached : Saves 1500 GAS, 15 SLODFILE: 2023-05-maia/src/ulysses-omnichain/BranchBridgeAgent.sol + uint8 _hTokensLength = uint8(getDeposit[_depositNonce].hTokens.length); - 332: if (uint8(getDeposit[_depositNonce].hTokens.length) == 1) { //@audit 1 SLOD + 332: if (_hTokensLength == 1) { - 365: } else if (uint8(getDeposit[_depositNonce].hTokens.length) > 1) { //@audit 2 SLOD + 365: } else if (_hTokensLength > 1) { 372: msg.sender, - 373: uint8(getDeposit[_depositNonce].hTokens.length), //@audit 3 SLOD + 373: _hTokensLength, 374: nonce,
FILE: 2023-05-maia/src/ulysses-omnichain/BranchBridgeAgent.sol if (uint8(getDeposit[_depositNonce].hTokens.length) == 1) { + address tokens_= getDeposit[_depositNonce].tokens[0]; + address hTokens_=getDeposit[_depositNonce].hTokens[0]; + uint256 amounts_=getDeposit[_depositNonce].amounts[0]; + uint256 deposits_= getDeposit[_depositNonce].deposits[0]; if (_isSigned) { packedData = abi.encodePacked( //@audit GAS is this possible to cache getDeposit[_depositNonce] bytes1(0x05), msg.sender, _depositNonce, - getDeposit[_depositNonce].hTokens[0], //@audit 1 SLOD + hTokens_, - getDeposit[_depositNonce].tokens[0], //@audit 1 SLOD + tokens_, - getDeposit[_depositNonce].amounts[0], //@audit 1 SLOD + amounts_, _normalizeDecimals( - getDeposit[_depositNonce].deposits[0],ERC20(getDeposit[_depositNonce].tokens[0]).decimals() //@audit 2 SLOD + deposits_,ERC20(tokens_).decimals() ), _toChain, _params, msg.value.toUint128(), _remoteExecutionGas ); } else { packedData = abi.encodePacked( bytes1(0x02), _depositNonce, - getDeposit[_depositNonce].hTokens[0], //@audit 2 SLOD + hTokens_, - getDeposit[_depositNonce].tokens[0], //@audit 3 SLOD + tokens_, - getDeposit[_depositNonce].amounts[0], //@audit 2 SLOD + amounts_, _normalizeDecimals( - getDeposit[_depositNonce].deposits[0],ERC20(getDeposit[_depositNonce].tokens[0]).decimals() //@audit 4 SLOD + deposits_,ERC20(tokens_).decimals()
FILE: 2023-05-maia/src/ulysses-omnichain/BranchBridgeAgent.sol } else if (uint8(getDeposit[_depositNonce].hTokens.length) > 1) { //Nonce uint32 nonce = _depositNonce; + address[] hTokens_=getDeposit[nonce].hTokens; + address[] tokens_=getDeposit[nonce].tokens; + uint256[] amounts_=getDeposit[nonce].amounts; + uint256[] deposits_=getDeposit[nonce].deposits; if (_isSigned) { packedData = abi.encodePacked( bytes1(0x06), msg.sender, uint8(getDeposit[_depositNonce].hTokens.length), nonce, - getDeposit[nonce].hTokens, //@audit 1 SLOD + hTokens_, - getDeposit[nonce].tokens, //@audit 1 SLOD + tokens_, - getDeposit[nonce].amounts, //@audit 1 SLOD + amounts_, - _normalizeDecimalsMultiple(getDeposit[nonce].deposits, getDeposit[nonce].tokens), //@audit 2 SLOD + _normalizeDecimalsMultiple(deposits_, tokens_), _toChain, _params, msg.value.toUint128(), _remoteExecutionGas ); } else { packedData = abi.encodePacked( bytes1(0x03), uint8(getDeposit[nonce].hTokens.length), _depositNonce, - getDeposit[nonce].hTokens, //@audit 2 SLOD + hTokens_, - getDeposit[nonce].tokens, //@audit 3 SLOD + tokens_, - getDeposit[nonce].amounts, //@audit 2 SLOD + amounts_, - _normalizeDecimalsMultiple(getDeposit[nonce].deposits, getDeposit[nonce].tokens), //@audit 4 SLOD + _normalizeDecimalsMultiple(deposits_, tokens_), _toChain, _params, msg.value.toUint128(), _remoteExecutionGas ); }
FILE: 2023-05-maia/src/ulysses-omnichain/BranchBridgeAgent.sol + uint256 depositedGas_= getDeposit[_depositNonce].depositedGas ; - 1069: if (minExecCost > getDeposit[_depositNonce].depositedGas) { + 1069: if (minExecCost > gdepositedGas_) { 1070: _forceRevert(); 1071: return; 1072: } 1073: 1074: //Update user deposit reverts if not enough gas => user must boost deposit with gas - 1075: getDeposit[_depositNonce].depositedGas -= minExecCost.toUint128(); + 1075: getDeposit[_depositNonce].depositedGas = depositedGas_ - minExecCost.toUint128();
RootBridgeAgent.sol: getSettlement[_settlementNonce].gasToBridgeOut mapping should be cached : Saves 100 GAS, 1 SLODFILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/RootBridgeAgent.sol 838: //Check if sufficient balance + uint128 _gasToBridgeOut = getSettlement[_settlementNonce].gasToBridgeOut ; - 839: if (minExecCost > getSettlement[_settlementNonce].gasToBridgeOut) { + 839: if (minExecCost > _gasToBridgeOut) { 840: _forceRevert(); 841: return; 842: } 843: 844: //Update user deposit reverts if not enough gas - 845: getSettlement[_settlementNonce].gasToBridgeOut -= minExecCost.toUint128(); + 845: getSettlement[_settlementNonce].gasToBridgeOut =_gasToBridgeOut - minExecCost.toUint128(); 846: }
4600 GASYou can save a Gcoldsload (2100 gas) in the address provider, plus the 100 gas overhead of the external call, for every _replenishGas(), by creating an immutable CONFIG variable which will store the IAnycallProxy(localAnyCallAddress).config() address. localAnyCallAddress is immutable variable so not changed any where in the contract. When ever we call IAnycallProxy(localAnyCallAddress).config() always return the same address. So its waste of external call instead we can store the IAnycallProxy(localAnyCallAddress).config() address with CONFIG immutable variable during constructor initialization time.
FILE: 2023-05-maia/src/ulysses-omnichain/RootBridgeAgent.sol + address public immutable CONFIG; Inside the constructor + CONFIG = IAnycallProxy(localAnyCallAddress).config(); - 851: IAnycallConfig(IAnycallProxy(localAnyCallAddress).config()).deposit{value: _executionGasSpent}(address(this)); + 851: IAnycallConfig(CONFIG).deposit{value: _executionGasSpent}(address(this)); - 1236: IAnycallConfig anycallConfig = IAnycallConfig(IAnycallProxy(localAnyCallAddress).config()); + 1236: IAnycallConfig anycallConfig = IAnycallConfig(CONFIG);
FILE: 2023-05-maia/src/ulysses-amm/UlyssesPool.sol + address public immutable OWNER; Inside the constructor + OWNER= factory.owner() - 154: asset.safeTransfer(factory.owner(), claimed); + 154: asset.safeTransfer(OWNER, claimed); - 324: if (msg.sender != factory.owner()) revert Unauthorized(); + 324: if (msg.sender != OWNER) revert Unauthorized();
1040 GAS, 8 Instancesrequire(a <= b); x = b - a => require(a <= b); unchecked { x = b - a }
Saves: 100-130 GAS
bHermes.sol: The subtractions should be unchecked . /// @dev Never overflows since balandeOf >= userClaimed As per comment the values not going to overflow : Saves 390 GASFILE: Breadcrumbs2023-05-maia/src/hermes/bHermes.sol 98: /// @dev Never overflows since balandeOf >= userClaimed. 99: claimWeight(balance - userClaimedWeight[msg.sender]); 100: claimBoost(balance - userClaimedBoost[msg.sender]); 101: claimGovernance(balance - userClaimedGovernance[msg.sender]);
UlyssesPool.sol: balance - assets should be unchecked since not possible to overflow because of this condition check if (balance > assets) : Saves 130 GASFILE: Breadcrumbs2023-05-maia/src/ulysses-amm/UlyssesPool.sol 138: if (balance > assets) { - 139: return balance - assets; + 139: return unchecked{balance - assets}; 140: } else {
UlyssesPool.sol: newRebalancingFee - oldRebalancingFee should be unchecked since not possible to overflow because of this condition check if (oldRebalancingFee < newRebalancingFee) : Saves 260 GASFILE: 2023-05-maia/src/ulysses-amm/UlyssesPool.sol 217: if (oldRebalancingFee < newRebalancingFee) { - 218: asset.safeTransferFrom(msg.sender, address(this), newRebalancingFee - oldRebalancingFee); + unchecked { + asset.safeTransferFrom(msg.sender, address(this), newRebalancingFee - oldRebalancingFee); + } 219: } 302: if (oldRebalancingFee < newRebalancingFee) { - 303: asset.safeTransferFrom(msg.sender, address(this), newRebalancingFee - oldRebalancingFee); + unchecked { + asset.safeTransferFrom(msg.sender, address(this), newRebalancingFee - oldRebalancingFee); + } 304: }
BranchPort.sol: currBalance - minReserves, minReserves - currBalance should be unchecked since not possible to overflow because of this condition check currBalance > minReserves,currBalance < minReserves : Saves 260 GASFILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/BranchPort.sol - 129: return currBalance > minReserves ? currBalance - minReserves : 0; + 129: unchecked {return currBalance > minReserves ? currBalance - minReserves : 0;} - 141: return currBalance < minReserves ? minReserves - currBalance : 0; + 129: unchecked {return currBalance < minReserves ? minReserves - currBalance : 0;}
25487 GASUnused internal state variable take up space in the contract's storage, which can increase the gas cost of deploying and calling the contract. If a state variable is never used, then it can be safely removed from the contract.
Lock modifier can also increase the gas cost of calling a contract. Lock modifiers are used to prevent a contract from being called by an unauthorized address. However, if the contract is only ever called by a trusted address, then the lock modifier can be safely removed. The lock modifier not used through out the contract can be safely removed.
CoreRootRouter.sol: After Removed unused code the deployment cost saved : 25487 GASFILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/CoreRootRouter.sol 435: uint256 internal _unlocked = 1; 436: 437: /// @notice Modifier for a simple re-entrancy check. 438: modifier lock() { 439: require(_unlocked == 1); 440: _unlocked = 2; 441: _; 442: _unlocked = 1; 443: }
81 GASUsing constants instead of type(uint256).max can save gas. This is because the compiler can embed constants directly into the bytecode of your contract, which saves on gas costs.
type(uint256).max this code will use 3 gas to get the maximum value of a uint256.
But constant will use 0 gas to get the maximum value of a uint256.
BranchBridgeAgent.sol: Use constants instead of type(uintx).max FILE: 2023-05-maia/src/uni-v3-staker/UniswapV3Staker.sol 70: if (liquidity == type(uint96).max) { 385: amount0Max: type(uint128).max, 386: amount1Max: type(uint128).max 456: if (liquidity >= type(uint96).max) stake.liquidityIfOverflow = 0; 506: if (liquidity >= type(uint96).max) { 509: liquidityNoOverflow: type(uint96).max,
FILE: 2023-05-maia/src/rewards/rewards/FlywheelGaugeRewards.sol 133: require(newRewards <= type(uint112).max); // safe cast 187: require(nextRewards <= type(uint112).max); // safe cast
FILE: 2023-05-maia/src/maia/tokens/ERC4626PartnerManager.sol 200: address(gaugeWeight).safeApprove(newPartnerVault, type(uint256).max); 201: address(gaugeBoost).safeApprove(newPartnerVault, type(uint256).max); 202: address(governance).safeApprove(newPartnerVault, type(uint256).max); 203: address(partnerGovernance).safeApprove(newPartnerVault, type(uint256).max);
FILE: Breadcrumbs2023-05-maia/src/erc-4626/ERC4626MultiToken.sol 143: if (allowed != type(uint256).max) allowance[owner][msg.sender] = allowed - shares; 165: if (allowed != type(uint256).max) allowance[owner][msg.sender] = allowed - shares; 200: shares = type(uint256).max; 270: return type(uint256).max; 275: return type(uint256).max;
FILE: 2023-05-maia/src/talos/base/TalosBaseStrategy.sol 130: address(_token0).safeApprove(address(_nonfungiblePositionManager), type(uint256).max); 131: address(_token1).safeApprove(address(_nonfungiblePositionManager), type(uint256).max); 249: if (allowed != type(uint256).max) allowance[_owner][msg.sender] = allowed - shares; 285: amount0Max: type(uint128).max, 286: amount1Max: type(uint128).max 367: amount0Max: type(uint128).max, 368: amount1Max: type(uint128).max
1000 GASMake sure Solidity’s optimizer is enabled. It reduces gas costs. If you want to gas optimize for contract deployment (costs less to deploy a contract) then set the Solidity optimizer at a low number. If you want to optimize for run-time gas costs (when functions are called on a contract) then set the optimizer to a high number.
Set the optimization value higher than 800 in your hardhat.config.ts file.
FILE: hardhat.config.js overrides: { 'src/talos/TalosStrategyVanilla.sol': { version: '0.8.18', settings: { viaIR: true, optimizer: { enabled: true, runs: 200, }, metadata: { bytecodeHash: 'none', }, }, }, 'src/talos/TalosStrategyStaked.sol': { version: '0.8.18', settings: { viaIR: true, optimizer: { enabled: true, runs: 200, }, metadata: { bytecodeHash: 'none', }, }, }, 'src/ulysses-omnichain/RootBridgeAgent.sol': { version: '0.8.18', settings: { viaIR: true, optimizer: { enabled: true, runs: 200, }, metadata: { bytecodeHash: 'none', }, }, }, 'src/ulysses-omnichain/BranchBridgeAgent.sol': { version: '0.8.18', settings: { viaIR: true, optimizer: { enabled: true, runs: 200, }, metadata: { bytecodeHash: 'none', }, }, }, 'src/ulysses-omnichain/BranchBridgeAgentFactory.sol': { version: '0.8.18', settings: { viaIR: true, optimizer: { enabled: true, runs: 200, }, metadata: { bytecodeHash: 'none', }, }, }, }, },
We can combine the events into one singular event to save Glogtopic (375 gas) that would otherwise be paid for the additional one event.
FILE: 2023-05-maia/src/governance/GovernorBravoDelegateMaia.sol 521: emit NewAdmin(oldAdmin, admin); 523: emit NewPendingAdmin(oldPendingAdmin, pendingAdmin);
public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted
/// @audit getDepositEntry(),callOut(),callOutAndBridge(),callOutAndBridgeMultiple(),callOutSigned(),callOutSignedAndBridge(),callOutSignedAndBridgeMultiple(),retryDeposit(),retrySettlement(),retrieveDeposit(),redeemDeposit(),performSystemCallOut(),performCallOut(),performCallOutAndBridge(),performCallOutAndBridgeMultiple(),clearToken() FILE: Breadcrumbs2023-05-maia/src/ulysses-omnichain/BranchBridgeAgent.sol /// @audit getSettlementEntry(),retrySettlement(),redeemSettlement(),callOut(),callOutAndBridge(),callOutAndBridgeMultiple(),bridgeInMultiple(), FILE: 2023-05-maia/src/ulysses-omnichain/RootBridgeAgent.sol
360 GAS, 24 Instances Using nested is cheaper than using && multiple check combinations. There are more advantages, such as easier to read code and better coverage reports. Saves 15 GAS, Per instance
FILE: Breadcrumbs2023-05-maia/src/talos/libraries/PoolVariables.sol 98: if (tick < 0 && tick % tickSpacing != 0) compressed--; FILE: 2023-05-maia/src/talos/base/TalosBaseStrategy.sol 151: if (amount0 == 0 && amount1 == 0) revert AmountsAreZero(); 121: if (amount0 == 0 && amount1 == 0) revert AmountsAreZero(); 273: if (amount0 == 0 && amount1 == 0) revert AmountsAreZero(); 335: if (amount0 == 0 && amount1 == 0) revert AmountsAreZero(); 408: require(balance0 >= amount0 && balance1 >= amount1); FILE: 2023-05-maia/src/erc-20/ERC20Gauges.sol 211: if (added && _userGauges[user].length() > maxGauges && !canContractExceedMaxGauges[user]) { 464: if (canExceedMax && account.code.length == 0) revert Errors.NonContractError(); FILE: 2023-05-maia/src/erc-20/ERC20MultiVotes.sol 257: if (pos > 0 && ckpts[pos - 1].fromBlock == block.number) { 112: if (canExceedMax && account.code.length == 0) revert Errors.NonContractError(); 201: if (newDelegate && delegateCount(delegator) > maxDelegates && !canContractExceedMaxDelegates[delegator]) { FILE: 2023-05-maia/src/governance/GovernorBravoDelegateMaia.sol 67: require( votingPeriod_ >= MIN_VOTING_PERIOD && votingPeriod_ <= MAX_VOTING_PERIOD, "GovernorBravo::initialize: invalid voting period" ); 71: require( votingDelay_ >= MIN_VOTING_DELAY && votingDelay_ <= MAX_VOTING_DELAY, "GovernorBravo::initialize: invalid voting delay" ); 75: require( proposalThreshold_ >= MIN_PROPOSAL_THRESHOLD && proposalThreshold_ <= MAX_PROPOSAL_THRESHOLD, "GovernorBravo::initialize: invalid proposal threshold" ); 119: require( targets.length == values.length && targets.length == signatures.length && targets.length == calldatas.length, "GovernorBravo::propose: proposal function information arity mismatch" ); 234: if (msg.sender != proposal.proposer && msg.sender != admin) { 237: require( (govToken.getPriorVotes(proposal.proposer, sub256(block.number, 1)) < getProposalThresholdAmount()) && msg.sender == whitelistGuardian, "GovernorBravo::cancel: whitelisted proposer" ); 298: require( proposalCount >= proposalId && proposalId > initialProposalId, "GovernorBravo::state: invalid proposal id" ); 399: require( newVotingDelay >= MIN_VOTING_DELAY && newVotingDelay <= MAX_VOTING_DELAY, "GovernorBravo::_setVotingDelay: invalid voting delay" ); 416: require( newVotingPeriod >= MIN_VOTING_PERIOD && newVotingPeriod <= MAX_VOTING_PERIOD, "GovernorBravo::_setVotingPeriod: invalid voting period" ); 433: require( newProposalThreshold >= MIN_PROPOSAL_THRESHOLD && newProposalThreshold <= MAX_PROPOSAL_THRESHOLD, "GovernorBravo::_setProposalThreshold: invalid proposal threshold" ); 507: require( msg.sender == pendingAdmin && msg.sender != address(0), "GovernorBravo:_acceptAdmin: pending admin only" //@audit-issue unreached check. dravee zksyn low );
#0 - c4-judge
2023-07-07T10:15:21Z
trust1995 marked the issue as grade-a
#1 - c4-sponsor
2023-07-12T18:22:13Z
0xBugsy marked the issue as sponsor confirmed
#2 - 0xLightt
2023-09-07T11:28:50Z