Platform: Code4rena
Start Date: 28/06/2022
Pot Size: $25,000 USDC
Total HM: 14
Participants: 50
Period: 4 days
Judge: GalloDaSballo
Total Solo HM: 7
Id: 141
League: ETH
Rank: 25/50
Findings: 2
Award: $67.18
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: zzzitron
Also found by: 0v3rf10w, 0x1f8b, 0x29A, AlleyCat, Bnke0x0, Chom, Funen, JC, Lambda, Limbooo, Meera, Picodes, Sm4rty, TerrierLover, TomJ, __141345__, asutorufos, aysha, c3phas, cccz, defsec, fatherOfBlocks, grGred, hake, ignacio, ladboy233, mrpathfindr, oyc_109, rfa, sach1r0, samruna, slywaters, ynnad
43.2289 USDC - $43.23
When casting to int from uint, the overflow might happen.
uint twapMantissa = oracle.getUnderlyingPrice(cNote); // returns price as mantissa //uint ir = (1 - twapMantissa).mul(adjusterCoefficient).add(baseRatePerYear); int diff = BASE - int(twapMantissa); //possible annoyance if 1e18 - twapMantissa > 2**255, differ
int(twapMantissa) can overflow depending on the value of uint twapMantissa. Even if this is not expected, handling this case should be good.
Static analysis
Consider using the logic of toInt256 provided by OpenZeppelin.
#0 - GalloDaSballo
2022-08-13T23:08:35Z
🌟 Selected for report: 0x1f8b
Also found by: 0x29A, 0xArshia, 0xKitsune, Bnke0x0, Chom, Fitraldys, Funen, JC, Lambda, Meera, Noah3o6, Picodes, RedOneN, Rohan16, Sm4rty, TerrierLover, TomJ, Tomio, Waze, ajtra, c3phas, cRat1st0s, defsec, durianSausage, fatherOfBlocks, grGred, hake, ladboy233, m_Rassska, mrpathfindr, oyc_109, rfa, ynnad
23.9548 USDC - $23.95
| Title | Counts |
|---|---|
[Gas-1] Use != 0 instead of > 0 on uint variables | 32 |
| [Gas-2] No need to set 0 on uint variables | 23 |
| [Gas-3] Potential usage of unchecked | 4 |
| [Gas-4] Gas cost of ++i/--i is smaller than i++/i-- | 11 |
| [Gas-5] Use custom errors | 102 |
[Gas-6] Can simplify the logic when setting name and symbol | 1 |
[Gas-7] Call block.number directly instead of calling getBlockNumber() function | 5 |
[Gas-8] Proposal.canceled seems not needed | 1 |
[Gas-9] Unnecessary console import and usage of console.log | 1 |
[Gas-10] Unnecessary usage of SafeMath at NoteInterest.sol | 1 |
!= 0 instead of > 0 on uint variablesuint variables will never be lower than 0. Therefore, > 0 and != 0 have same meanings. Using != 0 can reduce the gas deployment cost, so it is worth using != 0 wherever possible.
Here are list of the gas improvements by using != 0.
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Stableswap/BaseV1-core.sol
File: lending-market-v2/contracts/Stableswap/BaseV1-core.sol 159,25: if (timeElapsed > 0 && _reserve0 != 0 && _reserve1 != 0) { 256,27: require(liquidity > 0, 'ILM'); // BaseV1: INSUFFICIENT_LIQUIDITY_MINTED 275,25: require(amount0 > 0 && amount1 > 0, 'ILB'); // BaseV1: INSUFFICIENT_LIQUIDITY_BURNED 275,40: require(amount0 > 0 && amount1 > 0, 'ILB'); // BaseV1: INSUFFICIENT_LIQUIDITY_BURNED 289,28: require(amount0Out > 0 || amount1Out > 0, 'IOA'); // BaseV1: INSUFFICIENT_OUTPUT_AMOUNT 289,46: require(amount0Out > 0 || amount1Out > 0, 'IOA'); // BaseV1: INSUFFICIENT_OUTPUT_AMOUNT 298,24: if (amount0Out > 0) _safeTransfer(_token0, to, amount0Out); // optimistically transfer tokens 299,24: if (amount1Out > 0) _safeTransfer(_token1, to, amount1Out); // optimistically transfer tokens 300,25: if (data.length > 0) IBaseV1Callee(to).hook(msg.sender, amount0Out, amount1Out, data); // callback, used for flash loans 306,27: require(amount0In > 0 || amount1In > 0, 'IIA'); // BaseV1: INSUFFICIENT_INPUT_AMOUNT 306,44: require(amount0In > 0 || amount1In > 0, 'IIA'); // BaseV1: INSUFFICIENT_INPUT_AMOUNT 468,35: require(token.code.length > 0);
File: lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol 122,25: require(amountA > 0, "BaseV1Router: INSUFFICIENT_AMOUNT"); 123,26: require(reserveA > 0 && reserveB > 0, "BaseV1Router: INSUFFICIENT_LIQUIDITY"); 123,42: require(reserveA > 0 && reserveB > 0, "BaseV1Router: INSUFFICIENT_LIQUIDITY"); 474,35: require(token.code.length > 0); 481,35: require(token.code.length > 0, "token code length failure");
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Comptroller.sol
File: lending-market-v2/contracts/Comptroller.sol 314,23: if (shortfall > 0) { 334,47: if (redeemTokens == 0 && redeemAmount > 0) { 385,23: if (shortfall > 0) { 1134,34: if (amountToSubtract > 0) { 1199,25: if (deltaBlocks > 0 && supplySpeed > 0) { 1199,44: if (deltaBlocks > 0 && supplySpeed > 0) { 1202,48: Double memory ratio = supplyTokens > 0 ? fraction(compAccrued, supplyTokens) : Double({mantissa: 0}); 1205,32: } else if (deltaBlocks > 0) { 1220,25: if (deltaBlocks > 0 && borrowSpeed > 0) { 1220,44: if (deltaBlocks > 0 && borrowSpeed > 0) { 1223,48: Double memory ratio = borrowAmount > 0 ? fraction(compAccrued, borrowAmount) : Double({mantissa: 0}); 1226,32: } else if (deltaBlocks > 0) { 1316,25: if (deltaBlocks > 0 && compSpeed > 0) { 1316,42: if (deltaBlocks > 0 && compSpeed > 0) { 1384,20: if (amount > 0 && amount <= compRemaining) {
The default value of uint varibles are 0. Therefore, there is no need to set 0 on uint variables. Not setting 0 on uint variables can reduce the deployment gas cost.
Here are list of the gas improvements.
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Stableswap/BaseV1-core.sol
File: lending-market-v2/contracts/Stableswap/BaseV1-core.sol 48,29: uint public totalSupply = 0; 72,30: uint constant periodSize = 0; 210,21: for (uint i = 0; i < _prices.length; i++) { 226,24: uint nextIndex = 0; 227,20: uint index = 0; 340,21: for (uint i = 0; i < 255; i++) {
File: lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol 154,21: for (uint i = 0; i < routes.length; i++) { 176,27: uint _totalSupply = 0; 380,21: for (uint i = 0; i < routes.length; i++) { 496,20: stable = 0; // This line does not even need
File: lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol 66,21: for (uint i = 0; i < newProposal.targets.length; i++) { 88,21: for (uint i = 0; i < proposal.targets.length; i++) {
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Comptroller.sol
File: lending-market-v2/contracts/Comptroller.sol 131,21: for (uint i = 0; i < len; i++) { 211,21: for (uint i = 0; i < len; i++) { 742,21: for (uint i = 0; i < assets.length; i++) { 964,21: for (uint i = 0; i < allMarkets.length; i ++) { 1010,20: for(uint i = 0; i < numMarkets; i++) { 1111,21: for (uint i = 0; i < affectedUsers.length; ++i) { 1352,21: for (uint i = 0; i < cTokens.length; i++) { 1358,29: for (uint j = 0; j < holders.length; j++) { 1364,29: for (uint j = 0; j < holders.length; j++) { 1369,21: for (uint j = 0; j < holders.length; j++) { 1418,21: for (uint i = 0; i < numTokens; ++i) {
Following variables or operations can be wrapped by unchecked to reduce the gas cost.
[1] i++ or ++i used in the for loop when the end condition is uint or constant
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Stableswap/BaseV1-core.sol
File: lending-market-v2/contracts/Stableswap/BaseV1-core.sol 210,46: for (uint i = 0; i < _prices.length; i++) { 340,35: for (uint i = 0; i < 255; i++) {
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Comptroller.sol
File: lending-market-v2/contracts/Comptroller.sol 131,35: for (uint i = 0; i < len; i++) { 211,35: for (uint i = 0; i < len; i++) { 1010,41: for(uint i = 0; i < numMarkets; i++) {
[2] y > y_prev assures that y - y_prev in if statement, and y_prev - y in else statement will not underflow
if (y > y_prev) { if (y - y_prev <= 1) { return y; } } else { if (y_prev - y <= 1) { return y; } }
[3] if (msg.value > amountCANTO) assures that msg.value - amountCANTO will not underflow
unchecked { if (msg.value > amountCANTO) _safeTransferCANTO(msg.sender, msg.value - amountCANTO); }
[4] require(b <= a, ...) assures that a-b will not underflow
function sub256(uint256 a, uint256 b) internal pure returns (uint) { require(b <= a, "subtraction underflow"); return a - b; }
Here are gas optimization opportunities.
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Comptroller.sol
File: lending-market-v2/contracts/Comptroller.sol 131,35: for (uint i = 0; i < len; i++) { 211,35: for (uint i = 0; i < len; i++) { 742,45: for (uint i = 0; i < assets.length; i++) { 1010,41: for(uint i = 0; i < numMarkets; i++) { 1352,46: for (uint i = 0; i < cTokens.length; i++) {
File: lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol 66,58: for (uint i = 0; i < newProposal.targets.length; i++) { 88,55: for (uint i = 0; i < proposal.targets.length; i++) {
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Stableswap/BaseV1-core.sol
File: lending-market-v2/contracts/Stableswap/BaseV1-core.sol 210,46: for (uint i = 0; i < _prices.length; i++) { 340,35: for (uint i = 0; i < 255; i++) {
File: lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol 154,45: for (uint i = 0; i < routes.length; i++) { 380,45: for (uint i = 0; i < routes.length; i++) {
Using custom errors can reduce the gas cost.
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/CNote.sol
File: lending-market-v2/contracts/CNote.sol 17,9: require(msg.sender == admin, "CNote::_setAccountantContract:Only admin may call this function"); 94,9: require(success, "TOKEN_TRANSFER_IN_FAILED"); 105,13: require(balanceCur == 0, "Accountant has not been correctly supplied"); 147,9: require(success, "TOKEN_TRANSFER_OUT_FAILED"); 148,9: require(token.balanceOf(address(this)) == 0, "cNote::doTransferOut: TransferOut Failed"); 157,13: require(_notEntered, "re-entered");
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Comptroller.sol
File: lending-market-v2/contracts/Comptroller.sol 183,9: require(oErr == 0, "exitMarket: getAccountSnapshot failed"); // semi-opaque error code 242,9: require(!mintGuardianPaused[cToken], "mint is paused"); 348,9: require(!borrowGuardianPaused[cToken], "borrow is paused"); 356,13: require(msg.sender == cToken, "sender must be cToken"); 378,13: require(nextTotalBorrows < borrowCap, "market borrow cap reached"); 496,13: require(borrowBalance >= repayAmount, "Can not repay more than the total borrow"); 561,9: require(!seizeGuardianPaused, "seize is paused"); 619,9: require(!transferGuardianPaused, "transfer is paused"); 857,6: require(msg.sender == admin, "only admin can set close factor"); 965,13: require(allMarkets[i] != CToken(cToken), "market already added"); 1003,6: require(msg.sender == admin || msg.sender == borrowCapGuardian, "only admin or borrow cap guardian can set borrow caps"); 1008,9: require(numMarkets != 0 && numMarkets == numBorrowCaps, "invalid input"); 1021,9: require(msg.sender == admin, "only admin can set borrow cap guardian"); 1056,9: require(markets[address(cToken)].isListed, "cannot pause a market that is not listed"); 1057,9: require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); 1058,9: require(msg.sender == admin || state == true, "only admin can unpause"); 1066,9: require(markets[address(cToken)].isListed, "cannot pause a market that is not listed"); 1067,9: require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); 1068,9: require(msg.sender == admin || state == true, "only admin can unpause"); 1076,9: require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); 1077,9: require(msg.sender == admin || state == true, "only admin can unpause"); 1085,9: require(msg.sender == pauseGuardian || msg.sender == admin, "only pause guardian and admin can pause"); 1086,9: require(msg.sender == admin || state == true, "only admin can unpause"); 1094,9: require(msg.sender == unitroller.admin(), "only unitroller admin can change brains"); 1095,9: require(unitroller._acceptImplementation() == 0, "change not authorized"); 1100,9: require(msg.sender == admin, "Only admin can call this function"); // Only the timelock can call this function 1101,9: require(!proposal65FixExecuted, "Already executed this one-off function"); // Require that this function is only called once 1102,9: require(affectedUsers.length == amounts.length, "Invalid input"); 1163,9: require(market.isListed, "comp market is not listed"); 1354,13: require(markets[address(cToken)].isListed, "market must be listed"); 1400,9: require(adminOrInitializing(), "only admin can grant comp"); 1402,9: require(amountLeft == 0, "insufficient comp for grant"); 1413,9: require(adminOrInitializing(), "only admin can set comp speed"); 1416,9: require(numTokens == supplySpeeds.length && numTokens == borrowSpeeds.length, "Comptroller::_setCompSpeeds invalid input"); 1429,9: require(adminOrInitializing(), "only admin can set comp speed"); 1480,9: require(msg.sender == admin, "Only admin may initialize Weth Address");
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/NoteInterest.sol
File: lending-market-v2/contracts/NoteInterest.sol 167,9: require(msg.sender == admin, "only the admin may set the base rate"); 180,9: require(msg.sender == admin, "only the admin may set the adjuster coefficient"); 193,9: require(msg.sender == admin, "only the admin may set the update frequency");
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/WETH.sol
File: lending-market-v2/contracts/WETH.sol 29,9: require(_balanceOf[msg.sender] >= wamount, "sender balance insufficient for withdrawal"); 69,9: require(_balanceOf[src] >= wad, "WETH::transfeFrom"); 72,13: require(_allowance[src][msg.sender] >= wad, "WETH::transferFrom:allowance insufficient"); 89,9: require(owner != address(0), "ERC20: approve from the zero address"); 90,9: require(spender != address(0), "ERC20: approve to the zero address");
File: lending-market-v2/contracts/Governance/GovernorBravoDelegate.sol 25,9: require(address(timelock) == address(0), "GovernorBravo::initialize: can only initialize once"); 26,9: require(msg.sender == admin, "GovernorBravo::initialize: admin only"); 27,9: require(timelock_ != address(0), "GovernorBravo::initialize: invalid timelock address"); 42,9: require(unigovProposal.targets.length == unigovProposal.values.length && 46,9: require(unigovProposal.targets.length != 0, "GovernorBravo::propose: must provide actions"); 47,9: require(unigovProposal.targets.length <= proposalMaxOperations, "GovernorBravo::propose: too many actions"); 76,9: require(!timelock.queuedTransactions(keccak256(abi.encode(target, value, signature, data, eta))), "GovernorBravo::queueOrRevertInternal: identical proposal action already queued at eta"); 85,9: require(state(proposalId) == ProposalState.Queued, "GovernorBravo::execute: proposal can only be executed if it is queued"); 128,9: require(msg.sender == admin, "GovernorBravo::_initiate: admin only"); 129,9: require(initialProposalId == 0, "GovernorBravo::_initiate: can only initiate once"); 140,9: require(msg.sender == admin, "GovernorBravo:_setPendingAdmin: admin only"); 158,9: require(msg.sender == pendingAdmin, "GovernorBravo:_acceptAdmin: pending admin only"); 176,9: require(c >= a, "addition overflow"); 181,9: require(b <= a, "subtraction underflow");
File: lending-market-v2/contracts/Governance/GovernorBravoDelegator.sol 27,9: require(msg.sender == admin, "GovernorBravoDelegator::_setImplementation: admin only"); 28,9: require(implementation_ != address(0), "GovernorBravoDelegator::_setImplementation: invalid implementation address"); 40,9: require(msg.sender == admin, "GovernorBravoDelegator::_initiateDelegated: admin only"); 48,9: require(msg.sender == admin, "GovernorBravoDelegator::_acceptInitialAdminDelegated: admin only"); 56,9: require(msg.sender == admin, "GovernorBravoDelegator::_setPendingAdminDelegated: admin only");
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Stableswap/BaseV1-core.sol
File: lending-market-v2/contracts/Stableswap/BaseV1-core.sol 256,9: require(liquidity > 0, 'ILM'); // BaseV1: INSUFFICIENT_LIQUIDITY_MINTED 275,9: require(amount0 > 0 && amount1 > 0, 'ILB'); // BaseV1: INSUFFICIENT_LIQUIDITY_BURNED 289,9: require(amount0Out > 0 || amount1Out > 0, 'IOA'); // BaseV1: INSUFFICIENT_OUTPUT_AMOUNT 291,9: require(amount0Out < _reserve0 && amount1Out < _reserve1, 'IL'); // BaseV1: INSUFFICIENT_LIQUIDITY 297,9: require(to != _token0 && to != _token1, 'IT'); // BaseV1: INVALID_TO 306,9: require(amount0In > 0 || amount1In > 0, 'IIA'); // BaseV1: INSUFFICIENT_INPUT_AMOUNT 312,9: require(_k(_balance0, _balance1) >= _k(_reserve0, _reserve1), 'K'); // BaseV1: K 416,9: require(deadline >= block.timestamp, 'BaseV1: EXPIRED'); 434,9: require(recoveredAddress != address(0) && recoveredAddress == owner, 'BaseV1: INVALID_SIGNATURE'); 524,9: require(tokenA != tokenB, 'IA'); // BaseV1: IDENTICAL_ADDRESSES 526,9: require(token0 != address(0), 'ZA'); // BaseV1: ZERO_ADDRESS 527,9: require(getPair[token0][token1][stable] == address(0), 'PE'); // BaseV1: PAIR_EXISTS - single check is sufficient
File: lending-market-v2/contracts/Stableswap/BaseV1-periphery.sol 104,9: require(tokenA != tokenB, "BaseV1Router: IDENTICAL_ADDRESSES"); 106,9: require(token0 != address(0), "BaseV1Router: ZERO_ADDRESS"); 122,9: require(amountA > 0, "BaseV1Router: INSUFFICIENT_AMOUNT"); 123,9: require(reserveA > 0 && reserveB > 0, "BaseV1Router: INSUFFICIENT_LIQUIDITY"); 151,9: require(routes.length >= 1, "BaseV1Router: INVALID_PATH"); 241,17: require(amountBOptimal >= amountBMin, "BaseV1Router: INSUFFICIENT_B_AMOUNT"); 246,17: require(amountAOptimal >= amountAMin, "BaseV1Router: INSUFFICIENT_A_AMOUNT"); 309,9: require(IBaseV1Pair(pair).transferFrom(msg.sender, pair, liquidity)); // send liquidity to pair 313,9: require(amountA >= amountAMin, "BaseV1Router: INSUFFICIENT_A_AMOUNT"); 314,9: require(amountB >= amountBMin, "BaseV1Router: INSUFFICIENT_B_AMOUNT"); 405,9: require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT"); 420,9: require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT"); 433,9: require(routes[0].from == address(wcanto), "BaseV1Router: INVALID_PATH"); 435,9: require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT"); 446,9: require(routes[routes.length - 1].to == address(wcanto), "BaseV1Router: INVALID_PATH"); 448,9: require(amounts[amounts.length - 1] >= amountOutMin, "BaseV1Router: INSUFFICIENT_OUTPUT_AMOUNT"); 470,9: require(success, "TransferHelper: ETH_TRANSFER_FAILED"); 481,9: require(token.code.length > 0, "token code length failure");
File: lending-market-v2/contracts/Treasury/TreasuryDelegator.sol 31,9: require(msg.sender == admin, "GovernorBravoDelegator::setImplementation: admin only"); 32,9: require(implementation_ != address(0), "GovernorBravoDelegator::setImplementation: invalid implementation address"); 121,9: require(msg.value == 0, "TreasuryDelegator::fallback:cannot send value to fallback");
name and symbolif (_stable) { name = string(abi.encodePacked("StableV1 AMM - ", erc20(_token0).symbol(), "/", erc20(_token1).symbol())); symbol = string(abi.encodePacked("sAMM-", erc20(_token0).symbol(), "/", erc20(_token1).symbol())); } else { name = string(abi.encodePacked("VolatileV1 AMM - ", erc20(_token0).symbol(), "/", erc20(_token1).symbol())); symbol = string(abi.encodePacked("vAMM-", erc20(_token0).symbol(), "/", erc20(_token1).symbol())); }
This part can be written as follows:
string memory namePrefix; string memory symbolPrefix; if (_stable) { namePrefix = "StableV1 AMM - "; symbolPrefix = "sAMM-"; } else { namePrefix = "VolatileV1 AMM - "; symbolPrefix = "vAMM-"; } name = string(abi.encodePacked(namePrefix, erc20(_token0).symbol(), "/", erc20(_token1).symbol())); symbol = string(abi.encodePacked(symbolPrefix, erc20(_token0).symbol(), "/", erc20(_token1).symbol()));
block.number directly instead of calling getBlockNumber() functionCalling block.number directly instead of getBlockNumber() function can reduce the gas cost.
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/Comptroller.sol
File: lending-market-v2/contracts/Comptroller.sol 971,37: uint32 blockNumber = safe32(getBlockNumber(), "block number exceeds 32 bits"); 1197,37: uint32 blockNumber = safe32(getBlockNumber(), "block number exceeds 32 bits"); 1218,37: uint32 blockNumber = safe32(getBlockNumber(), "block number exceeds 32 bits"); 1314,28: uint blockNumber = getBlockNumber(); 1437,49: lastContributorBlock[contributor] = getBlockNumber();
Proposal.canceled seems not neededProposal.canceled seems not needed so it can be deleted to reduce the gas cost.
console import and usage of console.logIn the codebase, console.log is used but this should not be needed.
SafeMath at NoteInterest.solIn NoteInterest.sol, sub() function of SafeMath is only used. From solidity 8.0 version, underflow/overflow does not happen for subtraction - as a default.
https://github.com/Plex-Engineer/lending-market-v2/blob/main/contracts/NoteInterest.sol#L139
It can reduce the usage of SafeMath itself from NoteInterest.sol.
#0 - GalloDaSballo
2022-08-14T22:32:46Z
Report is very nice, however it missed the SLOADs and immutables that would have saved way more.
This will save around 500 gas in total