Platform: Code4rena
Start Date: 24/10/2023
Pot Size: $149,725 USDC
Total HM: 7
Participants: 52
Period: 21 days
Judge: ronnyx2017
Total Solo HM: 2
Id: 300
League: ETH
Rank: 37/52
Findings: 1
Award: $21.02
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: DavidGiladi
Also found by: 0xhex, 0xta, Collinsoden, JCK, Madalad, SAQ, SY_S, Sathish9098, cheatc0d3, codeslide, hihen, hunter_w3b, jamshed, lsaudit, mgf15, nonseodion, oualidpro, petrichor, sivanesh_808, tala7985, unique, ybansal2403, zabihullahazadzoi
21.0214 USDC - $21.02
Â
cachedSystemDebt and updating the systemDebtan additional read operation is performed to load the systemDebt value into the cachedSystemDebt variable. This read operation incurs a gas cost. This cost is typically around 2100 gas for a simple storage variable.
function increaseSystemDebt(uint256 _amount) external override { _requireCallerIsBOorCdpM(); - uint256 cachedSystemDebt = systemDebt + _amount; + systemDebt =systemDebt + _amount; - systemDebt = cachedSystemDebt; - emit ActivePoolEBTCDebtUpdated(cachedSystemDebt); + emit ActivePoolEBTCDebtUpdated(systemDebt); }
function decreaseSystemDebt(uint256 _amount) external override { _requireCallerIsBOorCdpM(); - uint256 cachedSystemDebt = systemDebt - _amount; + systemDebt -= _amount; - systemDebt = cachedSystemDebt; - emit ActivePoolEBTCDebtUpdated(cachedSystemDebt); + emit ActivePoolEBTCDebtUpdated(systemDebt); }
function increaseSystemCollShares(uint256 _value) external override { _requireCallerIsBorrowerOperations(); + systemCollShares += _value; - uint256 cachedSystemCollShares = systemCollShares + _value; - systemCollShares = cachedSystemCollShares; - emit SystemCollSharesUpdated(cachedSystemCollShares); + emit SystemCollSharesUpdated(systemCollShares); }
msg.sender use assembly to check msg.senderYou can use assembly to optimize the require statement to avoid duplicate msg.sender checks and save some gas.
using assembly to optimize this require might save around 100 to 200 gas.
require( msg.sender == borrowerOperationsAddress || msg.sender == cdpManagerAddress, "ActivePool: Caller is neither BorrowerOperations nor CdpManager" );
Here's how you can optimize the require statement using assembly:
require( assembly { let sender := msg.sender let borrowerOp := sload(borrowerOperationsAddress.slot) let cdpManager := sload(cdpManagerAddress.slot) iszero(or(eq(sender, borrowerOp), eq(sender, cdpManager))) }, "ActivePool: Caller is neither BorrowerOperations nor CdpManager" );
In this optimized code, assembly is used to load the borrowerOperationsAddress and cdpManagerAddress directly from storage and then check if msg.sender is neither of these two addresses. This reduces the need to check msg.sender twice and can potentially save gas.
Â
require( msg.sender == borrowerOperationsAddress || msg.sender == cdpManagerAddress || isAuthorized(msg.sender, msg.sig), "EBTC: Caller is neither BorrowerOperations nor CdpManager nor authorized" ); }
Instead of using address(
), it is more gas-efficient to pre-calculate and use the hardcoded address. Foundry’s script.sol and solmate’s LibRlp.sol contracts can help achieve this.
References: https://book.getfoundry.sh/reference/forge-std/compute-create-address
collateral.balanceOf(address(this)) >= collateral.getPooledEthByShares(systemCollShares),
collateral.sharesOf(address(this)) >= systemCollShares,
return collateral.balanceOf(address(this));
uint256 balance = IERC20(token).balanceOf(address(this));
uint256 balance = IERC20(token).balanceOf(address(this));
return keccak256(abi.encode(typeHash, name, version, _chainID(), address(this)));
_recipient != address(0) && _recipient != address(this),
constructor(address _owner) RolesAuthority(_owner, Authority(address(this))) {}
https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/Governor.sol#L36
address(this),
initialCdpIndex = sortedCdps.cdpCountOf(address(this));
IERC3156FlashBorrower(address(this)),
IERC3156FlashBorrower(address(this)),
bytes32 cdpId = sortedCdps.cdpOfOwnerByIndex(address(this), initialCdpIndex);
address(this),
IERC20(swapChecks[i].tokenToCheck).balanceOf(address(this)) >
require(addy != address(this)); // If it could call this it could fake the forwarded caller
address _owner = IOwnerLike(address(this)).owner();
require(msg.sender == address(this)); // Must call this via `execute` to explicitly set the flag
require(msg.sender == owner || authority.canCall(msg.sender, address(this), msg.sig));
(address(auth) != address(0) && auth.canCall(user, address(this), functionSig)) ||
return (address(auth) != address(0) && auth.canCall(user, address(this), functionSig));
require(_authority.canCall(msg.sender, address(this), msg.sig));
emit AuthorityUpdated(address(this), Authority(newAuthority));
return (amount * feeBps) / MAX_BPS;
pragma solidity 0.8.17;
https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/EBTCToken.sol#L3
https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/Governor.sol#L2
public is generally more gas-efficient than using the getter function.When you declare a state variable as public, the Solidity compiler automatically generates a getter function for that variable. This getter function allows you to access the value of the variable directly from outside the contract. Using this auto-generated getter is usually more gas-efficient than writing a custom getter function, as it avoids the overhead of function call execution.
function getSystemCollShares() external view override returns (uint256) { return systemCollShares; }
function getSystemDebt() external view override returns (uint256) { return systemDebt; }
function getFeeRecipientClaimableCollShares() external view override returns (uint256) { return feeRecipientCollShares; }
Â
Use a fixed size bytes datatype like bytes4 in place of string.
Instances of this issue :
function version() external pure override returns (string memory) { return _VERSION; }
function symbol() external pure override returns (string memory) { return _SYMBOL; }
function name() external pure override returns (string memory) { return _NAME; }
If the if statement has a logical AND and is not followed by an else statement, it can be replaced with 2 if statements
if (!_isDebtIncrease && _debtChange > 0) {
if ( !_fallbackIsBroken(fallbackResponse) && !_responseTimeout(fallbackResponse.timestamp, newFallbackCaler.fallbackTimeout()) ) {
if ( _checkHealthyCLResponse(chainlinkResponse.roundEthBtcId, ethBtcAnswer) && _checkHealthyCLResponse(chainlinkResponse.roundStEthEthId, stEthEthAnswer) ) {
if ( _checkHealthyCLResponse(prevChainlinkResponse.roundEthBtcId, ethBtcAnswer) && _checkHealthyCLResponse(prevChainlinkResponse.roundStEthEthId, stEthEthAnswer) ) {
if (maxNodes > 0 && i >= maxNodes) {
if (maxNodes > 0 && i >= maxNodes) {
if (maxNodes > 0 && i >= maxNodes) {
if (prevId == dummyId && nextId == dummyId) {
if (_prevId == dummyId && _nextId == dummyId) {
if (data.head == _startId && _NICR >= cdpManager.getCachedNominalICR(_startId)) {
if (data.tail == _startId && _NICR <= cdpManager.getCachedNominalICR(_startId)) {
if (prevId == dummyId && nextId == dummyId) {
When 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.html
Each 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.
uint8 v,
function decimals() external pure override returns (uint8) {
uint8 roleId;
https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/Governor.sol#L20
uint8[] roles;
https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/Governor.sol#L27
mapping(uint8 => string) internal roleNames;
https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/Governor.sol#L30
event RoleNameSet(uint8 indexed role, string indexed name);
https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/Governor.sol#L32
function getRoleName(uint8 role) external view returns (string memory roleName) {
function setRoleName(uint8 role, string memory roleName) external requiresAuth {
uint8 ethBtcDecimals; uint8 stEthEthDecimals;
try ETH_BTC_CL_FEED.decimals() returns (uint8 decimals) {
try STETH_ETH_CL_FEED.decimals() returns (uint8 decimals) {
uint80 _currentRoundEthBtcId, uint80 _currentRoundStEthEthId
uint8 ethBtcDecimals; uint8 stEthEthDecimals;
try ETH_BTC_CL_FEED.decimals() returns (uint8 decimals) {
try STETH_ETH_CL_FEED.decimals() returns (uint8 decimals) {
uint8 _ethBtcDecimals, uint8 _stEthEthDecimals
uint16 public feeBps = 3; // may be subject to future adjustments through protocol governance
abi.encode(
Instead of using the && operator in a single require statement to check multiple conditions, I suggest using multiple require statements with 1 condition per require statement (saving 3 gas per &).
See this issue which describes the fact that there is a larger deployment gas cost, but with enough runtime calls, the change ends up being cheaper.
recoveredAddress != address(0) && recoveredAddress == _borrower,
_maxFeePercentage >= redemptionFeeFloor && _maxFeePercentage <= DECIMAL_PRECISION,
_recipient != address(0) && _recipient != address(this),
_recipient != cdpManagerAddress && _recipient != borrowerOperationsAddress,
require( !_chainlinkIsBroken(chainlinkResponse, prevChainlinkResponse) && !_chainlinkIsFrozen(chainlinkResponse), "PriceFeed: Chainlink must be working and current" );
return _checkICRAgainstMCR(_icr) || (_checkICRAgainstTCR(_icr, _tcr) && _checkRecoveryModeForTCR(_tcr)); }
Proof Of Concept
function _requireSingularCollChange(
function _requireNonZeroAdjustment(
function _requireValidAdjustmentInCurrentMode(
760 function _processTokenMovesFromAdjustment(MoveTokensParams memory _varMvTokens) internal {
function _requireNoDecreaseOfICR(uint256 _newICR, uint256 _oldICR) internal pure {
function _requireValidDebtRepayment(uint256 _currentDebt, uint256 _debtRepayment) internal pure {
function _getNewNominalICRFromCdpChange(
function _getNewICRFromCdpChange(
function _requireCallerIsBorrowerOperations() internal view {
function _requireCallerIsCdpManager() internal view {
function _requireCallerIsActivePool() internal view {
function _requireCallerIsCdpM() internal view { require(msg.sender == cdpManagerAddress, "EBTC: Caller is not CdpManager"); }
Using assembly to check for zero can save gas by allowing more direct access to the evm and reducing some of the overhead associated with high-level operations in solidity.
if (newDebt == 0) {
if (_maxIterations == 0) {
if ( _response.timestampEthBtc == 0 || _response.timestampEthBtc > block.timestamp || _response.timestampStEthEth == 0 || _response.timestampStEthEth > block.timestamp ) {
if (_response.timestamp == 0 || _response.timestamp > block.timestamp) {
if (_response.answer == 0) {
if (minPrice == 0) return false;
if (_currentRoundEthBtcId == 0 || _currentRoundStEthEthId == 0) {
if (_size == 0) {
if (_answer <= 0) return false; if (_roundId == 0) return false;
if (maxArraySize == 0) {
require(cdpManager.getCdpStatus(_id) == 0, "SortedCdps: new id is NOT nonExistent!");
if (_maxIterations == 0) {
if (arrayLength == 0) {
if (_minutes == 0) {
if (n % 2 == 0) {
The compiler uses opcodes GT and ISZERO for solidity code that uses >, but only requires LT for >=, which saves 3 gas
There are 13 instances of this issue:
File: packages/contracts/contracts/CdpManagerStorage.sol 512 if (_newIndex > _oldIndex && totalStakes > 0) { 556 require(_newIndex > _prevIndex, "CDPManager: only take fee with bigger new index"); 585 require(activePool.getSystemCollShares() > _feeTaken, "CDPManager: fee split is too big"); 636 if (_scaledCdpColl > _feeSplitDistributed) { 654 CdpOwnersArrayLength > 1 && sortedCdps.getSize() > 1, 765 if (_newIndex > _oldIndex && totalStakes > 0) {
File: packages/contracts/contracts/LiquidationLibrary.sol 476 if (_partialState.ICR > LICR) { 553 if (_ICR > LICR) { 578 if (_ICR > LICR) {
File: packages/contracts/contracts/SortedCdps.sol 422 require(_len > 1, "SortedCdps: batchRemove() only apply to multiple cdpIds!"); 460 if (data.size > 1) {
File: packages/contracts/contracts/Dependencies/EbtcMath.sol 60 if (_minutes > 525600000) {
Â
Remove the parameter _initData in VariableInterestRate.getNewRate._initData because of it's not used in the function.
function batchLiquidateCdps(bytes32[] memory _cdpArray) external override {
The reason for this is that constant variables are evaluated at runtime and their value is included in the bytecode of the contract. This means that any expensive operations performed as part of the constant expression, such as a call to keccak256(), will be executed every time the contract is deployed, even if the result is always the same. This can result in higher gas costs.
bytes32 constant MY_HASH = keccak256("my string");
Alternatively, we could use an immutable variable, like so:
bytes32 immutable MY_HASH = keccak256("my string");
There are 4 instances of this issue:
File: packages/contracts/contracts/BorrowerOperations.sol 32 bytes32 private constant _PERMIT_POSITION_MANAGER_TYPEHASH = keccak256( "PermitPositionManagerApproval(address borrower,address positionManager,uint8 status,uint256 nonce,uint256 deadline)" );
File: packages/contracts/contracts/LeverageMacroBase.sol 37 bytes32 constant FLASH_LOAN_SUCCESS = keccak256("ERC3156FlashBorrower.onFlashLoan");
File: packages/contracts/contracts/SimplifiedDiamondLike.sol 39 bytes32 constant DIAMOND_STORAGE_POSITION = keccak256("diamond.standard.diamond.storage");
File: packages/contracts/contracts/Dependencies/ERC3156FlashLender.sol 11 bytes32 public constant FLASH_SUCCESS_VALUE = keccak256("ERC3156FlashBorrower.onFlashLoan");
External call cost is less expensive than of public functions.
Contracts are allowed to override their parents’ functions and change the visibility from external to public.
The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.
function getDeploymentStartTime() public view returns (uint256) {
function sweepToken(address token, uint256 amount) public {
function doesRoleHaveCapability(
function isPublicCapability(address target, bytes4 functionSig) public view returns (bool) {
function canCall(
We can use assembly to efficiently validate msg.sender for the didPay and uniswapV3SwapCallback functions with the least amount of opcodes necessary. Additionally, we can use xor() instead of iszero(eq()), saving 3 gas. We can also potentially save gas on the unhappy path by using scratch space to store the error selector, potentially avoiding memory expansion costs.
msg.sender == borrowerOperationsAddress,
require(msg.sender == cdpManagerAddress, "CollSurplusPool: Caller is not CdpManager");
require(msg.sender == activePoolAddress, "CollSurplusPool: Caller is not Active Pool");
msg.sender == borrowerOperationsAddress,
require( msg.sender == borrowerOperationsAddress || msg.sender == cdpManagerAddress || isAuthorized(msg.sender, msg.sig), "EBTC: Caller is neither BorrowerOperations nor CdpManager nor authorized" ); }
require(msg.sender == cdpManagerAddress, "EBTC: Caller is not CdpManager");
require(msg.sender == address(cdpManager), "SortedCdps: Caller is not the CdpManager");
msg.sender == borrowerOperationsAddress || msg.sender == address(cdpManager),
require(owner() == msg.sender, "Must be owner");
require(msg.sender == owner);
require(msg.sender == address(this)); // Must call this via `execute` to explicitly set the flag
require(msg.sender == owner, "Must be owner");
require(msg.sender == owner || authority.canCall(msg.sender, address(this), msg.sig));
Performing STATICCALLs that do not depend on variables incremented in loops should always try to be avoided within the loop. In the following instances, we are able to cache the external calls outside of the loop to save a STATICCALL (100 gas) per loop iteration.
There are 1 instances of this issue:
File: packages/contracts/contracts/Governor.sol 46 for (uint256 i = 0; i < users.length(); i++) {
function getUsersByRole(uint8 role) external view returns (address[] memory usersWithRole) { // Search over all users: O(n) * 2 uint256 count; uint256 ulenght = users.length(); - for (uint256 i = 0; i < users.length(); i++) { + for (uint256 i = 0; i < ulenght; i++) { address user = users.at(i); bool _canCall = doesUserHaveRole(user, role); if (_canCall) { count += 1; } }
Changing the abi.encode function to abi.encodePacked can save gas since the abi.encode function pads extra null bytes at the end of the call data, which is unnecessary. Also, in general, abi.encodePacked is more gas-efficient.
abi.encode(
return keccak256(abi.encode(typeHash, name, version, _chainID(), address(this)));
OpenCdpOperation memory flData = abi.decode(data, (OpenCdpOperation));
CloseCdpOperation memory flData = abi.decode(data, (CloseCdpOperation));
AdjustCdpOperation memory flData = abi.decode(data, (AdjustCdpOperation));
If needed, the values can be read from the verified contract source code, or if there are multiple values there can be a single getter function that returns a tuple of the values of all currently-public constants. Saves 3406-3606 gas in deployment gas due to the compiler not having to create non-payable getter functions for deployment calldata, not having to store the bytes of the value outside of where it’s used, and not adding another entry to the method ID table.
uint256 public constant TIMEOUT_ETH_BTC_FEED = 4800; // 1 hours & 20min: 60 * 80
uint256 public constant TIMEOUT_STETH_ETH_FEED = 90000; // 25 hours: 60 * 60 * 25
uint256 public constant MAX_PRICE_DEVIATION_FROM_PREVIOUS_ROUND = 5e17; // 50%
uint256 public constant MAX_PRICE_DIFFERENCE_BETWEEN_ORACLES = 5e16; // 5%
string public constant NAME = "HintHelpers";
ISortedCdps public immutable sortedCdps; ICdpManager public immutable cdpManager;
address public immutable owner;
uint256 public constant MAX_BPS = 10_000; uint256 public constant MAX_FEE_BPS = 1_000; // 10% bytes32 public constant FLASH_SUCCESS_VALUE = keccak256("ERC3156FlashBorrower.onFlashLoan");
uint256 public constant LICR = 1030000000000000000; // 103% // Minimum collateral ratio for individual cdps uint256 public constant MCR = 1100000000000000000; // 110% // Critical system collateral ratio. If the system's total collateral ratio (TCR) falls below the CCR, Recovery Mode is triggered. uint256 public constant CCR = 1250000000000000000; // 125% // Amount of stETH collateral to be locked in active pool on opening cdps uint256 public constant LIQUIDATOR_REWARD = 2e17; // Minimum amount of stETH collateral a CDP must have uint256 public constant MIN_NET_STETH_BALANCE = 2e18; uint256 public constant PERCENT_DIVISOR = 200; // dividing by 200 yields 0.5% uint256 public constant BORROWING_FEE_FLOOR = 0; // 0.5% uint256 public constant STAKING_REWARD_SPLIT = 5_000; // taking 50% cut from staking reward uint256 public constant MAX_REWARD_SPLIT = 10_000; IActivePool public immutable activePool; IPriceFeed public immutable override priceFeed; // the only collateral token allowed in CDP ICollateralToken public immutable collateral;
uint256 public constant MAX_TCR = type(uint256).max;
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 1 instances of this issue:
File: packages/contracts/contracts/SortedCdps.sol 451 emit NodeRemoved(_ids[i]);
block.number and block.timestamp are added to the event information by default, so adding them manually will waste additional gas.
Proof Of Concept
emit UnhealthyFallbackCaller(_fallbackCaller, block.timestamp);
This with 0.8.9 compiler and optimization enabled. As you can see it's cheaper to deploy with a modifier, and it will save you about 30 gas. But sometimes modifiers increase code size of the contract.
function _assertOwner() internal { // Reference will compare to variable, require(owner() == msg.sender, "Must be owner"); }
constants instead of enum can save gasEnum is expensive and it is <ins>more efficient to use constants</ins> instead. An illustrative example of this approach can be found in the <ins>ReentrancyGuard.sol</ins>.
enum FlashLoanType { stETH, eBTC, noFlashloan // Use this to not perform a FL and just `doOperation` } enum PostOperationCheck { openCdp, cdpStats, isClosed } enum Operator { skip, equal, gte, lte }
enum OperationType { OpenCdpOperation, AdjustCdpOperation, CloseCdpOperation }
enum OperationType { call, delegatecall }
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.
address public immutable borrowerOperations; address public immutable activePool; address public immutable cdpManager; address public immutable ebtcToken; address public immutable stETH; address public immutable sortedCdps;
Using both named returns and a return statement isn’t necessary, this consumes extra gas as one more variable that is not used is declared .
function getSystemCollShares() public view returns (uint256 entireSystemColl) {
function _getSystemDebt() internal view returns (uint256 entireSystemDebt) {
function _getCachedTCR(uint256 _price) internal view returns (uint256 TCR) {
function _getTCRWithSystemDebtAndCollShares( uint256 _price ) internal view returns (uint256 TCR, uint256 _coll, uint256 _debt) {
internal functions not called by the contract should be removedIf the functions are required by an interface, the contract should inherit from that interface and use the override keyword
Note: missed from bots
function _requireUserAcceptsFee(
function decMul(uint256 x, uint256 y) internal pure returns (uint256 decProd) {
When fetching data from a storage location, assigning the data to a memory variable causes all fields of the struct/array to be read from storage, which incurs a Gcoldsload (2100 gas) for each field of the struct/array. If the fields are read from the new memory variable, they incur an additional MLOAD rather than a cheap stack read. Instead of declearing the variable with the memory keyword, declaring the variable with the storage keyword and caching any fields that need to be re-read in stack variables, will be much cheaper, only incuring the Gcoldsload for the fields actually read. The only time it makes sense to read the whole struct/array into a memory variable, is if the full struct/array is being returned by the function, is being passed to a function that requires memory, or if the array/struct is being read from another memory array/struct
There are 36 instances of this issue:
File: packages/contracts/contracts/BorrowerOperations.sol 427 MoveTokensParams memory _varMvTokens = MoveTokensParams( 760 function _processTokenMovesFromAdjustment(MoveTokensParams memory _varMvTokens) internal { 856 AdjustCdpLocals memory _vars 987 AdjustCdpLocals memory vars,
File: packages/contracts/contracts/CdpManager.sol 136 SingleRedemptionInputs memory _redeemColFromCdp 150 CdpDebtAndCollShares memory _oldDebtAndColl = CdpDebtAndCollShares( 329 RedemptionTotals memory totals; 403 SingleRedemptionValues memory singleRedemption = _redeemCollateralFromCdp(
File: packages/contracts/contracts/HintHelpers.sol 62 LocalVariables_getRedemptionHints memory vars; 134 LocalVariables_getRedemptionHints memory vars,
File: packages/contracts/contracts/LeverageMacroBase.sol 176 ICdpManagerData.Cdp memory cdpInfo = cdpManager.Cdps(cdpId); 187 ICdpManagerData.Cdp memory cdpInfo = cdpManager.Cdps(checkParams.cdpId); 199 ICdpManagerData.Cdp memory cdpInfo = cdpManager.Cdps(checkParams.cdpId); 244 function _doCheckValueType(CheckValueAndType memory check, uint256 valueToCheck) internal { 291 function _handleOperation(LeverageMacroOperation memory operation) internal {
File: packages/contracts/contracts/PriceFeed.sol 73 ChainlinkResponse memory chainlinkResponse = _getCurrentChainlinkResponse(); 74 ChainlinkResponse memory prevChainlinkResponse = _getPrevChainlinkResponse( 100 ChainlinkResponse memory chainlinkResponse = _getCurrentChainlinkResponse(); 101 ChainlinkResponse memory prevChainlinkResponse = _getPrevChainlinkResponse( 105 FallbackResponse memory fallbackResponse = _getCurrentFallbackResponse(); 361 FallbackResponse memory fallbackResponse; 401 ChainlinkResponse memory _currentResponse, 402 ChainlinkResponse memory _prevResponse 412 function _badChainlinkResponse(ChainlinkResponse memory _response) internal view returns (bool) { 435 function _chainlinkIsFrozen(ChainlinkResponse memory _response) internal view returns (bool) { 446 ChainlinkResponse memory _currentResponse, ChainlinkResponse memory _prevResponse 465 function _fallbackIsBroken(FallbackResponse memory _response) internal view returns (bool) { 486 FallbackResponse memory _fallbackResponse 504 ChainlinkResponse memory _chainlinkResponse, ChainlinkResponse memory _prevChainlinkResponse, FallbackResponse memory _fallbackResponse 527 ChainlinkResponse memory _chainlinkResponse, FallbackResponse memory _fallbackResponse 562 FallbackResponse memory _fallbackResponse
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 4 instances of this issue:
File: packages/contracts/contracts/Governor.sol 47 address user = users.at(i); 48 bool _canCall = doesUserHaveRole(user, role); 58 address user = _usrs[i]; 59 bool _canCall = doesUserHaveRole(user, role);
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
File: packages/contracts/contracts/BorrowerOperations.sol 1133 return type(uint112).max;
File: packages/contracts/contracts/CdpManager.sol 278 _maxIterations = type(uint256).max;
File: packages/contracts/contracts/CdpManagerStorage.sol 21 uint128 public constant UNSET_TIMESTAMP = type(uint128).max;
File: packages/contracts/contracts/EBTCToken.sol 143 if (cachedAllowance != type(uint256).max) {
File: packages/contracts/contracts/Governor.sol 78 for (uint8 i = 0; i < type(uint8).max; i++) { 84 for (uint8 i = 0; i < type(uint8).max; i++) { 98 for (uint8 i = 0; i < type(uint8).max; i++) { 107 for (uint8 i = 0; i < type(uint8).max; i++) {
https://github.com/code-423n4/2023-10-badger/blob/main/packages/contracts/contracts/Governor.sol#L78
File: packages/contracts/contracts/HintHelpers.sol 79 _maxIterations = type(uint256).max;
File: packages/contracts/contracts/LeverageMacroReference.sol 39 ebtcToken.approve(_borrowerOperationsAddress, type(uint256).max); stETH.approve(_borrowerOperationsAddress, type(uint256).max); stETH.approve(_activePool, type(uint256).max); 53 ebtcToken.approve(address(borrowerOperations), type(uint256).max); stETH.approve(address(borrowerOperations), type(uint256).max); stETH.approve(address(activePool), type(uint256).max);
File: packages/contracts/contracts/SortedCdps.sol 90 _size = type(uint256).max;
File: packages/contracts/contracts/Dependencies/EbtcMath.sol 7 uint256 public constant MAX_TCR = type(uint256).max;
Â
#0 - c4-pre-sort
2023-11-17T14:44:05Z
bytes032 marked the issue as sufficient quality report
#1 - c4-judge
2023-11-28T03:04:48Z
jhsagd76 marked the issue as grade-c
#2 - jhsagd76
2023-12-06T17:20:13Z
29*1 + 1*4 - 7 *4
5
#3 - c4-judge
2023-12-06T20:56:41Z
jhsagd76 marked the issue as grade-b