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: 23/106
Findings: 5
Award: $1,155.32
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: carlitox477
Also found by: 0xDave, Jeiwan, Rolezn, __141345__, imare, nicobevi
102.8853 USDC - $102.89
Judge has assessed an item in Issue #80 as M risk. The relevant finding follows:
[LOW‑10] getPrice and combine will not work if expirationPeriod == 0 The following conditions will fail if expirationPeriod is set to 0. There is currently no limit that it cannot be set to 0.
Proof Of Concept 243: require( 244: (block.number - updatedAt) <= config.expirationPeriod, 245: "NFTOracle: asset price expired" 246: ); https://github.com/code-423n4/2022-11-paraspace/tree/main/paraspace-core/contracts/misc/NFTFloorOracle.sol#L243-L246
419: if (diffBlock <= config.expirationPeriod) { 420: validPriceList[validNum] = priceInfo.twap; 421: validNum++; 422: } https://github.com/code-423n4/2022-11-paraspace/tree/main/paraspace-core/contracts/misc/NFTFloorOracle.sol#L419-L422
#0 - c4-judge
2023-01-25T11:10:02Z
dmvt marked the issue as duplicate of #28
#1 - c4-judge
2023-01-25T11:10:08Z
dmvt marked the issue as partial-50
🌟 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_
18.3064 USDC - $18.31
According to Chainlink’s documentation, the latestAnswer
function is deprecated.
This function might suddenly stop working if Chainlink stop supporting deprecated APIs and the old API can return stale data.
The old API can return stale data.
It is indicated in the docs that the contract: "Use of Chainlink Aggregators as first source of price"
126: IEACAggregatorProxy source = IEACAggregatorProxy(assetsSources[asset]); 127: if (address(source) != address(0)) { 128: price = uint256(source.latestAnswer()); 129: }
Use the latestRoundData
function to get the price instead. Add checks on the return data with proper revert messages if the price is stale or the round is uncomplet.
See docs for reference: https://docs.chain.link/docs/price-feeds-api-reference/
#0 - JeffCX
2022-12-18T03:30:30Z
#1 - c4-judge
2022-12-20T17:44:29Z
dmvt marked the issue as duplicate of #5
#2 - c4-judge
2023-01-09T16:32:08Z
dmvt marked the issue as partial-50
#3 - c4-judge
2023-01-23T15:57:30Z
dmvt marked the issue as satisfactory
🌟 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
882.5476 USDC - $882.55
Issue | Contexts | |
---|---|---|
LOW‑1 | Missing Checks for Address(0x0) | 7 |
LOW‑2 | Unused receive() Function Will Lock Ether In Contract | 1 |
LOW‑3 | Missing Contract-existence Checks Before Low-level Calls | 1 |
LOW‑4 | Contracts are not using their OZ Upgradeable counterparts | 87 |
LOW‑5 | Low Level Calls With Solidity Version 0.8.14 Can Result In Optimiser Bug | 1 |
LOW‑6 | Missing parameter validation in constructor | 12 |
LOW‑7 | Prevent division by 0 | 2 |
LOW‑8 | Upgrade OpenZeppelin Contract Dependency | 2 |
LOW‑9 | The nonReentrant modifier should occur before all other modifiers | 25 |
LOW‑10 | getPrice and combine will not work if expirationPeriod == 0 | 2 |
Total: 140 contexts over 10 issues
Issue | Contexts | |
---|---|---|
NC‑1 | Critical Changes Should Use Two-step Procedure | 30 |
NC‑2 | Use a more recent version of Solidity | 32 |
NC‑3 | Event Is Missing Indexed Fields | 2 |
NC‑4 | Constants Should Be Defined Rather Than Using Magic Numbers | 1 |
NC‑5 | Missing event for critical parameter change | 8 |
NC‑6 | Implementation contract may not be initialized | 16 |
NC‑7 | Use of Block.Timestamp | 2 |
NC‑8 | Non-usage of specific imports | 20 |
NC‑9 | Expressions for constant values such as a call to keccak256() , should use immutable rather than constant | 3 |
NC‑10 | Use bytes.concat() | 12 |
NC‑11 | Open TODOs | 3 |
NC‑12 | No need to initialize uints to zero | 10 |
NC‑13 | Use delete to Clear Variables | 4 |
NC‑14 | Duplicate imports | 4 |
NC‑15 | Missing NATSPEC documentation | 4 |
NC‑16 | Insufficient NATSPEC documentation | 4 |
Total: 144 contexts over 14 issues
Lack of zero-address validation on address parameters may lead to transaction reverts, waste gas, require resubmission of transactions and may even force contract redeployments in certain cases within the protocol.
70: function setAddress: address newAddress
142: function setPriceOracle: address newPriceOracle
158: function setACLManager: address newAclManager
170: function setACLAdmin: address newAclAdmin
182: function setPriceOracleSentinel: address newPriceOracleSentinel
224: function setProtocolDataProvider: address newDataProvider
235: function setWETH: address newWETH
Consider adding explicit zero-address validation on input parameters of address type.
receive()
Function Will Lock Ether In ContractIf the intention is for the Ether to be used, the function should call another function, otherwise it should revert
149: receive() external payable {}
The function should call another function, otherwise it should revert
Low-level calls return success if there is no code present at the specified address.
145: (bool success, ) = to.call{value: value}(new bytes(0));
In addition to the zero-address checks, add a check to verify that <address>.code.length > 0
The non-upgradeable standard version of OpenZeppelin’s library are inherited / used by the contracts. It would be safer to use the upgradeable versions of the library contracts to avoid unexpected behaviour.
4: import "../dependencies/openzeppelin/contracts/AccessControl.sol"; 5: import "../dependencies/openzeppelin/upgradeability/Initializable.sol";
14: import {IERC20Detailed} from "../dependencies/openzeppelin/contracts/IERC20Detailed.sol";
12: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol"; 13: import {IERC1271} from "../../dependencies/openzeppelin/contracts/IERC1271.sol";
12: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol"; 13: import {IERC1271} from "../../dependencies/openzeppelin/contracts/IERC1271.sol";
13: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol"; 14: import {IERC1271} from "../../dependencies/openzeppelin/contracts/IERC1271.sol";
4: import {Ownable} from "../../dependencies/openzeppelin/contracts/Ownable.sol"; 10: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol";
5: import {SafeCast} from "../../../dependencies/openzeppelin/contracts/SafeCast.sol"; 6: import {IERC20} from "../../../dependencies/openzeppelin/contracts/IERC20.sol";
4: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol";
4: import {IERC20} from "../../../dependencies/openzeppelin/contracts/IERC20.sol"; 5: import {IERC721Enumerable} from "../../../dependencies/openzeppelin/contracts/IERC721Enumerable.sol"; 6: import {Math} from "../../../dependencies/openzeppelin/contracts/Math.sol";
4: import {IERC20} from "../../../dependencies/openzeppelin/contracts//IERC20.sol"; 8: import {Math} from "../../../dependencies/openzeppelin/contracts/Math.sol"; 17: import {Address} from "../../../dependencies/openzeppelin/contracts/Address.sol";
4: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol"; 16: import {SafeERC20} from "../../../dependencies/openzeppelin/contracts/SafeERC20.sol"; 17: import {IERC20} from "../../../dependencies/openzeppelin/contracts/IERC20.sol"; 18: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol"; 26: import {Address} from "../../../dependencies/openzeppelin/contracts/Address.sol";
5: import {Address} from "../../../dependencies/openzeppelin/contracts/Address.sol"; 6: import {IERC20} from "../../../dependencies/openzeppelin/contracts/IERC20.sol"; 7: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol";
4: import {IERC20} from "../../../dependencies/openzeppelin/contracts/IERC20.sol"; 5: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol";
4: import {IERC20} from "../../../dependencies/openzeppelin/contracts/IERC20.sol"; 5: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol"; 6: import {Address} from "../../../dependencies/openzeppelin/contracts/Address.sol"; 25: import {SafeCast} from "../../../dependencies/openzeppelin/contracts/SafeCast.sol";
4: import {IERC20} from "../../dependencies/openzeppelin/contracts/IERC20.sol";
22: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol"; 23: import {IERC721Receiver} from "../../dependencies/openzeppelin/contracts/IERC721Receiver.sol";
15: import {IERC20} from "../../dependencies/openzeppelin/contracts/IERC20.sol"; 16: import {SafeERC20} from "../../dependencies/openzeppelin/contracts/SafeERC20.sol"; 25: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol"; 26: import {IERC721Receiver} from "../../dependencies/openzeppelin/contracts/IERC721Receiver.sol";
21: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol"; 22: import {IERC721Receiver} from "../../dependencies/openzeppelin/contracts/IERC721Receiver.sol";
4: import {IERC20} from "../../dependencies/openzeppelin/contracts/IERC20.sol"; 5: import {IERC721} from "../../dependencies/openzeppelin/contracts/IERC721.sol"; 6: import {IERC1155} from "../../dependencies/openzeppelin/contracts/IERC1155.sol"; 7: import {IERC721Metadata} from "../../dependencies/openzeppelin/contracts/IERC721Metadata.sol"; 8: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol"; 10: import {SafeCast} from "../../dependencies/openzeppelin/contracts/SafeCast.sol"; 20: import {SafeERC20} from "../../dependencies/openzeppelin/contracts/SafeERC20.sol";
7: import {IERC20} from "../../dependencies/openzeppelin/contracts/IERC20.sol"; 8: import {IERC721} from "../../dependencies/openzeppelin/contracts/IERC721.sol";
4: import {IERC20} from "../../dependencies/openzeppelin/contracts/IERC20.sol"; 5: import {IERC721} from "../../dependencies/openzeppelin/contracts/IERC721.sol"; 6: import {IERC1155} from "../../dependencies/openzeppelin/contracts/IERC1155.sol"; 7: import {IERC721Metadata} from "../../dependencies/openzeppelin/contracts/IERC721Metadata.sol"; 8: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol"; 10: import {SafeCast} from "../../dependencies/openzeppelin/contracts/SafeCast.sol";
4: import {IERC20} from "../../dependencies/openzeppelin/contracts/IERC20.sol"; 5: import {IERC721} from "../../dependencies/openzeppelin/contracts/IERC721.sol"; 6: import {IERC1155} from "../../dependencies/openzeppelin/contracts/IERC1155.sol"; 7: import {IERC721Metadata} from "../../dependencies/openzeppelin/contracts/IERC721Metadata.sol"; 8: import {Address} from "../../dependencies/openzeppelin/contracts/Address.sol"; 9: import {SafeERC20} from "../../dependencies/openzeppelin/contracts/SafeERC20.sol"; 10: import {SafeCast} from "../../dependencies/openzeppelin/contracts/SafeCast.sol";
4: import {Context} from "../../../dependencies/openzeppelin/contracts/Context.sol"; 5: import {Strings} from "../../../dependencies/openzeppelin/contracts/Strings.sol"; 6: import {Address} from "../../../dependencies/openzeppelin/contracts/Address.sol"; 7: import {IERC165} from "../../../dependencies/openzeppelin/contracts/IERC165.sol"; 8: import {IERC721Metadata} from "../../../dependencies/openzeppelin/contracts/IERC721Metadata.sol"; 9: import {IERC721Receiver} from "../../../dependencies/openzeppelin/contracts/IERC721Receiver.sol"; 10: import {IERC721Enumerable} from "../../../dependencies/openzeppelin/contracts/IERC721Enumerable.sol"; 13: import {SafeCast} from "../../../dependencies/openzeppelin/contracts/SafeCast.sol"; 21: import {ReentrancyGuard} from "../../../dependencies/openzeppelin/contracts/ReentrancyGuard.sol";
4: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol"; 5: import {SafeERC20} from "../../../dependencies/openzeppelin/contracts/SafeERC20.sol"; 6: import {IERC20} from "../../../dependencies/openzeppelin/contracts/IERC20.sol"; 10: import {Math} from "../../../dependencies/openzeppelin/contracts/Math.sol";
4: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol"; 5: import {SafeERC20} from "../../../dependencies/openzeppelin/contracts/SafeERC20.sol"; 6: import {IERC20} from "../../../dependencies/openzeppelin/contracts/IERC20.sol";
4: import {OwnableUpgradeable} from "../dependencies/openzeppelin/upgradeability/OwnableUpgradeable.sol"; 11: import {IERC721} from "../dependencies/openzeppelin/contracts/IERC721.sol"; 12: import {IERC721Receiver} from "../dependencies/openzeppelin/contracts/IERC721Receiver.sol"; 17: import {ReentrancyGuard} from "../dependencies/openzeppelin/contracts/ReentrancyGuard.sol";
Where applicable, use the contracts from @openzeppelin/contracts-upgradeable
instead of @openzeppelin/contracts
.
The project contracts in scope are using low level calls with solidity version before 0.8.14 which can result in optimizer bug. https://medium.com/certora/overly-optimistic-optimizer-certora-bug-disclosure-2101e3f7994d
Simliar findings in Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#5-low-level-calls-with-solidity-version-0814-can-result-in-optimiser-bug
POC can be found in the above medium reference url.
Functions that execute low level calls in contracts with solidity version under 0.8.14
643: assembly { mstore(reservesList, sub(reservesListCount, droppedReservesCount)) }
Consider upgrading to at least solidity v0.8.15.
constructor
Some parameters of constructors are not checked for invalid values.
53: address fallbackOracle 54: address baseCurrency
24: address _factory 25: address _manager 26: address _addressProvider
45: string memory marketId 45: address owner
32: address apeCoinStaking
16: address apeCoinStaking
16: address apeCoinStaking
44: address _punk 45: address _wpunk 46: address _pool
Validate the parameters.
On several locations in the code precautions are not being taken for not dividing by 0, this will revert the code.
These functions can be called with 0 value in the input, this value is not checked for being bigger than 0, that means in some scenarios this can potentially trigger a division by zero.
349: return userTotalDebt / assetUnit
531: return (balance / assetUnit
Recommend making sure division by 0 won’t occur by checking the variables beforehand and handling this edge case.
An outdated OZ version is used (which has known vulnerabilities, see: https://github.com/OpenZeppelin/openzeppelin-contracts/security/advisories).
"@openzeppelin/contracts": "4.2.0"
https://github.com/code-423n4/2022-11-paraspace/tree/main/paraspace-core/package.json#L31
"@openzeppelin/contracts-upgradeable": "4.2.0"
https://github.com/code-423n4/2022-11-paraspace/tree/main/paraspace-core/package.json#L32
Update OpenZeppelin Contracts Usage in package.json
nonReentrant
modifier should occur before all other modifiersCurrently the nonReentrant
modifier is not the first to occur, it should occur before all other modifiers.
79: function mint( address onBehalfOf, DataTypes.ERC721SupplyParams[] calldata tokenData ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
87: function burn( address from, address receiverOfUnderlying, uint256[] calldata tokenIds ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
119: function transferOnLiquidation( address from, address to, uint256 value ) external onlyPool nonReentrant {
199: function transferUnderlyingTo(address target, uint256 tokenId) external virtual override onlyPool nonReentrant {
214: function handleRepayment(address user, uint256 amount) external virtual override onlyPool nonReentrant {
102: function burn( address from, address receiverOfUnderlying, uint256[] calldata tokenIds ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
159: function unstakePositionAndRepay(uint256 tokenId, address incentiveReceiver) external onlyPool nonReentrant {
26: function depositApeCoin(ApeCoinStaking.SingleNft[] calldata _nfts) external onlyPool nonReentrant {
39: function claimApeCoin(uint256[] calldata _nfts, address _recipient) external onlyPool nonReentrant {
52: function withdrawApeCoin( ApeCoinStaking.SingleNft[] calldata _nfts, address _recipient ) external onlyPool nonReentrant {
66: function depositBAKC(ApeCoinStaking.PairNftWithAmount[] calldata _nftPairs) external onlyPool nonReentrant {
82: function claimBAKC( ApeCoinStaking.PairNft[] calldata _nftPairs, address _recipient ) external onlyPool nonReentrant {
97: function withdrawBAKC( ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, address _apeRecipient ) external onlyPool nonReentrant {
26: function depositApeCoin(ApeCoinStaking.SingleNft[] calldata _nfts) external onlyPool nonReentrant {
39: function claimApeCoin(uint256[] calldata _nfts, address _recipient) external onlyPool nonReentrant {
52: function withdrawApeCoin( ApeCoinStaking.SingleNft[] calldata _nfts, address _recipient ) external onlyPool nonReentrant {
66: function depositBAKC(ApeCoinStaking.PairNftWithAmount[] calldata _nftPairs) external onlyPool nonReentrant {
82: function claimBAKC( ApeCoinStaking.PairNft[] calldata _nftPairs, address _recipient ) external onlyPool nonReentrant {
97: function withdrawBAKC( ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, address _apeRecipient ) external onlyPool nonReentrant {
40: function burn( address from, address receiverOfUnderlying, uint256[] calldata tokenIds ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
123: function decreaseUniswapV3Liquidity( address user, uint256 tokenId, uint128 liquidityDecrease, uint256 amount0Min, uint256 amount1Min, bool receiveEthAsWeth ) external onlyPool nonReentrant {
458: function setIsUsedAsCollateral( uint256 tokenId, bool useAsCollateral, address sender ) external virtual override onlyPool nonReentrant returns (bool) {
474: function batchSetIsUsedAsCollateral( uint256[] calldata tokenIds, bool useAsCollateral, address sender ) external virtual override onlyPool nonReentrant returns ( uint256 oldCollateralizedBalance, uint256 newCollateralizedBalance ) {
529: function startAuction(uint256 tokenId) external virtual override onlyPool nonReentrant {
540: function endAuction(uint256 tokenId) external virtual override onlyPool nonReentrant {
Re-sort modifiers so that the nonReentrant
modifier occurs first.
getPrice
and combine
will not work if expirationPeriod
== 0The following conditions will fail if expirationPeriod
is set to 0. There is currently no limit that it cannot be set to 0.
243: require( 244: (block.number - updatedAt) <= config.expirationPeriod, 245: "NFTOracle: asset price expired" 246: );
419: if (diffBlock <= config.expirationPeriod) { 420: validPriceList[validNum] = priceInfo.twap; 421: validNum++; 422: }
The critical procedures should be two step process.
See similar findings in previous Code4rena contests for reference: https://code4rena.com/reports/2022-06-illuminate/#2-critical-changes-should-use-two-step-procedure
175: function setConfig(uint128 expirationPeriod, uint128 maxPriceDeviation)
183: function setPause(address _asset, bool _flag)
195: function setPrice(address _asset, uint256 _twap)
221: function setMultiplePrices(
66: function setAssetSources(
74: function setFallbackOracle(address fallbackOracle)
56: function setMarketId(string memory newMarketId)
70: function setAddress(bytes32 id, address newAddress)
81: function setAddressAsProxy(bytes32 id, address newImplementationAddress)
121: function setPoolConfiguratorImpl(address newPoolConfiguratorImpl)
142: function setPriceOracle(address newPriceOracle)
158: function setACLManager(address newAclManager) external override onlyOwner {
170: function setACLAdmin(address newAclAdmin) external override onlyOwner {
182: function setPriceOracleSentinel(address newPriceOracleSentinel)
224: function setProtocolDataProvider(address newDataProvider)
235: function setWETH(address newWETH) external override onlyOwner {
242: function setMarketplace(
413: function setSApeUseAsCollateral(address user) internal {
378: function setUserUseERC20AsCollateral(address asset, bool useAsCollateral)
397: function setUserUseERC721AsCollateral(
151: function setReserveInterestRateStrategyAddress(
166: function setReserveAuctionStrategyAddress(
181: function setConfiguration(
206: function setAuctionRecoveryHealthFactor(uint64 value)
263: function setAuctionValidityTime(address user)
136: function setUnstakeApeIncentive(uint256 incentive) external onlyPoolAdmin {
131: function setIncentivesController(IRewardController controller)
142: function setBalanceLimit(uint64 limit) external onlyPoolAdmin {
224: function setApprovalForAll(address operator, bool approved)
458: function setIsUsedAsCollateral(
Lack of two-step procedure for critical operations leaves them error-prone. Consider adding two step procedure on the critical functions.
<a href="https://blog.soliditylang.org/2022/02/16/solidity-0.8.12-release-announcement/">0.8.12</a>: string.concat() instead of abi.encodePacked(<str>,<str>)
<a href="https://blog.soliditylang.org/2022/03/16/solidity-0.8.13-release-announcement/">0.8.13</a>: Ability to use using for with a list of free functions
<a href="https://blog.soliditylang.org/2022/05/18/solidity-0.8.14-release-announcement/">0.8.14</a>:
ABI Encoder: When ABI-encoding values from calldata that contain nested arrays, correctly validate the nested array length against calldatasize() in all cases. Override Checker: Allow changing data location for parameters only when overriding external functions.
<a href="https://blog.soliditylang.org/2022/06/15/solidity-0.8.15-release-announcement/">0.8.15</a>:
Code Generation: Avoid writing dirty bytes to storage when copying bytes arrays. Yul Optimizer: Keep all memory side-effects of inline assembly blocks.
<a href="https://blog.soliditylang.org/2022/08/08/solidity-0.8.16-release-announcement/">0.8.16</a>:
Code Generation: Fix data corruption that affected ABI-encoding of calldata values represented by tuples: structs at any nesting level; argument lists of external functions, events and errors; return value lists of external functions. The 32 leading bytes of the first dynamically-encoded value in the tuple would get zeroed when the last component contained a statically-encoded array.
<a href="https://blog.soliditylang.org/2022/09/08/solidity-0.8.17-release-announcement/">0.8.17</a>:
Yul Optimizer: Prevent the incorrect removal of storage writes before calls to Yul functions that conditionally terminate the external EVM call.
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
pragma solidity 0.8.10;
Consider updating to a more recent solidity version.
Index 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.
event OracleConfigSet(uint128 expirationPeriod, uint128 maxPriceDeviation);
event UnstakeApeIncentiveUpdated(uint256 oldValue, uint256 newValue);
It is not clear what the value 30
stands for and what's the rational behind its value.
133: ApeStakingLogic.executeSetUnstakeApeIncentive(dataStorage, 30);
When changing state variables events are not emitted. Emitting events allows monitoring activities with off-chain monitoring tools.
151: function setReserveInterestRateStrategyAddress(
166: function setReserveAuctionStrategyAddress(
181: function setConfiguration(
206: function setAuctionRecoveryHealthFactor(uint64 value)
263: function setAuctionValidityTime(address user)
131: function setIncentivesController(IRewardController controller)
142: function setBalanceLimit(uint64 limit) external onlyPoolAdmin {
458: function setIsUsedAsCollateral(
OpenZeppelin recommends that the initializer modifier be applied to constructors. Per OZs Post implementation contract should be initialized to avoid potential griefs or exploits. https://forum.openzeppelin.com/t/uupsupgradeable-vulnerability-post-mortem/15680/5
49: constructor( IPoolAddressesProvider provider, address[] memory assets, address[] memory sources, address fallbackOracle, address baseCurrency, uint256 baseCurrencyUnit )
23: constructor( address _factory, address _manager, address _addressProvider )
45: constructor(string memory marketId, address owner)
50: constructor( uint256 maxPriceMultiplier, uint256 minExpPriceMultiplier, uint256 minPriceMultiplier, uint256 stepLinear, uint256 stepExp, uint256 tickLength )
51: constructor(IPoolAddressesProvider provider)
64: constructor(IPoolAddressesProvider provider)
62: constructor(IPoolAddressesProvider provider)
89: constructor(IPoolAddressesProvider provider)
43: constructor(IPool pool, bool atomic_pricing) MintableIncentivizedERC721( pool, "NTOKEN_IMPL", "NTOKEN_IMPL", atomic_pricing )
32: constructor(IPool pool, address apeCoinStaking) NToken(pool, false)
16: constructor(IPool pool, address apeCoinStaking) NTokenApeStaking(pool, apeCoinStaking)
16: constructor(IPool pool, address apeCoinStaking) NTokenApeStaking(pool, apeCoinStaking)
32: constructor(IPool pool) NToken(pool, false)
33: constructor(IPool pool) NToken(pool, true)
83: constructor( IPool pool, string memory name_, string memory symbol_, bool atomic_pricing )
43: constructor( address _punk, address _wpunk, address _pool )
Block timestamps have historically been used for a variety of applications, such as entropy for random numbers (see the Entropy Illusion for further details), locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts. References: SWC ID: 116
778: .calculateAuctionPriceMultiplier(startTime, block.timestamp);
285: userConfig.auctionValidityTime = block.timestamp;
Block timestamps should not be used for entropy or generating random numbers—i.e., they should not be the deciding factor (either directly or through some derivation) for winning a game or changing an important state.
Time-sensitive logic is sometimes required; e.g., for unlocking contracts (time-locking), completing an ICO after a few weeks, or enforcing expiry dates. It is sometimes recommended to use block.number and an average block time to estimate times; with a 10 second block time, 1 week equates to approximately, 60480 blocks. Thus, specifying a block number at which to change a contract state can be more secure, as miners are unable to easily manipulate the block number.
The current form of relative path import is not recommended for use because it can unpredictably pollute the namespace. Instead, the Solidity docs recommend specifying imported symbols explicitly. https://docs.soliditylang.org/en/v0.8.15/layout-of-source-files.html#importing-other-source-files
A good example:
import {OwnableUpgradeable} from "openzeppelin-contracts-upgradeable/contracts/access/OwnableUpgradeable.sol"; import {SafeTransferLib} from "solmate/utils/SafeTransferLib.sol"; import {SafeCastLib} from "solmate/utils/SafeCastLib.sol"; import {ERC20} from "solmate/tokens/ERC20.sol"; import {IProducer} from "src/interfaces/IProducer.sol"; import {GlobalState, UserState} from "src/Common.sol";
4: import "../dependencies/openzeppelin/contracts/AccessControl.sol"; 5: import "../dependencies/openzeppelin/upgradeability/Initializable.sol"; 6: import "./interfaces/INFTFloorOracle.sol";
10: import "../../../interfaces/INTokenApeStaking.sol";
30: import "../../../interfaces/INTokenApeStaking.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";
12: import "../../interfaces/INTokenApeStaking.sol";
7: import "../../../interfaces/IPool.sol"; 11: import "./MintableERC721Logic.sol";
7: import "../../../interfaces/IRewardController.sol"; 8: import "../../libraries/types/DataTypes.sol"; 9: import "../../../interfaces/IPool.sol";
Use specific imports syntax per solidity docs recommendation.
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.
70: bytes32 public constant UPDATER_ROLE = keccak256("UPDATER_ROLE");
16: bytes32 constant POOL_STORAGE_POSITION = bytes32(uint256(keccak256("paraspace.proxy.pool.storage")) - 1);
23: bytes32 constant APE_STAKING_DATA_STORAGE_POSITION = bytes32( uint256(keccak256("paraspace.proxy.ntoken.apestaking.storage")) - 1
bytes.concat()
Solidity version 0.8.4 introduces bytes.concat()
(vs abi.encodePacked(<bytes>,<bytes>)
)
63: orderInfo.id = abi.encodePacked(makerAsk.r, makerAsk.s, makerAsk.v) 86: bytes memory data = abi.encodePacked(selector, params)
99: bytes memory data = abi.encodePacked(selector, params) 115: bytes memory data = abi.encodePacked(selector, params) 99: bytes memory data = abi.encodePacked(selector, params) 115: bytes memory data = abi.encodePacked(selector, params)
75: orderInfo.id = abi.encodePacked(order.r, order.s, order.v) 94: bytes memory data = abi.encodePacked(selector, params)
1076: keccak256(abi.encodePacked(params.orderInfo.id) 1077: keccak256(abi.encodePacked(params.credit.orderId) 1116: abi.encodePacked( "Credit(address token,uint256 amount,bytes orderId) 1128: keccak256(abi.encodePacked(credit.orderId)
Use bytes.concat()
and upgrade to at least Solidity version 0.8.4 if required.
An open TODO is present. It is recommended to avoid open TODOs as they may indicate programming errors that still need to be fixed.
238: // TODO using bit shifting for the 2^96
59: makerAsk.price, // TODO: take minPercentageToAsk into account
442: // TODO: support PToken
There is no need to initialize uint
variables to zero as their default value is 0
411: uint256 validNum = 0;
125: uint256 price = 0;
208: uint256 sum = 0;
380: uint256 price = 0;
71: uint256 amountToWithdraw = 0;
135: uint256 amountToWithdraw = 0; 137: uint256 actualTransferAmount = 0;
631: uint256 droppedReservesCount = 0;
205: uint64 collateralizedTokens = 0;
272: uint64 burntCollateralizedTokens = 0;
delete
to Clear Variablesdelete a
assigns the initial value for the type to a
. i.e. for integers it is equivalent to a = 0
, but it can also be used on arrays, where it assigns a dynamic array of length zero or a static array of the same length with all elements reset. For structs, it assigns a struct with all members reset. Similarly, it can also be used to set an address to zero address. It has no effect on whole mappings though (as the keys of mappings may be arbitrary and are generally unknown). However, individual keys and what they map to can be deleted: If a
is a mapping, then delete a[x]
will delete the value stored at x
.
The delete
key better conveys the intention and is also more idiomatic. Consider replacing assignments of zero with delete
statements.
380: price = 0; 410: downpayment = 0;
589: vars.ethLeft = 0;
123: reserve.accruedToTreasury = 0;
There are several occasions where there several imports of the same file
4: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol"; 18: import {IERC721} from "../../../dependencies/openzeppelin/contracts/IERC721.sol";
5: import {Errors} from "../libraries/helpers/Errors.sol"; 25: import {Errors} from "../libraries/helpers/Errors.sol";
5: import {Errors} from "../libraries/helpers/Errors.sol"; 28: import {Errors} from "../libraries/helpers/Errors.sol";
5: import {Errors} from "../libraries/helpers/Errors.sol"; 24: import {Errors} from "../libraries/helpers/Errors.sol";
25: function getAskOrderInfo(bytes memory params, address weth) 67: function getBidOrderInfo( 73: function matchAskWithTakerBid( 96: function matchBidWithTakerAsk(address, bytes calldata)
25: function getAskOrderInfo(bytes memory params, address weth) 60: function getBidOrderInfo( 93: function matchAskWithTakerBid( 109: function matchBidWithTakerAsk(address, bytes calldata) 124: function isBasicOrder(AdvancedOrder memory advancedOrder)
31: function getAskOrderInfo(bytes memory params, address weth) 79: function getBidOrderInfo( 88: function matchAskWithTakerBid( 104: function matchBidWithTakerAsk(address, bytes calldata)
261: function getFeederSize() public view returns (uint256) { 265: function _whenNotPaused(address _asset) internal view { 270: function _isAssetExisted(address _asset) internal view returns (bool) { 274: function _isFeederExisted(address _feeder) internal view returns (bool) { 278: function _addAsset(address _asset) 296: function _removeAsset(address _asset) 307: function _addFeeder(address _feeder) 326: function _removeFeeder(address _feeder) 351: function _checkValidity(address _asset, uint256 _twap) 376: function _finalizePrice(address _asset, uint256 _twap) internal { 388: function _addRawValue(address _asset, uint256 _twap) internal { 397: function _combine(address _asset, uint256 _twap) 432: function _quickSort(
138: function getTokenPrice(address asset, uint256 tokenId) 155: function getTokensPrices(address asset, uint256[] calldata tokenIds) 172: function getTokensPricesSum(address asset, uint256[] calldata tokenIds) 218: function _onlyAssetListingOrPoolAdmins() internal view {
399: function getLtvAndLTForUniswapV3( 450: function _getUserBalanceForUniswapV3( 535: function _getAssetPrice(address oracle, address currentReserveAddress) 543: function _getTokenPrice(
63: function executeBuyWithCredit( 160: function executeBatchBuyWithCredit( 229: function executeAcceptBidWithCre 267: function executeBatchAcceptBidWithCredit( 552: function _checkAllowance(address token, address operator) internal { 559: function _cache(DataTypes.ExecuteMarketplaceParams memory params) 569: function _refundETH(uint256 ethLeft) internal { 575: function _depositETH( 593: function _transferAndCollateralize(
214: function executeGetAssetLtvAndLT(
134: function executeSupplyERC721Base( 335: function executeWithdrawERC721( 396: function executeDecreaseUniswapV3Liquidity(
189: function validateWithdrawERC721( 442: function validateSetUseERC721AsCollateral( 801: function validateEndAuction( 1065: function validateBuyWithCredit( 1071: function validateAcceptBidWithCredit( 1092: function verifyCreditSignature( 1110: function hashCredit(DataTypes.Credit memory credit) 1133: function getDomainSeparator() internal view returns (bytes32) { 1155: function validateForUniswapV3(
90: function calculateAuctionPriceMultiplier( 101: function _calculateAuctionPriceMultiplierByTicks(uint256 ticks)
413: function setSApeUseAsCollateral(address user) internal { 428: function getUserHf(address user) internal view returns (uint256) { 443: function checkSApeIsNotPaused(DataTypes.PoolStorage storage ps)
237: function decreaseUniswapV3Liquidity( 397: function setUserUseERC721AsCollateral( 793: function onERC721Received(
251: function getAssetLtvAndLT(address asset, uint256 tokenId)
290: function _safeTransferFrom( 364: function _mintMultiple( 384: function _burnMultiple(address user, uint256[] calldata tokenIds)
95: function _burn( 127: function rescueERC20( 136: function rescueERC721( 151: function rescueERC1155( 168: function executeAirdrop( 271: function onERC721Received( 280: function onERC1155Received( 295: function onERC1155BatchReceived(
130: function initializeStakingData() internal { 136: function setUnstakeApeIncentive(uint256 incentive) external onlyPoolAdmin {
144: function _safeTransferETH(address to, uint256 value) internal {
80: function executeTransfer( 135: function executeTransferCollateralizable( 153: function executeSetIsUsedAsCollateral( 187: function executeMintMultiple( 260: function executeBurnMultiple( 340: function executeApprove( 348: function _approve( 357: function executeApprovalForAll( 368: function executeStartAuction( 386: function executeEndAuction( 402: function _checkBalanceLimit(
Missing @param
for expirationPeriod
and maxPriceDeviation
175: function setConfig(uint128 expirationPeriod, uint128 maxPriceDeviation)
Missing @param
for _asset
and _flag
183: setPause(address _asset, bool _flag)
Missing @param
for _twaps
183: function setMultiplePrices(
Missing @notice
236: getPrice(address _asset) 252: getLastUpdateTime(address _asset)
Missing @param
for params
and vars
362: function _getUserBalanceForERC721(
Missing @param
for reserve
, xTokenAddress
and assetPrice
362: function _getUserBalanceForERC721(
Missing @param
for reservesData
564: function _supplyNewCollateral(
Missing @return
for uint256
114: function _buyWithCredit(
Missing @return
for uint256
and uin256
376: function _delegateToPool(
Missing @param
for user
, ps
and oracle
166: function executeGetUserAccountData(
Missing @param
for assetType
63: function validateSupply(
Missing @param
for reservesData
590: function validateLiquidateERC721(
Missing @return
for uint256
182: function getUserApeStakingAmount(address user)
#0 - c4-judge
2023-01-25T11:04:40Z
dmvt marked the issue as grade-a
🌟 Selected for report: IllIllI
Also found by: B2, Deivitto, Dravee, PaludoX0, RaymondFam, Rolezn, Sathish9098, _Adam, ahmedov, c3phas, chrisdior4, cyberinn, rjs, saneryee
145.9382 USDC - $145.94
Issue | Contexts | Estimated Gas Saved | |
---|---|---|---|
GAS‑1 | Multiple Address Mappings Can Be Combined Into A Single Mapping Of An Address To A Struct, Where Appropriate | 1 | - |
GAS‑2 | Duplicated require() /revert() Checks Should Be Refactored To A Modifier Or Function | 40 | 1120 |
GAS‑3 | Empty Blocks Should Be Removed Or Emit Something | 2 | - |
GAS‑4 | <x> += <y> Costs More Gas Than <x> = <x> + <y> For State Variables | 26 | - |
GAS‑5 | ++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 | 1715 |
GAS‑6 | require() /revert() Strings Longer Than 32 Bytes Cost Extra Gas | 18 | - |
GAS‑7 | abi.encode() is less efficient than abi.encodepacked() | 2 | 200 |
GAS‑8 | Using 10**X for constants isn't gas efficient | 1 | - |
GAS‑9 | Public Functions To External | 16 | - |
GAS‑10 | Usage of uints /ints smaller than 32 bytes (256 bits) incurs overhead | 26 | - |
GAS‑11 | Optimize names to save gas | 15 | 330 |
GAS‑12 | Use immutable weth variable value | 1 | - |
GAS‑13 | State variables only set in the constructor should be declared immutable | 1 | - |
GAS‑14 | internal functions only called once can be inlined to save gas | 23 | - |
GAS‑15 | Setting the constructor to payable | 19 | 247 |
GAS‑16 | Functions guaranteed to revert when called by normal users can be marked payable | 65 | 1365 |
GAS‑17 | Using unchecked blocks to save gas | 2 | 272 |
GAS‑18 | Structs can be packed into fewer storage slots | 1 | - |
Total: 307 contexts over 17 issues
Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot.
74: mapping(address => PriceInformation) public assetPriceMap; 81: mapping(address => FeederPosition) private feederPositionMap; 88: mapping(address => FeederRegistrar) public assetFeederMap;
Can be changed to:
struct priceFeederStruct { PriceInformation assetPriceMap; FeederPosition feederPositionMap; FeederRegistrar assetFeederMap; } mapping(address => priceFeederStruct) public priceFeederInfo;
require()
/revert()
Checks Should Be Refactored To A Modifier Or FunctionSaves deployment costs
51: (orderInfo.offer.length > 0, Errors.INVALID_MARKETPLACE_ORDER); 84: (orderInfo.offer.length > 0, Errors.INVALID_MARKETPLACE_ORDER);
68: (amount != 0, Errors.INVALID_AMOUNT); 160: (amount != 0, Errors.INVALID_AMOUNT);
85: (isActive, Errors.RESERVE_INACTIVE); 185: (isActive, Errors.RESERVE_INACTIVE); 207: (isActive, Errors.RESERVE_INACTIVE); 390: (isActive, Errors.RESERVE_INACTIVE); 438: (isActive, Errors.RESERVE_INACTIVE); 460: (isActive, Errors.RESERVE_INACTIVE); 1029: (isActive, Errors.RESERVE_INACTIVE); 1057: (isActive, Errors.RESERVE_INACTIVE);
86: (!isPaused, Errors.RESERVE_PAUSED); 186: (!isPaused, Errors.RESERVE_PAUSED); 208: (!isPaused, Errors.RESERVE_PAUSED); 391: (!isPaused, Errors.RESERVE_PAUSED); 439: (!isPaused, Errors.RESERVE_PAUSED); 461: (!isPaused, Errors.RESERVE_PAUSED); 1030: (!isPaused, Errors.RESERVE_PAUSED); 1058: (!isPaused, Errors.RESERVE_PAUSED);
768: (vars.collateralReserveActive, Errors.RESERVE_INACTIVE); 824: (vars.collateralReserveActive, Errors.RESERVE_INACTIVE);
769: (!vars.collateralReservePaused, Errors.RESERVE_PAUSED); 825: (!vars.collateralReservePaused, Errors.RESERVE_PAUSED);
937: (!reserve.configuration.getPaused(), Errors.RESERVE_PAUSED); 950: (!reserve.configuration.getPaused(), Errors.RESERVE_PAUSED);
1068: (!params.marketplace.paused, Errors.MARKETPLACE_PAUSED); 1074: (!params.marketplace.paused, Errors.MARKETPLACE_PAUSED);
157: (asset != address(0), Errors.ZERO_ADDRESS_NOT_VALID); 172: (asset != address(0), Errors.ZERO_ADDRESS_NOT_VALID); 187: (asset != address(0), Errors.ZERO_ADDRESS_NOT_VALID);
The code should be refactored such that they no longer exist, or the block should do something useful, such as emitting an event or reverting. If the contract is meant to be extended, the contract should be abstract and the function signatures be added without any default implementation. If the block is an empty if-statement block to avoid doing subsequent checks in the else-if/else conditions, the else-if/else conditions should be nested under the negation of the if-statement, because they involve different classes of checks, which may lead to the introduction of errors when the code is later modified (if(x){}else if(y){...}else{...} => if(!x){if(y){...}else{...}})
50: {}
149: receive() external payable {}
<x> += <y>
Costs More Gas Than <x> = <x> + <y>
For State Variables149: token0Amount += positionData.tokensOwed0;
150: token1Amount += positionData.tokensOwed1;
211: sum += getTokenPrice(tokenIds[index]);
169: vars.payableDebtByERC20Assets += vars
176: vars.avgLtv += vars.userBalanceInBaseCurrency * vars.ltv;
178: vars.totalCollateralInBaseCurrency += vars
185: vars.avgLiquidationThreshold += vars.liquidationThreshold;
189: vars.totalDebtInBaseCurrency += _getUserDebtInBaseCurrency(
231: vars.avgERC721LiquidationThreshold += vars
233: vars.totalERC721CollateralInBaseCurrency += vars
237: vars.avgLtv += vars.ltv;
380: totalValue += _getTokenPrice(
479: totalValue += tokenPrice;
496: totalLTV += tmpLTV * tokenPrice;
497: totalLiquidationThreshold +=
83: vars.ethLeft -= _buyWithCredit(
205: vars.ethLeft -= _buyWithCredit(
397: price += item.startAmount;
77: amountToWithdraw += _nfts[index].amount;
166: amountToWithdraw += _nftPairs[index].amount;
215: totalAmount += getTokenIdStakingAmount(
257: apeStakedAmount += bakcStakedAmount;
146: erc721Data.userState[from].collateralizedBalance -= 1;
318: erc721Data.userState[user].balance -= balanceToBurn;
++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
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++) {
95: for (uint256 i = 0; i < assets.length; i++) {
197: for (uint256 i = 0; i < assets.length; i++) {
193: for (uint256 index = 0; index < tokenIds.length; index++) {
210: for (uint256 index = 0; index < tokenIds.length; index++) {
38: for (i = 0; i < params.nftTokenIds.length; i++) {
371: for (uint256 index = 0; index < totalBalance; index++) {
466: for (uint256 index = 0; index < totalBalance; index++) {
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++) {
58: for (uint16 i = 0; i < params.reservesCount; i++) {
104: for (uint256 i = 0; i < assets.length; i++) {
184: for (uint256 index = 0; index < params.tokenData.length; index++) {
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++) {
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++) {
279: for (uint256 index = 0; index < _nfts.length; index++) {
289: for (uint256 index = 0; index < _nftPairs.length; index++) {
634: for (uint256 i = 0; i < reservesListCount; i++) {
106: for (uint256 index = 0; index < tokenIds.length; index++) {
145: for (uint256 i = 0; i < ids.length; i++) {
107: for (uint256 index = 0; index < tokenIds.length; index++) {
51: for (uint256 index = 0; index < tokenIds.length; index++) {
97: for (uint256 index = 0; index < tokenIds.length; index++) {
493: for (uint256 index = 0; index < tokenIds.length; index++) {
213: for (uint256 index = 0; index < totalBalance; index++) {
207: for (uint256 index = 0; index < tokenData.length; index++) {
280: for (uint256 index = 0; index < tokenIds.length; index++) {
82: for (uint256 i = 0; i < punkIndexes.length; i++) {
109: 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 Gas118: require(_isAssetExisted(_asset), "NFTOracle: asset not existed");
128: require(_isFeederExisted(_feeder), "NFTOracle: feeder not existed");
207: require(dataValidity, "NFTOracle: invalid price data");
267: require(!_paused, "NFTOracle: nft price feed paused");
356: require(_twap > 0, "NFTOracle: price should be more than 0");
51: require(orderInfo.offer.length > 0, Errors.INVALID_MARKETPLACE_ORDER);
84: require(orderInfo.offer.length > 0, Errors.INVALID_MARKETPLACE_ORDER);
38: require(runInput.details.length == 1, Errors.INVALID_MARKETPLACE_ORDER);
51: require(nfts.length == 1, Errors.INVALID_MARKETPLACE_ORDER);
86: require(id != POOL, Errors.INVALID_ADDRESSES_PROVIDER_ID);
406: require((variableDebt != 0), Errors.NO_DEBT_OF_SELECTED_TYPE);
418: require(userBalance != 0, Errors.UNDERLYING_BALANCE_ZERO);
60: require(initializingPool == POOL, Errors.POOL_ADDRESSES_DO_NOT_MATCH);
176: require(airdropParams.length >= 4, Errors.INVALID_AIRDROP_PARAMETERS);
70: require(msg.sender == _underlyingAsset, Errors.OPERATION_NOT_SUPPORTED);
193: require(to != owner, "ERC721: approval to old owner");
92: require(to != address(0), "ERC721: transfer to the zero address");
199: require(to != address(0), "ERC721: mint to the zero address");
abi.encode()
is less efficient than abi.encodepacked()
See for more information: https://github.com/ConnorBlockchain/Solidity-Encode-Gas-Comparison
1124: abi.encode(
1136: abi.encode(
10**X
for constants isn't gas efficientIn Solidity, a constant expression in a variable will compute the expression everytime the variable is called. It's not the result of the expression that is stored, but the expression itself.
As Solidity supports the scientific notation, constants of form 10**X can be rewritten as 1eX to save the gas cost from the calculation with the exponentiation operator **.
245: ((oracleData.token0Price * (10**18)) /
Should be change to:
245: ((oracleData.token0Price * (1e18)) /
Replace 10**X with 1eX
The following functions could be set external to save gas and improve code quality. External call cost is less expensive than of public functions.
function initialize( address _admin, address[] memory _feeders, address[] memory _assets ) public initializer {
function getFeederSize() public view returns (uint256) {
function getLiquidityAmountFromPositionData( UinswapV3PositionData memory positionData ) public pure returns (uint256 token0Amount, uint256 token1Amount) {
function getLpFeeAmountFromPositionData( UinswapV3PositionData memory positionData ) public view returns (uint256 token0Amount, uint256 token1Amount) {
function getTokenPrice(uint256 tokenId) public view returns (uint256) {
function getAddress(bytes32 id) public view override returns (address) {
function executeBorrow( mapping(address => DataTypes.ReserveData) storage reservesData, mapping(uint256 => address) storage reservesList, DataTypes.UserConfigurationMap storage userConfig, DataTypes.ExecuteBorrowParams memory params ) public {
function initialize( IPool initializingPool, address underlyingAsset, IRewardController incentivesController, string calldata nTokenName, string calldata nTokenSymbol, bytes calldata params ) public virtual override initializer {
function initialize( IPool initializingPool, address underlyingAsset, IRewardController incentivesController, string calldata nTokenName, string calldata nTokenSymbol, bytes calldata params ) public virtual override initializer {
function getBAKC() public view returns (IERC721) {
function name() public view override returns (string memory) {
function totalSupply() public view virtual override returns (uint256) {
function getTokenIdStakingAmount( uint256 poolId, ApeCoinStaking _apeCoinStaking, uint256 tokenId ) public view returns (uint256) {
function executeTransfer( MintableERC721Data storage erc721Data, IPool POOL, bool ATOMIC_PRICING, address from, address to, uint256 tokenId ) public {
function isAuctioned( MintableERC721Data storage erc721Data, IPool POOL, uint256 tokenId ) public view returns (bool) {
function onERC721Received( address, address, uint256, bytes memory ) public virtual override returns (bytes4) {
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
18: uint128 expirationPeriod;
20: uint128 maxPriceDeviation;
47: uint8 index;
300: uint8 assetIndex = assetFeederMap[_asset].index;
330: uint8 feederIndex = feederPositionMap[_feeder].index;
43: uint8 token0Decimal;
44: uint8 token1Decimal;
45: uint160 sqrtPriceX96;
125: uint16 liquidationAssetReserveId;
581: interfaceId == type(IERC721Metadata).interfaceId;
13: uint64 balance;
14: uint64 collateralizedBalance;
15: uint128 additionalData;
42: uint64 balanceLimit;
103: uint64 oldSenderBalance = erc721Data.userState[from].balance;
105: uint64 oldRecipientBalance = erc721Data.userState[to].balance;
106: uint64 newRecipientBalance = oldRecipientBalance + 1;
200: uint64 oldBalance = erc721Data.userState[to].balance;
205: uint64 collateralizedTokens = 0;
247: uint64 newBalance = oldBalance + uint64(tokenData.length);
272: uint64 burntCollateralizedTokens = 0;
273: uint64 balanceToBurn;
408: uint64 balanceLimit = erc721Data.balanceLimit;
Contracts most called functions could simply save gas by function ordering via Method ID. Calling a function at runtime will be cheaper if the function is positioned earlier in the order (has a relatively lower Method ID) because 22 gas are added to the cost of a function for every position that came before it. The caller can save on gas if you prioritize most called functions.
See more <a href="https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92">here</a>
Relevant to all in-scope contracts, read more about it <a href="https://medium.com/joyso/solidity-how-does-function-name-affect-gas-consumption-in-smart-contract-47d270d8ac92">here</a>
Find a lower method ID name for the most called functions for example Call() vs. Call1() is cheaper by 22 gas For example, the function IDs in the Gauge.sol contract will be the most used; A lower method ID may be given.
weth
variable valueIt is cheaper to use weth
variable as immutable and store the value of WETH address. It saves one storage read and make the code clearer.
92: address weth = _addressesProvider.getWETH();
constructor
should be declared immutableAvoids a Gsset (20000 gas) in the constructor, and replaces each Gwarmacces (100 gas) with a PUSH32 (3 gas).
65: _underlyingAsset = underlyingAsset
internal
functions only called once can be inlined to save gas265: function _whenNotPaused
388: function _addRawValue
218: function _onlyAssetListingOrPoolAdmins
302: function _updateParaProxyImpl
435: function _burnCollateralPTokens
465: function _burnCollateralNTokens
488: function _liquidatePTokens
523: function _burnDebtTokens
564: function _supplyNewCollateral
605: function _calculateDebt
376: function _delegateToPool
552: function _checkAllowance
593: function _transferAndCollateralize
118: function validateSupplyFromNToken
740: function validateStartAuction
801: function validateEndAuction
944: function validateTransferERC721
1133: function getDomainSeparator
413: function setSApeUseAsCollateral
69: function _onlyPoolConfigurator
76: function _onlyPoolAdmin
130: function initializeStakingData
51: function _decreaseLiquidity
constructor
to payable
Saves ~13 gas per instance
49: constructor( IPoolAddressesProvider provider, address[] memory assets, address[] memory sources, address fallbackOracle, address baseCurrency, uint256 baseCurrencyUnit )
23: constructor( address _factory, address _manager, address _addressProvider )
45: constructor(string memory marketId, address owner)
50: constructor( uint256 maxPriceMultiplier, uint256 minExpPriceMultiplier, uint256 minPriceMultiplier, uint256 stepLinear, uint256 stepExp, uint256 tickLength )
51: constructor(IPoolAddressesProvider provider)
64: constructor(IPoolAddressesProvider provider)
62: constructor(IPoolAddressesProvider provider)
89: constructor(IPoolAddressesProvider provider)
43: constructor(IPool pool, bool atomic_pricing) MintableIncentivizedERC721( pool, "NTOKEN_IMPL", "NTOKEN_IMPL", atomic_pricing )
32: constructor(IPool pool, address apeCoinStaking) NToken(pool, false)
16: constructor(IPool pool, address apeCoinStaking) NTokenApeStaking(pool, apeCoinStaking)
16: constructor(IPool pool, address apeCoinStaking) NTokenApeStaking(pool, apeCoinStaking)
32: constructor(IPool pool) NToken(pool, false)
33: constructor(IPool pool) NToken(pool, true)
83: constructor( IPool pool, string memory name_, string memory symbol_, bool atomic_pricing )
43: constructor( address _punk, address _wpunk, address _pool )
payable
If a function modifier or require such as onlyOwner/onlyX 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.
139: function addAssets(address[] calldata _assets) external onlyRole(DEFAULT_ADMIN_ROLE) {
148: function removeAsset(address _asset) external onlyRole(DEFAULT_ADMIN_ROLE) onlyWhenAssetExisted(_asset) {
158: function addFeeders(address[] calldata _feeders) external onlyRole(DEFAULT_ADMIN_ROLE) {
167: function removeFeeder(address _feeder) external onlyWhenFeederExisted(_feeder) {
175: function setConfig(uint128 expirationPeriod, uint128 maxPriceDeviation) external onlyRole(DEFAULT_ADMIN_ROLE) {
183: function setPause(address _asset, bool _flag) external onlyRole(DEFAULT_ADMIN_ROLE) {
195: function setPrice(address _asset, uint256 _twap) public onlyRole(UPDATER_ROLE) onlyWhenAssetExisted(_asset) whenNotPaused(_asset) {
221: function setMultiplePrices( address[] calldata _assets, uint256[] calldata _twaps ) external onlyRole(UPDATER_ROLE) {
66: function setAssetSources( address[] calldata assets, address[] calldata sources ) external override onlyAssetListingOrPoolAdmins {
74: function setFallbackOracle(address fallbackOracle) external override onlyAssetListingOrPoolAdmins {
56: function setMarketId(string memory newMarketId) external override onlyOwner {
70: function setAddress(bytes32 id, address newAddress) external override onlyOwner {
81: function setAddressAsProxy(bytes32 id, address newImplementationAddress) external override onlyOwner {
105: function updatePoolImpl( IParaProxy.ProxyImplementation[] calldata implementationParams, address _init, bytes calldata _calldata ) external override onlyOwner {
121: function setPoolConfiguratorImpl(address newPoolConfiguratorImpl) external override onlyOwner {
142: function setPriceOracle(address newPriceOracle) external override onlyOwner {
158: function setACLManager(address newAclManager) external override onlyOwner {
170: function setACLAdmin(address newAclAdmin) external override onlyOwner {
182: function setPriceOracleSentinel(address newPriceOracleSentinel) external override onlyOwner {
224: function setProtocolDataProvider(address newDataProvider) external override onlyOwner {
235: function setWETH(address newWETH) external override onlyOwner {
242: function setMarketplace( bytes32 id, address marketplace, address adapter, address operator, bool paused ) external override onlyOwner {
110: function initReserve( address asset, address xTokenAddress, address variableDebtAddress, address interestRateStrategyAddress, address auctionStrategyAddress ) external virtual override onlyPoolConfigurator {
139: function dropReserve(address asset) external virtual override onlyPoolConfigurator {
151: function setReserveInterestRateStrategyAddress( address asset, address rateStrategyAddress ) external virtual override onlyPoolConfigurator {
166: function setReserveAuctionStrategyAddress( address asset, address auctionStrategyAddress ) external virtual override onlyPoolConfigurator {
181: function setConfiguration( address asset, DataTypes.ReserveConfigurationMap calldata configuration ) external virtual override onlyPoolConfigurator {
196: function rescueTokens( DataTypes.AssetType assetType, address token, address to, uint256 amountOrTokenId ) external virtual override onlyPoolAdmin {
206: function setAuctionRecoveryHealthFactor(uint64 value) external virtual override onlyPoolConfigurator {
79: function mint( address onBehalfOf, DataTypes.ERC721SupplyParams[] calldata tokenData ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
87: function burn( address from, address receiverOfUnderlying, uint256[] calldata tokenIds ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
119: function transferOnLiquidation( address from, address to, uint256 value ) external onlyPool nonReentrant {
127: function rescueERC20( address token, address to, uint256 amount ) external override onlyPoolAdmin {
136: function rescueERC721( address token, address to, uint256[] calldata ids ) external override onlyPoolAdmin {
151: function rescueERC1155( address token, address to, uint256[] calldata ids, uint256[] calldata amounts, bytes calldata data ) external override onlyPoolAdmin {
168: function executeAirdrop( address airdropContract, bytes calldata airdropParams ) external override onlyPoolAdmin {
199: function transferUnderlyingTo(address target, uint256 tokenId) external virtual override onlyPool nonReentrant {
214: function handleRepayment(address user, uint256 amount) external virtual override onlyPool nonReentrant {
102: function burn( address from, address receiverOfUnderlying, uint256[] calldata tokenIds ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
136: function setUnstakeApeIncentive(uint256 incentive) external onlyPoolAdmin {
159: function unstakePositionAndRepay(uint256 tokenId, address incentiveReceiver) external onlyPool nonReentrant {
26: function depositApeCoin(ApeCoinStaking.SingleNft[] calldata _nfts) external onlyPool nonReentrant {
39: function claimApeCoin(uint256[] calldata _nfts, address _recipient) external onlyPool nonReentrant {
52: function withdrawApeCoin( ApeCoinStaking.SingleNft[] calldata _nfts, address _recipient ) external onlyPool nonReentrant {
66: function depositBAKC(ApeCoinStaking.PairNftWithAmount[] calldata _nftPairs) external onlyPool nonReentrant {
82: function claimBAKC( ApeCoinStaking.PairNft[] calldata _nftPairs, address _recipient ) external onlyPool nonReentrant {
97: function withdrawBAKC( ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, address _apeRecipient ) external onlyPool nonReentrant {
26: function depositApeCoin(ApeCoinStaking.SingleNft[] calldata _nfts) external onlyPool nonReentrant {
39: function claimApeCoin(uint256[] calldata _nfts, address _recipient) external onlyPool nonReentrant {
52: function withdrawApeCoin( ApeCoinStaking.SingleNft[] calldata _nfts, address _recipient ) external onlyPool nonReentrant {
66: function depositBAKC(ApeCoinStaking.PairNftWithAmount[] calldata _nftPairs) external onlyPool nonReentrant {
82: function claimBAKC( ApeCoinStaking.PairNft[] calldata _nftPairs, address _recipient ) external onlyPool nonReentrant {
97: function withdrawBAKC( ApeCoinStaking.PairNftWithAmount[] memory _nftPairs, address _apeRecipient ) external onlyPool nonReentrant {
40: function burn( address from, address receiverOfUnderlying, uint256[] calldata tokenIds ) external virtual override onlyPool nonReentrant returns (uint64, uint64) {
123: function decreaseUniswapV3Liquidity( address user, uint256 tokenId, uint128 liquidityDecrease, uint256 amount0Min, uint256 amount1Min, bool receiveEthAsWeth ) external onlyPool nonReentrant {
131: function setIncentivesController(IRewardController controller) external onlyPoolAdmin {
142: function setBalanceLimit(uint64 limit) external onlyPoolAdmin {
458: function setIsUsedAsCollateral( uint256 tokenId, bool useAsCollateral, address sender ) external virtual override onlyPool nonReentrant returns (bool) {
474: function batchSetIsUsedAsCollateral( uint256[] calldata tokenIds, bool useAsCollateral, address sender ) external virtual override onlyPool nonReentrant returns ( uint256 oldCollateralizedBalance, uint256 newCollateralizedBalance ) {
529: function startAuction(uint256 tokenId) external virtual override onlyPool nonReentrant {
540: function endAuction(uint256 tokenId) external virtual override onlyPool nonReentrant {
202: function emergencyERC721TokenTransfer( address token, uint256 tokenId, address to ) external onlyOwner {
217: function emergencyPunkTransfer(address to, uint256 punkIndex) external onlyOwner {
Functions guaranteed to revert when called by normal users can be marked payable.
unchecked
blocks to save gasSolidity version 0.8+ comes with implicit overflow and underflow checks on unsigned integers. When an overflow or an underflow isn’t possible (as an example, when a comparison is made before the arithmetic operation), some gas can be saved by using an unchecked
block
243: require( 244: (block.number - updatedAt) <= config.expirationPeriod, 245: "NFTOracle: asset price expired" 246: );
400: uint256 downpayment = price - vars.creditAmount;
Each slot saved can avoid an extra Gsset (20000 gas) for the first setting of the struct. Subsequent reads as well as writes have smaller gas savings
There is no need to use uint256
for timestamp as it is unlikely to ever reach the max uint256
value. uint64
is enough for timestamps
struct PriceInformation { // last reported floor price(offchain twap) uint256 twap; // last updated blocknumber uint256 updatedAt; // last updated timestamp uint256 updatedTimestamp; }
Saving 2 slots by changing to:
struct PriceInformation { // last reported floor price(offchain twap) uint64 twap; // last updated blocknumber uint64 updatedAt; // last updated timestamp uint64 updatedTimestamp; }
#0 - c4-judge
2023-01-25T11:10:33Z
dmvt marked the issue as grade-b