Platform: Code4rena
Start Date: 28/11/2022
Pot Size: $192,500 USDC
Total HM: 33
Participants: 106
Period: 11 days
Judge: LSDan
Total Solo HM: 15
Id: 186
League: ETH
Rank: 5/106
Findings: 6
Award: $6,691.89
š Selected for report: 5
š Solo Findings: 1
š Selected for report: IllIllI
Also found by: 0xNazgul, Atarpara, Awesome, Aymen0909, BClabs, Kong, ali_shehab, bullseye, chaduke, csanuragjain, datapunk, fatherOfBlocks, hansfriese, kaliberpoziomka8552, nicobevi, pashov, pzeus, shark, unforgiven, web3er, xiaoming90
58.4142 USDC - $58.41
Contrary to what the function comments say, removeFeeder()
is able to be called by anyone, not just the owner. By removing all feeders (i.e. floor twap price oracle keepers), a malicious user can cause all queries for the price of NFTs reliant on the NFTFloorOracle
(all NFTs except for the UniswapV3 ones), to revert, which will cause all calls to liquidateERC721()
to revert.
If NFTs can't be liquidated, positions will remain open for longer than they should, and the protocol may become insolvent by the time the issue is resolved.
The onlyRole(DEFAULT_ADMIN_ROLE)
should have been used instead of onlyWhenFeederExisted
...
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #1 165 /// @notice Allows owner to remove feeder. 166 /// @param _feeder feeder to remove 167 function removeFeeder(address _feeder) 168 external 169 onlyWhenFeederExisted(_feeder) 170 { 171 _removeFeeder(_feeder); 172: }
... since onlyWhenFeederExisted
is already on the internal call to _removeFeeder()
(onlyWhenFeederExisted
doesn't do any authentication of the caller):
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #2 326 function _removeFeeder(address _feeder) 327 internal 328 onlyWhenFeederExisted(_feeder) 329 { 330 uint8 feederIndex = feederPositionMap[_feeder].index; 331 if (feederIndex >= 0 && feeders[feederIndex] == _feeder) { 332 feeders[feederIndex] = feeders[feeders.length - 1]; 333 feeders.pop(); 334 } 335 delete feederPositionMap[_feeder]; 336 revokeRole(UPDATER_ROLE, _feeder); 337 emit FeederRemoved(_feeder); 338: }
Note that feeders
must have the UPDATER_ROLE
(revoked above) in order to update the price.
The fetching of the price will revert if the price is stale:
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #3 234 /// @param _asset The nft contract 235 /// @return price The most recent price on chain 236 function getPrice(address _asset) 237 external 238 view 239 override 240 returns (uint256 price) 241 { 242 uint256 updatedAt = assetPriceMap[_asset].updatedAt; 243 require( 244 @> (block.number - updatedAt) <= config.expirationPeriod, 245 "NFTOracle: asset price expired" 246 ); 247 return assetPriceMap[_asset].twap; 248: }
And it will become stale if there are no feeders for enough time:
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #4 195 function setPrice(address _asset, uint256 _twap) 196 public 197 @> onlyRole(UPDATER_ROLE) 198 onlyWhenAssetExisted(_asset) 199 whenNotPaused(_asset) 200 { 201 bool dataValidity = false; 202 if (hasRole(DEFAULT_ADMIN_ROLE, msg.sender)) { 203 @> _finalizePrice(_asset, _twap); 204 return; 205 } 206 dataValidity = _checkValidity(_asset, _twap); 207 require(dataValidity, "NFTOracle: invalid price data"); 208 // add price to raw feeder storage 209 _addRawValue(_asset, _twap); 210 uint256 medianPrice; 211 // set twap price only when median value is valid 212 (dataValidity, medianPrice) = _combine(_asset, _twap); 213 if (dataValidity) { 214 @> _finalizePrice(_asset, medianPrice); 215 } 216: }
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #5 376 function _finalizePrice(address _asset, uint256 _twap) internal { 377 PriceInformation storage assetPriceMapEntry = assetPriceMap[_asset]; 378 assetPriceMapEntry.twap = _twap; 379 @> assetPriceMapEntry.updatedAt = block.number; 380 assetPriceMapEntry.updatedTimestamp = block.timestamp; 381 emit AssetDataSet( 382 _asset, 383 assetPriceMapEntry.twap, 384 assetPriceMapEntry.updatedAt 385 ); 386: }
Note that the default staleness interval is six hours:
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #6 10 //expirationPeriod at least the interval of client to feed data(currently 6h=21600s/12=1800 in mainnet) 11 //we do not accept price lags behind to much 12: uint128 constant EXPIRATION_PERIOD = 1800;
The reverting getPrice()
function is called from the ERC721OracleWrapper
where it is not caught:
File: /paraspace-core/contracts/misc/ERC721OracleWrapper.sol #7 44 function setOracle(address _oracleAddress) 45 external 46 onlyAssetListingOrPoolAdmins 47 { 48 @> oracleAddress = INFTFloorOracle(_oracleAddress); 49 } 50 ... 54 55 function latestAnswer() external view override returns (int256) { 56 @> return int256(oracleAddress.getPrice(asset)); 57: }
And neither is it caught from any of the callers further up the chain (note that the fallback oracle can't be hit since the call reverts before that):
File: /paraspace-core/contracts/misc/ERC721OracleWrapper.sol #8 10: contract ERC721OracleWrapper is IEACAggregatorProxy {
File: /paraspace-core/contracts/misc/ParaSpaceOracle.sol #9 114 /// @inheritdoc IPriceOracleGetter 115 function getAssetPrice(address asset) 116 public 117 view 118 override 119 returns (uint256) 120 { 121 if (asset == BASE_CURRENCY) { 122 return BASE_CURRENCY_UNIT; 123 } 124 125 uint256 price = 0; 126 @> IEACAggregatorProxy source = IEACAggregatorProxy(assetsSources[asset]); 127 if (address(source) != address(0)) { 128 @> price = uint256(source.latestAnswer()); 129 } 130 if (price == 0 && address(_fallbackOracle) != address(0)) { 131 price = _fallbackOracle.getAssetPrice(asset); 132 } 133 134 require(price != 0, Errors.ORACLE_PRICE_NOT_READY); 135 return price; 136: }
File: /paraspace-core/contracts/protocol/libraries/logic/GenericLogic.sol #10 535 function _getAssetPrice(address oracle, address currentReserveAddress) 536 internal 537 view 538 returns (uint256) 539 { 540 @> return IPriceOracleGetter(oracle).getAssetPrice(currentReserveAddress); 541: }
File: /paraspace-core/contracts/protocol/libraries/logic/GenericLogic.sol : _getUserBalanceForERC721() #11 388 @> uint256 assetPrice = _getAssetPrice( 389 params.oracle, 390 vars.currentReserveAddress 391 ); 392 totalValue = 393 ICollateralizableERC721(vars.xTokenAddress) 394 .collateralizedBalanceOf(params.user) * 395 assetPrice; 396: }
File: /paraspace-core/contracts/protocol/libraries/logic/GenericLogic.sol : calculateUserAccountData() #12 214 vars 215 .userBalanceInBaseCurrency = _getUserBalanceForERC721( 216 params, 217 vars 218: );
File: /paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol #13 286 function executeLiquidateERC721( 287 mapping(address => DataTypes.ReserveData) storage reservesData, 288 mapping(uint256 => address) storage reservesList, 289 mapping(address => DataTypes.UserConfigurationMap) storage usersConfig, 290 DataTypes.ExecuteLiquidateParams memory params 291 ) external returns (uint256) { 292 ExecuteLiquidateLocalVars memory vars; ... 311 ( 312 vars.userGlobalCollateral, 313 , 314 vars.userGlobalDebt, //in base currency 315 , 316 , 317 , 318 , 319 , 320 vars.healthFactor, 321 322 @> ) = GenericLogic.calculateUserAccountData( 323 reservesData, 324 reservesList, 325 DataTypes.CalculateUserAccountDataParams({ 326 userConfig: userConfig, 327 reservesCount: params.reservesCount, 328 user: params.borrower, 329 oracle: params.priceOracle 330 }) 331: );
File: /paraspace-core/contracts/protocol/pool/PoolCore.sol #14 457 /// @inheritdoc IPoolCore 458 function liquidateERC721( 459 address collateralAsset, 460 address borrower, 461 uint256 collateralTokenId, 462 uint256 maxLiquidationAmount, 463 bool receiveNToken 464 ) external payable virtual override nonReentrant { 465 DataTypes.PoolStorage storage ps = poolStorage(); 466 467 @> LiquidationLogic.executeLiquidateERC721( 468 ps._reserves, 469 ps._reservesList, 470: ps._usersConfig,
A person close to liquidation can remove all feeders, giving themselves a free option on whether the extra time it takes for the admins to resolve the issue, is enough time for their position to go back into the green. Alternatively, a competitor can analyze what price most liquidations will occur at (based on on-chain data about every user's account health), and can time the removal of feeders for maximum effect. Note that even if the admins re-add the feeders, the malicious user can just remove them again.
Code inspection
Add the onlyRole(DEFAULT_ADMIN_ROLE)
modifier to removeFeeder()
#0 - c4-judge
2022-12-20T16:59:10Z
dmvt marked the issue as duplicate of #31
#1 - c4-judge
2023-01-09T14:38:34Z
dmvt marked the issue as selected for report
#2 - c4-judge
2023-01-23T15:59:59Z
dmvt marked the issue as satisfactory
#3 - C4-Staff
2023-02-01T19:10:42Z
captainmangoC4 marked the issue as selected for report
š Selected for report: IllIllI
Also found by: 0x52, 0xNazgul, Franfran, IllIllI, Jeiwan, Lambda, RaymondFam, Rolezn, Trust, __141345__, codecustard, erictee, gzeon, hansfriese, imare, rbserver, rvierdiiev, seyni, skinz, ujamal_
23.7984 USDC - $23.80
If the feeders haven't updated their prices within the default six-hour time window, the floor oracle will revert when requests are made to get the current price, and these reverts are not caught by the wrapper oracle which handles the fallback oracle, so rather than using the fallback oracle in these cases, the calls will revert, leading to liquidations to fail (see my other submission for the full chain from the floor oracle to the liquidation function).
Additionally, the code uses the deprecated chainlink latestAnswer()
API for its token requests, which may revert once it is no longer supported
getPrice()
will fail if the values are stale:
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #1 236 function getPrice(address _asset) 237 external 238 view 239 override 240 returns (uint256 price) 241 { 242 @> uint256 updatedAt = assetPriceMap[_asset].updatedAt; 243 require( 244 (block.number - updatedAt) <= config.expirationPeriod, 245 @> "NFTOracle: asset price expired" 246 ); 247 return assetPriceMap[_asset].twap; 248: }
They can be stale due to too much price skew, or the feeders being down, e.g. due to another bug:
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #2 369 // config maxPriceDeviation as multiple directly(not percent) for simplicity 370 if (priceDeviation >= config.maxPriceDeviation) { 371 return false; 372: }
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #3 376 function _finalizePrice(address _asset, uint256 _twap) internal { 377 PriceInformation storage assetPriceMapEntry = assetPriceMap[_asset]; 378 assetPriceMapEntry.twap = _twap; 379 @> assetPriceMapEntry.updatedAt = block.number; 380 assetPriceMapEntry.updatedTimestamp = block.timestamp; 381 emit AssetDataSet( 382 _asset, 383 assetPriceMapEntry.twap, 384 assetPriceMapEntry.updatedAt 385 ); 386: }
The wrapper oracle does not use a try-catch, so it can't swallow these reverts and allow the fallback oracle to be used:
File: /paraspace-core/contracts/misc/ParaSpaceOracle.sol #4 114 /// @inheritdoc IPriceOracleGetter 115 function getAssetPrice(address asset) 116 public 117 view 118 override 119 returns (uint256) 120 { 121 if (asset == BASE_CURRENCY) { 122 return BASE_CURRENCY_UNIT; 123 } 124 125 uint256 price = 0; 126 IEACAggregatorProxy source = IEACAggregatorProxy(assetsSources[asset]); 127 if (address(source) != address(0)) { 128 @> price = uint256(source.latestAnswer()); 129 } 130 if (price == 0 && address(_fallbackOracle) != address(0)) { 131 @> price = _fallbackOracle.getAssetPrice(asset); 132 } 133 134 require(price != 0, Errors.ORACLE_PRICE_NOT_READY); 135 return price; 136: }
Code inspection
Use a try-catch when fetching the normal latestAnswer()
, and use the fallback if it failed
#0 - c4-judge
2022-12-20T17:45:49Z
dmvt marked the issue as duplicate of #5
#1 - c4-judge
2022-12-20T17:48:27Z
dmvt marked the issue as selected for report
#2 - c4-judge
2023-01-23T15:52:22Z
dmvt marked the issue as satisfactory
#3 - C4-Staff
2023-02-01T19:10:51Z
captainmangoC4 marked the issue as selected for report
š Selected for report: IllIllI
Also found by: 0x52, 0xNazgul, Franfran, IllIllI, Jeiwan, Lambda, RaymondFam, Rolezn, Trust, __141345__, codecustard, erictee, gzeon, hansfriese, imare, rbserver, rvierdiiev, seyni, skinz, ujamal_
23.7984 USDC - $23.80
Judge has assessed an item in Issue #404 as M risk. The relevant finding follows:
[Lā04] latestAnswer() is deprecated 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 is 1 instance of this issue:
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol
128: price = uint256(source.latestAnswer()); https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/misc/ParaSpaceOracle.sol#L128
#0 - c4-judge
2023-01-26T12:38:13Z
dmvt marked the issue as duplicate of #5
#1 - c4-judge
2023-01-26T12:38:26Z
dmvt marked the issue as partial-50
š Selected for report: IllIllI
3523.4633 USDC - $3,523.46
From a judge's contest in a previous contest:
Because Front-running is a key aspect of AMM design, deadline is a useful tool to ensure that your tx cannot be āsaved for laterā. Due to the removal of the check, it may be more profitable for a miner to deny the transaction from being mined until the transaction incurs the maximum amount of slippage.
Most of the functions that interact with AMM pools do not have a deadline parameter, but specifically the one shown below is passing block.timestamp
to a pool, which means that whenever the miner decides to include the txn in a block, it will be valid at that time, since block.timestamp
will be the current timestamp.
File: /paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol #1 51 function _decreaseLiquidity( 52 address user, 53 uint256 tokenId, 54 uint128 liquidityDecrease, 55 uint256 amount0Min, 56 uint256 amount1Min, 57 bool receiveEthAsWeth 58 ) internal returns (uint256 amount0, uint256 amount1) { 59 if (liquidityDecrease > 0) { 60 // amount0Min and amount1Min are price slippage checks 61 // if the amount received after burning is not greater than these minimums, transaction will fail 62 INonfungiblePositionManager.DecreaseLiquidityParams 63 memory params = INonfungiblePositionManager 64 .DecreaseLiquidityParams({ 65 tokenId: tokenId, 66 liquidity: liquidityDecrease, 67 amount0Min: amount0Min, 68 amount1Min: amount1Min, 69 @> deadline: block.timestamp 70 }); 71 72 INonfungiblePositionManager(_underlyingAsset).decreaseLiquidity( 73 params 74 ); 75 } 76:
A malicious miner can hold the transaction, which may be being done in order to free up capital to ensure that there are funds available to do operations to prevent a liquidation. It is highly likely that a liquidation is more profitable for a miner to mine, with its associated follow-on transactions, than to allow the decrease of liquidity. A miner can also just hold it until maximum slippage is incurred, as the judge stated.
Code inspection
Add deadline arguments to all functions that interact with AMMs, and pass it along to AMM calls
#0 - c4-judge
2022-12-20T14:22:11Z
dmvt marked the issue as primary issue
#1 - c4-judge
2023-01-09T23:52:00Z
dmvt marked the issue as selected for report
#2 - trust1995
2023-01-26T18:19:12Z
From a practical point of view, this would require the collusion of a large amount of miners which is extraordinarily unlikely. Each miner is incentivized to maximize TX fees in a block, rather than plan a theoretical liquidation some long time in the future. decreaseLiquidity() properly sends slippage thresholds. For these reasons, I would view this as a useful suggestion rather than M-level (assets are at theoretical risk).
#3 - vladbochok
2023-01-27T08:00:21Z
@trust1995 I didn't even participate in the contest, so I don't have any incentives. However, I see it very unfair to judge it as "suggestion".
The described report logically fully satisfies the description of the Medium severity bug. From the medium severity bug description.
Assets not at direct risk, but the function of the protocol or its availability could be impacted, or leak value with a hypothetical attack path with stated assumptions, but external requirements.
Doesn't the report show hypothetical attack paths with external requirements?
#4 - trust1995
2023-01-27T08:16:42Z
I don't see this attack as different from general miner collusion, i.e. when deadline parameters are passed by user "properly". It is a property of Eth style blockchains. The additional incentive presented for miner collusion is theoretical. Also, the funds in the NTokenUniswap are counted as collateral, so this action doesnt save user from a liquidation in any reasonable way.
#5 - dmvt
2023-01-27T18:10:49Z
To invalidate this or mark it as a suggestion is the same as asserting that the deadline timestamp is an unnecessary part of an AMM design. The recent decision linked in the issue states exactly the opposite.
I agree that:
Because Front-running is a key aspect of AMM design, deadline is a useful tool to ensure that your tx cannot be āsaved for laterā.
I also agree with the medium risk rating given to the issue in the previous contest. Those these two issues do diverge slightly, the risk and impact are the same.
#6 - trust1995
2023-01-27T18:17:36Z
Contrarily, to mark the decision as M means issue has demonstrated a core dis-functioning, or a theoretical risk of funds with stated hypotheticals. Neither is evident from the issue.
A malicious miner can hold the transaction, which may be being done in order to free up capital to ensure that there are funds available to do operations to prevent a liquidation. It is highly likely that a liquidation is more profitable for a miner to mine, with its associated follow-on transactions, than to allow the decrease of liquidity. A miner can also just hold it until maximum slippage is incurred, as the judge stated.
There is no actual risk of funds here. The NToken holding counts as collateral.
I agree deadline is a useful tool for AMMs. I disagree that the way in which it is used here equates to a M-level impact.
#7 - dmvt
2023-01-27T18:39:27Z
I agree deadline is a useful tool for AMMs. I disagree that the way in which it is used here equates to a M-level impact.
Your opinion is noted, but in the absence of further information, this issue is valid and will remain medium risk.
š Selected for report: IllIllI
Also found by: 0x4non, 0x52, 0xAgro, 0xNazgul, 0xSmartContract, 0xackermann, 9svR6w, Awesome, Aymen0909, B2, BRONZEDISC, Bnke0x0, Deekshith99, Deivitto, Diana, Dravee, HE1M, Jeiwan, Kaiziron, KingNFT, Lambda, Mukund, PaludoX0, RaymondFam, Rolezn, Sathish9098, Secureverse, SmartSek, __141345__, ahmedov, ayeslick, brgltd, cccz, ch0bu, chrisdior4, cryptonue, cryptostellar5, csanuragjain, datapunk, delfin454000, erictee, gz627, gzeon, helios, i_got_hacked, ignacio, imare, jadezti, jayphbee, joestakey, kankodu, ksk2345, ladboy233, martin, nadin, nicobevi, oyc_109, pashov, pavankv, pedr02b2, pzeus, rbserver, ronnyx2017, rvierdiiev, shark, unforgiven, xiaoming90, yjrwkk
1147.3119 USDC - $1,147.31
Issue | Instances | |
---|---|---|
[Lā01] | Misleading variable naming/documentation | 1 |
[Lā02] | Wrong interest during leap years | 1 |
[Lā03] | Fallback oracle may break with future NFTs | 1 |
[Lā04] | latestAnswer() is deprecated | 1 |
[Lā05] | tokenURI() does not follow EIP-721 | 2 |
[Lā06] | Owner can renounce while system is paused | 1 |
[Lā07] | Empty receive() /payable fallback() function does not authenticate requests | 1 |
[Lā08] | abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256() | 1 |
[Lā09] | Use Ownable2Step rather than Ownable | 1 |
[Lā10] | Open TODOs | 3 |
[Lā11] | Centralization risks | 2 |
Total: 13 instances over 10 issues
Issue | Instances | |
---|---|---|
[Nā01] | EIP-1967 storage slots should use the eip1967.proxy prefix | 2 |
[Nā02] | Input addresse to _safeTransferETH() should be payable | 1 |
[Nā03] | Functions that must be overridden should be virtual, with no body | 2 |
[Nā04] | Inconsistent safe transfer library used | 1 |
[Nā05] | Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions | 1 |
[Nā06] | Import declarations should import specific identifiers, rather than the whole file | 20 |
[Nā07] | Missing initializer modifier on constructor | 1 |
[Nā08] | Duplicate import statements | 9 |
[Nā09] | The nonReentrant modifier should occur before all other modifiers | 25 |
[Nā10] | override function arguments that are unused should have the variable name removed or commented out to avoid compiler warnings | 2 |
[Nā11] | 2**<n> - 1 should be re-written as type(uint<n>).max | 3 |
[Nā12] | constant s should be defined rather than using magic numbers | 16 |
[Nā13] | Numeric values having to do with time should use time units for readability | 1 |
[Nā14] | Use bit shifts in an imutable variable rather than long bit masks of a single bit, for readability | 1 |
[Nā15] | Events that mark critical parameter changes should contain both the old and the new value | 3 |
[Nā16] | Use a more recent version of solidity | 4 |
[Nā17] | Use a more recent version of solidity | 16 |
[Nā18] | Expressions for constant values such as a call to keccak256() , should use immutable rather than constant | 1 |
[Nā19] | Use scientific notation (e.g. 1e18 ) rather than exponentiation (e.g. 10**18 ) | 1 |
[Nā20] | Inconsistent spacing in comments | 33 |
[Nā21] | Lines are too long | 2 |
[Nā22] | Variable names that consist of all capital letters should be reserved for constant /immutable variables | 3 |
[Nā23] | NatSpec is incomplete | 39 |
[Nā24] | Not using the named return variables anywhere in the function is confusing | 18 |
[Nā25] | Duplicated require() /revert() checks should be refactored to a modifier or function | 32 |
[Nā26] | Consider using delete rather than assigning zero to clear values | 2 |
[Nā27] | Contracts should have full test coverage | 1 |
[Nā28] | Large or complicated code bases should implement fuzzing tests | 1 |
[Nā29] | Typos | 3 |
Total: 244 instances over 29 issues
The recieveEthAsWeth
argument, if true, will do what the comment says: convert WETH to ETH, even though the variable name says the opposite. There is no way to know which one is right, without reading the code, which will lead to problems for callers of the external version of the function, decreaseUniswapV3Liquidity()
There is 1 instance of this issue:
File: /paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol 41 /** 42 * @notice A function that decreases the current liquidity. 43 * @param tokenId The id of the erc721 token 44 * @param liquidityDecrease The amount of liquidity to remove of LP 45 * @param amount0Min The minimum amount to remove of token0 46 * @param amount1Min The minimum amount to remove of token1 47 * @param receiveEthAsWeth If convert weth to ETH 48 * @return amount0 The amount received back in token0 49 * @return amount1 The amount returned back in token1 50 */ 51 function _decreaseLiquidity( 52 address user, 53 uint256 tokenId, 54 uint128 liquidityDecrease, 55 uint256 amount0Min, 56 uint256 amount1Min, 57 bool receiveEthAsWeth 58: ) internal returns (uint256 amount0, uint256 amount1) {
The calculateLinearInterest()
function uses SECONDS_PER_YEAR
, which is a constant, so the constant will have the wrong value during leap years. While the function is out of scope, it's eventually called by PoolCore:getNormalizedIncome()
, which is in scope.
There is 1 instance of this issue:
File: /paraspace-core/contracts/protocol/libraries/math/MathUtils.sol 23 function calculateLinearInterest(uint256 rate, uint40 lastUpdateTimestamp) 24 internal 25 view 26 returns (uint256) 27 { 28 //solium-disable-next-line 29 uint256 result = rate * 30 (block.timestamp - uint256(lastUpdateTimestamp)); 31 unchecked { 32 result = result / SECONDS_PER_YEAR; 33 } 34 35 return WadRayMath.RAY + result; 36: }
In order for the fallback oracle to fall back to the Bend DAO oracle, the NFT in question must say that it in fact supportsInterface(0x80ac58cd)
. The EIP-721 standard says that the implementer MUST do this, and both Openzeppelin's and Solmate's impelemntations do, but in the future the ParaSpace protocol may want to support a token that does not. I believe this deserves low rather than non-critical, because the damage won't be known until one of the other oracles fail. The fallback oracle is supposed to be the last resort before the protocol is unable to price/liquidate anything, so if the issue isn't caught before then, things could get stuck at the worst possible time. I would suggest to require()
that the NFT supports the NFT at the point where it's added, to avoid having to event think about this edge case in the future
There is 1 instance of this issue:
File: /paraspace-core/contracts/misc/ParaSpaceFallbackOracle.sol 35 try IERC165(asset).supportsInterface(INTERFACE_ID_ERC721) returns ( 36 bool supported 37 ) { 38 if (supported == true) { 39 return INFTOracle(BEND_DAO).getAssetPrice(asset); 40 } 41: } catch {}
latestAnswer()
is deprecatedUse 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 is 1 instance of this issue:
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol 128: price = uint256(source.latestAnswer());
tokenURI()
does not follow EIP-721The EIP states that tokenURI()
"Throws if _tokenId
is not a valid NFT", which the code below does not do. If the NFT has not yet been minted, tokenURI()
should revert
There are 2 instances of this issue:
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 178 function tokenURI(uint256) 179 external 180 view 181 virtual 182 override 183 returns (string memory) 184 { 185 return ""; 186: }
File: paraspace-core/contracts/protocol/tokenization/NToken.sol 313 function tokenURI(uint256 tokenId) 314 public 315 view 316 virtual 317 override 318 returns (string memory) 319 { 320 return IERC721Metadata(_underlyingAsset).tokenURI(tokenId); 321: }
The contract owner or single user with a role is not prevented from renouncing the role/ownership while the contract is paused, which would cause any user assets stored in the protocol, to be locked indefinitely
There is 1 instance of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 183 function setPause(address _asset, bool _flag) 184 external 185 onlyRole(DEFAULT_ADMIN_ROLE) 186 { 187 assetFeederMap[_asset].paused = _flag; 188 emit AssetPaused(_asset, _flag); 189: }
receive()
/payable fallback()
function does not authenticate requestsIf 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 is 1 instance of this issue:
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol 149: receive() external payable {}
abi.encodePacked()
should not be used with dynamic types when passing the result to a hash function such as keccak256()
Use abi.encode()
instead which will pad items to 32 bytes, which will prevent hash collisions (e.g. abi.encodePacked(0x123,0x456)
=> 0x123456
=> abi.encodePacked(0x1,0x23456)
, but abi.encode(0x123,0x456)
=> 0x0...1230...456
). "Unless there is a compelling reason, abi.encode
should be preferred". If there is only one argument to abi.encodePacked()
it can often be cast to bytes()
or bytes32()
instead.
If all arguments are strings and or bytes, bytes.concat()
should be used instead
There is 1 instance of this issue:
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol 1115 bytes32 typeHash = keccak256( 1116 abi.encodePacked( 1117 "Credit(address token,uint256 amount,bytes orderId)" 1118 ) 1119: );
Ownable2Step
rather than Ownable
Ownable2Step
and Ownable2StepUpgradeable
prevent the contract ownership from mistakenly being transferred to an address that cannot handle it (e.g. due to a typo in the address), by requiring that the recipient of the owner permissions actively accept via a contract call of its own.
There is 1 instance of this issue:
File: paraspace-core/contracts/ui/WPunkGateway.sol 19 contract WPunkGateway is 20 ReentrancyGuard, 21 IWPunkGateway, 22 IERC721Receiver, 23: OwnableUpgradeable
Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment
There are 3 instances of this issue:
File: paraspace-core/contracts/misc/marketplaces/LooksRareAdapter.sol 59: makerAsk.price, // TODO: take minPercentageToAsk into account
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol 238: // TODO using bit shifting for the 2^96
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 442: // TODO: support PToken
The admin can set whatever price they wish, causing anyone with NFT collateral to be liquidatable. The admin can also set a WETH address that just steals the funds. These operations should have more checks for market conditions before being allowed.
File: /paraspace-core/contracts/misc/NFTFloorOracle.sol #1 195 function setPrice(address _asset, uint256 _twap) 196 public 197 onlyRole(UPDATER_ROLE) 198 onlyWhenAssetExisted(_asset) 199 whenNotPaused(_asset) 200 { 201 bool dataValidity = false; 202 if (hasRole(DEFAULT_ADMIN_ROLE, msg.sender)) { 203 _finalizePrice(_asset, _twap); 204 return; 205: }
File: /paraspace-core/contracts/protocol/configuration/PoolAddressesProvider.sol #2 235 function setWETH(address newWETH) external override onlyOwner { 236 address oldWETH = _addresses[WETH]; 237 _addresses[WETH] = newWETH; 238 emit WETHUpdated(oldWETH, newWETH); 239: }
eip1967.proxy
prefixUsing the prefix makes it easier to confirm that you are following the standard, and not altering the behavior in some incompatable way
There are 2 instances of this issue:
File: /paraspace-core/contracts/protocol/pool/PoolStorage.sol /// @audit use `eip1967.proxy.paraspace.pool.storage` 17: bytes32(uint256(keccak256("paraspace.proxy.pool.storage")) - 1);
File: /paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol /// @audit use `eip1967.proxy.paraspace.ntoken.apestaking.storage` 25: uint256(keccak256("paraspace.proxy.ntoken.apestaking.storage")) - 1
_safeTransferETH()
should be payableWhile it doesn't affect any contract operation, it adds a degree of type safety during compilation, and is done by OpenZeppelin
(https://github.com/code-423n4/2022-11-paraspace/blob/c6820a279c64a299a783955749fdc977de8f0449/paraspace-core/contracts/dependencies/openzeppelin/contracts/Address.sol#L60-L65)
There is 1 instance of this issue:
File: /paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol 144: function _safeTransferETH(address to, uint256 value) internal {
There are 2 instances of this issue:
File: /paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol 126: // should be overridden
File: /paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 185: return "";
Most places in the code use GPv2SafeERC20
, but this one uses SafeERC20
. All areas should use the same libraries
There is 1 instance of this issue:
File: /paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 16: import {SafeERC20} from "../../../dependencies/openzeppelin/contracts/SafeERC20.sol";
__gap[50]
storage variable to allow for new storage variables in later versionsSee this link for a description of this storage variable. While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.
There is 1 instance of this issue:
File: paraspace-core/contracts/ui/WPunkGateway.sol 19 contract WPunkGateway is 20 ReentrancyGuard, 21 IWPunkGateway, 22 IERC721Receiver, 23: OwnableUpgradeable
Using import declarations of the form import {<identifier_name>} from "some/file.sol"
avoids polluting the symbol namespace making flattened files smaller, and speeds up compilation
There are 20 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 4: import "../dependencies/openzeppelin/contracts/AccessControl.sol"; 5: import "../dependencies/openzeppelin/upgradeability/Initializable.sol"; 6: import "./interfaces/INFTFloorOracle.sol";
File: paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol 10: import "../../../interfaces/INTokenApeStaking.sol";
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol 30: import "../../../interfaces/INTokenApeStaking.sol";
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol 4: import "../libraries/paraspace-upgradeability/ParaReentrancyGuard.sol"; 5: import "../libraries/paraspace-upgradeability/ParaVersionedInitializable.sol"; 7: import "../../interfaces/IPoolApeStaking.sol"; 8: import "../../interfaces/IPToken.sol"; 9: import "../../dependencies/yoga-labs/ApeCoinStaking.sol"; 10: import "../../interfaces/IXTokenType.sol"; 11: import "../../interfaces/INTokenApeStaking.sol"; 20: import "../libraries/logic/BorrowLogic.sol"; 21: import "../libraries/logic/SupplyLogic.sol";
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol 7: import "../../../interfaces/IPool.sol"; 11: import "./MintableERC721Logic.sol";
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol 7: import "../../../interfaces/IRewardController.sol"; 8: import "../../libraries/types/DataTypes.sol"; 9: import "../../../interfaces/IPool.sol";
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol 12: import "../../interfaces/INTokenApeStaking.sol";
initializer
modifier on constructorOpenZeppelin recommends that the initializer
modifier be applied to constructors in order to avoid potential griefs, social engineering, or exploits. Ensure that the modifier is applied to the implementation contract. If the default constructor is currently being used, it should be changed to be an explicit one with the modifier applied.
There is 1 instance of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 55: contract NFTFloorOracle is Initializable, AccessControl, INFTFloorOracle {
There are 9 instances of this issue:
File: paraspace-core/contracts/misc/marketplaces/LooksRareAdapter.sol 11: import {AdvancedOrder, CriteriaResolver, Fulfillment, OfferItem, ItemType} from "../../dependencies/seaport/contracts/lib/ConsiderationStructs.sol";
File: paraspace-core/contracts/misc/marketplaces/SeaportAdapter.sol 11: import {AdvancedOrder, CriteriaResolver, Fulfillment, OfferItem, ItemType} from "../../dependencies/seaport/contracts/lib/ConsiderationStructs.sol";
File: paraspace-core/contracts/misc/marketplaces/X2Y2Adapter.sol 12: import {AdvancedOrder, CriteriaResolver, Fulfillment, OfferItem, ItemType} from "../../dependencies/seaport/contracts/lib/ConsiderationStructs.sol";
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 18: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol"; 21: import {AdvancedOrder, CriteriaResolver, Fulfillment} from "../../../dependencies/seaport/contracts/lib/ConsiderationStructs.sol";
File: paraspace-core/contracts/protocol/pool/PoolCore.sol 25: import {Errors} from "../libraries/helpers/Errors.sol";
File: paraspace-core/contracts/protocol/pool/PoolMarketplace.sol 28: import {Errors} from "../libraries/helpers/Errors.sol";
File: paraspace-core/contracts/protocol/pool/PoolParameters.sol 24: import {Errors} from "../libraries/helpers/Errors.sol";
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol 12: import {IMoonBird} from "../../dependencies/erc721-collections/IMoonBird.sol";
nonReentrant
modifier
should occur before all other modifiersThis is a best-practice to protect against reentrancy in other modifiers
There are 25 instances of this issue:
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 462: ) external virtual override onlyPool nonReentrant returns (bool) { 483: nonReentrant 534: nonReentrant 545: nonReentrant
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol 106: ) external virtual override onlyPool nonReentrant returns (uint64, uint64) { 162: nonReentrant
File: paraspace-core/contracts/protocol/tokenization/NTokenBAYC.sol 29: nonReentrant 42: nonReentrant 55: ) external onlyPool nonReentrant { 69: nonReentrant 85: ) external onlyPool nonReentrant { 100: ) external onlyPool nonReentrant {
File: paraspace-core/contracts/protocol/tokenization/NTokenMAYC.sol 29: nonReentrant 42: nonReentrant 55: ) external onlyPool nonReentrant { 69: nonReentrant 85: ) external onlyPool nonReentrant { 100: ) external onlyPool nonReentrant {
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol 44: ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
File: paraspace-core/contracts/protocol/tokenization/NToken.sol 82: ) external virtual override onlyPool nonReentrant returns (uint64, uint64) { 91: ) external virtual override onlyPool nonReentrant returns (uint64, uint64) { 123: ) external onlyPool nonReentrant { 204: nonReentrant 219: nonReentrant
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol 130: ) external onlyPool nonReentrant {
override
function arguments that are unused should have the variable name removed or commented out to avoid compiler warningsThere are 2 instances of this issue:
File: paraspace-core/contracts/protocol/tokenization/NToken.sol 214: function handleRepayment(address user, uint256 amount)
2**<n> - 1
should be re-written as type(uint<n>).max
Earlier versions of solidity can use uint<n>(-1)
instead. Expressions not including the - 1
can often be re-written to accomodate the change (e.g. by using a >
rather than a >=
, which will also save some gas)
There are 3 instances of this issue:
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol 247: ) * 2**96) / 1E9 259: ) * 2**96) / 1E9 271: ) * 2**96) /
constant
s should be defined rather than using magic numbersEven assembly can benefit from using readable constants instead of hex/numeric literals
There are 16 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit 100 366: ? (_twap * 100) / _priorTwap /// @audit 100 367: : (_priorTwap * 100) / _twap;
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol /// @audit 18 245: ((oracleData.token0Price * (10**18)) / /// @audit 96 /// @audit 1E9 247: ) * 2**96) / 1E9 /// @audit 96 /// @audit 1E9 259: ) * 2**96) / 1E9 /// @audit 96 271: ) * 2**96) / /// @audit 9 273: (9 +
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol /// @audit 0x0 143: ) == address(0x0), /// @audit 0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f 1137: 0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f, // keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)") /// @audit 0x88d989289235fb06c18e3c2f7ea914f41f773e86fb0073d632539f566f4df353 1138: 0x88d989289235fb06c18e3c2f7ea914f41f773e86fb0073d632539f566f4df353, // keccak256("ParaSpace") /// @audit 0x722c0e0c80487266e8c6a45e3a1a803aab23378a9c32e6ebe029d4fad7bfc965 1139: 0x722c0e0c80487266e8c6a45e3a1a803aab23378a9c32e6ebe029d4fad7bfc965, // keccak256(bytes("1.1")),
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol /// @audit 30 133: ApeStakingLogic.executeSetUnstakeApeIncentive(dataStorage, 30);
File: paraspace-core/contracts/protocol/tokenization/NToken.sol /// @audit 4 176: require(airdropParams.length >= 4, Errors.INVALID_AIRDROP_PARAMETERS);
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol /// @audit 30 34: _ERC721Data.balanceLimit = 30;
There are units for seconds, minutes, hours, days, and weeks, and since they're defined, they should be used
There is 1 instance of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit 1800 12: uint128 constant EXPIRATION_PERIOD = 1800;
There is 1 instance of this issue:
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol 21: uint256 internal constant Q128 = 0x100000000000000000000000000000000;
This should especially be done if the new value is not required to be different from the old value
There are 3 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit setPause() 188: emit AssetPaused(_asset, _flag);
File: paraspace-core/contracts/protocol/configuration/PoolAddressesProvider.sol /// @audit updatePoolImpl() 112: emit PoolUpdated(implementationParams, _init, _calldata); /// @audit setMarketplace() 255: emit MarketplaceUpdated(id, marketplace, adapter, operator, paused);
Use a solidity version of at least 0.8.12 to get string.concat()
to be used instead of abi.encodePacked(<str>,<str>)
There are 4 instances of this issue:
File: paraspace-core/contracts/misc/marketplaces/LooksRareAdapter.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/misc/marketplaces/SeaportAdapter.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/misc/marketplaces/X2Y2Adapter.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol 2: pragma solidity 0.8.10;
Use a solidity version of at least 0.8.13 to get the ability to use using for
with a list of free functions
There are 16 instances of this issue:
File: paraspace-core/contracts/protocol/libraries/logic/BorrowLogic.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/libraries/logic/GenericLogic.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/libraries/logic/SupplyLogic.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/pool/DefaultReserveAuctionStrategy.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/pool/PoolCore.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/pool/PoolMarketplace.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/pool/PoolParameters.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/tokenization/NToken.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol 2: pragma solidity 0.8.10;
File: paraspace-core/contracts/ui/WPunkGateway.sol 2: pragma solidity 0.8.10;
keccak256()
, should use immutable
rather than constant
While it doesn't save any gas because the compiler knows that developers often make this mistake, it's still best to use the right tool for the task at hand. There is a difference between constant
variables and immutable
variables, and they should each be used in their appropriate contexts. constants
should be used for literal values written into the code, and immutable
variables should be used for expressions, or values calculated in, or passed into the constructor.
There is 1 instance of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 70: bytes32 public constant UPDATER_ROLE = keccak256("UPDATER_ROLE");
1e18
) rather than exponentiation (e.g. 10**18
)While the compiler knows to optimize away the exponentiation, it's still better coding practice to use idioms that do not require compiler optimization, if they exist
There is 1 instance of this issue:
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol 245: ((oracleData.token0Price * (10**18)) /
Some lines use // x
and some use //x
. The instances below point out the usages that don't follow the majority, within each file
There are 33 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 105: //still need to grant update_role to admin for emergency call 361: //first price is always valid 404: //first time just use the feeding value 408: //use memory here so allocate with maximum length 412: //aggeregate with price from all feeders
File: paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol 100: //userCollateral from collateralReserve 102: //userGlobalCollateral from all reserves 104: //userDebt from liquadationReserve 106: //userGlobalDebt from all reserves 108: //actualLiquidationAmount to repay based on collateral 110: //actualCollateral allowed to liquidate 112: //liquidationBonusRate from reserve config 114: //user health factor 116: //liquidation protocol fee to be sent to treasury 118: //liquidation funds payer 120: //collateral P|N Token 122: //auction strategy 124: //liquidation asset reserve id 126: //whether auction is enabled 128: //liquidation reserve cache 314: vars.userGlobalDebt, //in base currency
File: paraspace-core/contracts/protocol/libraries/logic/SupplyLogic.sol 34: // See `IPool` for descriptions
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol 353: //add the current already borrowed amount to the amount requested to calculate the total collateral needed. 355: vars.amountInBaseCurrency).percentDiv(vars.currentLtv); //LTV is calculated in percentage 573: //if collateral isn't enabled as collateral by user, it cannot be liquidated 685: //if collateral isn't enabled as collateral by user, it cannot be liquidated 794: //if collateral isn't enabled as collateral by user, it cannot be auctioned
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol 155: //only partially withdraw need user's BAKC 171: ////transfer BAKC back for user 214: //transfer BAKC back for user 304: //transfer BAKC back for user 337: //7 checkout ape balance
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 258: //solhint-disable-next-line max-line-length
Usually lines in source code are limited to 80 characters. Today's screens are much larger so it's reasonable to stretch this in some cases. Since the files will most likely reside in GitHub, and GitHub starts using a scroll bar in all cases when the length is over 164 characters, the lines below should be split when they reach that length
There are 2 instances of this issue:
File: paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol 603: * @return The actual debt that is getting liquidated. If liquidation amount passed in by the liquidator is greater then the total user debt, then use the user total debt as the actual debt getting liquidated. If the user total debt is greater than the liquidation amount getting passed in by the liquidator, then use the liquidation amount the user is passing in.
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol 1137: 0x8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f, // keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)")
constant
/immutable
variablesIf the variable needs to be different based on which class it comes from, a view
/pure
function should be used instead (e.g. like this).
There are 3 instances of this issue:
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol 78: IPool POOL;
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol 426: IPool POOL, 404: bool ATOMIC_PRICING,
There are 39 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit Missing: '@param _twaps' 218 /// @notice Allows owner to set new price on PriceInformation and updates the 219 /// internal Median cumulativePrice. 220 /// @param _assets The nft contract to set a floor price for 221 function setMultiplePrices( 222 address[] calldata _assets, 223 uint256[] calldata _twaps 224: ) external onlyRole(UPDATER_ROLE) {
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol /// @audit Missing: '@return' 111 * @param positionData The specified position data 112 */ 113 function getLiquidityAmountFromPositionData( 114 UinswapV3PositionData memory positionData 115: ) public pure returns (uint256 token0Amount, uint256 token1Amount) { /// @audit Missing: '@return' 142 * @param positionData The specified position data 143 */ 144 function getLpFeeAmountFromPositionData( 145 UinswapV3PositionData memory positionData 146: ) public view returns (uint256 token0Amount, uint256 token1Amount) {
File: paraspace-core/contracts/protocol/libraries/logic/GenericLogic.sol /// @audit Missing: '@param params' /// @audit Missing: '@param vars' 356 /** 357 * @notice Calculates total xToken balance of the user in the based currency used by the price oracle 358 * @dev For gas reasons, the xToken balance is calculated by fetching `scaledBalancesOf` normalized debt, which 359 * is cheaper than fetching `balanceOf` 360 * @return totalValue The total xToken balance of the user normalized to the base currency of the price oracle 361 **/ 362 function _getUserBalanceForERC721( 363 DataTypes.CalculateUserAccountDataParams memory params, 364 CalculateUserAccountDataVars memory vars 365: ) private view returns (uint256 totalValue) { /// @audit Missing: '@param reserve' /// @audit Missing: '@param xTokenAddress' /// @audit Missing: '@param assetPrice' 504 /** 505 * @notice Calculates total xToken balance of the user in the based currency used by the price oracle 506 * @dev For gas reasons, the xToken balance is calculated by fetching `scaledBalancesOf` normalized debt, which 507 * is cheaper than fetching `balanceOf` 508 * @param user The address of the user 509 * @param assetUnit The value representing one full unit of the asset (10^decimals) 510 * @return The total xToken balance of the user normalized to the base currency of the price oracle 511 **/ 512 function _getUserBalanceForERC20( 513 address user, 514 DataTypes.ReserveData storage reserve, 515 address xTokenAddress, 516 uint256 assetUnit, 517 uint256 assetPrice 518: ) private view returns (uint256) {
File: paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol /// @audit Missing: '@param reservesData' 558 /** 559 * @notice Supply new collateral for taking out of borrower's another collateral 560 * @param userConfig The user configuration that track the supplied/borrowed assets 561 * @param params The additional parameters needed to execute the liquidation function 562 * @param vars the executeLiquidateERC20() function local vars 563 */ 564 function _supplyNewCollateral( 565 mapping(address => DataTypes.ReserveData) storage reservesData, 566 DataTypes.UserConfigurationMap storage userConfig, 567 DataTypes.ExecuteLiquidateParams memory params, 568: ExecuteLiquidateLocalVars memory vars
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol /// @audit Missing: '@return' 112 * @param params The additional parameters needed to execute the buyWithCredit function 113 */ 114 function _buyWithCredit( 115 mapping(address => DataTypes.ReserveData) storage reservesData, 116 mapping(uint256 => address) storage reservesList, 117 DataTypes.UserConfigurationMap storage userConfig, 118 DataTypes.ExecuteMarketplaceParams memory params 119: ) internal returns (uint256) { /// @audit Missing: '@return' 374 * @param vars The marketplace local vars for caching storage values for future reads 375 */ 376 function _delegateToPool( 377 DataTypes.ExecuteMarketplaceParams memory params, 378 MarketplaceLocalVars memory vars 379: ) internal returns (uint256, uint256) {
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol /// @audit Missing: '@param user' /// @audit Missing: '@param ps' /// @audit Missing: '@param oracle' 155 /** 156 * @notice Returns the user account data across all the reserves 157 * @return totalCollateralBase The total collateral of the user in the base currency used by the price feed 158 * @return totalDebtBase The total debt of the user in the base currency used by the price feed 159 * @return availableBorrowsBase The borrowing power left of the user in the base currency used by the price feed 160 * @return currentLiquidationThreshold The liquidation threshold of the user 161 * @return ltv The loan to value of The user 162 * @return healthFactor The current health factor of the user 163 * @return erc721HealthFactor The current erc721 health factor of the user 164 **/ 165 function executeGetUserAccountData( 166 address user, 167 DataTypes.PoolStorage storage ps, 168 address oracle 169 ) 170 external 171 view 172 returns ( 173 uint256 totalCollateralBase, 174 uint256 totalDebtBase, 175 uint256 availableBorrowsBase, 176 uint256 currentLiquidationThreshold, 177 uint256 ltv, 178 uint256 healthFactor, 179: uint256 erc721HealthFactor
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol /// @audit Missing: '@param assetType' 58 /** 59 * @notice Validates a supply action. 60 * @param reserveCache The cached data of the reserve 61 * @param amount The amount to be supplied 62 */ 63 function validateSupply( 64 DataTypes.ReserveCache memory reserveCache, 65 uint256 amount, 66: DataTypes.AssetType assetType /// @audit Missing: '@param reservesData' 584 /** 585 * @notice Validates the liquidation action. 586 * @param userConfig The user configuration mapping 587 * @param collateralReserve The reserve data of the collateral 588 * @param params Additional parameters needed for the validation 589 */ 590 function validateLiquidateERC721( 591 mapping(address => DataTypes.ReserveData) storage reservesData, 592 DataTypes.UserConfigurationMap storage userConfig, 593 DataTypes.ReserveData storage collateralReserve, 594: DataTypes.ValidateLiquidateERC721Params memory params /// @audit Missing: '@return' 700 * @param oracle The price oracle 701 */ 702 function validateHealthFactor( 703 mapping(address => DataTypes.ReserveData) storage reservesData, 704 mapping(uint256 => address) storage reservesList, 705 DataTypes.UserConfigurationMap memory userConfig, 706 address user, 707 uint256 reservesCount, 708 address oracle 709: ) internal view returns (uint256, bool) { /// @audit Missing: '@param reservesData' /// @audit Missing: '@param asset' /// @audit Missing: '@param tokenId' 940 /** 941 * @notice Validates a transfer action. 942 * @param reserve The reserve object 943 */ 944 function validateTransferERC721( 945 mapping(address => DataTypes.ReserveData) storage reservesData, 946 DataTypes.ReserveData storage reserve, 947 address asset, 948: uint256 tokenId
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol /// @audit Missing: '@param atomic_pricing' 77 /** 78 * @dev Constructor. 79 * @param pool The reference to the main Pool contract 80 * @param name_ The name of the token 81 * @param symbol_ The symbol of the token 82 */ 83 constructor( 84 IPool pool, 85 string memory name_, 86 string memory symbol_, 87: bool atomic_pricing
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol /// @audit Missing: '@return' 202 * @param _apeCoinStaking ApeCoinStaking contract address 203 */ 204 function getUserTotalStakingAmount( 205 mapping(address => UserState) storage userState, 206 mapping(address => mapping(uint256 => uint256)) storage ownedTokens, 207 address user, 208 uint256 poolId, 209 ApeCoinStaking _apeCoinStaking 210: ) external view returns (uint256) { /// @audit Missing: '@return' 229 * @param tokenId specified the tokenId for the position 230 */ 231 function getTokenIdStakingAmount( 232 uint256 poolId, 233 ApeCoinStaking _apeCoinStaking, 234 uint256 tokenId 235: ) public view returns (uint256) {
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol /// @audit Missing: '@param erc721Data' /// @audit Missing: '@param length' 478 /** 479 * @dev Private function to add a token to this extension's ownership-tracking data structures. 480 * @param to address representing the new owner of the given token ID 481 * @param tokenId uint256 ID of the token to be added to the tokens list of the given address 482 */ 483 function _addTokenToOwnerEnumeration( 484 MintableERC721Data storage erc721Data, 485 address to, 486 uint256 tokenId, 487: uint256 length /// @audit Missing: '@param erc721Data' /// @audit Missing: '@param length' 493 /** 494 * @dev Private function to add a token to this extension's token tracking data structures. 495 * @param tokenId uint256 ID of the token to be added to the tokens list 496 */ 497 function _addTokenToAllTokensEnumeration( 498 MintableERC721Data storage erc721Data, 499 uint256 tokenId, 500: uint256 length /// @audit Missing: '@param erc721Data' /// @audit Missing: '@param userBalance' 506 /** 507 * @dev Private function to remove a token from this extension's ownership-tracking data structures. Note that 508 * while the token is not assigned a new owner, the `_ownedTokensIndex` mapping is _not_ updated: this allows for 509 * gas optimizations e.g. when performing a transfer operation (avoiding double writes). 510 * This has O(1) time complexity, but alters the order of the _ownedTokens array. 511 * @param from address representing the previous owner of the given token ID 512 * @param tokenId uint256 ID of the token to be removed from the tokens list of the given address 513 */ 514 function _removeTokenFromOwnerEnumeration( 515 MintableERC721Data storage erc721Data, 516 address from, 517 uint256 tokenId, 518: uint256 userBalance /// @audit Missing: '@param erc721Data' /// @audit Missing: '@param length' 539 /** 540 * @dev Private function to remove a token from this extension's token tracking data structures. 541 * This has O(1) time complexity, but alters the order of the _allTokens array. 542 * @param tokenId uint256 ID of the token to be removed from the tokens list 543 */ 544 function _removeTokenFromAllTokensEnumeration( 545 MintableERC721Data storage erc721Data, 546 uint256 tokenId, 547: uint256 length
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol /// @audit Missing: '@param apeCoinStaking' 28 /** 29 * @dev Constructor. 30 * @param pool The address of the Pool contract 31 */ 32: constructor(IPool pool, address apeCoinStaking) NToken(pool, false) { /// @audit Missing: '@return' 180 * @param user user address 181 */ 182 function getUserApeStakingAmount(address user) 183 external 184 view 185: returns (uint256)
File: paraspace-core/contracts/protocol/tokenization/NTokenBAYC.sol /// @audit Missing: '@param _apeRecipient' 92 /** 93 * @notice Withdraw staked ApeCoin from the Pair pool. If withdraw is total staked amount, performs an automatic claim. 94 * @param _nftPairs Array of Paired BAYC NFT's with staked amounts 95 * @dev if pairs have split ownership and BAKC is attempting a withdraw, the withdraw must be for the total staked amount 96 */ 97 function withdrawBAKC( 98 ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, 99 address _apeRecipient 100: ) external onlyPool nonReentrant {
File: paraspace-core/contracts/protocol/tokenization/NTokenMAYC.sol /// @audit Missing: '@param _apeRecipient' 92 /** 93 * @notice Withdraw staked ApeCoin from the Pair pool. If withdraw is total staked amount, performs an automatic claim. 94 * @param _nftPairs Array of Paired MAYC NFT's with staked amounts 95 * @dev if pairs have split ownership and BAKC is attempting a withdraw, the withdraw must be for the total staked amount 96 */ 97 function withdrawBAKC( 98 ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, 99 address _apeRecipient 100: ) external onlyPool nonReentrant {
File: paraspace-core/contracts/protocol/tokenization/NToken.sol /// @audit Missing: '@param atomic_pricing' 39 /** 40 * @dev Constructor. 41 * @param pool The address of the Pool contract 42 */ 43 constructor(IPool pool, bool atomic_pricing) 44 MintableIncentivizedERC721( 45 pool, 46 "NTOKEN_IMPL", 47 "NTOKEN_IMPL", 48 atomic_pricing 49: )
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol /// @audit Missing: '@param user' 41 /** 42 * @notice A function that decreases the current liquidity. 43 * @param tokenId The id of the erc721 token 44 * @param liquidityDecrease The amount of liquidity to remove of LP 45 * @param amount0Min The minimum amount to remove of token0 46 * @param amount1Min The minimum amount to remove of token1 47 * @param receiveEthAsWeth If convert weth to ETH 48 * @return amount0 The amount received back in token0 49 * @return amount1 The amount returned back in token1 50 */ 51 function _decreaseLiquidity( 52 address user, 53 uint256 tokenId, 54 uint128 liquidityDecrease, 55 uint256 amount0Min, 56 uint256 amount1Min, 57 bool receiveEthAsWeth 58: ) internal returns (uint256 amount0, uint256 amount1) {
File: paraspace-core/contracts/ui/WPunkGateway.sol /// @audit Missing: '@param punkIndexes' 119 /** 120 * @notice Implements the acceptBidWithCredit feature. AcceptBidWithCredit allows users to 121 * accept a leveraged bid on ParaSpace NFT marketplace. Users can submit leveraged bid and pay 122 * at most (1 - LTV) * $NFT 123 * @dev The nft receiver just needs to do the downpayment 124 * @param marketplaceId The marketplace identifier 125 * @param payload The encoded parameters to be passed to marketplace contract (selector eliminated) 126 * @param credit The credit that user would like to use for this purchase 127 * @param referralCode The referral code used 128 */ 129 function acceptBidWithCredit( 130 bytes32 marketplaceId, 131 bytes calldata payload, 132 DataTypes.Credit calldata credit, 133 uint256[] calldata punkIndexes, 134 uint16 referralCode 135: ) external nonReentrant { /// @audit Missing: '@param punkIndexes' 157 /** 158 * @notice Implements the batchAcceptBidWithCredit feature. AcceptBidWithCredit allows users to 159 * accept a leveraged bid on ParaSpace NFT marketplace. Users can submit leveraged bid and pay 160 * at most (1 - LTV) * $NFT 161 * @dev The nft receiver just needs to do the downpayment 162 * @param marketplaceIds The marketplace identifiers 163 * @param payloads The encoded parameters to be passed to marketplace contract (selector eliminated) 164 * @param credits The credits that the makers have approved to use for this purchase 165 * @param referralCode The referral code used 166 */ 167 function batchAcceptBidWithCredit( 168 bytes32[] calldata marketplaceIds, 169 bytes[] calldata payloads, 170 DataTypes.Credit[] calldata credits, 171 uint256[] calldata punkIndexes, 172 uint16 referralCode 173: ) external nonReentrant {
Consider changing the variable to be an unnamed one
There are 18 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit price 236 function getPrice(address _asset) 237 external 238 view 239 override 240: returns (uint256 price) /// @audit timestamp 252 function getLastUpdateTime(address _asset) 253 external 254 view 255 override 256: returns (uint256 timestamp)
File: paraspace-core/contracts/protocol/pool/PoolParameters.sol /// @audit totalCollateralBase /// @audit totalDebtBase /// @audit availableBorrowsBase /// @audit currentLiquidationThreshold /// @audit ltv /// @audit healthFactor /// @audit erc721HealthFactor 226 function getUserAccountData(address user) 227 external 228 view 229 virtual 230 override 231 returns ( 232 uint256 totalCollateralBase, 233 uint256 totalDebtBase, 234 uint256 availableBorrowsBase, 235 uint256 currentLiquidationThreshold, 236 uint256 ltv, 237 uint256 healthFactor, 238: uint256 erc721HealthFactor /// @audit ltv /// @audit lt 251 function getAssetLtvAndLT(address asset, uint256 tokenId) 252 external 253 view 254 virtual 255 override 256: returns (uint256 ltv, uint256 lt)
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol /// @audit oldCollateralizedBalance /// @audit newCollateralizedBalance 364 function _mintMultiple( 365 address to, 366 DataTypes.ERC721SupplyParams[] calldata tokenData 367 ) 368 internal 369 virtual 370 returns ( 371 uint64 oldCollateralizedBalance, 372: uint64 newCollateralizedBalance /// @audit oldCollateralizedBalance /// @audit newCollateralizedBalance 384 function _burnMultiple(address user, uint256[] calldata tokenIds) 385 internal 386 virtual 387 returns ( 388 uint64 oldCollateralizedBalance, 389: uint64 newCollateralizedBalance
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol /// @audit nesting /// @audit current /// @audit total 111 function nestingPeriod(uint256 tokenId) 112 external 113 view 114 returns ( 115 bool nesting, 116 uint256 current, 117: uint256 total
require()
/revert()
checks should be refactored to a modifier or functionThe compiler will inline the function, which will avoid JUMP
instructions usually associated with functions
There are 32 instances of this issue:
File: paraspace-core/contracts/misc/marketplaces/LooksRareAdapter.sol 102: revert(Errors.CALL_MARKETPLACE_FAILED);
File: paraspace-core/contracts/misc/marketplaces/SeaportAdapter.sol 84: require(orderInfo.offer.length > 0, Errors.INVALID_MARKETPLACE_ORDER); 87 require( 88 orderInfo.consideration.length > 0, 89 Errors.INVALID_MARKETPLACE_ORDER 90: );
File: paraspace-core/contracts/misc/marketplaces/X2Y2Adapter.sol 110: revert(Errors.CALL_MARKETPLACE_FAILED);
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol 169: revert(Errors.ORACLE_PRICE_NOT_READY);
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 279 require( 280 marketplaceIds.length == payloads.length && 281 payloads.length == credits.length, 282 Errors.INCONSISTENT_PARAMS_LENGTH 283: ); 294 require( 295 vars.orderInfo.taker == onBehalfOf, 296 Errors.INVALID_ORDER_TAKER 297: );
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol 160: require(amount != 0, Errors.INVALID_AMOUNT); 168 require( 169 xToken.getXTokenType() != XTokenType.PTokenSApe, 170 Errors.SAPE_NOT_ALLOWED 171: ); 185: require(isActive, Errors.RESERVE_INACTIVE); 186: require(!isPaused, Errors.RESERVE_PAUSED); 434 require( 435 reserveAssetType == DataTypes.AssetType.ERC20, 436 Errors.INVALID_ASSET_TYPE 437: ); 456 require( 457 reserveAssetType == DataTypes.AssetType.ERC721, 458 Errors.INVALID_ASSET_TYPE 459: ); 1059 require( 1060 assetType == DataTypes.AssetType.ERC20, 1061 Errors.INVALID_ASSET_TYPE 1062: ); 636 require( 637 vars.collateralReserveActive && vars.principalReserveActive, 638 Errors.RESERVE_INACTIVE 639: ); 640 require( 641 !vars.collateralReservePaused && !vars.principalReservePaused, 642 Errors.RESERVE_PAUSED 643: ); 645 require( 646 params.priceOracleSentinel == address(0) || 647 params.healthFactor < 648 MINIMUM_HEALTH_FACTOR_LIQUIDATION_THRESHOLD || 649 IPriceOracleSentinel(params.priceOracleSentinel) 650 .isLiquidationAllowed(), 651 Errors.PRICE_ORACLE_SENTINEL_CHECK_FAILED 652: ); 686 require( 687 vars.isCollateralEnabled, 688 Errors.COLLATERAL_CANNOT_BE_AUCTIONED_OR_LIQUIDATED 689: ); 757 require( 758 vars.collateralReserveAssetType == DataTypes.AssetType.ERC721, 759 Errors.INVALID_ASSET_TYPE 760: ); 826 require( 827 IAuctionableERC721(params.xTokenAddress).isAuctioned( 828 params.tokenId 829 ), 830 Errors.AUCTION_NOT_STARTED 831: ); 819 require( 820 IERC721(params.xTokenAddress).ownerOf(params.tokenId) == 821 params.user, 822 Errors.NOT_THE_OWNER 823: ); 824: require(vars.collateralReserveActive, Errors.RESERVE_INACTIVE); 825: require(!vars.collateralReservePaused, Errors.RESERVE_PAUSED); 950: require(!reserve.configuration.getPaused(), Errors.RESERVE_PAUSED); 1074: require(!params.marketplace.paused, Errors.MARKETPLACE_PAUSED);
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol 112 require( 113 getUserHf(msg.sender) > 114 DataTypes.HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 115 Errors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD 116: ); 200 require( 201 INToken(xTokenAddress).ownerOf(_nftPairs[index].mainTokenId) == 202 msg.sender, 203 Errors.NOT_THE_OWNER 204: );
File: paraspace-core/contracts/protocol/pool/PoolCore.sol 726 require( 727 msg.sender == ps._reserves[asset].xTokenAddress, 728 Errors.CALLER_NOT_XTOKEN 729: );
File: paraspace-core/contracts/protocol/pool/PoolParameters.sol 172: require(asset != address(0), Errors.ZERO_ADDRESS_NOT_VALID); 173 require( 174 ps._reserves[asset].id != 0 || ps._reservesList[0] == asset, 175 Errors.ASSET_NOT_LISTED 176: );
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 296 require( 297 _isApprovedOrOwner(_msgSender(), tokenId), 298 "ERC721: transfer caller is not owner nor approved" 299: );
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol 167 require( 168 !isAuctioned(erc721Data, POOL, tokenId), 169 Errors.TOKEN_IN_AUCTION 170: );
delete
rather than assigning zero to clear valuesThe delete
keyword more closely matches the semantics of what is being done, and draws more attention to the changing of state, which may lead to a more thorough audit of its associated logic
There are 2 instances of this issue:
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 589: vars.ethLeft = 0;
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol 123: reserve.accruedToTreasury = 0;
While 100% code coverage does not guarantee that there are no bugs, it often will catch easy-to-find bugs, and will ensure that there are fewer regressions when the code invariably has to be modified. Furthermore, in order to get full coverage, code authors will often have to re-organize their code so that it is more modular, so that each component can be tested separately, which reduces interdependencies between modules and layers, and makes for code that is easier to reason about and audit.
There is 1 instance of this issue:
- What is the overall line coverage percentage provided by your tests?: 85
Large code bases, or code with lots of inline-assembly, complicated math, or complicated interactions between multiple contracts, should implement fuzzing tests. Fuzzers such as Echidna require the test writer to come up with invariants which should not be violated under any circumstances, and the fuzzer tests various inputs and function calls to ensure that the invariants always hold. Even code with 100% code coverage can still have bugs due to the order of the operations a user performs, and fuzzers, with properly and extensively-written invariants, can close this testing gap significantly.
There is 1 instance of this issue:
- How many contracts are in scope?: 32 - Total SLoC for these contracts?: 8408 - How many external imports are there?: 12 - How many separate interfaces and struct definitions are there for the contracts within scope?: 44 interfaces, 35 struct definitions - Does most of your code generally use composition or inheritance?: Yes - How many external calls?: 306
There are 3 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit aggeregate 412: //aggeregate with price from all feeders
File: paraspace-core/contracts/protocol/libraries/logic/AuctionLogic.sol /// @audit tsatr 34: * @notice Function to tsatr auction on an ERC721 of a position if its ERC721 Health Factor drops below 1.
File: paraspace-core/contracts/ui/WPunkGateway.sol /// @audit computated 213: * due selfdestructs or transfer punk to pre-computated contract address before deployment.
These findings are excluded from awards calculations because there are publicly-available automated tools that find them. The valid ones appear here for completeness
Issue | Instances | |
---|---|---|
[Lā01] | safeApprove() is deprecated | 1 |
[Lā02] | Missing checks for address(0x0) when assigning values to address state variables | 4 |
Total: 5 instances over 2 issues
Issue | Instances | |
---|---|---|
[Nā01] | Return values of approve() not checked | 2 |
[Nā02] | public functions not called by the contract should be declared external instead | 3 |
[Nā03] | constant s should be defined rather than using magic numbers | 2 |
[Nā04] | Event is missing indexed fields | 8 |
Total: 15 instances over 4 issues
safeApprove()
is deprecatedDeprecated in favor of safeIncreaseAllowance()
and safeDecreaseAllowance()
. If only setting the initial allowance to the value that means infinite, safeIncreaseAllowance()
can be used instead. The function may currently work, but if a bug is found in this version of OpenZeppelin, and the version that you're forced to upgrade to no longer has this function, you'll encounter unnecessary delays in porting and testing replacement contracts.
There is 1 instance of this issue:
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol /// @audit (valid but excluded finding) 555: IERC20(token).safeApprove(operator, type(uint256).max);
address(0x0)
when assigning values to address
state variablesThere are 4 instances of this issue:
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol /// @audit (valid but excluded finding) 58: BASE_CURRENCY = baseCurrency;
File: paraspace-core/contracts/ui/WPunkGateway.sol /// @audit (valid but excluded finding) 48: punk = _punk; /// @audit (valid but excluded finding) 49: wpunk = _wpunk; /// @audit (valid but excluded finding) 50: pool = _pool;
approve()
not checkedNot all IERC20
implementations revert()
when there's a failure in approve()
. The function signature has a boolean
return value and they indicate errors that way instead. By not checking the return value, operations that should have marked as failed, may potentially go through without actually approving anything
There are 2 instances of this issue:
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol /// @audit (valid but excluded finding) 45: _apeCoin.approve(address(_apeCoinStaking), type(uint256).max); /// @audit (valid but excluded finding) 46: _apeCoin.approve(address(POOL), type(uint256).max);
public
functions not called by the contract should be declared external
insteadContracts are allowed to override their parents' functions and change the visibility from external
to public
.
There are 3 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit (valid but excluded finding) 97 function initialize( 98 address _admin, 99 address[] memory _feeders, 100 address[] memory _assets 101: ) public initializer { /// @audit (valid but excluded finding) 261: function getFeederSize() public view returns (uint256) {
File: paraspace-core/contracts/protocol/libraries/logic/BorrowLogic.sol /// @audit (valid but excluded finding) 52 function executeBorrow( 53 mapping(address => DataTypes.ReserveData) storage reservesData, 54 mapping(uint256 => address) storage reservesList, 55 DataTypes.UserConfigurationMap storage userConfig, 56: DataTypes.ExecuteBorrowParams memory params
constant
s should be defined rather than using magic numbersEven assembly can benefit from using readable constants instead of hex/numeric literals
There are 2 instances of this issue:
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol /// @audit 18 - (valid but excluded finding) 255: (18 + /// @audit 18 - (valid but excluded finding) 267: (18 +
indexed
fieldsIndex event fields make the field more quickly accessible to off-chain tools that parse events. However, note that each index field costs extra gas during emission, so it's not necessarily best to index the maximum allowed per event (three fields). Each event
should use three indexed
fields if there are three or more fields, and gas usage is not particularly of concern for the events in question. If there are fewer than three fields, all of the fields should be indexed.
There are 8 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit (valid but excluded finding) 58: event AssetPaused(address indexed asset, bool paused); /// @audit (valid but excluded finding) 63: event OracleConfigSet(uint128 expirationPeriod, uint128 maxPriceDeviation); /// @audit (valid but excluded finding) 64 event AssetDataSet( 65 address indexed asset, 66 uint256 twap, 67 uint256 lastUpdatedBlock 68: );
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol /// @audit (valid but excluded finding) 38 event BuyWithCredit( 39 bytes32 indexed marketplaceId, 40 DataTypes.OrderInfo orderInfo, 41 DataTypes.Credit credit 42: ); /// @audit (valid but excluded finding) 44 event AcceptBidWithCredit( 45 bytes32 indexed marketplaceId, 46 DataTypes.OrderInfo orderInfo, 47 DataTypes.Credit credit 48: );
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol /// @audit (valid but excluded finding) 30: event MintedToTreasury(address indexed reserve, uint256 amountMinted);
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol /// @audit (valid but excluded finding) 29: event UnstakeApeIncentiveUpdated(uint256 oldValue, uint256 newValue);
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol /// @audit (valid but excluded finding) 74 event ApprovalForAll( 75 address indexed owner, 76 address indexed operator, 77 bool approved 78: );
#0 - c4-judge
2023-01-25T16:36:20Z
dmvt marked the issue as grade-a
#1 - c4-judge
2023-01-26T12:23:57Z
dmvt marked the issue as selected for report
#2 - dmvt
2023-01-26T14:32:24Z
N-23 should be low risk. L-04, L-06 & L-11 have been upgraded. Everything else is good.
š Selected for report: IllIllI
Also found by: B2, Deivitto, Dravee, PaludoX0, RaymondFam, Rolezn, Sathish9098, _Adam, ahmedov, c3phas, chrisdior4, cyberinn, rjs, saneryee
1933.2753 USDC - $1,933.28
Issue | Instances | Total Gas Saved | |
---|---|---|---|
[Gā01] | Save gas by checking against default WETH address | 1 | 2200 |
[Gā02] | Save gas by batching NToken operations | 2 | 200 |
[Gā03] | Using storage instead of memory for structs/arrays saves gas | 5 | 21000 |
[Gā04] | Multiple accesses of a mapping/array should use a local variable cache | 4 | 168 |
[Gā05] | The result of function calls should be cached rather than re-calling the function | 2 | - |
[Gā06] | internal functions only called once can be inlined to save gas | 15 | 300 |
[Gā07] | Add unchecked {} for subtractions where the operands cannot underflow because of a previous require() or if -statement | 1 | 85 |
[Gā08] | ++i /i++ should be unchecked{++i} /unchecked{i++} when it is not possible for them to overflow, as is the case when used in for - and while -loops | 49 | 2940 |
[Gā09] | require() /revert() strings longer than 32 bytes cost extra gas | 14 | - |
[Gā10] | Optimize names to save gas | 23 | 506 |
[Gā11] | ++i costs less gas than i++ , especially when it's used in for -loops (--i /i-- too) | 1 | 5 |
[Gā12] | Splitting require() statements that use && saves gas | 13 | 39 |
[Gā13] | Usage of uints /ints smaller than 32 bytes (256 bits) incurs overhead | 5 | - |
[Gā14] | Using private rather than public for constants, saves gas | 1 | - |
[Gā15] | Inverting the condition of an if -else -statement wastes gas | 2 | - |
[Gā16] | require() or revert() statements that check input arguments should be at the top of the function | 1 | - |
[Gā17] | Use custom errors rather than revert() /require() strings to save gas | 202 | - |
[Gā18] | Functions guaranteed to revert when called by normal users can be marked payable | 66 | 1386 |
[Gā19] | Don't use _msgSender() if not supporting EIP-2771 | 7 | 112 |
Total: 414 instances over 19 issues with 28941 gas saved
Gas totals use lower bounds of ranges and count two iterations of each for
-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.
You can save a Gcoldsload (2100 gas) in the address provider, plus the 100 gas overhead of the external call, for every receive()
, by creating an immutable DEFAULT_WETH
variable which will store the initial WETH address, and change the require statement to be: require(msg.ender == DEFAULT_WETH || msg.sender == <etc>)
.
There is 1 instance of this issue:
File: /paraspace-core/contracts/protocol/pool/PoolCore.sol 802 receive() external payable { 803 require( 804 msg.sender == 805 address(IPoolAddressesProvider(ADDRESSES_PROVIDER).getWETH()), 806 "Receive not allowed" 807 ); 808: }
NToken
operationsEvery external call made to a contract incurs at least 100 gas of overhead. Since all of the IDs belong to the same NToken, you can prevent this overhead by having a safeTransferFromBatch()
function, or by implementing EIP-1155 support, which natively supports batching.
There are 2 instances of this issue:
File: /paraspace-core/contracts/ui/WPunkGateway.sol 102 function withdrawPunk(uint256[] calldata punkIndexes, address to) 103 external 104 nonReentrant 105 { 106 INToken nWPunk = INToken( 107 Pool.getReserveData(address(WPunk)).xTokenAddress 108 ); 109 for (uint256 i = 0; i < punkIndexes.length; i++) { 110 nWPunk.safeTransferFrom(msg.sender, address(this), punkIndexes[i]); 111: }
File: /paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol 28 function executeFlashClaim( 29 DataTypes.PoolStorage storage ps, 30 DataTypes.ExecuteFlashClaimParams memory params 31 ) external { 32 DataTypes.ReserveData storage reserve = ps._reserves[params.nftAsset]; 33 address nTokenAddress = reserve.xTokenAddress; 34 ValidationLogic.validateFlashClaim(ps, params); 35 36 uint256 i; 37 // step 1: moving underlying asset forward to receiver contract 38 for (i = 0; i < params.nftTokenIds.length; i++) { 39 INToken(nTokenAddress).transferUnderlyingTo( 40 params.receiverAddress, 41 params.nftTokenIds[i] 42 ); 43: }
storage
instead of memory
for structs/arrays saves gasWhen 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 5 instances of this issue:
File: paraspace-core/contracts/misc/marketplaces/X2Y2Adapter.sol 40: IX2Y2.SettleDetail memory detail = runInput.details[0]; 41: IX2Y2.Order memory order = runInput.orders[detail.orderIdx]; 42: IX2Y2.OrderItem memory item = order.items[detail.itemIdx];
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 414 PriceInformation memory priceInfo = feederRegistrar.feederPrice[ 415 feeders[i] 416: ];
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol 109 DataTypes.ReserveConfigurationMap 110: memory reserveConfiguration = reserve.configuration;
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 4 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit assetPriceMap[_asset] on line 242 247: return assetPriceMap[_asset].twap; /// @audit assetFeederMap[_asset] on line 282 284: assetFeederMap[_asset].index = uint8(assets.length - 1); /// @audit feederPositionMap[_feeder] on line 312 313: feederPositionMap[_feeder].registered = true; /// @audit assetPriceMap[_asset] on line 405 426: return (false, assetPriceMap[_asset].twap);
The instances below point to the second+ call of the function within a single function
There are 2 instances of this issue:
File: paraspace-core/contracts/protocol/pool/PoolCore.sol /// @audit ADDRESSES_PROVIDER.getWETH() on line 475 477: liquidationAsset: ADDRESSES_PROVIDER.getWETH(),
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol /// @audit _apeCoinStaking.apeCoin() on line 53 55: _apeCoinStaking.apeCoin().safeTransfer(_apeRecipient, balance);
internal
functions only called once can be inlined to save gasNot inlining costs 20 to 40 gas because of two extra JUMP
instructions and additional stack operations needed for function calls.
There are 15 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 265: function _whenNotPaused(address _asset) internal view { 278 function _addAsset(address _asset) 279 internal 280: onlyWhenAssetNotExisted(_asset) 296 function _removeAsset(address _asset) 297 internal 298: onlyWhenAssetExisted(_asset) 307 function _addFeeder(address _feeder) 308 internal 309: onlyWhenFeederNotExisted(_feeder) 326 function _removeFeeder(address _feeder) 327 internal 328: onlyWhenFeederExisted(_feeder) 351 function _checkValidity(address _asset, uint256 _twap) 352 internal 353 view 354: returns (bool) 388: function _addRawValue(address _asset, uint256 _twap) internal { 397 function _combine(address _asset, uint256 _twap) 398 internal 399 view 400: returns (bool, uint256)
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol 221 function _getOracleData(UinswapV3PositionData memory positionData) 222 internal 223 view 224: returns (PairOracleData memory) 282 function _getPendingFeeAmounts(UinswapV3PositionData memory positionData) 283 internal 284 view 285: returns (uint256 token0Amount, uint256 token1Amount)
File: paraspace-core/contracts/protocol/configuration/PoolAddressesProvider.sol 302 function _updateParaProxyImpl( 303 bytes32 id, 304 IParaProxy.ProxyImplementation[] calldata implementationParams, 305 address _init, 306: bytes calldata _calldata
File: paraspace-core/contracts/protocol/pool/DefaultReserveAuctionStrategy.sol 101 function _calculateAuctionPriceMultiplierByTicks(uint256 ticks) 102 internal 103 view 104: returns (uint256)
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol 413: function setSApeUseAsCollateral(address user) internal {
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol 51 function _decreaseLiquidity( 52 address user, 53 uint256 tokenId, 54 uint128 liquidityDecrease, 55 uint256 amount0Min, 56 uint256 amount1Min, 57 bool receiveEthAsWeth 58: ) internal returns (uint256 amount0, uint256 amount1) { 144: function _safeTransferETH(address to, uint256 value) internal {
unchecked {}
for subtractions where the operands cannot underflow because of a previous require()
or if
-statementrequire(a <= b); x = b - a
=> require(a <= b); unchecked { x = b - a }
There is 1 instance of this issue:
File: paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol /// @audit if-condition on line 874 877: msg.value - vars.actualLiquidationAmount
++i
/i++
should be unchecked{++i}
/unchecked{i++}
when it is not possible for them to overflow, as is the case when used in for
- and while
-loopsThe unchecked
keyword is new in solidity version 0.8.0, so this only applies to that version or higher, which these instances are. This saves 30-40 gas per loop
There are 49 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 229: for (uint256 i = 0; i < _assets.length; i++) { 291: for (uint256 i = 0; i < _assets.length; i++) { 321: for (uint256 i = 0; i < _feeders.length; i++) { 413: for (uint256 i = 0; i < feederSize; i++) {
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol 95: for (uint256 i = 0; i < assets.length; i++) { 197: for (uint256 i = 0; i < assets.length; i++) {
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol 193: for (uint256 index = 0; index < tokenIds.length; index++) { 210: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol 38: for (i = 0; i < params.nftTokenIds.length; i++) { 56: for (i = 0; i < params.nftTokenIds.length; i++) {
File: paraspace-core/contracts/protocol/libraries/logic/GenericLogic.sol 371: for (uint256 index = 0; index < totalBalance; index++) { 466: for (uint256 index = 0; index < totalBalance; index++) {
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 177: for (uint256 i = 0; i < marketplaceIds.length; i++) { 284: for (uint256 i = 0; i < marketplaceIds.length; i++) { 382: for (uint256 i = 0; i < params.orderInfo.consideration.length; i++) { 470: for (uint256 i = 0; i < params.orderInfo.offer.length; i++) {
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol 58: for (uint16 i = 0; i < params.reservesCount; i++) { 104: for (uint256 i = 0; i < assets.length; i++) {
File: paraspace-core/contracts/protocol/libraries/logic/SupplyLogic.sol 184: for (uint256 index = 0; index < params.tokenData.length; index++) { 195: for (uint256 index = 0; index < params.tokenData.length; index++) {
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol 131: for (uint256 index = 0; index < amount; index++) { 212: for (uint256 index = 0; index < tokenIds.length; index++) { 465: for (uint256 index = 0; index < tokenIds.length; index++) { 910: for (uint256 index = 0; index < tokenIds.length; index++) { 1034: for (uint256 i = 0; i < params.nftTokenIds.length; i++) {
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol 72: for (uint256 index = 0; index < _nfts.length; index++) { 103: for (uint256 index = 0; index < _nfts.length; index++) { 138: for (uint256 index = 0; index < _nftPairs.length; index++) { 172: for (uint256 index = 0; index < actualTransferAmount; index++) { 199: for (uint256 index = 0; index < _nftPairs.length; index++) { 215: for (uint256 index = 0; index < _nftPairs.length; index++) { 279: for (uint256 index = 0; index < _nfts.length; index++) { 289: for (uint256 index = 0; index < _nftPairs.length; index++) { 305: for (uint256 index = 0; index < _nftPairs.length; index++) {
File: paraspace-core/contracts/protocol/pool/PoolCore.sol 634: for (uint256 i = 0; i < reservesListCount; i++) {
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 493: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol 213: for (uint256 index = 0; index < totalBalance; index++) {
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol 207: for (uint256 index = 0; index < tokenData.length; index++) { 280: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol 107: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol 51: for (uint256 index = 0; index < tokenIds.length; index++) { 97: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/NToken.sol 106: for (uint256 index = 0; index < tokenIds.length; index++) { 145: for (uint256 i = 0; i < ids.length; i++) {
File: paraspace-core/contracts/ui/WPunkGateway.sol 82: for (uint256 i = 0; i < punkIndexes.length; i++) { 109: for (uint256 i = 0; i < punkIndexes.length; i++) { 113: for (uint256 i = 0; i < punkIndexes.length; i++) { 136: for (uint256 i = 0; i < punkIndexes.length; i++) { 174: for (uint256 i = 0; i < punkIndexes.length; i++) {
require()
/revert()
strings longer than 32 bytes cost extra gasEach extra memory word of bytes past the original 32 incurs an MSTORE which costs 3 gas
There are 14 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 225 require( 226 _assets.length == _twaps.length, 227 "NFTOracle: Tokens and price length differ" 228: ); 356: require(_twap > 0, "NFTOracle: price should be more than 0");
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 195 require( 196 _msgSender() == owner || isApprovedForAll(owner, _msgSender()), 197 "ERC721: approve caller is not owner nor approved for all" 198: ); 213 require( 214 _exists(tokenId), 215 "ERC721: approved query for nonexistent token" 216: ); 259 require( 260 _isApprovedOrOwner(_msgSender(), tokenId), 261 "ERC721: transfer caller is not owner nor approved" 262: ); 296 require( 297 _isApprovedOrOwner(_msgSender(), tokenId), 298 "ERC721: transfer caller is not owner nor approved" 299: ); 354 require( 355 _exists(tokenId), 356 "ERC721: operator query for nonexistent token" 357: ); 594 require( 595 index < balanceOf(owner), 596 "ERC721Enumerable: owner index out of bounds" 597: ); 618 require( 619 index < totalSupply(), 620 "ERC721Enumerable: global index out of bounds" 621: );
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol 88 require( 89 erc721Data.owners[tokenId] == from, 90 "ERC721: transfer from incorrect owner" 91: ); 92: require(to != address(0), "ERC721: transfer to the zero address"); 377 require( 378 _exists(erc721Data, tokenId), 379 "ERC721: startAuction for nonexistent token" 380: ); 395 require( 396 _exists(erc721Data, tokenId), 397 "ERC721: endAuction for nonexistent token" 398: );
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol 98 require( 99 _isApprovedOrOwner(_msgSender(), tokenIds[index]), 100 "ERC721: transfer caller is not owner nor approved" 101: );
public
/external
function names and public
member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted
There are 23 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit initialize(), addAssets(), removeAsset(), addFeeders(), removeFeeder(), setConfig(), setPause(), setPrice(), setMultiplePrices(), getFeederSize() 55: contract NFTFloorOracle is Initializable, AccessControl, INFTFloorOracle {
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol /// @audit getFallbackOracle() 21: contract ParaSpaceOracle is IParaSpaceOracle {
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol /// @audit getOnchainPositionData(), getLiquidityAmount(), getLiquidityAmountFromPositionData(), getLpFeeAmount(), getLpFeeAmountFromPositionData(), getTokenPrice(), getTokensPrices(), getTokensPricesSum() 17: contract UniswapV3OracleWrapper is IUniswapV3OracleWrapper {
File: paraspace-core/contracts/protocol/libraries/logic/AuctionLogic.sol /// @audit executeStartAuction(), executeEndAuction() 15: library AuctionLogic {
File: paraspace-core/contracts/protocol/libraries/logic/BorrowLogic.sol /// @audit executeBorrow(), executeRepay() 21: library BorrowLogic {
File: paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol /// @audit executeFlashClaim() 14: library FlashClaimLogic {
File: paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol /// @audit executeLiquidateERC20(), executeLiquidateERC721() 36: library LiquidationLogic {
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol /// @audit executeBuyWithCredit(), executeBatchBuyWithCredit(), executeAcceptBidWithCredit(), executeBatchAcceptBidWithCredit() 33: library MarketplaceLogic {
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol /// @audit executeInitReserve(), executeRescueTokens(), executeMintToTreasury(), executeDropReserve(), executeGetUserAccountData(), executeGetAssetLtvAndLT() 23: library PoolLogic {
File: paraspace-core/contracts/protocol/libraries/logic/SupplyLogic.sol /// @audit executeSupply(), executeSupplyERC721(), executeSupplyERC721FromNToken(), executeWithdraw(), executeWithdrawERC721(), executeDecreaseUniswapV3Liquidity(), executeFinalizeTransferERC20(), executeFinalizeTransferERC721(), executeUseERC20AsCollateral(), executeCollateralizeERC721(), executeUncollateralizeERC721() 28: library SupplyLogic {
File: paraspace-core/contracts/protocol/pool/DefaultReserveAuctionStrategy.sol /// @audit getMaxPriceMultiplier(), getMinExpPriceMultiplier(), getMinPriceMultiplier(), getStepLinear(), getStepExp(), getTickLength() 20: contract DefaultReserveAuctionStrategy is IReserveAuctionStrategy {
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol /// @audit withdrawApeCoin(), claimApeCoin(), withdrawBAKC(), claimBAKC(), borrowApeAndStake(), unstakeApePositionAndRepay(), repayAndSupply() 23: contract PoolApeStaking is
File: paraspace-core/contracts/protocol/pool/PoolCore.sol /// @audit initialize(), getReserveAddressById() 45: contract PoolCore is
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol /// @audit setIncentivesController(), setBalanceLimit() 29: abstract contract MintableIncentivizedERC721 is
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol /// @audit withdrawBAKC(), executeSetUnstakeApeIncentive(), executeUnstakePositionAndRepay(), getUserTotalStakingAmount(), getTokenIdStakingAmount() 18: library ApeStakingLogic {
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol /// @audit executeTransfer(), executeTransferCollateralizable(), executeMintMultiple(), executeBurnMultiple(), executeApprove(), executeApprovalForAll(), executeStartAuction(), executeEndAuction(), isAuctioned() 52: library MintableERC721Logic {
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol /// @audit getBAKC(), getApeStaking(), setUnstakeApeIncentive(), unstakePositionAndRepay(), getUserApeStakingAmount() 20: abstract contract NTokenApeStaking is NToken, INTokenApeStaking {
File: paraspace-core/contracts/protocol/tokenization/NTokenBAYC.sol /// @audit depositApeCoin(), claimApeCoin(), withdrawApeCoin(), depositBAKC(), claimBAKC(), withdrawBAKC() 15: contract NTokenBAYC is NTokenApeStaking {
File: paraspace-core/contracts/protocol/tokenization/NTokenMAYC.sol /// @audit depositApeCoin(), claimApeCoin(), withdrawApeCoin(), depositBAKC(), claimBAKC(), withdrawBAKC() 15: contract NTokenMAYC is NTokenApeStaking {
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol /// @audit toggleNesting(), nestingPeriod(), nestingOpen() 27: contract NTokenMoonBirds is NToken, IMoonBirdBase {
File: paraspace-core/contracts/protocol/tokenization/NToken.sol /// @audit getAtomicPricingConfig() 29: contract NToken is VersionedInitializable, MintableIncentivizedERC721, INToken {
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol /// @audit decreaseUniswapV3Liquidity() 26: contract NTokenUniswapV3 is NToken, INTokenUniswapV3 {
File: paraspace-core/contracts/ui/WPunkGateway.sol /// @audit initialize(), supplyPunk(), withdrawPunk(), acceptBidWithCredit(), batchAcceptBidWithCredit(), emergencyERC721TokenTransfer(), emergencyPunkTransfer(), getWPunkAddress() 19: contract WPunkGateway is
++i
costs less gas than i++
, especially when it's used in for
-loops (--i
/i--
too)Saves 5 gas per loop
There is 1 instance of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 450: j--;
require()
statements that use &&
saves gasSee 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 by 3 gas
There are 13 instances of this issue:
File: paraspace-core/contracts/misc/marketplaces/SeaportAdapter.sol 41 require( 42 // NOT criteria based and must be basic order 43 resolvers.length == 0 && isBasicOrder(advancedOrder), 44 Errors.INVALID_MARKETPLACE_ORDER 45: ); 71 require( 72 // NOT criteria based and must be basic order 73 advancedOrders.length == 2 && 74 isBasicOrder(advancedOrders[0]) && 75 isBasicOrder(advancedOrders[1]), 76 Errors.INVALID_MARKETPLACE_ORDER 77: );
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 171 require( 172 marketplaceIds.length == payloads.length && 173 payloads.length == credits.length, 174 Errors.INCONSISTENT_PARAMS_LENGTH 175: ); 279 require( 280 marketplaceIds.length == payloads.length && 281 payloads.length == credits.length, 282 Errors.INCONSISTENT_PARAMS_LENGTH 283: );
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol 546 require( 547 vars.collateralReserveActive && vars.principalReserveActive, 548 Errors.RESERVE_INACTIVE 549: ); 550 require( 551 !vars.collateralReservePaused && !vars.principalReservePaused, 552 Errors.RESERVE_PAUSED 553: ); 636 require( 637 vars.collateralReserveActive && vars.principalReserveActive, 638 Errors.RESERVE_INACTIVE 639: ); 640 require( 641 !vars.collateralReservePaused && !vars.principalReservePaused, 642 Errors.RESERVE_PAUSED 643: ); 672 require( 673 params.maxLiquidationAmount >= params.actualLiquidationAmount && 674 (msg.value == 0 || msg.value >= params.maxLiquidationAmount), 675 Errors.LIQUIDATION_AMOUNT_NOT_ENOUGH 676: ); 1196 require( 1197 vars.token0IsActive && vars.token1IsActive, 1198 Errors.RESERVE_INACTIVE 1199: ); 1202 require( 1203 !vars.token0IsPaused && !vars.token1IsPaused, 1204 Errors.RESERVE_PAUSED 1205: ); 1208 require( 1209 !vars.token0IsFrozen && !vars.token1IsFrozen, 1210 Errors.RESERVE_FROZEN 1211: );
File: paraspace-core/contracts/protocol/pool/PoolParameters.sol 216 require( 217 value > MIN_AUCTION_HEALTH_FACTOR && 218 value <= MAX_AUCTION_HEALTH_FACTOR, 219 Errors.INVALID_AMOUNT 220: );
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.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
There are 5 instances of this issue:
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol /// @audit uint64 collateralizedBalance 177 collateralizedBalance = useAsCollateral 178 ? collateralizedBalance + 1 179: : collateralizedBalance - 1; /// @audit uint64 oldCollateralizedBalance 201 oldCollateralizedBalance = erc721Data 202 .userState[to] 203: .collateralizedBalance; /// @audit uint64 newCollateralizedBalance 240 newCollateralizedBalance = 241 oldCollateralizedBalance + 242: collateralizedTokens; /// @audit uint64 oldCollateralizedBalance 276 oldCollateralizedBalance = erc721Data 277 .userState[user] 278: .collateralizedBalance; /// @audit uint64 newCollateralizedBalance 319 newCollateralizedBalance = 320 oldCollateralizedBalance - 321: burntCollateralizedTokens;
private
rather than public
for constants, saves gasIf 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
There is 1 instance of this issue:
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 73: bool public immutable ATOMIC_PRICING;
if
-else
-statement wastes gasFlipping the true
and false
blocks instead saves 3 gas
There are 2 instances of this issue:
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 401 if (!vars.isETH) { 402 IERC20(vars.creditToken).safeTransferFrom( 403 params.orderInfo.taker, 404 address(this), 405 downpayment 406 ); 407 _checkAllowance(vars.creditToken, params.marketplace.operator); 408 // convert to (priceEth, downpaymentEth) 409 price = 0; 410 downpayment = 0; 411 } else { 412 require(params.ethLeft >= downpayment, Errors.PAYNOW_NOT_ENOUGH); 413: }
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 562 if (!_isAuctioned) { 563 auction = DataTypes.Auction({startTime: 0}); 564 } else { 565 auction = _ERC721Data.auctions[tokenId]; 566: }
require()
or revert()
statements that check input arguments should be at the top of the functionChecks that involve constants should come before checks that involve state variables, function calls, and calculations. By doing these checks first, the function is able to revert before wasting a Gcoldsload (2100 gas*) in a function that may ultimately revert in the unhappy case.
There is 1 instance of this issue:
File: paraspace-core/contracts/protocol/pool/PoolParameters.sol /// @audit expensive op on line 212 214: require(value != 0, Errors.INVALID_AMOUNT);
revert()
/require()
strings to save gasCustom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas
There are 202 instances of this issue:
File: paraspace-core/contracts/misc/marketplaces/SeaportAdapter.sol 41 require( 42 // NOT criteria based and must be basic order 43 resolvers.length == 0 && isBasicOrder(advancedOrder), 44 Errors.INVALID_MARKETPLACE_ORDER 45: ); 51: require(orderInfo.offer.length > 0, Errors.INVALID_MARKETPLACE_ORDER); 54 require( 55 orderInfo.consideration.length > 0, 56 Errors.INVALID_MARKETPLACE_ORDER 57: ); 71 require( 72 // NOT criteria based and must be basic order 73 advancedOrders.length == 2 && 74 isBasicOrder(advancedOrders[0]) && 75 isBasicOrder(advancedOrders[1]), 76 Errors.INVALID_MARKETPLACE_ORDER 77: ); 84: require(orderInfo.offer.length > 0, Errors.INVALID_MARKETPLACE_ORDER); 87 require( 88 orderInfo.consideration.length > 0, 89 Errors.INVALID_MARKETPLACE_ORDER 90: );
File: paraspace-core/contracts/misc/marketplaces/X2Y2Adapter.sol 38: require(runInput.details.length == 1, Errors.INVALID_MARKETPLACE_ORDER); 44 require( 45 IX2Y2.Op.COMPLETE_SELL_OFFER == detail.op, 46 Errors.INVALID_MARKETPLACE_ORDER 47: ); 51: require(nfts.length == 1, Errors.INVALID_MARKETPLACE_ORDER);
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 225 require( 226 _assets.length == _twaps.length, 227 "NFTOracle: Tokens and price length differ" 228: ); 243 require( 244 (block.number - updatedAt) <= config.expirationPeriod, 245 "NFTOracle: asset price expired" 246: );
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol 91 require( 92 assets.length == sources.length, 93 Errors.INCONSISTENT_PARAMS_LENGTH 94: ); 96 require( 97 assets[i] != BASE_CURRENCY, 98 Errors.SET_ORACLE_SOURCE_NOT_ALLOWED 99: ); 134: require(price != 0, Errors.ORACLE_PRICE_NOT_READY); 222 require( 223 aclManager.isAssetListingAdmin(msg.sender) || 224 aclManager.isPoolAdmin(msg.sender), 225 Errors.CALLER_NOT_ASSET_LISTING_OR_POOL_ADMIN 226: );
File: paraspace-core/contracts/protocol/configuration/PoolAddressesProvider.sol 86: require(id != POOL, Errors.INVALID_ADDRESSES_PROVIDER_ID); 268: require(newAddress != address(0), Errors.ZERO_ADDRESS_NOT_VALID);
File: paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol 46 require( 47 IFlashClaimReceiver(params.receiverAddress).executeOperation( 48 params.nftAsset, 49 params.nftTokenIds, 50 params.params 51 ), 52 Errors.INVALID_FLASH_CLAIM_RECEIVER 53: ); 86 require( 87 healthFactor > DataTypes.HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 88 Errors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD 89: );
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol 171 require( 172 marketplaceIds.length == payloads.length && 173 payloads.length == credits.length, 174 Errors.INCONSISTENT_PARAMS_LENGTH 175: ); 245: require(vars.orderInfo.taker == onBehalfOf, Errors.INVALID_ORDER_TAKER); 279 require( 280 marketplaceIds.length == payloads.length && 281 payloads.length == credits.length, 282 Errors.INCONSISTENT_PARAMS_LENGTH 283: ); 294 require( 295 vars.orderInfo.taker == onBehalfOf, 296 Errors.INVALID_ORDER_TAKER 297: ); 384 require( 385 item.startAmount == item.endAmount, 386 Errors.INVALID_MARKETPLACE_ORDER 387: ); 388 require( 389 item.itemType == ItemType.ERC20 || 390 (vars.isETH && item.itemType == ItemType.NATIVE), 391 Errors.INVALID_ASSET_TYPE 392: ); 393 require( 394 item.token == params.credit.token, 395 Errors.CREDIT_DOES_NOT_MATCH_ORDER 396: ); 412: require(params.ethLeft >= downpayment, Errors.PAYNOW_NOT_ENOUGH); 440: require(vars.xTokenAddress != address(0), Errors.ASSET_NOT_LISTED); 472 require( 473 item.itemType == ItemType.ERC721, 474 Errors.INVALID_ASSET_TYPE 475: ); 489: require(isNToken, Errors.ASSET_NOT_LISTED); 494 require( 495 INToken(vars.xTokenAddress).getXTokenType() != 496 XTokenType.NTokenUniswapV3, 497 Errors.UNIV3_NOT_ALLOWED 498: );
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol 45: require(Address.isContract(params.asset), Errors.NOT_CONTRACT); 56: require(!reserveAlreadyAdded, Errors.RESERVE_ALREADY_ADDED); 66 require( 67 params.reservesCount < params.maxNumberReserves, 68 Errors.NO_MORE_RESERVES_ALLOWED 69: );
File: paraspace-core/contracts/protocol/libraries/logic/SupplyLogic.sol 409 require( 410 nToken.getXTokenType() == XTokenType.NTokenUniswapV3, 411 Errors.ONLY_UNIV3_ALLOWED 412: );
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol 68: require(amount != 0, Errors.INVALID_AMOUNT); 71 require( 72 xToken.getXTokenType() != XTokenType.PTokenSApe, 73 Errors.SAPE_NOT_ALLOWED 74: ); 84: require(reserveAssetType == assetType, Errors.INVALID_ASSET_TYPE); 85: require(isActive, Errors.RESERVE_INACTIVE); 86: require(!isPaused, Errors.RESERVE_PAUSED); 87: require(!isFrozen, Errors.RESERVE_FROZEN); 92 require( 93 supplyCap == 0 || 94 (IPToken(reserveCache.xTokenAddress) 95 .scaledTotalSupply() 96 .rayMul(reserveCache.nextLiquidityIndex) + amount) <= 97 supplyCap * 98 (10**reserveCache.reserveConfiguration.getDecimals()), 99 Errors.SUPPLY_CAP_EXCEEDED 100: ); 102 require( 103 supplyCap == 0 || 104 (INToken(reserveCache.xTokenAddress).totalSupply() + 105 amount <= 106 supplyCap), 107 Errors.SUPPLY_CAP_EXCEEDED 108: ); 123 require( 124 msg.sender == reserveCache.xTokenAddress, 125 Errors.CALLER_NOT_XTOKEN 126: ); 133 require( 134 IERC721(params.asset).ownerOf( 135 params.tokenData[index].tokenId 136 ) == reserveCache.xTokenAddress, 137 Errors.NOT_THE_OWNER 138: ); 140 require( 141 IERC721(reserveCache.xTokenAddress).ownerOf( 142 params.tokenData[index].tokenId 143 ) == address(0x0), 144 Errors.NOT_THE_OWNER 145: ); 160: require(amount != 0, Errors.INVALID_AMOUNT); 162 require( 163 amount <= userBalance, 164 Errors.NOT_ENOUGH_AVAILABLE_USER_BALANCE 165: ); 168 require( 169 xToken.getXTokenType() != XTokenType.PTokenSApe, 170 Errors.SAPE_NOT_ALLOWED 171: ); 181 require( 182 reserveAssetType == DataTypes.AssetType.ERC20, 183 Errors.INVALID_ASSET_TYPE 184: ); 185: require(isActive, Errors.RESERVE_INACTIVE); 186: require(!isPaused, Errors.RESERVE_PAUSED); 203 require( 204 reserveAssetType == DataTypes.AssetType.ERC721, 205 Errors.INVALID_ASSET_TYPE 206: ); 207: require(isActive, Errors.RESERVE_INACTIVE); 208: require(!isPaused, Errors.RESERVE_PAUSED); 258: require(params.amount != 0, Errors.INVALID_AMOUNT); 269 require( 270 vars.assetType == DataTypes.AssetType.ERC20, 271 Errors.INVALID_ASSET_TYPE 272: ); 273: require(vars.isActive, Errors.RESERVE_INACTIVE); 274: require(!vars.isPaused, Errors.RESERVE_PAUSED); 275: require(!vars.isFrozen, Errors.RESERVE_FROZEN); 276: require(vars.borrowingEnabled, Errors.BORROWING_NOT_ENABLED); 278 require( 279 params.priceOracleSentinel == address(0) || 280 IPriceOracleSentinel(params.priceOracleSentinel) 281 .isBorrowAllowed(), 282 Errors.PRICE_ORACLE_SENTINEL_CHECK_FAILED 283: ); 306 require( 307 vars.totalDebt <= vars.borrowCap * vars.assetUnit, 308 Errors.BORROW_CAP_EXCEEDED 309: ); 335 require( 336 vars.userCollateralInBaseCurrency != 0, 337 Errors.COLLATERAL_BALANCE_IS_ZERO 338: ); 339: require(vars.currentLtv != 0, Errors.LTV_VALIDATION_FAILED); 341 require( 342 vars.healthFactor > HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 343 Errors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD 344: ); 357 require( 358 vars.collateralNeededInBaseCurrency <= 359 vars.userCollateralInBaseCurrency, 360 Errors.COLLATERAL_CANNOT_COVER_NEW_BORROW 361: ); 377: require(amountSent != 0, Errors.INVALID_AMOUNT); 378 require( 379 amountSent != type(uint256).max || msg.sender == onBehalfOf, 380 Errors.NO_EXPLICIT_AMOUNT_TO_REPAY_ON_BEHALF 381: ); 390: require(isActive, Errors.RESERVE_INACTIVE); 391: require(!isPaused, Errors.RESERVE_PAUSED); 392 require( 393 assetType == DataTypes.AssetType.ERC20, 394 Errors.INVALID_ASSET_TYPE 395: ); 401 require( 402 (variableDebtPreviousIndex < reserveCache.nextVariableBorrowIndex), 403 Errors.SAME_BLOCK_BORROW_REPAY 404: ); 406: require((variableDebt != 0), Errors.NO_DEBT_OF_SELECTED_TYPE); 418: require(userBalance != 0, Errors.UNDERLYING_BALANCE_ZERO); 421 require( 422 xToken.getXTokenType() != XTokenType.PTokenSApe, 423 Errors.SAPE_NOT_ALLOWED 424: ); 434 require( 435 reserveAssetType == DataTypes.AssetType.ERC20, 436 Errors.INVALID_ASSET_TYPE 437: ); 438: require(isActive, Errors.RESERVE_INACTIVE); 439: require(!isPaused, Errors.RESERVE_PAUSED); 456 require( 457 reserveAssetType == DataTypes.AssetType.ERC721, 458 Errors.INVALID_ASSET_TYPE 459: ); 460: require(isActive, Errors.RESERVE_INACTIVE); 461: require(!isPaused, Errors.RESERVE_PAUSED); 515 require( 516 vars.collateralReserveAssetType == DataTypes.AssetType.ERC20, 517 Errors.INVALID_ASSET_TYPE 518: ); 520 require( 521 msg.value == 0 || params.liquidationAsset == params.weth, 522 Errors.INVALID_LIQUIDATION_ASSET 523: ); 525 require( 526 msg.value == 0 || msg.value >= params.actualLiquidationAmount, 527 Errors.LIQUIDATION_AMOUNT_NOT_ENOUGH 528: ); 533 require( 534 xToken.getXTokenType() != XTokenType.PTokenSApe, 535 Errors.SAPE_NOT_ALLOWED 536: ); 546 require( 547 vars.collateralReserveActive && vars.principalReserveActive, 548 Errors.RESERVE_INACTIVE 549: ); 550 require( 551 !vars.collateralReservePaused && !vars.principalReservePaused, 552 Errors.RESERVE_PAUSED 553: ); 555 require( 556 params.priceOracleSentinel == address(0) || 557 params.healthFactor < 558 MINIMUM_HEALTH_FACTOR_LIQUIDATION_THRESHOLD || 559 IPriceOracleSentinel(params.priceOracleSentinel) 560 .isLiquidationAllowed(), 561 Errors.PRICE_ORACLE_SENTINEL_CHECK_FAILED 562: ); 564 require( 565 params.healthFactor < HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 566 Errors.HEALTH_FACTOR_NOT_BELOW_THRESHOLD 567: ); 574 require( 575 vars.isCollateralEnabled, 576 Errors.COLLATERAL_CANNOT_BE_AUCTIONED_OR_LIQUIDATED 577: ); 578 require( 579 params.totalDebt != 0, 580 Errors.SPECIFIED_CURRENCY_NOT_BORROWED_BY_USER 581: ); 596 require( 597 params.liquidator != params.borrower, 598 Errors.LIQUIDATOR_CAN_NOT_BE_SELF 599: ); 611 require( 612 vars.collateralReserveAssetType == DataTypes.AssetType.ERC721, 613 Errors.INVALID_ASSET_TYPE 614: ); 636 require( 637 vars.collateralReserveActive && vars.principalReserveActive, 638 Errors.RESERVE_INACTIVE 639: ); 640 require( 641 !vars.collateralReservePaused && !vars.principalReservePaused, 642 Errors.RESERVE_PAUSED 643: ); 645 require( 646 params.priceOracleSentinel == address(0) || 647 params.healthFactor < 648 MINIMUM_HEALTH_FACTOR_LIQUIDATION_THRESHOLD || 649 IPriceOracleSentinel(params.priceOracleSentinel) 650 .isLiquidationAllowed(), 651 Errors.PRICE_ORACLE_SENTINEL_CHECK_FAILED 652: ); 655 require( 656 params.healthFactor < params.auctionRecoveryHealthFactor, 657 Errors.ERC721_HEALTH_FACTOR_NOT_BELOW_THRESHOLD 658: ); 659 require( 660 IAuctionableERC721(params.xTokenAddress).isAuctioned( 661 params.tokenId 662 ), 663 Errors.AUCTION_NOT_STARTED 664: ); 666 require( 667 params.healthFactor < HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 668 Errors.ERC721_HEALTH_FACTOR_NOT_BELOW_THRESHOLD 669: ); 672 require( 673 params.maxLiquidationAmount >= params.actualLiquidationAmount && 674 (msg.value == 0 || msg.value >= params.maxLiquidationAmount), 675 Errors.LIQUIDATION_AMOUNT_NOT_ENOUGH 676: ); 686 require( 687 vars.isCollateralEnabled, 688 Errors.COLLATERAL_CANNOT_BE_AUCTIONED_OR_LIQUIDATED 689: ); 690: require(params.globalDebt != 0, Errors.GLOBAL_DEBT_IS_ZERO); 732 require( 733 healthFactor >= HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 734 Errors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD 735: ); 757 require( 758 vars.collateralReserveAssetType == DataTypes.AssetType.ERC721, 759 Errors.INVALID_ASSET_TYPE 760: ); 762 require( 763 IERC721(params.xTokenAddress).ownerOf(params.tokenId) == 764 params.user, 765 Errors.NOT_THE_OWNER 766: ); 768: require(vars.collateralReserveActive, Errors.RESERVE_INACTIVE); 769: require(!vars.collateralReservePaused, Errors.RESERVE_PAUSED); 771 require( 772 collateralReserve.auctionStrategyAddress != address(0), 773 Errors.AUCTION_NOT_ENABLED 774: ); 775 require( 776 !IAuctionableERC721(params.xTokenAddress).isAuctioned( 777 params.tokenId 778 ), 779 Errors.AUCTION_ALREADY_STARTED 780: ); 782 require( 783 params.erc721HealthFactor < HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 784 Errors.ERC721_HEALTH_FACTOR_NOT_BELOW_THRESHOLD 785: ); 795 require( 796 vars.isCollateralEnabled, 797 Errors.COLLATERAL_CANNOT_BE_AUCTIONED_OR_LIQUIDATED 798: ); 815 require( 816 vars.collateralReserveAssetType == DataTypes.AssetType.ERC721, 817 Errors.INVALID_ASSET_TYPE 818: ); 819 require( 820 IERC721(params.xTokenAddress).ownerOf(params.tokenId) == 821 params.user, 822 Errors.NOT_THE_OWNER 823: ); 824: require(vars.collateralReserveActive, Errors.RESERVE_INACTIVE); 825: require(!vars.collateralReservePaused, Errors.RESERVE_PAUSED); 826 require( 827 IAuctionableERC721(params.xTokenAddress).isAuctioned( 828 params.tokenId 829 ), 830 Errors.AUCTION_NOT_STARTED 831: ); 833 require( 834 params.erc721HealthFactor > params.auctionRecoveryHealthFactor, 835 Errors.ERC721_HEALTH_FACTOR_NOT_ABOVE_THRESHOLD 836: ); 869 require( 870 !hasZeroLtvCollateral || reserve.configuration.getLtv() == 0, 871 Errors.LTV_VALIDATION_FAILED 872: ); 918: require(assetLTV == 0, Errors.LTV_VALIDATION_FAILED); 921 require( 922 reserve.configuration.getLtv() == 0, 923 Errors.LTV_VALIDATION_FAILED 924: ); 937: require(!reserve.configuration.getPaused(), Errors.RESERVE_PAUSED); 950: require(!reserve.configuration.getPaused(), Errors.RESERVE_PAUSED); 975: require(asset != address(0), Errors.ZERO_ADDRESS_NOT_VALID); 976 require( 977 reserve.id != 0 || reservesList[0] == asset, 978 Errors.ASSET_NOT_LISTED 979: ); 980 require( 981 IToken(reserve.variableDebtTokenAddress).totalSupply() == 0, 982 Errors.VARIABLE_DEBT_SUPPLY_NOT_ZERO 983: ); 984 require( 985 IToken(reserve.xTokenAddress).totalSupply() == 0, 986 Errors.XTOKEN_SUPPLY_NOT_ZERO 987: ); 1000 require( 1001 reserve.configuration.getAssetType() == DataTypes.AssetType.ERC721, 1002 Errors.INVALID_ASSET_TYPE 1003: ); 1004 require( 1005 params.receiverAddress != address(0), 1006 Errors.ZERO_ADDRESS_NOT_VALID 1007: ); 1011 require( 1012 tokenType != XTokenType.NTokenUniswapV3, 1013 Errors.UNIV3_NOT_ALLOWED 1014: ); 1029: require(isActive, Errors.RESERVE_INACTIVE); 1030: require(!isPaused, Errors.RESERVE_PAUSED); 1035 require( 1036 nToken.ownerOf(params.nftTokenIds[i]) == msg.sender, 1037 Errors.NOT_THE_OWNER 1038: ); 1057: require(isActive, Errors.RESERVE_INACTIVE); 1058: require(!isPaused, Errors.RESERVE_PAUSED); 1059 require( 1060 assetType == DataTypes.AssetType.ERC20, 1061 Errors.INVALID_ASSET_TYPE 1062: ); 1068: require(!params.marketplace.paused, Errors.MARKETPLACE_PAUSED); 1074: require(!params.marketplace.paused, Errors.MARKETPLACE_PAUSED); 1075 require( 1076 keccak256(abi.encodePacked(params.orderInfo.id)) == 1077 keccak256(abi.encodePacked(params.credit.orderId)), 1078 Errors.CREDIT_DOES_NOT_MATCH_ORDER 1079: ); 1080 require( 1081 verifyCreditSignature( 1082 params.credit, 1083 params.orderInfo.maker, 1084 params.credit.v, 1085 params.credit.r, 1086 params.credit.s 1087 ), 1088 Errors.INVALID_CREDIT_SIGNATURE 1089: ); 1196 require( 1197 vars.token0IsActive && vars.token1IsActive, 1198 Errors.RESERVE_INACTIVE 1199: ); 1202 require( 1203 !vars.token0IsPaused && !vars.token1IsPaused, 1204 Errors.RESERVE_PAUSED 1205: ); 1208 require( 1209 !vars.token0IsFrozen && !vars.token1IsFrozen, 1210 Errors.RESERVE_FROZEN 1211: );
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol 73 require( 74 nToken.ownerOf(_nfts[index].tokenId) == msg.sender, 75 Errors.NOT_THE_OWNER 76: ); 85 require( 86 getUserHf(msg.sender) > 87 DataTypes.HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 88 Errors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD 89: ); 104 require( 105 nToken.ownerOf(_nfts[index]) == msg.sender, 106 Errors.NOT_THE_OWNER 107: ); 112 require( 113 getUserHf(msg.sender) > 114 DataTypes.HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 115 Errors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD 116: ); 139 require( 140 INToken(xTokenAddress).ownerOf(_nftPairs[index].mainTokenId) == 141 msg.sender, 142 Errors.NOT_THE_OWNER 143: ); 180 require( 181 getUserHf(msg.sender) > 182 DataTypes.HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 183 Errors.HEALTH_FACTOR_LOWER_THAN_LIQUIDATION_THRESHOLD 184: ); 200 require( 201 INToken(xTokenAddress).ownerOf(_nftPairs[index].mainTokenId) == 202 msg.sender, 203 Errors.NOT_THE_OWNER 204: ); 280 require( 281 INToken(localVar.nTokenAddress).ownerOf(_nfts[index].tokenId) == 282 msg.sender, 283 Errors.NOT_THE_OWNER 284: ); 290 require( 291 INToken(localVar.nTokenAddress).ownerOf( 292 _nftPairs[index].mainTokenId 293 ) == msg.sender, 294 Errors.NOT_THE_OWNER 295: ); 338 require( 339 localVar.apeCoin.balanceOf(localVar.nTokenAddress) == 340 localVar.beforeBalance, 341 Errors.TOTAL_STAKING_AMOUNT_WRONG 342: ); 356 require( 357 getUserHf(positionOwner) < 358 DataTypes.HEALTH_FACTOR_LIQUIDATION_THRESHOLD, 359 Errors.HEALTH_FACTOR_NOT_BELOW_THRESHOLD 360: ); 380 require( 381 msg.sender == ps._reserves[underlyingAsset].xTokenAddress, 382 Errors.CALLER_NOT_XTOKEN 383: ); 453: require(isActive, Errors.RESERVE_INACTIVE); 454: require(!isPaused, Errors.RESERVE_PAUSED);
File: paraspace-core/contracts/protocol/pool/PoolCore.sol 80 require( 81 provider == ADDRESSES_PROVIDER, 82 Errors.INVALID_ADDRESSES_PROVIDER 83: ); 692 require( 693 msg.sender == ps._reserves[asset].xTokenAddress, 694 Errors.CALLER_NOT_XTOKEN 695: ); 726 require( 727 msg.sender == ps._reserves[asset].xTokenAddress, 728 Errors.CALLER_NOT_XTOKEN 729: ); 761 require( 762 reserve.id != 0 || ps._reservesList[0] == underlyingAsset, 763 Errors.ASSET_NOT_LISTED 764: ); 803 require( 804 msg.sender == 805 address(IPoolAddressesProvider(ADDRESSES_PROVIDER).getWETH()), 806 "Receive not allowed" 807: );
File: paraspace-core/contracts/protocol/pool/PoolParameters.sol 70 require( 71 ADDRESSES_PROVIDER.getPoolConfigurator() == msg.sender, 72 Errors.CALLER_NOT_POOL_CONFIGURATOR 73: ); 77 require( 78 IACLManager(ADDRESSES_PROVIDER.getACLManager()).isPoolAdmin( 79 msg.sender 80 ), 81 Errors.CALLER_NOT_POOL_ADMIN 82: ); 157: require(asset != address(0), Errors.ZERO_ADDRESS_NOT_VALID); 158 require( 159 ps._reserves[asset].id != 0 || ps._reservesList[0] == asset, 160 Errors.ASSET_NOT_LISTED 161: ); 172: require(asset != address(0), Errors.ZERO_ADDRESS_NOT_VALID); 173 require( 174 ps._reserves[asset].id != 0 || ps._reservesList[0] == asset, 175 Errors.ASSET_NOT_LISTED 176: ); 187: require(asset != address(0), Errors.ZERO_ADDRESS_NOT_VALID); 188 require( 189 ps._reserves[asset].id != 0 || ps._reservesList[0] == asset, 190 Errors.ASSET_NOT_LISTED 191: ); 214: require(value != 0, Errors.INVALID_AMOUNT); 216 require( 217 value > MIN_AUCTION_HEALTH_FACTOR && 218 value <= MAX_AUCTION_HEALTH_FACTOR, 219 Errors.INVALID_AMOUNT 220: ); 271: require(user != address(0), Errors.ZERO_ADDRESS_NOT_VALID); 281 require( 282 erc721HealthFactor > ps._auctionRecoveryHealthFactor, 283 Errors.ERC721_HEALTH_FACTOR_NOT_ABOVE_THRESHOLD 284: );
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 49 require( 50 aclManager.isPoolAdmin(msg.sender), 51 Errors.CALLER_NOT_POOL_ADMIN 52: ); 60: require(_msgSender() == address(POOL), Errors.CALLER_MUST_BE_POOL); 195 require( 196 _msgSender() == owner || isApprovedForAll(owner, _msgSender()), 197 "ERC721: approve caller is not owner nor approved for all" 198: ); 213 require( 214 _exists(tokenId), 215 "ERC721: approved query for nonexistent token" 216: ); 259 require( 260 _isApprovedOrOwner(_msgSender(), tokenId), 261 "ERC721: transfer caller is not owner nor approved" 262: ); 296 require( 297 _isApprovedOrOwner(_msgSender(), tokenId), 298 "ERC721: transfer caller is not owner nor approved" 299: ); 354 require( 355 _exists(tokenId), 356 "ERC721: operator query for nonexistent token" 357: ); 594 require( 595 index < balanceOf(owner), 596 "ERC721Enumerable: owner index out of bounds" 597: ); 618 require( 619 index < totalSupply(), 620 "ERC721Enumerable: global index out of bounds" 621: );
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol 67 require( 68 incentive < PercentageMath.HALF_PERCENTAGE_FACTOR, 69 "Value Too High" 70: );
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol 88 require( 89 erc721Data.owners[tokenId] == from, 90 "ERC721: transfer from incorrect owner" 91: ); 93 require( 94 !isAuctioned(erc721Data, POOL, tokenId), 95 Errors.TOKEN_IN_AUCTION 96: ); 167 require( 168 !isAuctioned(erc721Data, POOL, tokenId), 169 Errors.TOKEN_IN_AUCTION 170: ); 210 require( 211 !_exists(erc721Data, tokenId), 212 "ERC721: token already minted" 213: ); 284 require( 285 !isAuctioned(erc721Data, POOL, tokenId), 286 Errors.TOKEN_IN_AUCTION 287: ); 373 require( 374 !isAuctioned(erc721Data, POOL, tokenId), 375 Errors.AUCTION_ALREADY_STARTED 376: ); 377 require( 378 _exists(erc721Data, tokenId), 379 "ERC721: startAuction for nonexistent token" 380: ); 391 require( 392 isAuctioned(erc721Data, POOL, tokenId), 393 Errors.AUCTION_NOT_STARTED 394: ); 395 require( 396 _exists(erc721Data, tokenId), 397 "ERC721: endAuction for nonexistent token" 398: ); 409 require( 410 balanceLimit == 0 || balance <= balanceLimit, 411 Errors.NTOKEN_BALANCE_EXCEEDED 412: );
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol 70: require(msg.sender == _underlyingAsset, Errors.OPERATION_NOT_SUPPORTED); 98 require( 99 _isApprovedOrOwner(_msgSender(), tokenIds[index]), 100 "ERC721: transfer caller is not owner nor approved" 101: );
File: paraspace-core/contracts/protocol/tokenization/NToken.sol 60: require(initializingPool == POOL, Errors.POOL_ADDRESSES_DO_NOT_MATCH); 64: require(underlyingAsset != address(0), Errors.ZERO_ADDRESS_NOT_VALID); 141 require( 142 token != _underlyingAsset, 143 Errors.UNDERLYING_ASSET_CAN_NOT_BE_TRANSFERRED 144: ); 172 require( 173 airdropContract != address(0), 174 Errors.INVALID_AIRDROP_CONTRACT_ADDRESS 175: ); 176: require(airdropParams.length >= 4, Errors.INVALID_AIRDROP_PARAMETERS);
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol 131: require(user == ownerOf(tokenId), Errors.NOT_THE_OWNER);
payable
If a function modifier such as onlyOwner
is used, the function will revert if a normal user tries to pay the function. Marking the function as payable
will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. The extra opcodes avoided are
CALLVALUE
(2),DUP1
(3),ISZERO
(3),PUSH2
(3),JUMPI
(10),PUSH1
(3),DUP1
(3),REVERT
(0),JUMPDEST
(1),POP
(2), which costs an average of about 21 gas per call to the function, in addition to the extra deployment cost
There are 66 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol 139 function addAssets(address[] calldata _assets) 140 external 141: onlyRole(DEFAULT_ADMIN_ROLE) 148 function removeAsset(address _asset) 149 external 150 onlyRole(DEFAULT_ADMIN_ROLE) 151: onlyWhenAssetExisted(_asset) 158 function addFeeders(address[] calldata _feeders) 159 external 160: onlyRole(DEFAULT_ADMIN_ROLE) 175 function setConfig(uint128 expirationPeriod, uint128 maxPriceDeviation) 176 external 177: onlyRole(DEFAULT_ADMIN_ROLE) 183 function setPause(address _asset, bool _flag) 184 external 185: onlyRole(DEFAULT_ADMIN_ROLE) 195 function setPrice(address _asset, uint256 _twap) 196 public 197 onlyRole(UPDATER_ROLE) 198 onlyWhenAssetExisted(_asset) 199: whenNotPaused(_asset) 221 function setMultiplePrices( 222 address[] calldata _assets, 223 uint256[] calldata _twaps 224: ) external onlyRole(UPDATER_ROLE) { 278 function _addAsset(address _asset) 279 internal 280: onlyWhenAssetNotExisted(_asset) 296 function _removeAsset(address _asset) 297 internal 298: onlyWhenAssetExisted(_asset) 307 function _addFeeder(address _feeder) 308 internal 309: onlyWhenFeederNotExisted(_feeder) 326 function _removeFeeder(address _feeder) 327 internal 328: onlyWhenFeederExisted(_feeder)
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol 66 function setAssetSources( 67 address[] calldata assets, 68 address[] calldata sources 69: ) external override onlyAssetListingOrPoolAdmins { 74 function setFallbackOracle(address fallbackOracle) 75 external 76 override 77: onlyAssetListingOrPoolAdmins
File: paraspace-core/contracts/protocol/configuration/PoolAddressesProvider.sol 56 function setMarketId(string memory newMarketId) 57 external 58 override 59: onlyOwner 70 function setAddress(bytes32 id, address newAddress) 71 external 72 override 73: onlyOwner 81 function setAddressAsProxy(bytes32 id, address newImplementationAddress) 82 external 83 override 84: onlyOwner 105 function updatePoolImpl( 106 IParaProxy.ProxyImplementation[] calldata implementationParams, 107 address _init, 108 bytes calldata _calldata 109: ) external override onlyOwner { 121 function setPoolConfiguratorImpl(address newPoolConfiguratorImpl) 122 external 123 override 124: onlyOwner 142 function setPriceOracle(address newPriceOracle) 143 external 144 override 145: onlyOwner 158: function setACLManager(address newAclManager) external override onlyOwner { 170: function setACLAdmin(address newAclAdmin) external override onlyOwner { 182 function setPriceOracleSentinel(address newPriceOracleSentinel) 183 external 184 override 185: onlyOwner 224 function setProtocolDataProvider(address newDataProvider) 225 external 226 override 227: onlyOwner 235: function setWETH(address newWETH) external override onlyOwner { 242 function setMarketplace( 243 bytes32 id, 244 address marketplace, 245 address adapter, 246 address operator, 247 bool paused 248: ) external override onlyOwner {
File: paraspace-core/contracts/protocol/pool/PoolParameters.sol 110 function initReserve( 111 address asset, 112 address xTokenAddress, 113 address variableDebtAddress, 114 address interestRateStrategyAddress, 115 address auctionStrategyAddress 116: ) external virtual override onlyPoolConfigurator { 139 function dropReserve(address asset) 140 external 141 virtual 142 override 143: onlyPoolConfigurator 151 function setReserveInterestRateStrategyAddress( 152 address asset, 153 address rateStrategyAddress 154: ) external virtual override onlyPoolConfigurator { 166 function setReserveAuctionStrategyAddress( 167 address asset, 168 address auctionStrategyAddress 169: ) external virtual override onlyPoolConfigurator { 181 function setConfiguration( 182 address asset, 183 DataTypes.ReserveConfigurationMap calldata configuration 184: ) external virtual override onlyPoolConfigurator { 196 function rescueTokens( 197 DataTypes.AssetType assetType, 198 address token, 199 address to, 200 uint256 amountOrTokenId 201: ) external virtual override onlyPoolAdmin { 206 function setAuctionRecoveryHealthFactor(uint64 value) 207 external 208 virtual 209 override 210: onlyPoolConfigurator
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 131 function setIncentivesController(IRewardController controller) 132 external 133: onlyPoolAdmin 142: function setBalanceLimit(uint64 limit) external onlyPoolAdmin { 458 function setIsUsedAsCollateral( 459 uint256 tokenId, 460 bool useAsCollateral, 461 address sender 462: ) external virtual override onlyPool nonReentrant returns (bool) { 474 function batchSetIsUsedAsCollateral( 475 uint256[] calldata tokenIds, 476 bool useAsCollateral, 477 address sender 478 ) 479 external 480 virtual 481 override 482 onlyPool 483 nonReentrant 484 returns ( 485 uint256 oldCollateralizedBalance, 486: uint256 newCollateralizedBalance 529 function startAuction(uint256 tokenId) 530 external 531 virtual 532 override 533 onlyPool 534: nonReentrant 540 function endAuction(uint256 tokenId) 541 external 542 virtual 543 override 544 onlyPool 545: nonReentrant
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol 102 function burn( 103 address from, 104 address receiverOfUnderlying, 105 uint256[] calldata tokenIds 106: ) external virtual override onlyPool nonReentrant returns (uint64, uint64) { 136: function setUnstakeApeIncentive(uint256 incentive) external onlyPoolAdmin { 159 function unstakePositionAndRepay(uint256 tokenId, address incentiveReceiver) 160 external 161 onlyPool 162: nonReentrant
File: paraspace-core/contracts/protocol/tokenization/NTokenBAYC.sol 26 function depositApeCoin(ApeCoinStaking.SingleNft[] calldata _nfts) 27 external 28 onlyPool 29: nonReentrant 39 function claimApeCoin(uint256[] calldata _nfts, address _recipient) 40 external 41 onlyPool 42: nonReentrant 52 function withdrawApeCoin( 53 ApeCoinStaking.SingleNft[] calldata _nfts, 54 address _recipient 55: ) external onlyPool nonReentrant { 66 function depositBAKC(ApeCoinStaking.PairNftWithAmount[] calldata _nftPairs) 67 external 68 onlyPool 69: nonReentrant 82 function claimBAKC( 83 ApeCoinStaking.PairNft[] calldata _nftPairs, 84 address _recipient 85: ) external onlyPool nonReentrant { 97 function withdrawBAKC( 98 ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, 99 address _apeRecipient 100: ) external onlyPool nonReentrant {
File: paraspace-core/contracts/protocol/tokenization/NTokenMAYC.sol 26 function depositApeCoin(ApeCoinStaking.SingleNft[] calldata _nfts) 27 external 28 onlyPool 29: nonReentrant 39 function claimApeCoin(uint256[] calldata _nfts, address _recipient) 40 external 41 onlyPool 42: nonReentrant 52 function withdrawApeCoin( 53 ApeCoinStaking.SingleNft[] calldata _nfts, 54 address _recipient 55: ) external onlyPool nonReentrant { 66 function depositBAKC(ApeCoinStaking.PairNftWithAmount[] calldata _nftPairs) 67 external 68 onlyPool 69: nonReentrant 82 function claimBAKC( 83 ApeCoinStaking.PairNft[] calldata _nftPairs, 84 address _recipient 85: ) external onlyPool nonReentrant { 97 function withdrawBAKC( 98 ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, 99 address _apeRecipient 100: ) external onlyPool nonReentrant {
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol 40 function burn( 41 address from, 42 address receiverOfUnderlying, 43 uint256[] calldata tokenIds 44: ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
File: paraspace-core/contracts/protocol/tokenization/NToken.sol 79 function mint( 80 address onBehalfOf, 81 DataTypes.ERC721SupplyParams[] calldata tokenData 82: ) external virtual override onlyPool nonReentrant returns (uint64, uint64) { 87 function burn( 88 address from, 89 address receiverOfUnderlying, 90 uint256[] calldata tokenIds 91: ) external virtual override onlyPool nonReentrant returns (uint64, uint64) { 119 function transferOnLiquidation( 120 address from, 121 address to, 122 uint256 value 123: ) external onlyPool nonReentrant { 127 function rescueERC20( 128 address token, 129 address to, 130 uint256 amount 131: ) external override onlyPoolAdmin { 136 function rescueERC721( 137 address token, 138 address to, 139 uint256[] calldata ids 140: ) external override onlyPoolAdmin { 151 function rescueERC1155( 152 address token, 153 address to, 154 uint256[] calldata ids, 155 uint256[] calldata amounts, 156 bytes calldata data 157: ) external override onlyPoolAdmin { 168 function executeAirdrop( 169 address airdropContract, 170 bytes calldata airdropParams 171: ) external override onlyPoolAdmin { 199 function transferUnderlyingTo(address target, uint256 tokenId) 200 external 201 virtual 202 override 203 onlyPool 204: nonReentrant 214 function handleRepayment(address user, uint256 amount) 215 external 216 virtual 217 override 218 onlyPool 219: nonReentrant
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol 123 function decreaseUniswapV3Liquidity( 124 address user, 125 uint256 tokenId, 126 uint128 liquidityDecrease, 127 uint256 amount0Min, 128 uint256 amount1Min, 129 bool receiveEthAsWeth 130: ) external onlyPool nonReentrant {
File: paraspace-core/contracts/ui/WPunkGateway.sol 202 function emergencyERC721TokenTransfer( 203 address token, 204 uint256 tokenId, 205 address to 206: ) external onlyOwner { 217 function emergencyPunkTransfer(address to, uint256 punkIndex) 218 external 219: onlyOwner
_msgSender()
if not supporting EIP-2771Use msg.sender
if the code does not implement EIP-2771 trusted forwarder support
There are 7 instances of this issue:
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol 60: require(_msgSender() == address(POOL), Errors.CALLER_MUST_BE_POOL); 196: _msgSender() == owner || isApprovedForAll(owner, _msgSender()), 231: _msgSender(), 260: _isApprovedOrOwner(_msgSender(), tokenId), 297: _isApprovedOrOwner(_msgSender(), tokenId),
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol 99: _isApprovedOrOwner(_msgSender(), tokenIds[index]),
These findings are excluded from awards calculations because there are publicly-available automated tools that find them. The valid ones appear here for completeness
Issue | Instances | Total Gas Saved | |
---|---|---|---|
[Gā01] | Using calldata instead of memory for read-only arguments in external functions saves gas | 34 | 4080 |
[Gā02] | State variables should be cached in stack variables rather than re-reading them from storage | 1 | 97 |
[Gā03] | <array>.length should not be looked up in every loop of a for -loop | 41 | 123 |
[Gā04] | Using bool s for storage incurs overhead | 1 | 17100 |
[Gā05] | Using > 0 costs more gas than != 0 when used on a uint in a require() statement | 1 | 6 |
[Gā06] | ++i costs less gas than i++ , especially when it's used in for -loops (--i /i-- too) | 57 | 285 |
[Gā07] | Using private rather than public for constants, saves gas | 8 | - |
[Gā08] | Division by two should use bit shifting | 2 | 40 |
[Gā09] | Use custom errors rather than revert() /require() strings to save gas | 15 | - |
Total: 160 instances over 9 issues with 21731 gas saved
Gas totals use lower bounds of ranges and count two iterations of each for
-loop. All values above are runtime, not deployment, values; deployment values are listed in the individual issue descriptions. The table above as well as its gas numbers do not include any of the excluded findings.
calldata
instead of memory
for read-only arguments in external
functions saves gasWhen a function with a memory
array is called externally, the abi.decode()
step has to use a for-loop to copy each index of the calldata
to the memory
index. Each iteration of this for-loop costs at least 60 gas (i.e. 60 * <mem_array>.length
). Using calldata
directly, obliviates the need for such a loop in the contract code and runtime execution. Note that even if an interface defines a function as having memory
arguments, it's still valid for implementation contracs to use calldata
arguments instead.
If the array is passed to an internal
function which passes the array to another internal function where the array is modified and therefore memory
is used in the external
call, it's still more gass-efficient to use calldata
when the external
function uses modifiers, since the modifiers may prevent the internal functions from being called. Structs have the same overhead as an array of length one
Note that I've also flagged instances where the function is public
but can be marked as external
since it's not called by the contract, and cases where a constructor is involved
There are 34 instances of this issue:
File: paraspace-core/contracts/misc/marketplaces/LooksRareAdapter.sol /// @audit params - (valid but excluded finding) 25 function getAskOrderInfo(bytes memory params, address weth) 26 external 27 pure 28 override 29: returns (DataTypes.OrderInfo memory orderInfo) /// @audit (valid but excluded finding) 67 function getBidOrderInfo( 68 bytes memory /*params*/ 69: ) external pure override returns (DataTypes.OrderInfo memory) {
File: paraspace-core/contracts/misc/marketplaces/SeaportAdapter.sol /// @audit params - (valid but excluded finding) 25 function getAskOrderInfo(bytes memory params, address) 26 external 27 pure 28 override 29: returns (DataTypes.OrderInfo memory orderInfo) /// @audit params - (valid but excluded finding) 60 function getBidOrderInfo(bytes memory params) 61 external 62 pure 63 override 64: returns (DataTypes.OrderInfo memory orderInfo)
File: paraspace-core/contracts/misc/marketplaces/X2Y2Adapter.sol /// @audit params - (valid but excluded finding) 31 function getAskOrderInfo(bytes memory params, address) 32 external 33 pure 34 override 35: returns (DataTypes.OrderInfo memory orderInfo) /// @audit (valid but excluded finding) 79 function getBidOrderInfo(bytes memory) 80 external 81 pure 82 override 83: returns (DataTypes.OrderInfo memory)
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit _feeders - (valid but excluded finding) /// @audit _assets - (valid but excluded finding) 97 function initialize( 98 address _admin, 99 address[] memory _feeders, 100 address[] memory _assets 101: ) public initializer {
File: paraspace-core/contracts/protocol/configuration/PoolAddressesProvider.sol /// @audit newMarketId - (valid but excluded finding) 56 function setMarketId(string memory newMarketId) 57 external 58 override 59: onlyOwner
File: paraspace-core/contracts/protocol/libraries/logic/AuctionLogic.sol /// @audit params - (valid but excluded finding) 41 function executeStartAuction( 42 mapping(address => DataTypes.ReserveData) storage reservesData, 43 mapping(uint256 => address) storage reservesList, 44 mapping(address => DataTypes.UserConfigurationMap) storage usersConfig, 45: DataTypes.ExecuteAuctionParams memory params /// @audit params - (valid but excluded finding) 101 function executeEndAuction( 102 mapping(address => DataTypes.ReserveData) storage reservesData, 103 mapping(uint256 => address) storage reservesList, 104 mapping(address => DataTypes.UserConfigurationMap) storage usersConfig, 105: DataTypes.ExecuteAuctionParams memory params
File: paraspace-core/contracts/protocol/libraries/logic/BorrowLogic.sol /// @audit params - (valid but excluded finding) 52 function executeBorrow( 53 mapping(address => DataTypes.ReserveData) storage reservesData, 54 mapping(uint256 => address) storage reservesList, 55 DataTypes.UserConfigurationMap storage userConfig, 56: DataTypes.ExecuteBorrowParams memory params /// @audit params - (valid but excluded finding) 127 function executeRepay( 128 mapping(address => DataTypes.ReserveData) storage reservesData, 129 DataTypes.UserConfigurationMap storage userConfig, 130 DataTypes.ExecuteRepayParams memory params 131: ) external returns (uint256) {
File: paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol /// @audit params - (valid but excluded finding) 28 function executeFlashClaim( 29 DataTypes.PoolStorage storage ps, 30: DataTypes.ExecuteFlashClaimParams memory params
File: paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol /// @audit params - (valid but excluded finding) 161 function executeLiquidateERC20( 162 mapping(address => DataTypes.ReserveData) storage reservesData, 163 mapping(uint256 => address) storage reservesList, 164 mapping(address => DataTypes.UserConfigurationMap) storage usersConfig, 165 DataTypes.ExecuteLiquidateParams memory params 166: ) external returns (uint256) { /// @audit params - (valid but excluded finding) 286 function executeLiquidateERC721( 287 mapping(address => DataTypes.ReserveData) storage reservesData, 288 mapping(uint256 => address) storage reservesList, 289 mapping(address => DataTypes.UserConfigurationMap) storage usersConfig, 290 DataTypes.ExecuteLiquidateParams memory params 291: ) external returns (uint256) {
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol /// @audit params - (valid but excluded finding) 39 function executeInitReserve( 40 mapping(address => DataTypes.ReserveData) storage reservesData, 41 mapping(uint256 => address) storage reservesList, 42 DataTypes.InitReserveParams memory params 43: ) external returns (bool) {
File: paraspace-core/contracts/protocol/libraries/logic/SupplyLogic.sol /// @audit params - (valid but excluded finding) 81 function executeSupply( 82 mapping(address => DataTypes.ReserveData) storage reservesData, 83 DataTypes.UserConfigurationMap storage userConfig, 84: DataTypes.ExecuteSupplyParams memory params /// @audit params - (valid but excluded finding) 167 function executeSupplyERC721( 168 mapping(address => DataTypes.ReserveData) storage reservesData, 169 DataTypes.UserConfigurationMap storage userConfig, 170: DataTypes.ExecuteSupplyERC721Params memory params /// @audit params - (valid but excluded finding) 228 function executeSupplyERC721FromNToken( 229 mapping(address => DataTypes.ReserveData) storage reservesData, 230 DataTypes.UserConfigurationMap storage userConfig, 231: DataTypes.ExecuteSupplyERC721Params memory params /// @audit params - (valid but excluded finding) 270 function executeWithdraw( 271 mapping(address => DataTypes.ReserveData) storage reservesData, 272 mapping(uint256 => address) storage reservesList, 273 DataTypes.UserConfigurationMap storage userConfig, 274 DataTypes.ExecuteWithdrawParams memory params 275: ) external returns (uint256) { /// @audit params - (valid but excluded finding) 335 function executeWithdrawERC721( 336 mapping(address => DataTypes.ReserveData) storage reservesData, 337 mapping(uint256 => address) storage reservesList, 338 DataTypes.UserConfigurationMap storage userConfig, 339 DataTypes.ExecuteWithdrawERC721Params memory params 340: ) external returns (uint256) { /// @audit params - (valid but excluded finding) 396 function executeDecreaseUniswapV3Liquidity( 397 mapping(address => DataTypes.ReserveData) storage reservesData, 398 mapping(uint256 => address) storage reservesList, 399 DataTypes.UserConfigurationMap storage userConfig, 400: DataTypes.ExecuteDecreaseUniswapV3LiquidityParams memory params /// @audit params - (valid but excluded finding) 462 function executeFinalizeTransferERC20( 463 mapping(address => DataTypes.ReserveData) storage reservesData, 464 mapping(uint256 => address) storage reservesList, 465 mapping(address => DataTypes.UserConfigurationMap) storage usersConfig, 466: DataTypes.FinalizeTransferParams memory params /// @audit params - (valid but excluded finding) 523 function executeFinalizeTransferERC721( 524 mapping(address => DataTypes.ReserveData) storage reservesData, 525 mapping(uint256 => address) storage reservesList, 526 mapping(address => DataTypes.UserConfigurationMap) storage usersConfig, 527: DataTypes.FinalizeTransferERC721Params memory params
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol /// @audit _nftPairs - (valid but excluded finding) 120 function withdrawBAKC( 121 address nftAsset, 122 ApeCoinStaking.PairNftWithAmount[] memory _nftPairs 123: ) external nonReentrant {
File: paraspace-core/contracts/protocol/pool/PoolCore.sol /// @audit (valid but excluded finding) 793 function onERC721Received( 794 address, 795 address, 796 uint256, 797 bytes memory 798: ) external virtual returns (bytes4) {
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol /// @audit _data - (valid but excluded finding) 281 function safeTransferFrom( 282 address from, 283 address to, 284 uint256 tokenId, 285 bytes memory _data 286: ) external virtual override nonReentrant {
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol /// @audit _nftPairs - (valid but excluded finding) 38 function withdrawBAKC( 39 ApeCoinStaking _apeCoinStaking, 40 uint256 poolId, 41 ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, 42: address _apeRecipient /// @audit params - (valid but excluded finding) 92 function executeUnstakePositionAndRepay( 93 mapping(uint256 => address) storage _owners, 94 APEStakingParameter storage stakingParameter, 95: UnstakeAndRepayParams memory params
File: paraspace-core/contracts/protocol/tokenization/NTokenBAYC.sol /// @audit _nftPairs - (valid but excluded finding) 97 function withdrawBAKC( 98 ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, 99 address _apeRecipient 100: ) external onlyPool nonReentrant {
File: paraspace-core/contracts/protocol/tokenization/NTokenMAYC.sol /// @audit _nftPairs - (valid but excluded finding) 97 function withdrawBAKC( 98 ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, 99 address _apeRecipient 100: ) external onlyPool nonReentrant {
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol /// @audit (valid but excluded finding) 63 function onERC721Received( 64 address operator, 65 address from, 66 uint256 id, 67 bytes memory 68: ) external virtual override returns (bytes4) {
File: paraspace-core/contracts/protocol/tokenization/NToken.sol /// @audit (valid but excluded finding) 271 function onERC721Received( 272 address, 273 address, 274 uint256, 275 bytes memory 276: ) external virtual override returns (bytes4) {
The instances below point to the second+ access of a state variable within a function. Caching of a state variable replaces each Gwarmaccess (100 gas) with a much cheaper stack read. Other less obvious fixes/optimizations include having local memory caches of state variable structs, or having local caches of state variable contracts/addresses.
There is 1 instance of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit assetPriceMap[_asset].twap on line 405 - (valid but excluded finding) 426: return (false, assetPriceMap[_asset].twap);
<array>.length
should not be looked up in every loop of a for
-loopThe overheads outlined below are PER LOOP, excluding the first loop
MLOAD
(3 gas)CALLDATALOAD
(3 gas)Caching the length changes each of these to a DUP<N>
(3 gas), and gets rid of the extra DUP<N>
needed to store the stack offset
There are 41 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit (valid but excluded finding) 229: for (uint256 i = 0; i < _assets.length; i++) { /// @audit (valid but excluded finding) 291: for (uint256 i = 0; i < _assets.length; i++) { /// @audit (valid but excluded finding) 321: for (uint256 i = 0; i < _feeders.length; i++) {
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol /// @audit (valid but excluded finding) 95: for (uint256 i = 0; i < assets.length; i++) { /// @audit (valid but excluded finding) 197: for (uint256 i = 0; i < assets.length; i++) {
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol /// @audit (valid but excluded finding) 193: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 210: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol /// @audit (valid but excluded finding) 38: for (i = 0; i < params.nftTokenIds.length; i++) { /// @audit (valid but excluded finding) 56: for (i = 0; i < params.nftTokenIds.length; i++) {
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol /// @audit (valid but excluded finding) 177: for (uint256 i = 0; i < marketplaceIds.length; i++) { /// @audit (valid but excluded finding) 284: for (uint256 i = 0; i < marketplaceIds.length; i++) { /// @audit (valid but excluded finding) 382: for (uint256 i = 0; i < params.orderInfo.consideration.length; i++) { /// @audit (valid but excluded finding) 470: for (uint256 i = 0; i < params.orderInfo.offer.length; i++) {
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol /// @audit (valid but excluded finding) 104: for (uint256 i = 0; i < assets.length; i++) {
File: paraspace-core/contracts/protocol/libraries/logic/SupplyLogic.sol /// @audit (valid but excluded finding) 184: for (uint256 index = 0; index < params.tokenData.length; index++) { /// @audit (valid but excluded finding) 195: for (uint256 index = 0; index < params.tokenData.length; index++) {
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol /// @audit (valid but excluded finding) 212: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 465: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 910: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 1034: for (uint256 i = 0; i < params.nftTokenIds.length; i++) {
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol /// @audit (valid but excluded finding) 72: for (uint256 index = 0; index < _nfts.length; index++) { /// @audit (valid but excluded finding) 103: for (uint256 index = 0; index < _nfts.length; index++) { /// @audit (valid but excluded finding) 138: for (uint256 index = 0; index < _nftPairs.length; index++) { /// @audit (valid but excluded finding) 199: for (uint256 index = 0; index < _nftPairs.length; index++) { /// @audit (valid but excluded finding) 215: for (uint256 index = 0; index < _nftPairs.length; index++) { /// @audit (valid but excluded finding) 279: for (uint256 index = 0; index < _nfts.length; index++) { /// @audit (valid but excluded finding) 289: for (uint256 index = 0; index < _nftPairs.length; index++) { /// @audit (valid but excluded finding) 305: for (uint256 index = 0; index < _nftPairs.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol /// @audit (valid but excluded finding) 493: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol /// @audit (valid but excluded finding) 207: for (uint256 index = 0; index < tokenData.length; index++) { /// @audit (valid but excluded finding) 280: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol /// @audit (valid but excluded finding) 107: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol /// @audit (valid but excluded finding) 51: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 97: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/NToken.sol /// @audit (valid but excluded finding) 106: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 145: for (uint256 i = 0; i < ids.length; i++) {
File: paraspace-core/contracts/ui/WPunkGateway.sol /// @audit (valid but excluded finding) 82: for (uint256 i = 0; i < punkIndexes.length; i++) { /// @audit (valid but excluded finding) 109: for (uint256 i = 0; i < punkIndexes.length; i++) { /// @audit (valid but excluded finding) 113: for (uint256 i = 0; i < punkIndexes.length; i++) { /// @audit (valid but excluded finding) 136: for (uint256 i = 0; i < punkIndexes.length; i++) { /// @audit (valid but excluded finding) 174: for (uint256 i = 0; i < punkIndexes.length; i++) {
bool
s for storage incurs overhead// Booleans are more expensive than uint256 or any type that takes up a full // word because each write operation emits an extra SLOAD to first read the // slot's contents, replace the bits taken up by the boolean, and then write // back. This is the compiler's defense against contract upgrades and // pointer aliasing, and it cannot be disabled.
https://github.com/OpenZeppelin/openzeppelin-contracts/blob/58f635312aa21f947cae5f8578638a85aa2519f5/contracts/security/ReentrancyGuard.sol#L23-L27
Use uint256(1)
and uint256(2)
for true/false to avoid a Gwarmaccess (100 gas) for the extra SLOAD, and to avoid Gsset (20000 gas) when changing from false
to true
, after having been true
in the past
There is 1 instance of this issue:
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol /// @audit (valid but excluded finding) 73: bool public immutable ATOMIC_PRICING;
> 0
costs more gas than != 0
when used on a uint
in a require()
statementThis change saves 6 gas per instance. The optimization works until solidity version 0.8.13 where there is a regression in gas costs.
There is 1 instance of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit (valid but excluded finding) 356: require(_twap > 0, "NFTOracle: price should be more than 0");
++i
costs less gas than i++
, especially when it's used in for
-loops (--i
/i--
too)Saves 5 gas per loop
There are 57 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit (valid but excluded finding) 229: for (uint256 i = 0; i < _assets.length; i++) { /// @audit (valid but excluded finding) 291: for (uint256 i = 0; i < _assets.length; i++) { /// @audit (valid but excluded finding) 321: for (uint256 i = 0; i < _feeders.length; i++) { /// @audit (valid but excluded finding) 413: for (uint256 i = 0; i < feederSize; i++) { /// @audit (valid but excluded finding) 421: validNum++; /// @audit (valid but excluded finding) 449: i++;
File: paraspace-core/contracts/misc/ParaSpaceOracle.sol /// @audit (valid but excluded finding) 95: for (uint256 i = 0; i < assets.length; i++) { /// @audit (valid but excluded finding) 197: for (uint256 i = 0; i < assets.length; i++) {
File: paraspace-core/contracts/misc/UniswapV3OracleWrapper.sol /// @audit (valid but excluded finding) 193: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 210: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/libraries/logic/FlashClaimLogic.sol /// @audit (valid but excluded finding) 38: for (i = 0; i < params.nftTokenIds.length; i++) { /// @audit (valid but excluded finding) 56: for (i = 0; i < params.nftTokenIds.length; i++) {
File: paraspace-core/contracts/protocol/libraries/logic/GenericLogic.sol /// @audit (valid but excluded finding) 371: for (uint256 index = 0; index < totalBalance; index++) { /// @audit (valid but excluded finding) 466: for (uint256 index = 0; index < totalBalance; index++) {
File: paraspace-core/contracts/protocol/libraries/logic/MarketplaceLogic.sol /// @audit (valid but excluded finding) 177: for (uint256 i = 0; i < marketplaceIds.length; i++) { /// @audit (valid but excluded finding) 284: for (uint256 i = 0; i < marketplaceIds.length; i++) { /// @audit (valid but excluded finding) 382: for (uint256 i = 0; i < params.orderInfo.consideration.length; i++) { /// @audit (valid but excluded finding) 470: for (uint256 i = 0; i < params.orderInfo.offer.length; i++) {
File: paraspace-core/contracts/protocol/libraries/logic/PoolLogic.sol /// @audit (valid but excluded finding) 58: for (uint16 i = 0; i < params.reservesCount; i++) { /// @audit (valid but excluded finding) 104: for (uint256 i = 0; i < assets.length; i++) {
File: paraspace-core/contracts/protocol/libraries/logic/SupplyLogic.sol /// @audit (valid but excluded finding) 184: for (uint256 index = 0; index < params.tokenData.length; index++) { /// @audit (valid but excluded finding) 195: for (uint256 index = 0; index < params.tokenData.length; index++) {
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol /// @audit (valid but excluded finding) 131: for (uint256 index = 0; index < amount; index++) { /// @audit (valid but excluded finding) 212: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 465: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 910: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 1034: for (uint256 i = 0; i < params.nftTokenIds.length; i++) {
File: paraspace-core/contracts/protocol/pool/PoolApeStaking.sol /// @audit (valid but excluded finding) 72: for (uint256 index = 0; index < _nfts.length; index++) { /// @audit (valid but excluded finding) 103: for (uint256 index = 0; index < _nfts.length; index++) { /// @audit (valid but excluded finding) 138: for (uint256 index = 0; index < _nftPairs.length; index++) { /// @audit (valid but excluded finding) 164: actualTransferAmount++; /// @audit (valid but excluded finding) 172: for (uint256 index = 0; index < actualTransferAmount; index++) { /// @audit (valid but excluded finding) 199: for (uint256 index = 0; index < _nftPairs.length; index++) { /// @audit (valid but excluded finding) 215: for (uint256 index = 0; index < _nftPairs.length; index++) { /// @audit (valid but excluded finding) 279: for (uint256 index = 0; index < _nfts.length; index++) { /// @audit (valid but excluded finding) 289: for (uint256 index = 0; index < _nftPairs.length; index++) { /// @audit (valid but excluded finding) 305: for (uint256 index = 0; index < _nftPairs.length; index++) {
File: paraspace-core/contracts/protocol/pool/PoolCore.sol /// @audit (valid but excluded finding) 634: for (uint256 i = 0; i < reservesListCount; i++) { /// @audit (valid but excluded finding) 638: droppedReservesCount++;
File: paraspace-core/contracts/protocol/pool/PoolParameters.sol /// @audit (valid but excluded finding) 134: ps._reservesCount++;
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol /// @audit (valid but excluded finding) 493: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol /// @audit (valid but excluded finding) 213: for (uint256 index = 0; index < totalBalance; index++) {
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol /// @audit (valid but excluded finding) 207: for (uint256 index = 0; index < tokenData.length; index++) { /// @audit (valid but excluded finding) 234: collateralizedTokens++; /// @audit (valid but excluded finding) 280: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 304: balanceToBurn++; /// @audit (valid but excluded finding) 313: burntCollateralizedTokens++;
File: paraspace-core/contracts/protocol/tokenization/NTokenApeStaking.sol /// @audit (valid but excluded finding) 107: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/NTokenMoonBirds.sol /// @audit (valid but excluded finding) 51: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 97: for (uint256 index = 0; index < tokenIds.length; index++) {
File: paraspace-core/contracts/protocol/tokenization/NToken.sol /// @audit (valid but excluded finding) 106: for (uint256 index = 0; index < tokenIds.length; index++) { /// @audit (valid but excluded finding) 145: for (uint256 i = 0; i < ids.length; i++) {
File: paraspace-core/contracts/ui/WPunkGateway.sol /// @audit (valid but excluded finding) 82: for (uint256 i = 0; i < punkIndexes.length; i++) { /// @audit (valid but excluded finding) 109: for (uint256 i = 0; i < punkIndexes.length; i++) { /// @audit (valid but excluded finding) 113: for (uint256 i = 0; i < punkIndexes.length; i++) { /// @audit (valid but excluded finding) 136: for (uint256 i = 0; i < punkIndexes.length; i++) { /// @audit (valid but excluded finding) 174: for (uint256 i = 0; i < punkIndexes.length; i++) {
private
rather than public
for constants, saves gasIf 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
There are 8 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit (valid but excluded finding) 70: bytes32 public constant UPDATER_ROLE = keccak256("UPDATER_ROLE");
File: paraspace-core/contracts/protocol/libraries/logic/LiquidationLogic.sol /// @audit (valid but excluded finding) 57: uint256 public constant MAX_LIQUIDATION_CLOSE_FACTOR = 1e4; /// @audit (valid but excluded finding) 64: uint256 public constant CLOSE_FACTOR_HF_THRESHOLD = 0.95e18;
File: paraspace-core/contracts/protocol/libraries/logic/ValidationLogic.sol /// @audit (valid but excluded finding) 45: uint256 public constant REBALANCE_UP_LIQUIDITY_RATE_THRESHOLD = 0.9e4; /// @audit (valid but excluded finding) 49 uint256 public constant MINIMUM_HEALTH_FACTOR_LIQUIDATION_THRESHOLD = 50: 0.95e18; /// @audit (valid but excluded finding) 56: uint256 public constant HEALTH_FACTOR_LIQUIDATION_THRESHOLD = 1e18;
File: paraspace-core/contracts/protocol/pool/PoolCore.sol /// @audit (valid but excluded finding) 53: uint256 public constant POOL_REVISION = 1;
File: paraspace-core/contracts/protocol/tokenization/NToken.sol /// @audit (valid but excluded finding) 32: uint256 public constant NTOKEN_REVISION = 0x1;
<x> / 2
is the same as <x> >> 1
. While the compiler uses the SHR
opcode to accomplish both, the version that uses division incurs an overhead of 20 gas due to JUMP
s to and from a compiler utility function that introduces checks which can be avoided by using unchecked {}
around the division by two
There are 2 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit (valid but excluded finding) 429: return (true, validPriceList[validNum / 2]); /// @audit (valid but excluded finding) 440: uint256 pivot = arr[uint256(left + (right - left) / 2)];
revert()
/require()
strings to save gasCustom errors are available from solidity version 0.8.4. Custom errors save ~50 gas each time they're hit by avoiding having to allocate and store the revert string. Not defining the strings also save deployment gas
There are 15 instances of this issue:
File: paraspace-core/contracts/misc/NFTFloorOracle.sol /// @audit (valid but excluded finding) 118: require(_isAssetExisted(_asset), "NFTOracle: asset not existed"); /// @audit (valid but excluded finding) 123: require(!_isAssetExisted(_asset), "NFTOracle: asset existed"); /// @audit (valid but excluded finding) 128: require(_isFeederExisted(_feeder), "NFTOracle: feeder not existed"); /// @audit (valid but excluded finding) 133: require(!_isFeederExisted(_feeder), "NFTOracle: feeder existed"); /// @audit (valid but excluded finding) 207: require(dataValidity, "NFTOracle: invalid price data"); /// @audit (valid but excluded finding) 267: require(!_paused, "NFTOracle: nft price feed paused"); /// @audit (valid but excluded finding) 356: require(_twap > 0, "NFTOracle: price should be more than 0");
File: paraspace-core/contracts/protocol/tokenization/base/MintableIncentivizedERC721.sol /// @audit (valid but excluded finding) 193: require(to != owner, "ERC721: approval to old owner");
File: paraspace-core/contracts/protocol/tokenization/libraries/ApeStakingLogic.sol /// @audit (valid but excluded finding) 72: require(oldValue != incentive, "Same Value");
File: paraspace-core/contracts/protocol/tokenization/libraries/MintableERC721Logic.sol /// @audit (valid but excluded finding) 92: require(to != address(0), "ERC721: transfer to the zero address"); /// @audit (valid but excluded finding) 164: require(owner == sender, "not owner"); /// @audit (valid but excluded finding) 199: require(to != address(0), "ERC721: mint to the zero address"); /// @audit (valid but excluded finding) 283: require(owner == user, "not the owner of Ntoken"); /// @audit (valid but excluded finding) 363: require(owner != operator, "ERC721: approve to caller");
File: paraspace-core/contracts/protocol/tokenization/NTokenUniswapV3.sol /// @audit (valid but excluded finding) 146: require(success, "ETH_TRANSFER_FAILED");
#0 - c4-judge
2023-01-25T16:35:50Z
dmvt marked the issue as grade-a
#1 - c4-judge
2023-01-26T12:14:31Z
dmvt marked the issue as selected for report