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: 23/36
Findings: 2
Award: $184.90
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: ether_sky
Also found by: 0x11singh99, 0xE1, 0xJaeger, Bauchibred, Bigsam, Bozho, Breeje, DarkTower, HChang26, SpicyMeatball, Trust, ZanyBonzy, albahaca, bareli, blutorque, grearlake, hals, hassan-truscova, hihen, oualidpro, pfapostol, ravikiranweb3, slvDev, zhaojie
15.328 USDC - $15.33
| issue | instance | |
|---|---|---|
| [L-01] | _setupRole is Deprecated: | 6 |
| [L-02] | Missing checks for address(0x0) when assigning values to address state variables: | 2 |
| [L-03] | Unsafe downcast: | 7 |
| [L-04] | Array does not have a pop function: | 5 |
| [L-05] | Approve type(uint256).max may not work with some tokens: | 2 |
| [L-06] | Setters should have initial value check: | 19 |
| [L-07] | Empty receive()/payable fallback() function does not authorize requests: | 2 |
| [L-08] | Contracts are designed to receive ETH but do not implement function for withdrawal: | 2 |
| [L-09] | latestAnswer() is deprecated: | 1 |
| [L-10] | Int casting block.timestamp can reduce the lifespan of a contract: | 1 |
| [L-11] | Contract inherits Pausable but does not expose pausing/unpausing functionality: | 1 |
| [L-12] | No limits when setting state variable amounts: | 21 |
| [L-13] | Pausing withdrawals is unfair to the users: | 1 |
| [L-14] | Allowed fees/rates should be capped by smart contracts: | 6 |
| [L-15] | Owner can renounce ownership while system is paused: | 2 |
| [L-16] | Constructor contains no validation: | 2 |
| [L-17] | For loops in public or external functions should be avoided due to high gas costs and possible DOS: | 2 |
| [L-18] | Function calls within for loops: | 4 |
| [L-19] | External calls in an unbounded for-loop may result in a DoS: | 3 |
| [L-20] | Multiplication on the result of a division: | 4 |
| [L-21] | Governance functions should be controlled by time locks: | 35 |
| [L-22] | Code does not follow the best practice of check-effects-interaction: | 4 |
| [L-23] | The additions/multiplications may silently overflow because they're in unchecked blocks with no preceding value checks, which may lead to unexpected results.: | 1 |
| [L-24] | Unbounded state array which is iterated upon: | 14 |
| [L-25] | Prefer continue over revert model in iteration: | 2 |
| [L-26] | Contracts use infinite approvals with no means to revoke: | 1 |
| [L-27] | External calls in modifiers should be avoided: | 1 |
_setupRole is Deprecated:_setupRole is deprecated in favor of grantRole and renounceRole.
There are 6 instances of this issue:
File: src/blast/BlastOnboardingBoot.sol MIM.safeApprove(address(router), type(uint256).max); USDB.safeApprove(address(router), type(uint256).max); pool.safeApprove(address(staking), totalPoolShares); MIM.safeApprove(address(router), type(uint256).max); USDB.safeApprove(address(router), type(uint256).max); pool.safeApprove(address(staking), totalPoolShares);
address(0x0) when assigning values to address state variables:This issue arises when an address state variable is assigned a value without a preceding check to ensure it isn't address(0x0). This can lead to unexpected behavior as address(0x0) often represents an uninitialized address.
There are 2 instances of this issue:
File: src/blast/BlastOnboarding.sol 191 bootstrapper = bootstrapper_;
File: src/mimswap/auxiliary/FeeRateModel.sol 58 implementation = implementation_;
When a type is downcast to a smaller type, the higher order bits are truncated, effectively applying a modulo to the original value. Without any other checks, this wrapping will lead to unexpected behavior and bugs.
There are 7 instances of this issue
File: src/mimswap/MagicLP.sol 438 _BASE_TARGET_ = uint112(uint256(_BASE_TARGET_) - (uint256(_BASE_TARGET_) * shareAmount).divCeil(totalShares)); 125 _BLOCK_TIMESTAMP_LAST_ = uint32(block.timestamp % 2 ** 32); 495 _I_ = uint128(newI); 494 _K_ = uint64(newK); 439 _QUOTE_TARGET_ = uint112(uint256(_QUOTE_TARGET_) - (uint256(_QUOTE_TARGET_) * shareAmount).divCeil(totalShares)); 493 _LP_FEE_RATE_ = uint64(newLpFeeRate);
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L493:493
File: src/staking/LockingMultiRewards.sol 389 reward.lastUpdateTime = uint248(block.timestamp);
pop function:Arrays without the pop operation in Solidity can lead to inefficient memory management and increase the likelihood of out-of-gas errors.
There are 5 instances of this issue
File: src/mimswap/periphery/Factory.sol 149 pools[baseToken][quoteToken].push(pool); 150 userPools[creator].push(pool);
File: src/staking/LockingMultiRewards.sol 313 rewardTokens.push(rewardToken); 501 _userLocks[user].push(LockedBalance({amount: amount, unlockTime: _nextUnlockTime})); 648 _userRewardLock[user].items.push(RewardLockItem({token: rewardToken, amount: rewardAmount}));
type(uint256).max may not work with some tokens:Source: https://github.com/d-xo/weird-erc20#revert-on-large-approvals--transfers
There are 2 instances of this issue
File: src/blast/BlastOnboardingBoot.sol 103 MIM.safeApprove(address(router), type(uint256).max); 104 USDB.safeApprove(address(router), type(uint256).max);
Setters should have initial value check to prevent assigning wrong value to the variable. Assginment of wrong value can lead to unexpected behavior of the contract.
There are 19 instances of this issue
File: src/blast/BlastBox.sol 72 function setFeeTo(address feeTo_) external onlyOwner { 73 if (feeTo_ == address(0)) { 74 revert ErrZeroAddress(); 75 } 76 77 feeTo = feeTo_; 78 emit LogFeeToChanged(feeTo_); 79 } 81 function setTokenEnabled(address token, bool enabled) external onlyOwner { 82 enabledTokens[token] = enabled; 83 84 // enable native yields if token is recognized 85 // no matter if it's enabled or not 86 if (registry.nativeYieldTokens(token)) { 87 BlastYields.enableTokenClaimable(token); 88 } 89 90 emit LogTokenDepositEnabled(token, enabled); 91 }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastBox.sol#L81:91
File: src/blast/BlastGovernor.sol 40 function setFeeTo(address _feeTo) external onlyOwner { 41 if(_feeTo == address(0)) { 42 revert ErrZeroAddress(); 43 } 44 45 feeTo = _feeTo; 46 emit LogFeeToChanged(_feeTo); 47 }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastGovernor.sol#L40:47
File: src/blast/BlastMagicLP.sol 80 function setFeeTo(address feeTo_) external onlyImplementation onlyImplementationOwner { 81 if (feeTo_ == address(0)) { 82 revert ErrZeroAddress(); 83 } 84 85 feeTo = feeTo_; 86 emit LogFeeToChanged(feeTo_); 87 } 89 function setOperator(address operator, bool status) external onlyImplementation onlyImplementationOwner { 90 operators[operator] = status; 91 emit LogOperatorChanged(operator, status); 92 }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastMagicLP.sol#L89:92
File: src/blast/BlastOnboarding.sol 147 function setFeeTo(address feeTo_) external onlyOwner { 148 if (feeTo_ == address(0)) { 149 revert ErrZeroAddress(); 150 } 151 152 feeTo = feeTo_; 153 emit LogFeeToChanged(feeTo_); 154 } 175 function setTokenSupported(address token, bool supported) external onlyOwner { 176 supportedTokens[token] = supported; 177 178 if (registry.nativeYieldTokens(token)) { 179 BlastYields.enableTokenClaimable(token); 180 } 181 182 emit LogTokenSupported(token, supported); 183 } 185 function setCap(address token, uint256 cap) external onlyOwner onlySupportedTokens(token) { 186 caps[token] = cap; 187 emit LogTokenCapChanged(token, cap); 188 } 190 function setBootstrapper(address bootstrapper_) external onlyOwner { 191 bootstrapper = bootstrapper_; 192 emit LogBootstrapperChanged(bootstrapper_); 193 }
File: src/blast/BlastOnboardingBoot.sol 137 function setStaking(LockingMultiRewards _staking) external onlyOwner { 138 if (ready) { 139 revert ErrCannotChangeOnceReady(); 140 } 141 142 staking = _staking; 143 emit LogStakingChanged(address(_staking)); 144 } 146 function setReady(bool _ready) external onlyOwner onlyState(State.Closed) { 147 ready = _ready; 148 emit LogReadyChanged(ready); 149 }
File: src/blast/BlastTokenRegistry.sol 14 function setNativeYieldTokenEnabled(address token, bool enabled) external onlyOwner { 15 if (token == address(0)) { 16 revert ErrZeroAddress(); 17 } 18 19 nativeYieldTokens[token] = enabled; 20 emit LogNativeYieldTokenEnabled(token, enabled); 21 }
File: src/mimswap/MagicLP.sol 470 function setParameters( 471 address assetTo, 472 uint256 newLpFeeRate, 473 uint256 newI, 474 uint256 newK, 475 uint256 baseOutAmount, 476 uint256 quoteOutAmount, 477 uint256 minBaseReserve, 478 uint256 minQuoteReserve 479 ) public nonReentrant onlyImplementationOwner { 480 if (_BASE_RESERVE_ < minBaseReserve || _QUOTE_RESERVE_ < minQuoteReserve) { 481 revert ErrReserveAmountNotEnough(); 482 } 483 if (newI == 0 || newI > MAX_I) { 484 revert ErrInvalidI(); 485 } 486 if (newK > MAX_K) { 487 revert ErrInvalidK(); 488 } 489 if (newLpFeeRate < MIN_LP_FEE_RATE || newLpFeeRate > MAX_LP_FEE_RATE) { 490 revert ErrInvalidLPFeeRate(); 491 } 492 493 _LP_FEE_RATE_ = uint64(newLpFeeRate); 494 _K_ = uint64(newK); 495 _I_ = uint128(newI); 496 497 _transferBaseOut(assetTo, baseOutAmount); 498 _transferQuoteOut(assetTo, quoteOutAmount); 499 (uint256 newBaseBalance, uint256 newQuoteBalance) = _resetTargetAndReserve(); 500 501 emit ParametersChanged(newLpFeeRate, newI, newK, newBaseBalance, newQuoteBalance); 502 } 560 function _setReserve(uint256 baseReserve, uint256 quoteReserve) internal { 561 _BASE_RESERVE_ = baseReserve.toUint112(); 562 _QUOTE_RESERVE_ = quoteReserve.toUint112(); 563 564 _twapUpdate(); 565 }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L560:565
File: src/mimswap/auxiliary/FeeRateModel.sol 46 function setMaintainer(address maintainer_) external onlyOwner { 47 if (maintainer_ == address(0)) { 48 revert ErrZeroAddress(); 49 } 50 51 maintainer = maintainer_; 52 emit LogMaintainerChanged(maintainer_); 53 } 57 function setImplementation(address implementation_) public onlyOwner { 58 implementation = implementation_; 59 emit LogImplementationChanged(implementation_); 60 }
File: src/mimswap/periphery/Factory.sol 96 function setLpImplementation(address implementation_) external onlyOwner { 97 if (implementation_ == address(0)) { 98 revert ErrZeroAddress(); 99 } 100 101 implementation = implementation_; 102 emit LogSetImplementation(implementation_); 103 } 105 function setMaintainerFeeRateModel(IFeeRateModel maintainerFeeRateModel_) external onlyOwner { 106 if (address(maintainerFeeRateModel_) == address(0)) { 107 revert ErrZeroAddress(); 108 } 109 110 maintainerFeeRateModel = maintainerFeeRateModel_; 111 emit LogSetMaintainerFeeRateModel(maintainerFeeRateModel_); 112 }
File: src/staking/LockingMultiRewards.sol 317 function setMinLockAmount(uint256 _minLockAmount) external onlyOwner { 318 emit LogSetMinLockAmount(minLockAmount, _minLockAmount); 319 minLockAmount = _minLockAmount; 320 }
receive()/payable fallback() function does not authorize requests:If the intention is for the Ether to be used, the function should call another function, otherwise it should revert (e.g. require(msg.sender == address(weth))). 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. If the concern is having to spend a small amount of gas to check the sender against an immutable address, the code should at least have a function to rescue unused Ether.
There are 2 instances of this issue
File: src/blast/BlastGovernor.sol 13 receive() external payable {}
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastGovernor.sol#L13:13
File: src/mimswap/periphery/Router.sol 36 receive() external payable {}
The following contracts can receive ETH but can not withdraw, ETH is occasionally sent by users will be stuck in those contracts. This functionality also applies to baseTokens resulting in locked tokens and loss of funds. There are 2 instances of this issue
File: src/blast/BlastGovernor.sol 13 receive() external payable {}
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastGovernor.sol#L13:13
File: src/mimswap/periphery/Router.sol 36 receive() external payable {}
Use latestRoundData() instead so that you can tell whether the answer is stale or not. The latestAnswer() function returns zero if it is unable to fetch data, which may be the case if ChainLink stops supporting this API. The API and its deprecation message no longer even appear on the ChainLink website, so it is dangerous to continue using it.
There are 1 instances of this issue
File: src/oracles/aggregators/MagicLpAggregator.sol function latestAnswer() public view override returns (int256) { uint256 baseAnswerNomalized = uint256(baseOracle.latestAnswer()) * (10 ** (WAD - baseOracle.decimals())); uint256 quoteAnswerNormalized = uint256(quoteOracle.latestAnswer()) * (10 ** (WAD - quoteOracle.decimals())); return (0, latestAnswer(), 0, 0, 0);
block.timestamp can reduce the lifespan of a contract:Consider removing casting to ensure future functionality.
File: src/staking/LockingMultiRewards.sol 389 reward.lastUpdateTime = uint248(block.timestamp);
When a contract inherits from the Pausable contract in Solidity, it gains the ability to pause and resume certain actions, which can be crucial in mitigating potential risks or issues. However, the _pause and _unpause functions are internal and can only be called from within the contract or derived contracts. If the functionality to pause and unpause isn't exposed through public or external functions, it can't be called by the contract owner or designated pauser account. To take advantage of the Pausable contract, create public or external functions that call _pause and _unpause.
File: src/blast/BlastOnboarding.sol 12 contract BlastOnboardingData is Owned, Pausable {
It is important to ensure state variables numbers are set to a reasonable value.
File: src/mimswap/MagicLP.sol 125 _BLOCK_TIMESTAMP_LAST_ = uint32(block.timestamp % 2 ** 32); 140 _RState_ = uint32(PMMPricing.RState.ONE); 142 _QUOTE_TARGET_ = _QUOTE_RESERVE_; 145 _RState_ = uint32(PMMPricing.RState.ONE); 194 state.i = _I_; 195 state.K = _K_; 196 state.B = _BASE_RESERVE_; 197 state.Q = _QUOTE_RESERVE_; 198 state.B0 = _BASE_TARGET_; // will be calculated in adjustedTarget 199 state.Q0 = _QUOTE_TARGET_; 212 R = uint256(state.R); 220 baseReserve = _BASE_RESERVE_; 221 quoteReserve = _QUOTE_RESERVE_; 258 _RState_ = uint32(newRState); 281 _RState_ = uint32(newRState); 322 _RState_ = uint32(newRState); 344 _RState_ = uint32(newRState); 493 _LP_FEE_RATE_ = uint64(newLpFeeRate); 494 _K_ = uint64(newK); 495 _I_ = uint128(newI); 540 _RState_ = uint32(PMMPricing.RState.ONE); 557 _BLOCK_TIMESTAMP_LAST_ = blockTimestamp;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L557:557
File: src/mimswap/libraries/Math.sol 31 y = x;
File: src/mimswap/periphery/Router.sol 155 quoteAdjustedInAmount = quoteInAmount; 524 baseAdjustedInAmount = shares; 531 baseAdjustedInAmount = baseInAmount;
Users should always have the possibility of accessing their own funds, but when these functions are paused, their funds will be locked until the contracts are unpaused.
File: src/blast/BlastOnboarding.sol 132 function withdraw(address token, uint256 amount) external whenNotPaused onlySupportedTokens(token) {
Fees/rates should be required to be below 100%, preferably at a much lower limit, to ensure users don't have to monitor the blockchain for changes prior to using the protocol.
File: src/blast/BlastBox.sol 72 function setFeeTo(address feeTo_) external onlyOwner { 73 if (feeTo_ == address(0)) { 74 revert ErrZeroAddress(); 75 } 76 77 feeTo = feeTo_; 78 emit LogFeeToChanged(feeTo_); 79 }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastBox.sol#L72:79
File: src/blast/BlastGovernor.sol 40 function setFeeTo(address _feeTo) external onlyOwner { 41 if(_feeTo == address(0)) { 42 revert ErrZeroAddress(); 43 } 44 45 feeTo = _feeTo; 46 emit LogFeeToChanged(_feeTo); 47 }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastGovernor.sol#L40:47
File: src/blast/BlastMagicLP.sol 80 function setFeeTo(address feeTo_) external onlyImplementation onlyImplementationOwner { 81 if (feeTo_ == address(0)) { 82 revert ErrZeroAddress(); 83 } 84 85 feeTo = feeTo_; 86 emit LogFeeToChanged(feeTo_); 87 }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastMagicLP.sol#L80:87
File: src/blast/BlastOnboarding.sol 147 function setFeeTo(address feeTo_) external onlyOwner { 148 if (feeTo_ == address(0)) { 149 revert ErrZeroAddress(); 150 } 151 152 feeTo = feeTo_; 153 emit LogFeeToChanged(feeTo_); 154 }
File: src/mimswap/periphery/Factory.sol 105 function setMaintainerFeeRateModel(IFeeRateModel maintainerFeeRateModel_) external onlyOwner { 106 if (address(maintainerFeeRateModel_) == address(0)) { 107 revert ErrZeroAddress(); 108 } 109 110 maintainerFeeRateModel = maintainerFeeRateModel_; 111 emit LogSetMaintainerFeeRateModel(maintainerFeeRateModel_); 112 }
The contract owner is not prevented from renouncing the ownership while the contract is paused, which would cause any user assets stored in the protocol to be locked indefinitely.
File: src/blast/BlastOnboarding.sol 214 function pause() external onlyOwner {
File: src/staking/LockingMultiRewards.sol 337 function pause() external onlyOwner {
In Solidity, when values are being assigned in constructors to unsigned or integer variables, it's crucial to ensure the provided values adhere to the protocol's specific operational boundaries as laid out in the project specifications and documentation. If the constructors lack appropriate validation checks, there's a risk of setting state variables with values that could cause unexpected and potentially detrimental behavior within the contract's operations, violating the intended logic of the protocol. This can compromise the contract's security and impact the maintainability and reliability of the system. In order to avoid such issues, it is recommended to incorporate rigorous validation checks in constructors. These checks should align with the project's defined rules and constraints, making use of Solidity's built-in require function to enforce these conditions. If the validation checks fail, the require function will cause the transaction to revert, ensuring the integrity and adherence to the protocol's expected behavior.
File: src/mimswap/MagicLP.sol 84 constructor(address owner_) Owned(owner_) { 85 implementation = this; 86 87 // prevents the implementation contract initialization 88 _INITIALIZED_ = true; 89 }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L84:89
File: src/oracles/aggregators/MagicLpAggregator.sol 21 constructor(IMagicLP pair_, IAggregator baseOracle_, IAggregator quoteOracle_) { 22 pair = pair_; 23 baseOracle = baseOracle_; 24 quoteOracle = quoteOracle_; 25 baseDecimals = IERC20Metadata(pair_._BASE_TOKEN_()).decimals(); 26 quoteDecimals = IERC20Metadata(pair_._QUOTE_TOKEN_()).decimals(); 27 }
public or external functions should be avoided due to high gas costs and possible DOS:In Solidity, for loops can potentially cause Denial of Service (DoS) attacks if not handled carefully. DoS attacks can occur when an attacker intentionally exploits the gas cost of a function, causing it to run out of gas or making it too expensive for other users to call. Below are some scenarios where for loops can lead to DoS attacks: Nested for loops can become exceptionally gas expensive and should be used sparingly.
File: src/blast/BlastOnboarding.sol 164 function claimTokenYields(address[] memory tokens) external onlyOwner { 165 for (uint256 i = 0; i < tokens.length; i++) { 166 if (!supportedTokens[tokens[i]]) { 167 revert ErrUnsupportedToken(); 168 } 169 if (registry.nativeYieldTokens(tokens[i])) { 170 BlastYields.claimAllTokenYields(tokens[i], feeTo); 171 } 172 } 173 }
File: src/staking/LockingMultiRewards.sol 397 function processExpiredLocks(address[] memory users, uint256[] calldata lockIndexes) external onlyOperators { 398 if (users.length != lockIndexes.length) { 399 revert ErrLengthMismatch(); 400 } 401 402 _updateRewardsForUsers(users); 403 404 // Release all expired users' locks 405 for (uint256 i; i < users.length; ) { 406 address user = users[i]; 407 Balances storage bal = _balances[user]; 408 LockedBalance[] storage locks = _userLocks[user]; 409 410 if (locks.length == 0) { 411 revert ErrNoLocks(); 412 } 413 414 uint256 index = lockIndexes[i]; 415 416 // Prevents processing `lastLockIndex` out of order 417 if (index == lastLockIndex[user] && locks.length > 1) { 418 revert ErrInvalidLockIndex(); 419 } 420 421 // prohibit releasing non-expired locks 422 if (locks[index].unlockTime > block.timestamp) { 423 revert ErrLockNotExpired(); 424 } 425 426 uint256 amount = locks[index].amount; 427 uint256 lastIndex = locks.length - 1; 428 429 /// Last lock index changed place with the one we just swapped. 430 if (lastLockIndex[user] == lastIndex) { 431 lastLockIndex[user] = index; 432 } 433 434 if (index != lastIndex) { 435 locks[index] = locks[lastIndex]; 436 emit LogLockIndexChanged(user, lastIndex, index); 437 } 438 439 locks.pop(); 440 441 unlockedSupply += amount; 442 lockedSupply -= amount; 443 444 bal.unlocked += amount; 445 bal.locked -= amount; 446 447 emit LogUnlocked(user, amount, index); 448 449 unchecked { 450 ++i; 451 } 452 } 453 }
Making function calls within loops in Solidity can lead to inefficient gas usage, potential bottlenecks, and increased vulnerability to attacks. Each function call or external call consumes gas, and when executed within a loop, the gas cost multiplies, potentially causing the transaction to run out of gas or exceed block gas limits. This can result in transaction failure or unpredictable behavior.
File: src/staking/LockingMultiRewards.sol 580 for (uint256 j; j < users.length; ) { 581 address user = users[j]; 582 _udpateUserRewards(user, balanceOf(user), token, rewardPerToken_); 583 584 unchecked { 585 ++j; 586 } 587 } 561 for (uint256 i; i < rewardTokens.length; ) { 562 address token = rewardTokens[i]; 563 _udpateUserRewards(user, balance, token, _updateRewardsGlobal(token, totalSupply_)); 564 565 unchecked { 566 ++i; 567 } 568 } 547 for (uint256 i; i < rewardTokens.length; ) { 548 _updateRewardsGlobal(rewardTokens[i], totalSupply_); 549 unchecked { 550 ++i; 551 } 552 } 575 for (uint256 i; i < rewardTokens.length; ) { 576 address token = rewardTokens[i]; 577 uint256 rewardPerToken_ = _updateRewardsGlobal(token, totalSupply_); 578 579 // Record each user's rewards 580 for (uint256 j; j < users.length; ) { 581 address user = users[j]; 582 _udpateUserRewards(user, balanceOf(user), token, rewardPerToken_); 583 584 unchecked { 585 ++j; 586 } 587 } 588 589 unchecked { 590 ++i; 591 } 592 }
Consider limiting the number of iterations in for-loops that make external calls.
File: src/blast/BlastOnboarding.sol 165 for (uint256 i = 0; i < tokens.length; i++) {
File: src/staking/LockingMultiRewards.sol 405 for (uint256 i; i < users.length; ) { 614 for (uint256 i; i < rewardTokens.length; ) {
Dividing an integer by another integer will often result in loss of precision. When the result is multiplied by another number, the loss of precision is magnified, often to material magnitudes. (X / Z) * Y should be re-written as (X * Y) / Z.
File: src/mimswap/libraries/Math.sol 99 _sqrt = sqrt(((ki / V1) * delta) + DecimalMath.ONE2); 158 temp = (((delta * V1) / V0) * i) / V0; 170 uint256 part2 = (((k * V0) / V1) * V0) + (i * delta); // kQ0^2/Q1-i*deltaB
File: src/staking/LockingMultiRewards.sol 258 return (block.timestamp / rewardsDuration) * rewardsDuration;
Governance functions (such as upgrading contracts, setting critical parameters) should be controlled using time locks to introduce a delay between a proposal and its execution. This gives users time to exit before a potentially dangerous or malicious operation is applied.
File: src/blast/BlastBox.sol 81 function setTokenEnabled(address token, bool enabled) external onlyOwner { 68 function callBlastPrecompile(bytes calldata data) external onlyOwner { 72 function setFeeTo(address feeTo_) external onlyOwner {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastBox.sol#L72:79
File: src/blast/BlastGovernor.sol 40 function setFeeTo(address _feeTo) external onlyOwner { 53 function execute(address to, uint256 value, bytes calldata data) external onlyOwner returns (bool success, bytes memory result) { 49 function callBlastPrecompile(bytes calldata data) external onlyOwner {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastGovernor.sol#L49:51
File: src/blast/BlastOnboarding.sol 175 function setTokenSupported(address token, bool supported) external onlyOwner { 156 function callBlastPrecompile(bytes calldata data) external onlyOwner { 218 function unpause() external onlyOwner { 160 function claimGasYields() external onlyOwner returns (uint256) { 205 function rescue(address token, address to, uint256 amount) external onlyOwner { 147 function setFeeTo(address feeTo_) external onlyOwner { 214 function pause() external onlyOwner { 195 function open() external onlyOwner onlyState(State.Idle) { 200 function close() external onlyOwner onlyState(State.Opened) { 185 function setCap(address token, uint256 cap) external onlyOwner onlySupportedTokens(token) { 190 function setBootstrapper(address bootstrapper_) external onlyOwner { 164 function claimTokenYields(address[] memory tokens) external onlyOwner {
File: src/blast/BlastOnboardingBoot.sol 129 function initialize(Router _router) external onlyOwner { 96 function bootstrap(uint256 minAmountOut) external onlyOwner onlyState(State.Closed) returns (address, address, uint256) { 137 function setStaking(LockingMultiRewards _staking) external onlyOwner { 146 function setReady(bool _ready) external onlyOwner onlyState(State.Closed) {
File: src/blast/BlastTokenRegistry.sol 14 function setNativeYieldTokenEnabled(address token, bool enabled) external onlyOwner {
File: src/mimswap/auxiliary/FeeRateModel.sol 46 function setMaintainer(address maintainer_) external onlyOwner { 57 function setImplementation(address implementation_) public onlyOwner {
File: src/mimswap/periphery/Factory.sol 105 function setMaintainerFeeRateModel(IFeeRateModel maintainerFeeRateModel_) external onlyOwner { 120 function removePool( 121 address creator, 122 address baseToken, 123 address quoteToken, 124 uint256 poolIndex, 125 uint256 userPoolIndex 126 ) external onlyOwner { 96 function setLpImplementation(address implementation_) external onlyOwner { 116 function addPool(address creator, address baseToken, address quoteToken, address pool) external onlyOwner {
File: src/staking/LockingMultiRewards.sol 337 function pause() external onlyOwner { 300 function addReward(address rewardToken) public onlyOwner { 317 function setMinLockAmount(uint256 _minLockAmount) external onlyOwner { 324 function recover(address tokenAddress, uint256 tokenAmount) external onlyOwner { 341 function unpause() external onlyOwner {
Code should follow the best-practice of check-effects-interaction, where state variables are updated before any external calls are made. Doing so prevents a large class of reentrancy bugs.
File: src/blast/BlastOnboarding.sol /// @audit token.safeTransferFrom() called prior to this assignment 118 balances[msg.sender][token].total += amount;
File: src/mimswap/MagicLP.sol /// @audit _BASE_TOKEN_.balanceOf() called prior to this assignment 557 _BLOCK_TIMESTAMP_LAST_ = blockTimestamp; /// @audit _BASE_TOKEN_.balanceOf() called prior to this assignment /// @audit State change when _sync() calls _twapUpdate() /// @audit at line 553 _BASE_PRICE_CUMULATIVE_LAST_ += getMidPrice() * timeElapsed; 578 _twapUpdate();
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L578:578
File: src/staking/LockingMultiRewards.sol /// @audit _getRewards() called prior to this assignment /// @audit contains nested function call rewardToken.safeTransfer() /// @audit located in file 2024-03-abracadabra-money/src/staking/LockingMultiRewards.sol 619 rewards[user][rewardToken] = 0;
The additions/multiplications may silently overflow because they're in unchecked blocks with no preceding value checks, which may lead to unexpected results.
File: src/mimswap/MagicLP.sol 553 _BASE_PRICE_CUMULATIVE_LAST_ += getMidPrice() * timeElapsed;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L553:553
Iterating over an unbounded state array in Solidity can result in excessive gas consumption, especially if the array size exceeds the block gas limit. This issue commonly arises in tasks like token distribution. To address this, it is recommended to limit array sizes for iteration, consider alternative data structures like linked lists, adopt paginated processing for smaller batches over multiple transactions, or use a 'state array' with a separate index-tracking array to manage large datasets and avoid gas-related problems.
File: src/staking/LockingMultiRewards.sol 407 Balances storage bal = _balances[user]; 408 LockedBalance[] storage locks = _userLocks[user]; 417 if (index == lastLockIndex[user] && locks.length > 1) { 430 if (lastLockIndex[user] == lastIndex) { 431 lastLockIndex[user] = index; 548 _updateRewardsGlobal(rewardTokens[i], totalSupply_); 562 address token = rewardTokens[i]; 576 address token = rewardTokens[i]; 615 address rewardToken = rewardTokens[i]; 616 uint256 rewardAmount = rewards[user][rewardToken]; 616 uint256 rewardAmount = rewards[user][rewardToken]; 619 rewards[user][rewardToken] = 0; 619 rewards[user][rewardToken] = 0; 648 _userRewardLock[user].items.push(RewardLockItem({token: rewardToken, amount: rewardAmount}));
Preferably, it's better to skip operations on array indices when a condition is not met instead of reverting the entire transaction. Reverting could be exploited by malicious actors who might intentionally introduce array objects failing conditional checks, disrupting group operations. It's advisable to skip array indices rather than revert, unless there are valid security or logic reasons for doing otherwise
File: src/blast/BlastOnboarding.sol 165 for (uint256 i = 0; i < tokens.length; i++) { 166 if (!supportedTokens[tokens[i]]) { 167 revert ErrUnsupportedToken(); 168 } 169 if (registry.nativeYieldTokens(tokens[i])) { 170 BlastYields.claimAllTokenYields(tokens[i], feeTo); 171 } 172 }
File: src/staking/LockingMultiRewards.sol 405 for (uint256 i; i < users.length; ) { 406 address user = users[i]; 407 Balances storage bal = _balances[user]; 408 LockedBalance[] storage locks = _userLocks[user]; 409 410 if (locks.length == 0) { 411 revert ErrNoLocks(); 412 } 413 414 uint256 index = lockIndexes[i]; 415 416 // Prevents processing `lastLockIndex` out of order 417 if (index == lastLockIndex[user] && locks.length > 1) { 418 revert ErrInvalidLockIndex(); 419 } 420 421 // prohibit releasing non-expired locks 422 if (locks[index].unlockTime > block.timestamp) { 423 revert ErrLockNotExpired(); 424 } 425 426 uint256 amount = locks[index].amount; 427 uint256 lastIndex = locks.length - 1; 428 429 /// Last lock index changed place with the one we just swapped. 430 if (lastLockIndex[user] == lastIndex) { 431 lastLockIndex[user] = index; 432 } 433 434 if (index != lastIndex) { 435 locks[index] = locks[lastIndex]; 436 emit LogLockIndexChanged(user, lastIndex, index); 437 } 438 439 locks.pop(); 440 441 unlockedSupply += amount; 442 lockedSupply -= amount; 443 444 bal.unlocked += amount; 445 bal.locked -= amount; 446 447 emit LogUnlocked(user, amount, index); 448 449 unchecked { 450 ++i; 451 } 452 }
Infinite approvals on external contracts can be dangerous if the target becomes compromised. See here for a list of approval exploits. The following contracts are vulnerable to such attacks since they have no functionality to revoke the approval (call with amount ). Consider enabling the contract to revoke approval in emergency situations.
File: src/cauldrons/CauldronV4.sol 143 magicInternetMoney.approve(address(bentoBox), type(uint256).max);
Using external calls within modifiers can introduce reentrancy risks and make a contract's logic less transparent. Modifiers are meant for pre-execution checks, and external calls can lead to unpredictable flow and potential reentrancy issues. Avoiding external calls in modifiers and placing them directly in the function body enhances code clarity, aids in auditing, and minimizes unexpected behaviors. This practice ensures a more explicit execution order and clearer understanding of the code's effects.
File: src/blast/BlastMagicLP.sol 119 if (!BlastMagicLP(address(implementation)).operators(msg.sender) && msg.sender != implementation.owner()) {
#0 - c4-pre-sort
2024-03-15T15:00:44Z
141345 marked the issue as sufficient quality report
#1 - 141345
2024-03-16T01:04:37Z
214 albahaca l r nc 3 0 11
L 1 n L 2 l L 3 n L 4 n L 5 i L 6 i L 7 n L 8 n L 9 d L 10 n L 11 n L 12 i L 13 i L 14 i L 15 i L 16 i L 17 i L 18 i L 19 i L 20 i L 21 l L 22 l L 23 n L 24 n L 25 n L 26 n dup of https://github.com/code-423n4/2024-03-abracadabra-money-findings/issues/82 L 27 i
#2 - c4-judge
2024-03-29T16:55:44Z
thereksfour marked the issue as grade-b
🌟 Selected for report: hihen
Also found by: 0x11singh99, Bozho, Sathish9098, albahaca, clara, dharma09, oualidpro, pfapostol, slvDev
169.5732 USDC - $169.57
| issue | instance | |
|---|---|---|
| [G‑01] | Avoid contract existence checks by using low level calls | 51 |
| [G‑02] | State variables that are used multiple times in a function should be cached in stack variables | 8 |
| [G‑03] | Multiple address/ID mappings can be combined into a single mapping of an address/ID to a struct, where appropriate | 3 |
| [G‑04] | Use Assembly To Check For address(0) | 26 |
| [G‑05] | Use hardcode address instead address(this) | 41 |
| [G‑06] | Constructors can be marked payable | 16 |
| [G‑07] | Use assembly to write address storage values | 14 |
| [G‑08] | Avoid updating storage when the value hasn't changed | 18 |
| [G‑09] | Use calldata instead of memory for function arguments that do not get mutated | 2 |
| [G‑10] | Can Make The Variable Outside The Loop To Save Gas | 9 |
| [G‑11] | Use nested if statements instead of && | 8 |
| [G‑12] | Sort Solidity operations using short-circuit mode | 1 |
| [G‑13] | Amounts should be checked for 0 before calling a transfer | 5 |
| [G‑14] | Do not calculate constants | 4 |
| [G‑15] | With assembly, .call (bool success) transfer can be done gas-optimized | 1 |
| [G‑16] | Internal functions only called once can be inlined to save gas | 3 |
| [G‑17] | Use constants instead of type(uintx).max | 6 |
| [G‑18] | Multiple accesses of a mapping/array should use a local variable cache | 5 |
| [G‑19] | Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if-statement | 4 |
| [G‑20] | The result of function calls should be cached rather than re-calling the function | 2 |
| [G‑21] | Stack variable used as a cheaper cache for a state variable is only used once | 2 |
| [G‑22] | Use local variables for emitting | 4 |
| [G‑23] | State variables can be packed into fewer storage slots | 2 |
| [G‑24] | Storage re-read via storage pointer | 4 |
| [G‑25] | State variable read in a loop | 4 |
| [G‑26] | Initializers can be marked payable | 1 |
| [G‑27] | Use assembly to emit events | 57 |
| [G‑28] | Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead | 2 |
| [G‑29] | Emit Used In Loop | 4 |
| [G‑30] | Use assembly to calculate hashes to save gas | 1 |
| [G‑31] | Unused named return variables without optimizer waste gas | 18 |
| [G‑32] | Structs can be modified to fit in fewer storage slots | 2 |
| [G‑33] | Use assembly to validate msg.sender | 1 |
| [G‑34] | Unlimited gas consumption risk due to external call recipients | 1 |
| [G‑35] | Same cast is done multiple times | 5 |
| [G‑36] | Using constants instead of enum can save gas | 2 |
| [G‑37] | Move storage pointer to top of function to avoid offset calculation | 1 |
| [G‑38] | Consider using OZ EnumerateSet in place of nested mappings | 5 |
| [G‑39] | Instead of counting down in for statements, count up | 8 |
| [G‑40] | Use the inputs/results of assignments rather than re-reading state variables | 44 |
Prior to 0.8.10 the compiler inserted extra code, including EXTCODESIZE (100 gas), to check for contract existence for external function calls. In more recent solidity versions, the compiler will not insert these checks if the external call has a return value. Similar behavior can be achieved in earlier versions by using low-level calls, since low level calls never check for contract existence.
Note: these instances are missed from bots
There are 51 instances of this issue:
Total save gas : 100 * 51 = 5100
File: src/mimswap/auxiliary/FeeRateModel.sol 39 return IFeeRateImpl(implementation).getFeeRate(msg.sender, trader, lpFeeRate);
File: src/mimswap/periphery/Factory.sol 86 IMagicLP(clone).init(address(baseToken_), address(quoteToken_), lpFeeRate_, address(maintainerFeeRateModel), i_, k_);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Factory.sol#L86:86
File: src/mimswap/periphery/Router.sol 286 token = IMagicLP(lp)._QUOTE_TOKEN_(); 70 (shares, , ) = IMagicLP(clone).buyShares(to); 295 } else if (IMagicLP(lp)._QUOTE_TOKEN_() == address(weth)) { 328 IMagicLP(firstLp)._BASE_TOKEN_().safeTransferFrom(msg.sender, address(firstLp), amountIn); 208 } else if (IMagicLP(lp)._QUOTE_TOKEN_() == address(weth)) { 66 clone = IFactory(factory).create(baseToken, quoteToken, lpFeeRate, i, k); 411 IMagicLP(lp)._BASE_TOKEN_().safeTransferFrom(msg.sender, lp, amountIn); 204 token = IMagicLP(lp)._QUOTE_TOKEN_(); 186 IMagicLP(lp)._BASE_TOKEN_().safeTransferFrom(msg.sender, lp, baseInAmount); 236 address token = IMagicLP(lp)._BASE_TOKEN_(); 172 IMagicLP(lp)._BASE_TOKEN_().safeTransferFrom(msg.sender, lp, baseAdjustedInAmount); 117 (uint256 baseReserve, uint256 quoteReserve) = IMagicLP(lp).getReserves(); 287 (ethAmountOut, tokenAmountOut) = IMagicLP(lp).sellShares( 296 (tokenAmountOut, ethAmountOut) = IMagicLP(lp).sellShares( 351 inToken = IMagicLP(firstLp)._QUOTE_TOKEN_(); 489 IMagicLP(lp)._QUOTE_TOKEN_().safeTransferFrom(msg.sender, lp, amountIn); 136 uint256 i = IMagicLP(lp)._I_(); 443 IMagicLP(lp)._BASE_TOKEN_().safeTransferFrom(msg.sender, lp, amountIn); 572 amountOut = IMagicLP(lp).sellBase(to); 173 IMagicLP(lp)._QUOTE_TOKEN_().safeTransferFrom(msg.sender, lp, quoteAdjustedInAmount); 439 if (IMagicLP(lp)._QUOTE_TOKEN_() != address(weth)) { 393 IMagicLP(firstLp)._BASE_TOKEN_().safeTransferFrom(msg.sender, firstLp, amountIn); 467 address quoteToken = IMagicLP(lp)._QUOTE_TOKEN_(); 522 uint256 i = IMagicLP(lp)._I_(); 563 amountOut = IMagicLP(path[iterations]).sellQuote(to); 380 outToken = IMagicLP(lastLp)._QUOTE_TOKEN_(); 421 address baseToken = IMagicLP(lp)._BASE_TOKEN_(); 187 IMagicLP(lp)._QUOTE_TOKEN_().safeTransferFrom(msg.sender, lp, quoteInAmount); 485 if (IMagicLP(lp)._BASE_TOKEN_() != address(weth)) { 550 IMagicLP(path[i]).sellQuote(address(path[i + 1])); 252 uint256 baseBalance = IMagicLP(lp)._BASE_TOKEN_().balanceOf(address(lp)); 238 token = IMagicLP(lp)._QUOTE_TOKEN_(); 500 (shares, , ) = IMagicLP(lp).buyShares(to); 202 address token = IMagicLP(lp)._BASE_TOKEN_(); 284 address token = IMagicLP(lp)._BASE_TOKEN_(); 330 IMagicLP(firstLp)._QUOTE_TOKEN_().safeTransferFrom(msg.sender, address(firstLp), amountIn); 349 inToken = IMagicLP(firstLp)._BASE_TOKEN_(); 239 } else if (IMagicLP(lp)._QUOTE_TOKEN_() != address(weth)) { 514 (uint256 baseReserve, uint256 quoteReserve) = IMagicLP(lp).getReserves(); 271 return IMagicLP(lp).sellShares(sharesIn, to, minimumBaseAmount, minimumQuoteAmount, "", deadline); 395 IMagicLP(firstLp)._QUOTE_TOKEN_().safeTransferFrom(msg.sender, firstLp, amountIn); 253 uint256 quoteBalance = IMagicLP(lp)._QUOTE_TOKEN_().balanceOf(address(lp)); 547 IMagicLP(path[i]).sellBase(address(path[i + 1])); 579 amountOut = IMagicLP(lp).sellQuote(to); 382 outToken = IMagicLP(lastLp)._BASE_TOKEN_(); 93 (shares, , ) = IMagicLP(clone).buyShares(to); 561 amountOut = IMagicLP(path[iterations]).sellBase(to); 456 IMagicLP(lp)._QUOTE_TOKEN_().safeTransferFrom(msg.sender, lp, amountIn); 88 clone = IFactory(factory).create(useTokenAsQuote ? address(weth) : token, useTokenAsQuote ? token : address(weth), lpFeeRate, i, k);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Router.sol#L88:88
When performing multiple operations on a state variable in a function, it is recommended to cache it first. Either multiple reads or multiple writes to a state variable can save gas by caching it on the stack. 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. Saves 100 gas per instance.
There are 8 instances of this issue:
Total save gas : 100 * 8 = 800
File: src/blast/BlastMagicLP.sol // @audit `_BASE_TOKEN_` is State variable should be cached in stack variables 56 if (registry.nativeYieldTokens(_BASE_TOKEN_)) { token0Amount = BlastYields.claimAllTokenYields(_BASE_TOKEN_, feeTo_); } // @audit `_QUOTE_TOKEN_` is State variable should be cached in stack variables 59 if (registry.nativeYieldTokens(_QUOTE_TOKEN_)) { token1Amount = BlastYields.claimAllTokenYields(_QUOTE_TOKEN_, feeTo_); }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastMagicLP.sol#L56
File: src/mimswap/MagicLP.sol // @audit `_BASE_RESERVE_` and `_QUOTE_RESERVE_` are State variables should be cached in stack variables to save gas 138 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_; } } // @audit `_BASE_TOKEN_` is State variable should be cached in stack variables 285 _setReserve(_BASE_TOKEN_.balanceOf(address(this)), quoteBalance); emit Swap(address(_QUOTE_TOKEN_), address(_BASE_TOKEN_), quoteInput, receiveBaseAmount, msg.sender, to);
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L138-L148
File: src/mimswap/MagicLP.sol // @audit `_QUOTE_RESERVE_` and `_BASE_RESERVE_` are State variables should be cached in stack variables to save gas 290 function flashLoan(uint256 baseAmount, uint256 quoteAmount, address assetTo, bytes calldata data) external nonReentrant { _transferBaseOut(assetTo, baseAmount); _transferQuoteOut(assetTo, quoteAmount); if (data.length > 0) { ICallee(assetTo).FlashLoanCall(msg.sender, baseAmount, quoteAmount, data); } uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)); uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)); // no input -> pure loss if (baseBalance < _BASE_RESERVE_ && quoteBalance < _QUOTE_RESERVE_) { revert ErrFlashLoanFailed(); } // sell quote case // quote input + base output if (baseBalance < _BASE_RESERVE_) { uint256 quoteInput = quoteBalance - uint256(_QUOTE_RESERVE_); (uint256 receiveBaseAmount, uint256 mtFee, PMMPricing.RState newRState, uint256 newQuoteTarget) = querySellQuote( tx.origin, quoteInput ); if (uint256(_BASE_RESERVE_) - baseBalance > receiveBaseAmount) { revert ErrFlashLoanFailed(); } _transferBaseOut(_MT_FEE_RATE_MODEL_.maintainer(), mtFee); if (_RState_ != uint32(newRState)) { _QUOTE_TARGET_ = newQuoteTarget.toUint112(); _RState_ = uint32(newRState); emit RChange(newRState); } emit Swap(address(_QUOTE_TOKEN_), address(_BASE_TOKEN_), quoteInput, receiveBaseAmount, msg.sender, assetTo); } // sell base case // base input + quote output if (quoteBalance < _QUOTE_RESERVE_) { uint256 baseInput = baseBalance - uint256(_BASE_RESERVE_); (uint256 receiveQuoteAmount, uint256 mtFee, PMMPricing.RState newRState, uint256 newBaseTarget) = querySellBase( tx.origin, baseInput ); if (uint256(_QUOTE_RESERVE_) - quoteBalance > receiveQuoteAmount) { revert ErrFlashLoanFailed(); } _transferQuoteOut(_MT_FEE_RATE_MODEL_.maintainer(), mtFee); if (_RState_ != uint32(newRState)) { _BASE_TARGET_ = newBaseTarget.toUint112(); _RState_ = uint32(newRState); emit RChange(newRState); } emit Swap(address(_BASE_TOKEN_), address(_QUOTE_TOKEN_), baseInput, receiveQuoteAmount, msg.sender, assetTo); } _sync(); emit FlashLoan(msg.sender, assetTo, baseAmount, quoteAmount); }
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L290-L353
File: src/mimswap/periphery/Factory.sol // @audit `maintainerFeeRateModel` is State variable should be cached in stack variables 86 IMagicLP(clone).init(address(baseToken_), address(quoteToken_), lpFeeRate_, address(maintainerFeeRateModel), i_, k_); emit LogCreated(clone, baseToken_, quoteToken_, creator, lpFeeRate_, maintainerFeeRateModel, i_, k_);
Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations.
There are 3 instances of this issue:
File: src/blast/BlastOnboarding.sol 37 mapping(address token => Balances) public totals; 38 mapping(address token => uint256 cap) public caps; 39 40 // Per-user 41 mapping(address user => mapping(address token => Balances)) public balances;
File: src/mimswap/periphery/Factory.sol 35 mapping(address base => mapping(address quote => address[] pools)) public pools; 36 mapping(address creator => address[] pools) public userPools;
File: src/staking/LockingMultiRewards.sol 89 mapping(address token => Reward info) private _rewardData; 90 mapping(address user => Balances balances) private _balances; 91 mapping(address user => LockedBalance[] locks) private _userLocks; 92 mapping(address user => RewardLock rewardLock) private _userRewardLock; 93 94 mapping(address user => mapping(address token => uint256 amount)) public userRewardPerTokenPaid; 95 mapping(address user => mapping(address token => uint256 amount)) public rewards; 96 mapping(address user => uint256 index) public lastLockIndex;
Saves 6 gas per instance if using assembly to check for address(0)
There are 26 instances of this issue: Total save gas : 6 * 26 = 156
File: src/blast/BlastBox.sol 25 if (feeTo_ == address(0)) { 28 if (address(registry_) == address(0)) { 73 if (feeTo_ == address(0)) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastBox.sol#L25
File: src/blast/BlastGovernor.sol 16 if (feeTo_ == address(0)) { 41 if(_feeTo == address(0)) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastGovernor.sol#L16
File: src/blast/BlastMagicLP.sol 24 if (feeTo_ == address(0)) { 27 if (address(registry_) == address(0)) { 81 if (feeTo_ == address(0)) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastMagicLP.sol#L24
File: src/blast/BlastOnboarding.sol 85 if (address(registry_) == address(0)) { 89 if (feeTo_ == address(0)) { 148 if (feeTo_ == address(0)) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastOnboarding.sol#L85
File: src/blast/BlastOnboardingBoot.sol 97 if (pool != address(0)) {
File: src/blast/BlastTokenRegistry.sol 15 if (token == address(0)) {
File: src/blast/BlastWrappers.sol 21 if (governor_ == address(0)) { 35 if (governor_ == address(0)) { 48 if (governor_ == address(0)) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastWrappers.sol#L48:48
File: src/mimswap/MagicLP.sol 102 if (mtFeeRateModel == address(0) || baseTokenAddress == address(0) || quoteTokenAddress == address(0)) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L102:102
File: src/mimswap/auxiliary/FeeRateModel.sol 23 if (maintainer_ == address(0)) { 35 if (implementation == address(0)) { 47 if (maintainer_ == address(0)) {
File: src/mimswap/periphery/Factory.sol 39 if (implementation_ == address(0)) { 42 if (address(maintainerFeeRateModel_) == address(0)) { 97 if (implementation_ == address(0)) { 106 if (address(maintainerFeeRateModel_) == address(0)) {
File: src/mimswap/periphery/Router.sol 39 if (address(weth_) == address(0) || address(factory_) == address(0)) {
File: src/staking/LockingMultiRewards.sol 301 if (rewardToken == address(0)) {
it can be more gas-efficient to use a hardcoded address instead of the address(this) expression, especially if you need to use the same address multiple times in your contract.
There are 41 instances of this issue:
File: src/blast/BlastOnboardingBoot.sol 106 (pool, totalPoolShares) = router.createPool(MIM, USDB, FEE_RATE, I, K, address(this), baseAmount, quoteAmount); 117 staking = new LockingMultiRewards(pool, 30_000, 7 days, 13 weeks, address(this)); staking.setOperator(address(this), true);
File: src/blast/libraries/BlastYields.sol 21 if (IERC20Rebasing(token).getConfiguration(address(this)) == YieldMode.CLAIMABLE) { 26 emit LogBlastTokenClaimableEnabled(address(this), token); 31 emit LogBlastNativeClaimableEnabled(address(this)); 39 return claimMaxGasYields(address(this), recipient); 52 return claimAllNativeYields(address(this), recipient); 71 amount = IERC20Rebasing(token).claim(recipient, IERC20Rebasing(token).getClaimableAmount(address(this)));
File: src/mimswap/MagicLP.sol 229 return _BASE_TOKEN_.balanceOf(address(this)) - uint256(_BASE_RESERVE_); 233 return _QUOTE_TOKEN_.balanceOf(address(this)) - uint256(_QUOTE_RESERVE_); 245 uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)); 262 _setReserve(baseBalance, _QUOTE_TOKEN_.balanceOf(address(this))); 268 uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)); 285 _setReserve(_BASE_TOKEN_.balanceOf(address(this)), quoteBalance); 298 uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)); uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)); 361 uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)); uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)); 427 if (to == address(this)) { 431 uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)); uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)); 505 uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)); uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)); 530 baseBalance = _BASE_TOKEN_.balanceOf(address(this)); quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)); 568 uint256 baseBalance = _BASE_TOKEN_.balanceOf(address(this)); uint256 quoteBalance = _QUOTE_TOKEN_.balanceOf(address(this)); 615 if (address(this) == address(implementation)) { 622 if (address(this) != address(implementation)) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L229
File: src/mimswap/periphery/Router.sol 92 address(weth).safeTransferFrom(address(this), clone, msg.value); 269 lp.safeTransferFrom(msg.sender, address(this), sharesIn); 283 lp.safeTransferFrom(msg.sender, address(this), sharesIn); 289 address(this), 298 address(this), 398 amountOut = _swap(address(this), path, directions, minimumOut); 444 amountOut = _sellBase(lp, address(this), minimumOut); 490 amountOut = _sellQuote(lp, address(this), minimumOut);
File: src/staking/LockingMultiRewards.sol 326 if (tokenAddress == stakingToken && tokenAmount > stakingToken.balanceOf(address(this)) - stakingTokenBalance) { 367 rewardToken.safeTransferFrom(msg.sender, address(this), amount); 464 stakingToken.safeTransferFrom(msg.sender, address(this), amount);
Payable functions cost less gas to execute, since the compiler does not have to add extra checks to ensure that a payment wasn't provided. A constructor can safely be marked as payable, since only the deployer would be able to pass funds, and the project itself would not pass any funds.
There are 16 instances of this issue:
File: src/blast/BlastBox.sol 24 constructor(IERC20 weth_, BlastTokenRegistry registry_, address feeTo_) DegenBox(weth_) { 25 if (feeTo_ == address(0)) { 26 revert ErrZeroAddress(); 27 } 28 if (address(registry_) == address(0)) { 29 revert ErrZeroAddress(); 30 } 31 32 registry = registry_; 33 feeTo = feeTo_; 34 }
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastBox.sol#L24:34
File: src/blast/BlastDapp.sol 7 constructor() { 8 BlastPoints.configure(); 9 }
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastDapp.sol#L7:9
File: src/blast/BlastGovernor.sol 15 constructor(address feeTo_, address _owner) OperatableV2(_owner) { 16 if (feeTo_ == address(0)) { 17 revert ErrZeroAddress(); 18 } 19 20 feeTo = feeTo_; 21 BlastYields.configureDefaultClaimables(address(this)); 22 }
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastGovernor.sol#L15:22
File: src/blast/BlastMagicLP.sol 23 constructor(BlastTokenRegistry registry_, address feeTo_, address owner_) MagicLP(owner_) { 24 if (feeTo_ == address(0)) { 25 revert ErrZeroAddress(); 26 } 27 if (address(registry_) == address(0)) { 28 revert ErrZeroAddress(); 29 } 30 31 registry = registry_; 32 feeTo = feeTo_; 33 }
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastMagicLP.sol#L23:33
File: src/blast/BlastOnboarding.sol 58 constructor() Owned(msg.sender) { 59 BlastYields.configureDefaultClaimables(address(this)); 60 BlastPoints.configure(); 61 } 84 constructor(BlastTokenRegistry registry_, address feeTo_) { 85 if (address(registry_) == address(0)) { 86 revert ErrZeroAddress(); 87 } 88 89 if (feeTo_ == address(0)) { 90 revert ErrZeroAddress(); 91 } 92 93 registry = registry_; 94 feeTo = feeTo_; 95 }
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboarding.sol#L84:95
File: src/blast/BlastTokenRegistry.sol 12 constructor(address _owner) Owned(_owner) {}
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastTokenRegistry.sol#L12:12
File: src/blast/BlastWrappers.sol 20 constructor(IWETH weth_, IFactory factory, address governor_) Router(weth_, factory) { 21 if (governor_ == address(0)) { 22 revert ErrZeroAddress(); 23 } 24 BlastYields.configureDefaultClaimables(governor_); 25 } 29 constructor( 30 address implementation_, 31 IFeeRateModel maintainerFeeRateModel_, 32 address owner_, 33 address governor_ 34 ) Factory(implementation_, maintainerFeeRateModel_, owner_) { 35 if (governor_ == address(0)) { 36 revert ErrZeroAddress(); 37 } 38 BlastYields.configureDefaultClaimables(governor_); 39 } 47 constructor(address box_, address mim_, address governor_) CauldronV4(IBentoBoxV1(box_), IERC20(mim_)) { 48 if (governor_ == address(0)) { 49 revert ErrZeroAddress(); 50 } 51 if (governor_ == address(this)) { 52 revert ErrInvalidGovernorAddress(); 53 } 54 55 _governor = governor_; 56 _setupBlacklist(); 57 }
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastWrappers.sol#L47:57
File: src/mimswap/MagicLP.sol 84 constructor(address owner_) Owned(owner_) { 85 implementation = this; 86 87 // prevents the implementation contract initialization 88 _INITIALIZED_ = true; 89 }
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/MagicLP.sol#L84:89
File: src/mimswap/auxiliary/FeeRateModel.sol 22 constructor(address maintainer_, address owner_) Owned(owner_) { 23 if (maintainer_ == address(0)) { 24 revert ErrZeroAddress(); 25 } 26 27 maintainer = maintainer_; 28 }
File: src/mimswap/periphery/Factory.sol 38 constructor(address implementation_, IFeeRateModel maintainerFeeRateModel_, address owner_) Owned(owner_) { 39 if (implementation_ == address(0)) { 40 revert ErrZeroAddress(); 41 } 42 if (address(maintainerFeeRateModel_) == address(0)) { 43 revert ErrZeroAddress(); 44 } 45 implementation = implementation_; 46 maintainerFeeRateModel = maintainerFeeRateModel_; 47 }
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Factory.sol#L38:47
File: src/mimswap/periphery/Router.sol 38 constructor(IWETH weth_, IFactory factory_) { 39 if (address(weth_) == address(0) || address(factory_) == address(0)) { 40 revert ErrZeroAddress(); 41 } 42 43 weth = weth_; 44 factory = factory_; 45 }
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Router.sol#L38:45
File: src/oracles/aggregators/MagicLpAggregator.sol 21 constructor(IMagicLP pair_, IAggregator baseOracle_, IAggregator quoteOracle_) { 22 pair = pair_; 23 baseOracle = baseOracle_; 24 quoteOracle = quoteOracle_; 25 baseDecimals = IERC20Metadata(pair_._BASE_TOKEN_()).decimals(); 26 quoteDecimals = IERC20Metadata(pair_._QUOTE_TOKEN_()).decimals(); 27 }
File: src/staking/LockingMultiRewards.sol 112 constructor( 113 address _stakingToken, 114 uint256 _lockingBoostMultiplerInBips, 115 uint256 _rewardsDuration, 116 uint256 _lockDuration, 117 address _owner 118 ) OperatableV2(_owner) { 119 if (_lockingBoostMultiplerInBips <= BIPS) { 120 revert ErrInvalidBoostMultiplier(); 121 } 122 123 if (_lockDuration < MIN_LOCK_DURATION) { 124 revert ErrInvalidLockDuration(); 125 } 126 127 if (_rewardsDuration < MIN_REWARDS_DURATION) { 128 revert ErrInvalidRewardDuration(); 129 } 130 131 if (_lockDuration % _rewardsDuration != 0) { 132 revert ErrInvalidDurationRatio(); 133 } 134 135 stakingToken = _stakingToken; 136 lockingBoostMultiplerInBips = _lockingBoostMultiplerInBips; 137 rewardsDuration = _rewardsDuration; 138 lockDuration = _lockDuration; 139 140 // kocks are combined into the same `rewardsDuration` epoch. So, if 141 // a user stake with locking every `rewardsDuration` this should reach the 142 // maximum number of possible simultaneous because the first lock gets expired, 143 // freeing up a slot. 144 maxLocks = _lockDuration / _rewardsDuration; 145 }
By using assembly to write to address storage values, you can bypass some of these operations and lower the gas cost of writing to storage. Assembly code allows you to directly access the Ethereum Virtual Machine (EVM) and perform low-level operations that are not possible in Solidity.
example of using assembly to write to address storage values:
contract MyContract { address private myAddress; function setAddressUsingAssembly(address newAddress) public { assembly { sstore(0, newAddress) } } }
There are 14 instances of this issue:
File: src/blast/BlastBox.sol 33 feeTo = feeTo_; 77 feeTo = feeTo_;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastBox.sol#L33
File: src/blast/BlastGovernor.sol 20 feeTo = feeTo_; 45 feeTo = _feeTo;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastGovernor.sol#L45
File: src/blast/BlastMagicLP.sol 32 feeTo = feeTo_; 85 feeTo = feeTo_;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastMagicLP.sol#L32
File: src/blast/BlastOnboarding.sol 191 bootstrapper = bootstrapper_; 94 feeTo = feeTo_; 152 feeTo = feeTo_;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastOnboarding.sol#L191
File: src/mimswap/MagicLP.sol 119 _BASE_TOKEN_ = baseTokenAddress; 120 _QUOTE_TOKEN_ = quoteTokenAddress;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L119
File: src/mimswap/auxiliary/FeeRateModel.sol 27 maintainer = maintainer_; 51 maintainer = maintainer_; 58 implementation = implementation_;
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).
There are 18 instances of this issue:
File: src/blast/BlastBox.sol 72 function setFeeTo(address feeTo_) external onlyOwner {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastBox.sol#L72:72
File: src/blast/BlastGovernor.sol 40 function setFeeTo(address _feeTo) external onlyOwner {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastGovernor.sol#L40:40
File: src/blast/BlastMagicLP.sol 80 function setFeeTo(address feeTo_) external onlyImplementation onlyImplementationOwner {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastMagicLP.sol#L80:80
File: src/blast/BlastOnboarding.sol 147 function setFeeTo(address feeTo_) external onlyOwner { 195 function open() external onlyOwner onlyState(State.Idle) { 190 function setBootstrapper(address bootstrapper_) external onlyOwner { 200 function close() external onlyOwner onlyState(State.Opened) {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboarding.sol#L200:200
File: src/blast/BlastOnboardingBoot.sol 96 function bootstrap(uint256 minAmountOut) external onlyOwner onlyState(State.Closed) returns (address, address, uint256) { 146 function setReady(bool _ready) external onlyOwner onlyState(State.Closed) {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboardingBoot.sol#L146:146
File: src/mimswap/auxiliary/FeeRateModel.sol 57 function setImplementation(address implementation_) public onlyOwner { 46 function setMaintainer(address maintainer_) external onlyOwner {
File: src/mimswap/periphery/Factory.sol 96 function setLpImplementation(address implementation_) external onlyOwner { 105 function setMaintainerFeeRateModel(IFeeRateModel maintainerFeeRateModel_) external onlyOwner {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Factory.sol#L105:105
File: src/staking/LockingMultiRewards.sol 317 function setMinLockAmount(uint256 _minLockAmount) external onlyOwner {
File: test/mocks/ERC20Mock.sol 19 function setDecimals(uint8 decimals_) external {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mocks/ERC20Mock.sol#L19:19
File: utils/BaseScript.sol 33 function setNewDeploymentEnabled(bool _enabled) public {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/BaseScript.sol#L33:33
File: utils/BaseTest.sol 28 function setUpNoMocks() public virtual {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/BaseTest.sol#L28:28
File: utils/Toolkit.sol 273 function setTesting(bool _testing) public {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/Toolkit.sol#L273:273
Mark data types as calldata instead of memory where possible. This makes it so that Mark data types as calldata instead of memory where possible. This makes it so that the data is not automatically loaded into memory. If the data passed into the function does not need to be changed (like updating values in an array), it can be passed in as calldata. The one exception to this is if the argument must later be passed into another function that takes an argument that specifies memory storage..
There are 2 instances of this issue:
File: src/blast/BlastOnboarding.sol 164 function claimTokenYields(address[] memory tokens) external onlyOwner {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastOnboarding.sol#L164
File: src/staking/LockingMultiRewards.sol 397 function processExpiredLocks(address[] memory users, uint256[] calldata lockIndexes) external onlyOperators {
When you declare a variable inside a loop, Solidity creates a new instance of the variable for each iteration of the loop. This can lead to unnecessary gas costs, especially if the loop is executed frequently or iterates over a large number of elements.
By declaring the variable outside the loop, you can avoid the creation of multiple instances of the variable and reduce the gas cost of your contract. Here's an example:
contract MyContract { function sum(uint256[] memory values) public pure returns (uint256) { uint256 total = 0; for (uint256 i = 0; i < values.length; i++) { total += values[i]; } return total; } }
There are 9 instances of this issue:
File: src/staking/LockingMultiRewards.sol 406 address user = users[i]; 414 uint256 index = lockIndexes[i]; 426 uint256 amount = locks[index].amount; 427 uint256 lastIndex = locks.length - 1; 562 address token = rewardTokens[i]; 576 address token = rewardTokens[i]; 577 uint256 rewardPerToken_ = _updateRewardsGlobal(token, totalSupply_); 615 address rewardToken = rewardTokens[i]; 616 uint256 rewardAmount = rewards[user][rewardToken];
If the if statement has a logical AND and is not followed by an else statement, it can be replaced with 2 if statements.
contract NestedIfTest { //Execution cost: 22334 gas function funcBad(uint256 input) public pure returns (string memory) { if (input<10 && input>0 && input!=6){ return "If condition passed"; } } //Execution cost: 22294 gas function funcGood(uint256 input) public pure returns (string memory) { if (input<10) { if (input>0){ if (input!=6){ return "If condition passed"; } } } } }
There are 8 instances of this issue:
File: src/blast/BlastMagicLP.sol 119 if (!BlastMagicLP(address(implementation)).operators(msg.sender) && msg.sender != implementation.owner()) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastMagicLP.sol#L119
File: src/blast/BlastOnboarding.sol 114 if (caps[token] > 0 && totals[token].total > caps[token]) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastOnboarding.sol#L114
File: src/mimswap/MagicLP.sol 139 if (_RState_ == uint32(PMMPricing.RState.BELOW_ONE) && _BASE_RESERVE_ < _BASE_TARGET_) { 144 if (_RState_ == uint32(PMMPricing.RState.ABOVE_ONE) && _QUOTE_RESERVE_ < _QUOTE_TARGET_) { 302 if (baseBalance < _BASE_RESERVE_ && quoteBalance < _QUOTE_RESERVE_) { 549 if (timeElapsed > 0 && _BASE_RESERVE_ != 0 && _QUOTE_RESERVE_ != 0) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L139
File: src/mimswap/periphery/Router.sol 527 if (quoteReserve > 0 && baseReserve > 0) {
File: src/staking/LockingMultiRewards.sol 326 if (tokenAddress == stakingToken && tokenAmount > stakingToken.balanceOf(address(this)) - stakingTokenBalance) { 417 if (index == lastLockIndex[user] && locks.length > 1) {
Short-circuiting is a solidity contract development model that uses OR/AND logic to sequence different cost operations. It puts low gas cost operations in the front and high gas cost operations in the back, so that if the front is low If the cost operation is feasible, you can skip (short-circuit) the subsequent high-cost Ethereum virtual machine operation.
//f(x) is a low gas cost operation //g(y) is a high gas cost operation //Sort operations with different gas costs as follows f(x) || g(y) f(x) && g(y)
There are 1 instances of this issue:
File: src/blast/BlastMagicLP.sol 119 if (!BlastMagicLP(address(implementation)).operators(msg.sender) && msg.sender != implementation.owner()) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastMagicLP.sol#L119
Checking non-zero transfer values can avoid an expensive external call and save gas.
There are 5 instances of this issue:
File: src/blast/BlastOnboarding.sol 138 token.safeTransfer(msg.sender, amount); 210 token.safeTransfer(to, amount);
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastOnboarding.sol#L138
File: src/staking/LockingMultiRewards.sol 217 address(weth).safeTransfer(lp, wethAdjustedAmount); 311 token.safeTransfer(to, tokenAmountOut); 359 weth.deposit{value: msg.value}(); inToken.safeTransfer(address(firstLp), msg.value);
Due to how constant variables are implemented (replacements at compile-time), an expression assigned to a constant variable is recomputed each time that the variable is used, which wastes some gas Reffrence
There are 4 instances of this issue:
File: src/mimswap/MagicLP.sol 63 uint256 public constant MAX_I = 10 ** 36; 64 uint256 public constant MAX_K = 10 ** 18;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L63-L64
File: src/mimswap/libraries/DecimalMath.sol 20 uint256 internal constant ONE = 10 ** 18; 21 uint256 internal constant ONE2 = 10 ** 36;
return data (bool success,) has to be stored due to EVM architecture, but in a usage like below, ‘out’ and ‘outsize’ values are given (0,0), this storage disappears and gas optimization is provided. https://code4rena.com/reports/2023-01-biconomy#g-01-with-assembly-call-bool-success-transfer-can-be-done-gas-optimized
There are 1 instances of this issue:
File: src/blast/BlastGovernor.sol 54 (success, result) = to.call{value: value}(data);
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastGovernor.sol#L54
Not inlining costs 20 to 40 gas because of two extra JUMP instructions and additional stack operations needed for function calls.
There are 3 instances of this issue:
File: src/mimswap/MagicLP.sol 528 function _resetTargetAndReserve() internal returns (uint256 baseBalance, uint256 quoteBalance) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L528:528
File: src/staking/LockingMultiRewards.sol 544 function _updateRewards() internal { 572 function _updateRewardsForUsers(address[] memory users) internal {
Using constants instead of type(uintx).max can save gas in some cases because constants are precomputed at compile-time and can be reused throughout the code, whereas type(uintx).max requires computation at runtime
There are 6 instances of this issue:
File: src/blast/BlastOnboardingBoot.sol 103 MIM.safeApprove(address(router), type(uint256).max); 104 USDB.safeApprove(address(router), type(uint256).max);
File: src/mimswap/MagicLP.sol 508 if (baseBalance > type(uint112).max || quoteBalance > type(uint112).max) { 532 if (baseBalance > type(uint112).max || quoteBalance > type(uint112).max) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L508
The instances below point to the second+ access of a value inside a mapping/array, within a function. Caching 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 recalculate the key's keccak256 hash (Gkeccak256 - 30 gas) and that calculation's associated stack operations. Caching an array's struct avoids recalculating the array offsets into memory/calldata
There are 5 instances of this issue:
File: src/blast/BlastOnboarding.sol // @audit `caps[token]` is mapping 114 if (caps[token] > 0 && totals[token].total > caps[token]) {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboarding.sol#L170:170
function notifyRewardAmount(address rewardToken, uint256 amount, uint minRemainingTime) public onlyOperators { if (!_rewardData[rewardToken].exists) { revert ErrInvalidTokenAddress(); } _updateRewards(); rewardToken.safeTransferFrom(msg.sender, address(this), amount); Reward storage reward = _rewardData[rewardToken];
Fix code
function notifyRewardAmount(address rewardToken, uint256 amount, uint minRemainingTime) public onlyOperators { + Reward storage reward = _rewardData[rewardToken]; if (!reward.exists) { revert ErrInvalidTokenAddress(); } _updateRewards(); rewardToken.safeTransferFrom(msg.sender, address(this), amount); - Reward storage reward = _rewardData[rewardToken];
File: src/staking/LockingMultiRewards.sol // @audit `_rewardData[rewardToken].rewardPerTokenStored` is mapping also use in 285 279 return _rewardData[rewardToken].rewardPerTokenStored; // @audit `lastLockIndex[user]` is mapping also use in 430 417 if (index == lastLockIndex[user] && locks.length > 1) {
require(a <= b); x = b - a => require(a <= b); unchecked { x = b - a }
There are 4 instances of this issue:
File: src/mimswap/libraries/Math.sol 56 uint256 fairAmount = i * (V1 - V2); 178 bAbs = part2 - bAbs; 191 numerator = squareRoot - bAbs; 203 return V1 - V2;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/libraries/Math.sol#L56
Caching the result of a function call in a local variable when the function is called multiple times can save gas due to avoiding the need to execute the function code multiple times. There are 2 instances of this issue:
File: src/mimswap/MagicLP.sol // @audit `_MT_FEE_RATE_MODEL_.maintainer()` function call also use in line 319 341 _transferQuoteOut(_MT_FEE_RATE_MODEL_.maintainer(), mtFee);
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L341:341
File: src/mimswap/libraries/Math.sol // @audit `DecimalMath.mulFloor(i, delta)` 141 return DecimalMath.mulFloor(i, delta) > V1 ? V1 : DecimalMath.mulFloor(i, delta);
If the variable is only accessed once, it's cheaper to use the state variable directly that one time, and save the 3 gas the extra stack assignment would spend.
There are 2 instances of this issue:
File: src/mimswap/MagicLP.sol // @audit `baseBalance` and `quoteBalance` only used once 435 baseAmount = (baseBalance * shareAmount) / totalShares; 436 quoteAmount = (quoteBalance * shareAmount) / totalShares;
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/MagicLP.sol#L436:436
Use the function/modifier's local copy of the state variable, rather than incurring an extra Gwarmaccess (100 gas). In the unlikely event that the state variable hasn't already been used by the function/modifier, consider whether it is really necessary to include it in the event, given the fact that it incurs a Gcoldsload (2100 gas), or whether it can be passed in to or back out of the functions that do use it
There are 4 instances of this issue:
File: src/blast/BlastOnboardingBoot.sol 124 emit LogLiquidityBootstrapped(pool, address(staking), totalPoolShares); 148 emit LogReadyChanged(ready);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboardingBoot.sol#L148:148
File: src/mimswap/periphery/Factory.sol 88 emit LogCreated(clone, baseToken_, quoteToken_, creator, lpFeeRate_, maintainerFeeRateModel, i_, k_);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Factory.sol#L88:88
File: src/staking/LockingMultiRewards.sol 318 emit LogSetMinLockAmount(minLockAmount, _minLockAmount);
If variables occupying the same slot are both written in the same function or by the constructor, avoids a separate Gsset (20000 gas). Reads of the variables can also be cheaper
There are 2 instances of this issue:
File: src/blast/BlastOnboardingBoot.sol 23 contract BlastOnboardingBootDataV1 is BlastOnboardingData {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboardingBoot.sol#L23:23
File: src/mimswap/MagicLP.sol 25 contract MagicLP is ERC20, ReentrancyGuard, Owned {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/MagicLP.sol#L25:25
The instances below point to the second+ access of a state variable, via a storage pointer, within a function. Caching the value replaces each Gwarmaccess (100 gas) with a much cheaper stack read.
There are 4 instances of this issue:
File: src/staking/LockingMultiRewards.sol 241 return bal.unlocked + ((bal.locked * lockingBoostMultiplerInBips) / BIPS); 380 amount += _remainingRewardTime * reward.rewardRate; 427 uint256 lastIndex = locks.length - 1; 605 uint256 rewardItemLength = _rewardLock.items.length;
The state variable should be cached in and read from a local variable, or accumulated in a local variable then written to storage once outside of the loop, rather than reading/updating it on every iteration of the loop, which will replace each Gwarmaccess (100 gas) with a much cheaper stack read.
There are 4 instances of this issue:
File: src/staking/LockingMultiRewards.sol 547 for (uint256 i; i < rewardTokens.length; ) { 561 for (uint256 i; i < rewardTokens.length; ) { 575 for (uint256 i; i < rewardTokens.length; ) { 614 for (uint256 i; i < rewardTokens.length; ) {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/staking/LockingMultiRewards.
Payable functions cost less gas to execute, since the compiler does not have to add extra checks to ensure that a payment wasn't provided. An initializer can safely be marked as payable, since only the deployer would be able to pass funds, and the project itself would not pass any funds.
There are 1 instances of this issue:
File: src/blast/BlastOnboardingBoot.sol 129 function initialize(Router _router) external onlyOwner { 130 router = Router(payable(_router)); 131 factory = IFactory(router.factory()); 132 emit LogInitialized(_router); 133 }
Using the scratch space for event arguments (two words or fewer) will save gas over needing Solidity's full abi memory expansion used for emitting normally.
There are 57 instances of this issue:
File: src/blast/BlastBox.sol 78 emit LogFeeToChanged(feeTo_); 90 emit LogTokenDepositEnabled(token, enabled);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastBox.sol#L90:90
File: src/blast/BlastGovernor.sol 46 emit LogFeeToChanged(_feeTo);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastGovernor.sol#L46:46
File: src/blast/BlastMagicLP.sol 86 emit LogFeeToChanged(feeTo_); 91 emit LogOperatorChanged(operator, status);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastMagicLP.sol#L91:91
File: src/blast/BlastOnboarding.sol 120 emit LogDeposit(msg.sender, token, amount, lock_); 129 emit LogLock(msg.sender, token, amount); 140 emit LogWithdraw(msg.sender, token, amount); 153 emit LogFeeToChanged(feeTo_); 182 emit LogTokenSupported(token, supported); 187 emit LogTokenCapChanged(token, cap); 192 emit LogBootstrapperChanged(bootstrapper_); 197 emit LogStateChange(State.Opened); 202 emit LogStateChange(State.Closed); 211 emit LogTokenRescue(token, to, amount);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboarding.sol#L211:211
File: src/blast/BlastOnboardingBoot.sol 71 emit LogClaimed(msg.sender, shares, lock); 124 emit LogLiquidityBootstrapped(pool, address(staking), totalPoolShares); 132 emit LogInitialized(_router); 143 emit LogStakingChanged(address(_staking)); 148 emit LogReadyChanged(ready);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboardingBoot.sol#L148:148
File: src/blast/BlastTokenRegistry.sol 20 emit LogNativeYieldTokenEnabled(token, enabled);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastTokenRegistry.sol#L20:20
File: src/blast/libraries/BlastYields.sol 26 emit LogBlastTokenClaimableEnabled(address(this), token); 31 emit LogBlastNativeClaimableEnabled(address(this)); 44 emit LogBlastGasClaimed(recipient, amount); 57 emit LogBlastETHClaimed(recipient, amount); 62 emit LogBlastETHClaimed(recipient, amount); 72 emit LogBlastTokenClaimed(recipient, address(token), amount); 77 emit LogBlastTokenClaimed(recipient, address(token), amount);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/libraries/BlastYields.sol#L77:77
File: src/mimswap/MagicLP.sol 259 emit RChange(newRState); 264 emit Swap(address(_BASE_TOKEN_), address(_QUOTE_TOKEN_), baseInput, receiveQuoteAmount, msg.sender, to); 282 emit RChange(newRState); 287 emit Swap(address(_QUOTE_TOKEN_), address(_BASE_TOKEN_), quoteInput, receiveBaseAmount, msg.sender, to); 323 emit RChange(newRState); 325 emit Swap(address(_QUOTE_TOKEN_), address(_BASE_TOKEN_), quoteInput, receiveBaseAmount, msg.sender, assetTo); 345 emit RChange(newRState); 347 emit Swap(address(_BASE_TOKEN_), address(_QUOTE_TOKEN_), baseInput, receiveQuoteAmount, msg.sender, assetTo); 352 emit FlashLoan(msg.sender, assetTo, baseAmount, quoteAmount); 409 emit BuyShares(to, shares, balanceOf(to)); 454 emit SellShares(msg.sender, to, shareAmount, balanceOf(msg.sender)); 467 emit TokenRescue(token, to, amount); 501 emit ParametersChanged(newLpFeeRate, newI, newK, newBaseBalance, newQuoteBalance);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/MagicLP.sol#L501:501
File: src/mimswap/periphery/Factory.sol 88 emit LogCreated(clone, baseToken_, quoteToken_, creator, lpFeeRate_, maintainerFeeRateModel, i_, k_); 102 emit LogSetImplementation(implementation_); 111 emit LogSetMaintainerFeeRateModel(maintainerFeeRateModel_); 141 emit LogPoolRemoved(pool); 152 emit LogPoolAdded(baseToken, quoteToken, creator, pool);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Factory.sol#L152:152
File: src/staking/LockingMultiRewards.sol 183 emit LogWithdrawn(msg.sender, amount); 318 emit LogSetMinLockAmount(minLockAmount, _minLockAmount); 331 emit LogRecovered(tokenAddress, tokenAmount); 392 emit LogRewardAdded(amount); 436 emit LogLockIndexChanged(user, lastIndex, index); 447 emit LogUnlocked(user, amount, index); 475 emit LogStaked(account, amount); 513 emit LogLocked(user, amount, _nextUnlockTime, lockCount); 611 emit LogRewardLockCreated(user, _rewardLock.unlockTime); 638 emit LogRewardPaid(user, rewardToken, amount); 651 emit LogRewardLocked(user, rewardToken, rewardAmount);
uints/ints smaller than 32 bytes (256 bits) incurs overheadWhen using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size.https://docs.soliditylang.org/en/v0.8.11/internals/layout_in_storage.htmlEach operation involving a uint8 costs an extra 22-28 gas (depending on whether the other operand is also a variable of type uint8) as compared to ones involving uint256, due to the compiler having to clear the higher bits of the memory word before operating on the uint8, as well as the associated stack operations of doing so. Use a larger size then downcast where needed.
There are 2 instances of this issue:
File: src/mimswap/MagicLP.sol 546 uint32 blockTimestamp = uint32(block.timestamp % 2 ** 32); 547 uint32 timeElapsed = blockTimestamp - _BLOCK_TIMESTAMP_LAST_;
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/MagicLP.sol#L547:547
Emitting an event inside a loop performs a LOG op N times, where N is the loop length. Consider refactoring the code to emit the event only once at the end of loop. Gas savings should be multiplied by the average loop length.
There are 4 instances of this issue:
File: src/staking/LockingMultiRewards.sol 436 emit LogLockIndexChanged(user, lastIndex, index); 447 emit LogUnlocked(user, amount, index); 638 emit LogRewardPaid(user, rewardToken, amount); 651 emit LogRewardLocked(user, rewardToken, rewardAmount);
Using assembly to calculate hashes can save 80 gas per instance
There are 1 instances of this issue:
File: src/mimswap/periphery/Factory.sol 163 return keccak256(abi.encodePacked(sender_, implementation, baseToken_, quoteToken_, lpFeeRate_, i_, k_));
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Factory.sol#L163:163
Consider changing the variable to be an unnamed one, since the variable is never assigned, nor is it returned by name. If the optimizer is not turned on, leaving the code as it is will also waste gas for the stack variable.
There are 18 instances of this issue:
File: src/blast/BlastOnboardingBoot.sol 78 function claimable(address user) external view returns (uint256 shares) { 86 function previewTotalPoolShares() external view returns (uint256 baseAdjustedInAmount, uint256 quoteAdjustedInAmount, uint256 shares) { 155 function _claimable(address user) internal view returns (uint256 shares) {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboardingBoot.sol#L155:155
File: src/blast/libraries/BlastYields.sol 51 function claimAllNativeYields(address recipient) internal returns (uint256 amount) {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/libraries/BlastYields.sol#L51:51
File: src/mimswap/MagicLP.sol 215 function getMidPrice() public view returns (uint256 midPrice) { 224 function getUserFeeRate(address user) external view returns (uint256 lpFeeRate, uint256 mtFeeRate) { 228 function getBaseInput() public view returns (uint256 input) { 232 function getQuoteInput() public view returns (uint256 input) {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/MagicLP.sol#L232:232
File: src/mimswap/auxiliary/FeeRateModel.sol 34 function getFeeRate(address trader, uint256 lpFeeRate) external view returns (uint256 adjustedLpFeeRate, uint256 mtFeeRate) {
File: src/mimswap/periphery/Router.sol 185 ) external ensureDeadline(deadline) returns (uint256 shares) { 235 ) external payable ensureDeadline(deadline) returns (uint256 shares) { 268 ) external returns (uint256 baseAmountOut, uint256 quoteAmountOut) { 321 ) external ensureDeadline(deadline) returns (uint256 amountOut) { 342 ) external payable ensureDeadline(deadline) returns (uint256 amountOut) { 410 ) external ensureDeadline(deadline) returns (uint256 amountOut) { 420 ) external payable ensureDeadline(deadline) returns (uint256 amountOut) { 455 ) external ensureDeadline(deadline) returns (uint256 amountOut) { 466 ) external payable ensureDeadline(deadline) returns (uint256 amountOut) {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Router.sol#L466:466
Some member types can be safely modified, and as result, these struct will fit in fewer storage slots. Each slot saved can avoid an extra Gsset (20000 gas) for the first setting of the struct. Subsequent reads as well as writes have smaller gas savings.
There are 2 instances of this issue:
File: src/blast/BlastOnboarding.sol 24 struct Balances { uint256 unlocked; uint256 locked; uint256 total; }
File: src/mimswap/libraries/PMMPricing.sol 27 struct PMMState { uint256 i; uint256 K; uint256 B; uint256 Q; uint256 B0; uint256 Q0; RState R; }
We can use assembly to efficiently validate msg.sender with the least amount of opcodes necessary.
There are 1 instances of this issue:
File: src/mimswap/MagicLP.sol 608 if (msg.sender != implementation.owner()) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/mimswap/MagicLP.sol#L608
When calling an external function without specifying a gas limit , the called contract may consume all the remaining gas, causing the tx to be reverted. To mitigate this, it is recommended to explicitly set a gas limit when making low level external calls.
There are 1 instances of this issue:
File: src/blast/BlastGovernor.sol 54 (success, result) = to.call{value: value}(data);
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastGovernor.sol#L54
It's cheaper to do it once, and store the result to a variable. The examples below are the second+ instance of a given cast of the variable.
There are 5 instances of this issue:
address(router)File: src/blast/BlastOnboardingBoot.sol 103 MIM.safeApprove(address(router), type(uint256).max); USDB.safeApprove(address(router), type(uint256).max);
address(staking)File: src/blast/BlastOnboardingBoot.sol pool.safeApprove(address(staking), totalPoolShares); emit LogLiquidityBootstrapped(pool, address(staking), totalPoolShares); return (pool, address(staking), totalPoolShares);
Enum is expensive and it is more efficient to use constants instead. An illustrative example of this approach can be found in the ReentrancyGuard.sol.
There are 2 instances of this issue:
File: src/blast/BlastOnboarding.sol 18 enum State { Idle, Opened, Closed }
File: src/mimswap/libraries/PMMPricing.sol 21 enum RState { ONE, ABOVE_ONE, BELOW_ONE }
We can avoid unnecessary offset calculations by moving the storage pointer to the top of the function.
There are 1 instances of this issue:
File: src/staking/LockingMultiRewards.sol 369 Reward storage reward = _rewardData[rewardToken];
Nested mappings and multi-dimensional arrays in Solidity operate through a process of double hashing, wherein the original storage slot and the first key are concatenated and hashed, and then this hash is again concatenated with the second key and hashed. This process can be quite gas expensive due to the double-hashing operation and subsequent storage operation (sstore). A possible optimization involves manually concatenating the keys followed by a single hash operation and an sstore. However, this technique introduces the risk of storage collision, especially when there are other nested hash maps in the contract that use the same key types. Because Solidity is unaware of the number and structure of nested hash maps in a contract, it follows a conservative approach in computing the storage slot to avoid possible collisions. OpenZeppelin's EnumerableSet provides a potential solution to this problem. It creates a data structure that combines the benefits of set operations with the ability to enumerate stored elements, which is not natively available in Solidity. EnumerableSet handles the element uniqueness internally and can therefore provide a more gas-efficient and collision-resistant alternative to nested mappings or multi-dimensional arrays in certain scenarios.
There are 5 instances of this issue:
File: src/blast/BlastOnboarding.sol 41 mapping(address user => mapping(address token => Balances)) public balances;
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastOnboarding.sol#L41
File: src/mimswap/periphery/Factory.sol 35 mapping(address base => mapping(address quote => address[] pools)) public pools;
File: src/staking/LockingMultiRewards.sol 94 mapping(address user => mapping(address token => uint256 amount)) public userRewardPerTokenPaid; 95 mapping(address user => mapping(address token => uint256 amount)) public rewards;
Counting down is more gas efficient than counting up because neither we are making zero variable to non-zero variable and also we will get gas refund in the last transaction when making non-zero to zero variable.
There are 8 instances of this issue:
File: src/blast/BlastOnboarding.sol 165 for (uint256 i = 0; i < tokens.length; i++) {
https://github.com/code-423n4/2024-03-abracadabra-money/blob/main/src/blast/BlastOnboarding.sol#L165
File: src/mimswap/periphery/Router.sol 544 for (uint256 i = 0; i < iterations; ) {
File: src/staking/LockingMultiRewards.sol 504 for (uint256 i; i < users.length; ) { 547 for (uint256 i; i < rewardTokens.length; ) { 561 for (uint256 i; i < rewardTokens.length; ) { 575 for (uint256 i; i < rewardTokens.length; ) { 580 for (uint256 j; j < users.length; ) { 614 for (uint256 i; i < rewardTokens.length; ) {
When a state variable is assigned, it saves gas to use the value being assigned, later in the function, rather than re-reading the state variable itself. If needed, it can also be stored to a local variable, and be used in that way. Both options avoid a Gwarmaccess (100 gas). Note that if the operation is, say +=, the assignment also results in a value which can be used. The instances below point to the first reference after the assignment, since later references are already covered by issues describing the caching of state variable values.
There are 44 instances of this issue:
File: src/blast/BlastOnboardingBoot.sol 103 MIM.safeApprove(address(router), type(uint256).max); 104 USDB.safeApprove(address(router), type(uint256).max); 117 staking = new LockingMultiRewards(pool, 30_000, 7 days, 13 weeks, address(this)); 122 pool.safeApprove(address(staking), totalPoolShares); 122 pool.safeApprove(address(staking), totalPoolShares); 122 pool.safeApprove(address(staking), totalPoolShares); 124 emit LogLiquidityBootstrapped(pool, address(staking), totalPoolShares); 126 return (pool, address(staking), totalPoolShares); 148 emit LogReadyChanged(ready);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastOnboardingBoot.sol#L148:148
File: src/blast/BlastWrappers.sol 67 BlastYields.configureDefaultClaimables(_governor);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/blast/BlastWrappers.sol#L67:67
File: src/mimswap/MagicLP.sol 141 _BASE_TARGET_ = _BASE_RESERVE_; 142 _QUOTE_TARGET_ = _QUOTE_RESERVE_; 146 _BASE_TARGET_ = _BASE_RESERVE_; 147 _QUOTE_TARGET_ = _QUOTE_RESERVE_; 144 if (_RState_ == uint32(PMMPricing.RState.ABOVE_ONE) && _QUOTE_RESERVE_ < _QUOTE_TARGET_) { 144 if (_RState_ == uint32(PMMPricing.RState.ABOVE_ONE) && _QUOTE_RESERVE_ < _QUOTE_TARGET_) { 309 uint256 quoteInput = quoteBalance - uint256(_QUOTE_RESERVE_); 315 if (uint256(_BASE_RESERVE_) - baseBalance > receiveBaseAmount) { 325 emit Swap(address(_QUOTE_TOKEN_), address(_BASE_TOKEN_), quoteInput, receiveBaseAmount, msg.sender, assetTo); 325 emit Swap(address(_QUOTE_TOKEN_), address(_BASE_TOKEN_), quoteInput, receiveBaseAmount, msg.sender, assetTo); 331 uint256 baseInput = baseBalance - uint256(_BASE_RESERVE_); 337 if (uint256(_QUOTE_RESERVE_) - quoteBalance > receiveQuoteAmount) { 347 emit Swap(address(_BASE_TOKEN_), address(_QUOTE_TOKEN_), baseInput, receiveQuoteAmount, msg.sender, assetTo); 342 if (_RState_ != uint32(newRState)) { 381 shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_) ? DecimalMath.divFloor(quoteBalance, _I_) : baseBalance; 381 shares = quoteBalance < DecimalMath.mulFloor(baseBalance, _I_) ? DecimalMath.divFloor(quoteBalance, _I_) : baseBalance; 383 _QUOTE_TARGET_ = DecimalMath.mulFloor(shares, _I_).toUint112(); 402 _BASE_TARGET_ = (uint256(_BASE_TARGET_) + DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)).toUint112(); 402 _BASE_TARGET_ = (uint256(_BASE_TARGET_) + DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)).toUint112(); 402 _BASE_TARGET_ = (uint256(_BASE_TARGET_) + DecimalMath.mulFloor(uint256(_BASE_TARGET_), mintRatio)).toUint112(); 403 _QUOTE_TARGET_ = (uint256(_QUOTE_TARGET_) + DecimalMath.mulFloor(uint256(_QUOTE_TARGET_), mintRatio)).toUint112(); 403 _QUOTE_TARGET_ = (uint256(_QUOTE_TARGET_) + DecimalMath.mulFloor(uint256(_QUOTE_TARGET_), mintRatio)).toUint112(); 385 if (_QUOTE_TARGET_ == 0) { 438 _BASE_TARGET_ = uint112(uint256(_BASE_TARGET_) - (uint256(_BASE_TARGET_) * shareAmount).divCeil(totalShares)); 438 _BASE_TARGET_ = uint112(uint256(_BASE_TARGET_) - (uint256(_BASE_TARGET_) * shareAmount).divCeil(totalShares)); 513 _BASE_TARGET_ = uint112((uint256(_BASE_TARGET_) * baseBalance) / uint256(_BASE_RESERVE_)); 517 _QUOTE_TARGET_ = uint112((uint256(_QUOTE_TARGET_) * quoteBalance) / uint256(_QUOTE_RESERVE_));
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/MagicLP.sol#L517:517
File: src/mimswap/auxiliary/FeeRateModel.sol 39 return IFeeRateImpl(implementation).getFeeRate(msg.sender, trader, lpFeeRate);
File: src/mimswap/periphery/Factory.sol 86 IMagicLP(clone).init(address(baseToken_), address(quoteToken_), lpFeeRate_, address(maintainerFeeRateModel), i_, k_);
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Factory.sol#L86:86
File: src/mimswap/periphery/Router.sol 88 clone = IFactory(factory).create(useTokenAsQuote ? address(weth) : token, useTokenAsQuote ? token : address(weth), lpFeeRate, i, k); 203 if (token == address(weth)) { 237 if (token == address(weth)) { 285 if (token == address(weth)) {
https://github.com/r109n/2024-03-abracadabra-money/tree/main/mimswap/periphery/Router.sol#L285:285
#0 - c4-pre-sort
2024-03-15T14:58:44Z
141345 marked the issue as sufficient quality report
#1 - 141345
2024-03-16T00:22:21Z
204 albahaca l r nc 3 0 25
G 1 i G 2 l G 3 n G 4 n G 5 n G 6 n G 7 i G 8 n G 9 n G 10 n G 11 n G 12 n G 13 n G 14 n G 15 i G 16 n G 17 n G 18 n G 19 n G 20 l G 21 n G 22 n G 23 i G 24 i G 25 i G 26 i G 27 i G 28 n G 29 n G 30 i G 31 n G 32 i G 33 i G 34 i G 35 n G 36 n G 37 n G 38 n G 39 n G 40 l
#2 - c4-judge
2024-03-29T17:00:32Z
thereksfour marked the issue as grade-b
#3 - albahaca0000
2024-04-04T10:09:50Z
Hi @thereksfour thanks for judging
I dont' know why my report has grade b , I suggest a lot of gas optimization techniques, That are similar to the grade a reports and with more unique ones
These issues marked in my report as i but in other report marked as L
[G-23] State variables can be packed into fewer storage slots
[G-25] State variable read in a loop
[G-32] Structs can be modified to fit in fewer storage slots
These issue makred in my report as but NC but in other report makred as L
[G-22] Use local variables for emitting
These issue makred in my report as i but in other report makred as NC
[G‑33] Use asembly to validate msg.sender
[G‑34] Unlimited gas consumption risk due to external call recipients
so finally in my report there are 7 L that no other grade a report have.
#4 - c4-judge
2024-04-06T07:34:39Z
thereksfour marked the issue as grade-a