Platform: Code4rena
Start Date: 07/03/2024
Pot Size: $63,000 USDC
Total HM: 20
Participants: 36
Period: 5 days
Judge: cccz
Total Solo HM: 11
Id: 349
League: BLAST
Rank: 27/36
Findings: 1
Award: $169.57
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: hihen
Also found by: 0x11singh99, Bozho, Sathish9098, albahaca, clara, dharma09, oualidpro, pfapostol, slvDev
169.5732 USDC - $169.57
When setting values down casted as per implementation in setParameters() functions.
_LP_FEE_RATE_ = uint64(newLpFeeRate); _K_ = uint64(newK); _I_ = uint128(newI);
So its safe to down casting as per implementations. This saves 4000 GAS
, 2 SLOT
FILE: 2024-03-abracadabra-money/src/mimswap/MagicLP.sol - 80: uint256 public _LP_FEE_RATE_; + 80: uint64 public _LP_FEE_RATE_; - 81: uint256 public _K_; + 81: uint64 public _K_; - 82: uint256 public _I_; + 82: uint128 public _I_;
pool
and totalPoolShares
inherited state variables can be cached with memory
variablesCaching state variables in memory (often by assigning them to local variables) can be beneficial, especially when those state variables are accessed multiple times within a function. This practice reduces the cost associated with repeated state reads, which are more expensive than reading from memory . Saves 800 GAS
, 8 SLOD
FILE:2024-03-abracadabra-money/src/blast/BlastOnboardingBoot.sol function bootstrap(uint256 minAmountOut) external onlyOwner onlyState(State.Closed) returns (address, address, uint256) { Address _pool = pool ; - if (pool != address(0)) { + if (_pool != address(0)) { revert ErrAlreadyBootstrapped(); } uint256 baseAmount = totals[MIM].locked; uint256 quoteAmount = totals[USDB].locked; MIM.safeApprove(address(router), type(uint256).max); USDB.safeApprove(address(router), type(uint256).max); + uint256 totalPoolShares_ = totalPoolShares ; - (pool, totalPoolShares) = router.createPool(MIM, USDB, FEE_RATE, I, K, address(this), baseAmount, quoteAmount); + (_pool, totalPoolShares_ ) = router.createPool(MIM, USDB, FEE_RATE, I, K, address(this), baseAmount, quoteAmount); - if (totalPoolShares < minAmountOut) { + if (totalPoolShares_ < minAmountOut) { revert ErrInsufficientAmountOut(); } // Create staking contract // 3x boosting for locker, 7 days reward duration, 13 weeks lp locking // make this contract temporary the owner the set it as an operator // for permissionned `stakeFor` during the claiming process and then // transfer the ownership to the onboarding owner. staking = new LockingMultiRewards(pool, 30_000, 7 days, 13 weeks, address(this)); staking.setOperator(address(this), true); staking.transferOwnership(owner); // Approve staking contract - pool.safeApprove(address(staking), totalPoolShares); + _pool.safeApprove(address(staking), totalPoolShares_ ); - emit LogLiquidityBootstrapped(pool, address(staking), totalPoolShares); + emit LogLiquidityBootstrapped(_pool, address(staking), totalPoolShares_); - return (pool, address(staking), totalPoolShares); + return (_pool, address(staking), totalPoolShares_); }
It's common to use mappings for storing data associated with specific addresses or IDs. If multiple mappings are used to store related data, combining them into a single mapping that maps addresses or IDs to a struct can optimize gas usage and improve code readability.
FILE: 2024-03-abracadabra-money/src/blast/BlastOnboarding.sol mapping(address token => bool) public supportedTokens; mapping(address token => Balances) public totals; mapping(address token => uint256 cap) public caps; // Per-user mapping(address user => mapping(address token => Balances)) public balances;
FILE: 2024-03-abracadabra-money/src/staking/LockingMultiRewards.sol mapping(address user => mapping(address token => uint256 amount)) public userRewardPerTokenPaid; mapping(address user => mapping(address token => uint256 amount)) public rewards; mapping(address user => uint256 index) public lastLockIndex;
The instances below point to the second+ call of the function within a single function
totalSupply()
value should be cached instead calling recursively
FILE: 2024-03-abracadabra-money/src/mimswap/MagicLP.sol // buy shares [round down] function buyShares(address to) external nonReentrant returns (uint256 shares, uint256 baseInput, uint256 quoteInput) { uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)); uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)); uint256 baseReserve = _BASE_RESERVE_; uint256 quoteReserve = _QUOTE_RESERVE_; baseInput = baseBalance - baseReserve; quoteInput = quoteBalance - quoteReserve; if (baseInput == 0) { revert ErrNoBaseInput(); } // Round down when withdrawing. Therefore, never be a situation occurring balance is 0 but totalsupply is not 0 // But May Happen,reserve >0 But totalSupply = 0 + uint256 result = totalSupply(); - if (totalSupply() == 0) { + if (result == 0) { // case 1. initial supply if (quoteBalance == 0) { revert ErrZeroQuoteAmount(); } shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_) ? DecimalMath.divFloor(quoteBalance, _I_) : baseBalance; _BASE_TARGET_ = shares.toUint112(); _QUOTE_TARGET_ = DecimalMath.mulFloor(shares, _I_).toUint112(); if (_QUOTE_TARGET_ == 0) { revert ErrZeroQuoteTarget(); } if (shares <= 2001) { revert ErrMintAmountNotEnough(); } _mint(address(0), 1001); shares -= 1001; } else if (baseReserve > 0 && quoteReserve > 0) { // case 2. normal case uint256 baseInputRatio = DecimalMath.divFloor(baseInput, baseReserve); uint256 quoteInputRatio = DecimalMath.divFloor(quoteInput, quoteReserve); uint256 mintRatio = quoteInputRatio < baseInputRatio ? quoteInputRatio : baseInputRatio; - shares = DecimalMath.mulFloor(totalSupply(), mintRatio); + shares = DecimalMath.mulFloor(result , mintRatio); _BASE_TARGET_ = (uint256(_BASE_TARGET_) + DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)).toUint112(); _QUOTE_TARGET_ = (uint256(_QUOTE_TARGET_) + DecimalMath.mulFloor(uint256(_QUOTE_TARGET_), mintRatio)).toUint112(); } _mint(to, shares); _setReserve(baseBalance, quoteBalance); emit BuyShares(to, shares, balanceOf(to)); }
Using nested if statements instead of combining conditions with logical AND operators (&&) can result in gas savings, particularly in scenarios where most conditions are false. This approach ensures that once a condition is found to be false, subsequent conditions are not evaluated, reducing the overall computational effort. Saves 13 Gas
FILE: 2024-03-abracadabra-money/src/blast/BlastOnboarding.sol 114: if (caps[token] > 0 && totals[token].total > caps[token]) {
deposit()
function can be more gas optimizedFunction deposit can be optimized for gas efficiency by minimizing redundant storage operations and optimizing conditional logic.
FILE: 2024-03-abracadabra-money/src/blast/BlastOnboarding.sol function deposit(address token, uint256 amount, bool lock_) external whenNotPaused onlyState(State.Opened) onlySupportedTokens(token) { token.safeTransferFrom(msg.sender, address(this), amount); if (lock_) { totals[token].locked += amount; balances[msg.sender][token].locked += amount; } else { totals[token].unlocked += amount; balances[msg.sender][token].unlocked += amount; } totals[token].total += amount; if (caps[token] > 0 && totals[token].total > caps[token]) { revert ErrCapReached(); } balances[msg.sender][token].total += amount; emit LogDeposit(msg.sender, token, amount, lock_); }
function deposit(address token, uint256 amount, bool lock_) external whenNotPaused onlyState(State.Opened) onlySupportedTokens(token) { token.safeTransferFrom(msg.sender, address(this), amount); // Combine the locked and unlocked updates to reduce redundant code uint256 previousTotal = totals[token].total; // Cache the previous total for cap comparison Balance storage userBalance = balances[msg.sender][token]; // Use storage reference to reduce repeated lookups if (lock_) { totals[token].locked += amount; userBalance.locked += amount; } else { totals[token].unlocked += amount; userBalance.unlocked += amount; } uint256 updatedTotal = previousTotal + amount; totals[token].total = updatedTotal; // Update total once after calculation // Check the cap with updated total to save gas on unnecessary condition evaluation if (caps[token] > 0 && updatedTotal > caps[token]) { revert ErrCapReached(); } userBalance.total += amount; // Update user's total once after calculation emit LogDeposit(msg.sender, token, amount, lock_); }
In the optimized code:
State variable is only used once in a function, directly use the state variable without storing it in a local variable. Saves 13 GAS per instance
FILE: 2024-03-abracadabra-money/src/blast /BlastOnboardingBoot.sol function previewTotalPoolShares() external view returns (uint256 baseAdjustedInAmount, uint256 quoteAdjustedInAmount, uint256 shares) { - uint256 baseAmount = totals[MIM].locked; - uint256 quoteAmount = totals[USDB].locked; - return router.previewCreatePool(I, baseAmount, quoteAmount); + return router.previewCreatePool(I, totals[MIM].locked, totals[USDB].locked); } - int256 baseAmount = totals[MIM].locked; - uint256 quoteAmount = totals[USDB].locked; MIM.safeApprove(address(router), type(uint256).max); USDB.safeApprove(address(router), type(uint256).max); - (pool, totalPoolShares) = router.createPool(MIM, USDB, FEE_RATE, I, K, address(this), baseAmount, quoteAmount); + (pool, totalPoolShares) = router.createPool(MIM, USDB, FEE_RATE, I, K, address(this), totals[MIM].locked, totals[USDB].locked);
type(uint256).max
Using constants can lead to significant gas savings, especially when you replace commonly used expressions like type(uint256).max.
FILE: 2024-03-abracadabra-money/src/blast/BlastOnboardingBoot.sol 103: MIM.safeApprove(address(router), type(uint256).max); 104: USDB.safeApprove(address(router), type(uint256).max);
_QUOTE_RESERVE_
and _BASE_RESERVE_
values can be cached with memory variable . Saves 800 GAS
, 8 SLOD
In the given function correctRState, caching _QUOTE_RESERVE_
,_BASE_RESERVE_
as a local variable can reduce gas consumption since QUOTE_RESERVE and BASE_RESERVE is accessed multiple times. This is because reading from a local variable is cheaper in terms of gas than reading from storage.
FILE: 2024-03-abracadabra-money/src/mimswap/MagicLP.sol function correctRState() external { + uint112 _BASE_Reserve_ = _BASE_RESERVE_ ; + uint112 _QUOTE_Reserve_ = _QUOTE_RESERVE_ ; - if (_RState_ == uint32(PMMPricing.RState.BELOW_ONE) && _BASE_RESERVE_ < _BASE_TARGET_) { + if (_RState_ == uint32(PMMPricing.RState.BELOW_ONE) && _BASE_Reserve_ < _BASE_TARGET_) { _RState_ = uint32(PMMPricing.RState.ONE); - _BASE_TARGET_ = _BASE_RESERVE_; + _BASE_TARGET_ = _BASE_Reserve_ ; - _QUOTE_TARGET_ = _QUOTE_RESERVE_; + _QUOTE_TARGET_ = _QUOTE_Reserve_ ; } - if (_RState_ == uint32(PMMPricing.RState.ABOVE_ONE) && _QUOTE_RESERVE_ < _QUOTE_TARGET_) { + if (_RState_ == uint32(PMMPricing.RState.ABOVE_ONE) && _QUOTE_Reserve_< _QUOTE_TARGET_) { _RState_ = uint32(PMMPricing.RState.ONE); - _BASE_TARGET_ = _BASE_RESERVE_; + _BASE_TARGET_ = _BASE_Reserve_ ; - _QUOTE_TARGET_ = _QUOTE_RESERVE_; + _QUOTE_TARGET_ = _QUOTE_Reserve_ ; } } function correctRState() external { if (_RState_ == uint32(PMMPricing.RState.BELOW_ONE) && _BASE_RESERVE_ < _BASE_TARGET_) { _RState_ = uint32(PMMPricing.RState.ONE); _BASE_TARGET_ = _BASE_RESERVE_; _QUOTE_TARGET_ = _QUOTE_RESERVE_; } if (_RState_ == uint32(PMMPricing.RState.ABOVE_ONE) && _QUOTE_RESERVE_ < _QUOTE_TARGET_) { _RState_ = uint32(PMMPricing.RState.ONE); _BASE_TARGET_ = _BASE_RESERVE_; _QUOTE_TARGET_ = _QUOTE_RESERVE_; }
If the old value is equal to the new value, not re-storing the value will avoid a Gsreset (2900 gas), potentially at the expense of a Gcoldsload (2100 gas) or a Gwarmaccess (100 gas)
FILE: 2024-03-abracadabra-money/src/blast/BlastBox.sol function setFeeTo(address feeTo_) external onlyOwner { if (feeTo_ == address(0)) { revert ErrZeroAddress(); } feeTo = feeTo_; emit LogFeeToChanged(feeTo_); }
FILE: 2024-03-abracadabra-money/src/blast/BlastGovernor.sol function setFeeTo(address _feeTo) external onlyOwner { if(_feeTo == address(0)) { revert ErrZeroAddress(); } feeTo = _feeTo; emit LogFeeToChanged(_feeTo); }
FILE: 2024-03-abracadabra-money/src/blast /BlastMagicLP.sol function setFeeTo(address feeTo_) external onlyImplementation onlyImplementationOwner { if (feeTo_ == address(0)) { revert ErrZeroAddress(); } feeTo = feeTo_; emit LogFeeToChanged(feeTo_); } function setOperator(address operator, bool status) external onlyImplementation onlyImplementationOwner { operators[operator] = status; emit LogOperatorChanged(operator, status); }
The division cannot overflow, since both the numerator and the denominator are non-negative
FILE: 2024-03-abracadabra-money/src/blast/BlastOnboardingBoot.sol 163: return (userLocked * totalPoolShares) / totalLocked;
FILE: 2024-03-abracadabra-money/src/mimswap/MagicLP.sol 435: baseAmount = (baseBalance * shareAmount) / totalShares; 436: quoteAmount = (quoteBalance * shareAmount) / totalShares; 513: _BASE_TARGET_ = uint112((uint256(_BASE_TARGET_) * baseBalance) / uint256(_BASE_RESERVE_)); 517: _QUOTE_TARGET_ = uint112((uint256(_QUOTE_TARGET_) * quoteBalance) / uint256(_QUOTE_RESERVE_));
Use a solidity version of at least 0.8.0 to get simple compiler automatic inlining Use a solidity version of at least 0.8.3 to get better struct packing and cheaper multiple storage reads Use a solidity version of at least 0.8.4 to get custom errors, which are cheaper at deployment than revert()/require() strings Use a solidity version of at least 0.8.10 to have external calls skip contract existence checks if the external call has a return value.
FILE: 2024-03-abracadabra-money/src/mimswap/MagicLP.sol 8: pragma solidity >=0.8.0;
divCeil()
function can be optimized more gas efficientThe divCeil function can be optimized for gas efficiency by reducing the operations performed within the function. One common optimization is to combine the calculation and the conditional check into fewer steps. This Saves 50-100 Gas approximately for every call
FILE: 2024-03-abracadabra-money/src/mimswap/libraries /Math.sol function divCeil(uint256 a, uint256 b) internal pure returns (uint256) { uint256 quotient = a / b; uint256 remainder = a - quotient * b; if (remainder > 0) { return quotient + 1; } else { return quotient; }
unction divCeil(uint256 a, uint256 b) internal pure returns (uint256) { require(b > 0, "Division by zero"); // Added to prevent division by zero uint256 quotient = a / b; if (a % b > 0) { // Directly use modulo to check for a remainder return quotient + 1; } return quotient; }
Optimizing the _SolveQuadraticFunctionForTrade function involves reducing redundant calculations, reusing computed values, and ensuring operations are performed efficiently.
FILE: 2024-03-abracadabra-money/src/mimswap/libraries /Math.sol function _SolveQuadraticFunctionForTrade(uint256 V0, uint256 V1, uint256 delta, uint256 i, uint256 k) internal pure returns (uint256) { if (V0 == 0) { revert ErrIsZero(); } if (delta == 0) { return 0; } if (k == 0) { return DecimalMath.mulFloor(i, delta) > V1 ? V1 : DecimalMath.mulFloor(i, delta); } if (k == DecimalMath.ONE) { // if k==1 // Q2=Q1/(1+ideltaBQ1/Q0/Q0) // temp = ideltaBQ1/Q0/Q0 // Q2 = Q1/(1+temp) // Q1-Q2 = Q1*(1-1/(1+temp)) = Q1*(temp/(1+temp)) // uint256 temp = i.mul(delta).mul(V1).div(V0.mul(V0)); uint256 temp; uint256 idelta = i * delta; if (idelta == 0) { temp = 0; } else if ((idelta * V1) / idelta == V1) { temp = (idelta * V1) / (V0 * V0); } else { temp = (((delta * V1) / V0) * i) / V0; } return (V1 * temp) / (temp + DecimalMath.ONE); } // calculate -b value and sig // b = kQ0^2/Q1-i*deltaB-(1-k)Q1 // part1 = (1-k)Q1 >=0 // part2 = kQ0^2/Q1-i*deltaB >=0 // bAbs = abs(part1-part2) // if part1>part2 => b is negative => bSig is false // if part2>part1 => b is positive => bSig is true uint256 part2 = (((k * V0) / V1) * V0) + (i * delta); // kQ0^2/Q1-i*deltaB uint256 bAbs = (DecimalMath.ONE - k) * V1; // (1-k)Q1 bool bSig; if (bAbs >= part2) { bAbs = bAbs - part2; bSig = false; } else { bAbs = part2 - bAbs; bSig = true; } bAbs = bAbs / DecimalMath.ONE; // calculate sqrt uint256 squareRoot = DecimalMath.mulFloor((DecimalMath.ONE - k) * 4, DecimalMath.mulFloor(k, V0) * V0); // 4(1-k)kQ0^2 squareRoot = sqrt((bAbs * bAbs) + squareRoot); // sqrt(b*b+4(1-k)kQ0*Q0) // final res uint256 denominator = (DecimalMath.ONE - k) * 2; // 2(1-k) uint256 numerator; if (bSig) { numerator = squareRoot - bAbs; if (numerator == 0) { revert ErrIsZero(); } } else { numerator = bAbs + squareRoot; } uint256 V2 = DecimalMath.divCeil(numerator, denominator); if (V2 > V1) { return 0; } else { return V1 - V2; } }
function _SolveQuadraticFunctionForTrade(uint256 V0, uint256 V1, uint256 delta, uint256 i, uint256 k) internal pure returns (uint256) { require(V0 != 0, "ErrIsZero"); if (delta == 0) { return 0; } uint256 idelta = i * delta; if (k == 0) { uint256 product = DecimalMath.mulFloor(i, delta); // Reuse this value to avoid duplication return product > V1 ? V1 : product; } if (k == DecimalMath.ONE) { uint256 temp = 0; if (idelta != 0) { uint256 ideltaV1 = idelta * V1; temp = (ideltaV1 / idelta == V1) ? (ideltaV1 / (V0 * V0)) : ((delta * V1) / V0 * i) / V0; } // No need to check temp == 0 as it doesn't lead to revert return (V1 * temp) / (temp + DecimalMath.ONE); } uint256 part1 = (DecimalMath.ONE - k) * V1; uint256 part2 = idelta + (k * V0 * V0) / V1; uint256 bAbs; bool bSig; if (part1 >= part2) { bAbs = part1 - part2; bSig = false; } else { bAbs = part2 - part1; bSig = true; } // Simplify square root calculation by combining constants and removing redundant operations uint256 squareRoot = sqrt(bAbs * bAbs + DecimalMath.mulFloor((DecimalMath.ONE - k) * 4, DecimalMath.mulFloor(k, V0) * V0)); // Denominator is the same for both conditions uint256 denominator = (DecimalMath.ONE - k) * 2; uint256 numerator = bSig ? squareRoot - bAbs : bAbs + squareRoot; uint256 V2 = DecimalMath.divCeil(numerator, denominator); return V2 > V1 ? 0 : V1 - V2; }
Remove Redundant Checks: I've replaced the initial if (V0 == 0) revert condition with a require statement for clarity and gas efficiency.
Combine and Reuse Calculations: Whenever possible, combine similar calculations and reuse results to avoid duplicating expensive operations. For example, idelta (i * delta) is calculated once and reused.
Simplify Conditional Checks: Directly compare parts instead of recalculating them (for temp and bAbs calculations). This reduces the overall computational load.
Optimize Square Root Calculation: Group and simplify calculations related to the square root to avoid unnecessary multiplications.
Consolidate Return Statements: By computing V2 and directly returning the result of V1 - V2 (if appropriate), the function avoids unnecessary branches and makes the logic clearer.
#0 - c4-pre-sort
2024-03-15T14:58:46Z
141345 marked the issue as sufficient quality report
#1 - 141345
2024-03-16T00:17:19Z
89 Sathish9098 l r nc 4 3 7
G 1 l G 2 l G 3 n G 4 l G 5 n G 6 r G 7 n G 8 n G 9 l G 10 n G 11 n G 12 n G 13 r G 14 r
#2 - c4-judge
2024-03-29T16:46:20Z
thereksfour marked the issue as grade-b
#3 - sathishpic22
2024-04-04T06:41:21Z
Hi @thereksfour
Thank you for your evaluation.
Upon reviewing the reports that received a grade of A, I've observed that, although I submitted fewer findings, mine have a higher impact, particularly in terms of gas savings. The grade A reports have a greater number of findings, but their overall impact is lower compared to mine. It appears that these reports might have utilized bots to identify findings, which explains their higher count but not necessarily greater relevance or impact. Specifically, my four low-severity findings alone could save nearly 8,000 gas, surpassing the total gas savings of some grade A reports.
My report offers more value by avoiding irrelevant and false findings, and it also presents high-value insights with 4 low-severity findings and 3 refactor suggestions.
If the grading is based on the NC (Non-Critical) score, then prioritizing quantity over quality does not truly value the work of those who find more genuine findings that result in significant gas savings for the protocol. A good report with substantial findings can be overshadowed by a large number of NC findings. Often, NC findings are not implemented in the original code due to the minimal gas savings they offer. Many of these NC findings are typically ignored by sponsors, underscoring the importance of focusing on impactful improvements rather than the sheer number of findings.
I am confident that my report provides more value to the sponsor and deserves a higher grade than it currently has. Thank you for this opportunity to express my views. If my understanding is incorrect, please advise me.
Thank you.
#4 - c4-judge
2024-04-06T07:33:56Z
thereksfour marked the issue as grade-a