Known issues & Tips

Made 3 parts: random things for people to know, known issues related to oracle, and things I went over looking at the previous audit

More Docs/Known Complexity/Optimizations

Only plan to deploy to L1/mainnet
Using Diamond Proxy (can upgrade later with timelock)
Governance: not fully implemented yet. Plan is to start with admin access, create a DAO with token voting and time locks, and then upgrade contracts to have less parameters and be immutable. Also parameters are bounded with a min and max values that the admin can set to.
Orderbook design:
- For matching Short Orders (have to be above oracle price), it is approximate matching, and can temporarily match downwards from a short hint. It is expensive to determine the first short order that is matchable because the price oracle can jump due to chainlink. There is a 1 to 1 relationship between the oracle price and the first short that is matchable.
- Uses a hint system for placing orders because looping through orders takes a lot of gas
- unbounded loops: Orderbook matching can be expensive so large orders can hit the gas limit. Protocol attempts to help with this by having minimum order amounts and gas optimizations.
Struct Packing (uint80 instead of uint256), because of that I have custom .mul to make sure values don't overflow uint80
Margin Calls (primary liquidating requires a waiting period, handled by secondary)
- gas fee paid by shorter is determined by base fee
Multiple "Vaults" but only using one right now
Multiple Assets but only using one right now
Yield: nuances around updateYield: Since it's callable by anyone, someone can come in with a large ShortRecord position, and then call updateYield to get a disporportionate amount of yield.
Yield: to help against flash loan, both ETH yield and Ditto yield are based on time. If a ShortRecord is updated, a minimum amount of time needs to pass to be able to claim rewards.

Known Issues

Oracle is very dependent on Chainlink: similar to Liquity. stale/invalid prices fallback to uniswap TWAP. 2 hours staleness means it can be somewhat out of date
Oracle: non ETH/USD oracle assets currently have no Uniswap TWAP fallback, it reverts.
Need boostrap phase: when there are few shorts/TAPP is low, easy to fall into black swan scenario
Bootstrap for ditto rewards: first claimer gets 100% of ditto reward
Events: still adding core events, mostly admin events for now.
Not done with governance setup
Not finished with mainnet deployment setup

Known Considerations from previous stablecoin Codehawk report

https://www.codehawks.com/report/cljx3b9390009liqwuedkn0m0

H-01: Collateral tokens < 18 decimals: Project assumes collateral tokens are 18 decimals because they will all specifically be ETH LSTs like rETH. stETH and rETH are both 18. diamondCut upgrade can be used otherwise. (see old reports)
H-02: Liquidation reverts: Liquidation shouldn't revert the due to lack of collateral or the extra liquidation fee/bonus because the TAPP can be used. Primary liquidation can revert if there are no orders, secondary liquidation doesn't have an associated fee
H-03: Liquidation of small shorts: Protocol attempts to cover liquidating small positions in a few ways: there is a minimum short amount upon creation, gas fee is paid by shorter not liquidator. Shorts with too small fee to get liquidated are also too small to effect the market with bad debt. If needed, the shutdownMarket can be called when market CR is < minimumCR to freeze that market and allow people to redeemErc. Worst case, TAPP/DAO can also secondary liquidate small positions
M-01: protocol is only planned to be on L1
M-02 stale price: stale period is intentionally set at 2 hours for Ethereum/mainnet for base oracle (ETH/USD), which fallsback to TWAP. For multi-asset oracle, considering adding a mapping to track different stale heartbeats (Gold is 24 hours)
M-03 revert if outside of min/max answer: Shouldn't happen for a feed like ETH/USD, but will add checks for minAnswer and maxAnswer according to chainlink docs
M-04 decimals: protocol assumes same decimals, not calling decimals() to save a SLOAD of gas. Can use diamond upgrade if needed for multi-asset if necessary.
M-05 burnFrom: protocol doesn't inherit OZ ERC20Burnable, just calls _burn directly with owner modifier.
M-06 duplicate inputs: protocol uses arrays for inputs for orderhints which isn't an issue, batches for secondary margin call (checks for deleted shorts), and combineShorts ids which checks deleted shorts
M-07 oracle fallback: protocol uses Uniswap TWAP
M-08 fee-on-transfer: protocol's bridges for reth/steth account checks for fees on deposit of eth into steth or reth by checking balanceOf. It doesn't do it for depositing the token itself, protocol assumes steth/reth won't add a fee on transfer.
M-09 liquidate revert: protocol can use TAPP
M-10 upgradable collateral: diamond upgradable, bridges are whitelisted already, will think about detecting upgrades
M-11 liquidate front run: No particular protections against frontrunning an order/liquidation, and oracle manipulation (unlikely for ETH/USD pair for both chainlink/uniswap)
M-12 dos liquidation: secondary liquidation doesn't need to be precise, can't be blocked

4nalyzer known issues

Report

Issues

	Issue	Instances
GAS-1	Using bools for storage incurs overhead	1
GAS-2	Cache array length outside of loop	5
GAS-3	Use Custom Errors	28
GAS-4	Don't initialize variables with default value	2
GAS-5	Functions guaranteed to revert when called by normal users can be marked `payable`	21
GAS-6	`++i` costs less gas than `i++`, especially when it's used in `for`-loops (`--i`/`i--` too)	8
GAS-7	Use != 0 instead of > 0 for unsigned integer comparison	24
NC-1	Return values of `approve()` not checked	1
NC-2	Constants should be defined rather than using magic numbers	1
NC-3	Typos	49
L-1	`abi.encodePacked()` should not be used with dynamic types when passing the result to a hash function such as `keccak256()`	2
L-2	Empty Function Body - Consider commenting why	2
L-3	Unsafe ERC20 operation(s)	5

Gas Optimizations

	Issue	Instances
GAS-1	Using bools for storage incurs overhead	1
GAS-2	Cache array length outside of loop	5
GAS-3	Use Custom Errors	28
GAS-4	Don't initialize variables with default value	2
GAS-5	Functions guaranteed to revert when called by normal users can be marked `payable`	21
GAS-6	`++i` costs less gas than `i++`, especially when it's used in `for`-loops (`--i`/`i--` too)	8
GAS-7	Use != 0 instead of > 0 for unsigned integer comparison	24

[GAS-1] Using bools for storage incurs overhead

Use uint256(1) and uint256(2) for true/false to avoid a Gwarmaccess (100 gas), and to avoid Gsset (20000 gas) when changing from ‘false’ to ‘true’, after having been ‘true’ in the past. See source.

Instances (1):

File: contracts/libraries/AppStorage.sol

41:     mapping(address owner => mapping(address operator => bool)) isApprovedForAll;

[GAS-2] Cache array length outside of loop

If not cached, the solidity compiler will always read the length of the array during each iteration. That is, if it is a storage array, this is an extra sload operation (100 additional extra gas for each iteration except for the first) and if it is a memory array, this is an extra mload operation (3 additional gas for each iteration except for the first).

Instances (5):

File: contracts/facets/ERC721Facet.sol

31:             for (uint256 j; j < shortRecords.length;) {

File: contracts/facets/MarginCallSecondaryFacet.sol

52:         for (uint256 i; i < batches.length;) {

File: contracts/facets/ShortRecordFacet.sol

136:         for (uint256 i = ids.length - 1; i > 0; i--) {

File: contracts/libraries/LibOrders.sol

827:             for (uint256 i = 0; i < shortHintArray.length;) {

932:         for (uint256 i; i < orderHintArray.length; i++) {

[GAS-3] Use Custom Errors

Source
Instead of using error strings, to reduce deployment and runtime cost, you should use Custom Errors. This would save both deployment and runtime cost.

Instances (28):

File: contracts/facets/OwnerFacet.sol

175:         require(value > s.asset[asset].primaryLiquidationCR, "below primary liquidation");

330:         require(rewardRate <= 100, "above 100");

335:         require(rewardRate <= 100, "above 100");

340:         require(value > 100, "below 1.0");

342:         require(LibAsset.initialMargin(asset) < Constants.CRATIO_MAX, "above max CR");

346:         require(value > 100, "below 1.0");

347:         require(value <= 500, "above 5.0");

348:         require(value < s.asset[asset].initialMargin, "above initial margin");

353:         require(value > 100, "below 1.0");

354:         require(value <= 500, "above 5.0");

355:         require(value < s.asset[asset].primaryLiquidationCR, "above primary liquidation");

360:         require(value >= 100, "below 1.0");

361:         require(value <= 200, "above 2.0");

366:         require(value >= 100, "below 1.0");

367:         require(value <= 200, "above 2.0");

379:         require(value >= 100, "below 1.00");

380:         require(value <= 4800, "above 48.00");

385:         require(value >= 100, "below 1.00");

393:         require(value >= 100, "below 1.00");

401:         require(value > 0, "Can't be zero");

402:         require(value <= 250, "above 25.0");

407:         require(value > 0, "Can't be zero");

408:         require(value <= 250, "above 25.0");

414:         require(value > 0, "Can't be zero");

420:         require(value > 0, "Can't be zero");

426:         require(value > 0, "Can't be zero");

431:         require(withdrawalFee <= 1500, "above 15.00%");

436:         require(unstakeFee <= 250, "above 2.50%");

[GAS-4] Don't initialize variables with default value

Instances (2):

File: contracts/libraries/LibOrders.sol

74:         for (uint256 i = 0; i < size; i++) {

827:             for (uint256 i = 0; i < shortHintArray.length;) {

[GAS-5] Functions guaranteed to revert when called by normal users can be marked `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.

Instances (21):

File: contracts/bridges/BridgeReth.sol

98:     function unstake(address to, uint256 amount) external onlyDiamond {

File: contracts/bridges/BridgeSteth.sol

91:     function unstake(address to, uint256 amount) external onlyDiamond {

File: contracts/facets/OwnerFacet.sol

47:     function createMarket(address asset, STypes.Asset memory a) external onlyDAO {

111:     function transferOwnership(address newOwner) external onlyDAO {

124:     function transferAdminship(address newAdmin) external onlyAdminOrDAO {

130:     function setAssetOracle(address asset, address oracle) external onlyDAO {

149:     function setTithe(uint256 vault, uint16 zethTithePercent) external onlyAdminOrDAO {

174:     function setInitialMargin(address asset, uint16 value) external onlyAdminOrDAO {

211:     function setMinimumCR(address asset, uint8 value) external onlyAdminOrDAO {

249:     function setTappFeePct(address asset, uint8 value) external onlyAdminOrDAO {

254:     function setCallerFeePct(address asset, uint8 value) external onlyAdminOrDAO {

259:     function setMinBidEth(address asset, uint8 value) external onlyAdminOrDAO {

264:     function setMinAskEth(address asset, uint8 value) external onlyAdminOrDAO {

269:     function setMinShortErc(address asset, uint16 value) external onlyAdminOrDAO {

287:     function deleteBridge(address bridge) external onlyDAO {

314:     function setUnstakeFee(address bridge, uint8 unstakeFee) external onlyAdminOrDAO {

File: contracts/libraries/AppStorage.sol

94:     function _onlyValidShortRecord(address asset, address shorter, uint8 id)

File: contracts/tokens/Asset.sol

24:     function mint(address to, uint256 amount) external onlyDiamond {

28:     function burnFrom(address account, uint256 amount) external onlyDiamond {

File: contracts/tokens/Ditto.sol

26:     function mint(address to, uint256 amount) external onlyDiamond {

30:     function burnFrom(address account, uint256 amount) external onlyDiamond {

[GAS-6] `++i` costs less gas than `i++`, especially when it's used in `for`-loops (`--i`/`i--` too)

Saves 5 gas per loop

Instances (8):

File: contracts/facets/ERC721Facet.sol

33:                     balance++;

36:                     j++;

File: contracts/facets/OwnerFacet.sol

293:         for (uint256 i; i < length; i++) {

File: contracts/libraries/LibOrders.sol

67:             size++;

74:         for (uint256 i = 0; i < size; i++) {

932:         for (uint256 i; i < orderHintArray.length; i++) {

File: contracts/libraries/LibShortRecord.sol

59:             shortRecordCount++;

400:                 s.flaggerIdCounter++;

[GAS-7] Use != 0 instead of > 0 for unsigned integer comparison

Instances (24):

File: contracts/facets/BidOrdersFacet.sol

283:             if (lowestSell.shortRecordId > 0) {

346:         if (matchTotal.shortFillEth > 0) {

File: contracts/facets/BridgeRouterFacet.sol

104:         if (withdrawalFee > 0) {

132:         if (fee > 0) {

File: contracts/facets/ERC721Facet.sol

304:         if (to.code.length > 0) {

File: contracts/facets/MarketShutdownFacet.sol

69:         if (amtWallet > 0) {

73:         if (amtEscrow > 0) {

File: contracts/facets/OwnerFacet.sol

86:         _setMinBidEth(asset, a.minBidEth); //1 -> 0.001 ether

87:         _setMinAskEth(asset, a.minAskEth); //1 -> 0.001 ether

401:         require(value > 0, "Can't be zero");

407:         require(value > 0, "Can't be zero");

414:         require(value > 0, "Can't be zero");

420:         require(value > 0, "Can't be zero");

426:         require(value > 0, "Can't be zero");

File: contracts/facets/ShortRecordFacet.sol

136:         for (uint256 i = ids.length - 1; i > 0; i--) {

File: contracts/facets/YieldFacet.sol

169:         if (elapsedTime > 0) {

File: contracts/libraries/LibOracle.sol

39:             uint256 basePriceInEth = basePrice > 0

78:             protocolPrice > 0 && chainlinkDiff.div(protocolPrice) > 0.5 ether;

78:             protocolPrice > 0 && chainlinkDiff.div(protocolPrice) > 0.5 ether;

File: contracts/libraries/LibOrders.sol

879:                 (incomingOrder.price - oraclePrice).div(oraclePrice) > 0.005 ether;

882:                 (oraclePrice - incomingOrder.price).div(oraclePrice) > 0.005 ether;

File: contracts/libraries/LibShortRecord.sol

292:         if (ercDebt > 0) {

305:         if (ercDebt > 0) {

348:         if (yield > 0) {

Non Critical Issues

	Issue	Instances
NC-1	Return values of `approve()` not checked	1
NC-2	Constants should be defined rather than using magic numbers	1
NC-3	Typos	49

[NC-1] Return values of `approve()` not checked

Not 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

Instances (1):

File: contracts/bridges/BridgeSteth.sol

26:         steth.approve(

[NC-2] Constants should be defined rather than using magic numbers

Instances (1):

File: contracts/libraries/DataTypes.sol

111:         uint16 minShortErc; // 2000 -> (2000 * 10**18) -> 2000 ether

[NC-3] Typos

Instances (49):

File: contracts/bridges/BridgeReth.sol

- 55:     // Bring rETH to system and credit zETH to user
+ 55:     // Bring rETH to system and credit zETH to the user

- 69:     // Deposit ETH and mint rETH (to system) and credit zETH to user
+ 69:     // Deposit ETH and mint rETH (to system) and credit zETH to the user

- 83:     // Exchange system rETH to fulfill zETH obligation to user
+ 83:     // Exchange system rETH to fulfill zETH obligation to the user

File: contracts/bridges/BridgeSteth.sol

- 73:         // @edv address(0) means no fee taken by the referring protocol
+ 73:         // @dev address(0) means no fee taken by the referring protocol

File: contracts/facets/AskOrdersFacet.sol

- 52:         //@dev asks don't need to be concerned with shortHintId
+ 52:         //@dev asks don't need to be concerned with shortHintIds

File: contracts/facets/BidOrdersFacet.sol

- 88:         //@dev leave empty, don't need hint for market buys
+ 88:         //@dev Leave empty, don't need hint for market buys

- 131:         //@dev setting initial shortId to match "backwards" (See _shortDirectionHandler() below)
+ 131:         //@dev Setting initial shortId to match "backwards" (See _shortDirectionHandler() below)

- 143:             //@dev if match and match price is gt .5% to saved oracle in either direction, update startingShortId
+ 143:             //@dev If match and match price is gt .5% to saved oracle in either direction, update startingShortId

- 150:             //@dev no match, add to market if limit order
+ 150:             //@dev No match, add to market if limit order

- 188:                 // Consider bid filled if only dust amount left
+ 188:                 // Consider bid filled if only a dust amount left

- 266:             // Match short
+ 266:             // Match shorts

- 309:             // Match ask
+ 309:             // Match asks

- 356:             //@dev Approximates the startingShortId after bid is fully executed
+ 356:             //@dev Approximates the startingShortId after bids are fully executed

- 377:         // Match bid
+ 377:         // Match bids

File: contracts/facets/BridgeRouterFacet.sol

- 41:     //@dev does not need read only re-entrancy
+ 41:     //@dev does not need read-only re-entrancy

- 180:             // when yield is positive 1 zeth = 1 eth
+ 180:             // when yield is positive, 1 zeth = 1 eth

- 183:             // negative yield means 1 zeth < 1 eth
+ 183:             // negative yield means 1 zeth < 1 eth (i.e. 1 eth > 1 zeth)

File: contracts/facets/ExitShortFacet.sol

- 64:         // refund the rest of the collateral if ercDebt is fully paid back
+ 64:         // Refund the rest of the collateral if ercDebt is fully paid back

- 118:         // refund the rest of the collateral if ercDebt is fully paid back
+ 118:         // Refund the rest of the collateral if ercDebt is fully paid back

File: contracts/facets/MarginCallPrimaryFacet.sol

- 251:         //@dev By basing gasFee off of baseFee instead of priority, adversaries are prevent from draining the TAPP
+ 251:         //@dev By basing gasFee off of baseFee instead of priority, adversaries are prevented from draining the TAPP

File: contracts/facets/MarginCallSecondaryFacet.sol

- 37:     //@dev If you want to liquidated more than uint88.max worth of erc in shorts, you must call liquidateSecondary multiple times
+ 37:     //@dev If you want to liquidate more than uint88.max worth of erc in shorts, you must call liquidateSecondary multiple times

- 159:     // | 1.0 < c 1.1    | 1             | 0       | c - 1 |
+ 159:     // | 1.0 < c < 1.1  | 1             | 0       | c - 1 |

File: contracts/facets/MarketShutdownFacet.sol

- 46:                 // More than enough collateral to redeem ERC 1:1, send extras to TAPP
+ 46:                 // More than enough collateral to redeem ERC 1:1, send extra to TAPP

File: contracts/facets/ShortOrdersFacet.sol

- 73:         //@dev if match and match price is gt .5% to saved oracle in either direction, update startingShortId
+ 73:         //@dev if match and match price is gt .5% to saved oracle in either direction, update startingShortId and reading spot oracle price

82:         //@dev reading spot oracle price

File: contracts/facets/ShortRecordFacet.sol

- 58:         //@dev reset flag info if new cratio is above primaryLiquidationCR
+ 58:         //@dev reset flag info if new c-ratio is above primaryLiquidationCR

- 175:         // Merge all short records into the short at position id[0]
+ 175:         // Merge all short records into the short at position ids[0]

File: contracts/facets/VaultFacet.sol

- 13: // import {console} from "contracts/libraries/console.sol";
+ 13: // import {console} from "contracts/libraries/console.sol"

File: contracts/facets/YieldFacet.sol

- 57:         // distribute yield for the first order book
+ 57:         // Distribute yield for the first order book

- 60:         // distribute yield for remaining order books
+ 60:         // Distribute yield for remaining order books

- 70:         // claim all distributed yield
+ 70:         // Claim all distributed yield

- 92:         // Loop through all shorter's shorts of this asset
+ 92:         // Loop through all shorters' shorts of this asset

File: contracts/libraries/DataTypes.sol

- 103:         uint16 orderId; // max is uint16 but need to throw/handle that?
+ 103:         uint16 orderId; // max is uint16 but need to throw/handle that

File: contracts/libraries/LibAsset.sol

- 10:     // @dev used in ExitShortWallet and MarketShutDown
+ 10:     // @dev used in ExitShortWallet and MarketShutdown

- 101:     // @dev cRatio where a shorter loses all collateral on liquidation
+ 101:     // @dev cRatio where a short loses all collateral on liquidation

File: contracts/libraries/LibBridge.sol

- 17:     // @dev fee to withdrawal from that bridge
+ 17:     // @dev fee to withdraw from that bridge

File: contracts/libraries/LibOracle.sol

- 38:             //@dev multiply base oracle by 10**10 to give it 18 decimals of precision
+ 38:             //@dev multiply base oracle by 10**10 to give it 18 decimal places of precision

- 80:         //@dev if there is issue with chainlink, get twap price. Compare twap and chainlink
+ 80:         //@dev if there is an issue with chainlink, get twap price. Compare twap and chainlink

File: contracts/libraries/LibOrders.sol

- 425:         // otherwise it could be off because a tx could of modified state
+ 425:         // otherwise it could be off because a tx could have modified state

- 582:                 //@handles moving backwards only.
+ 582:                 //@dev Handles moving backwards only.

File: contracts/libraries/LibShortRecord.sol

- 191:             // remove the links of ID in the market
+ 191:             // remove the links of ID in the market (shortRecordId)

- 193:             // BEFORE: PREV <-> (ID) <-> NEXT
+ 193:             // BEFORE: PREV <-> (ID) <-> NEXT (shortRecordId)

- 194:             // AFTER : PREV <----------> NEXT
+ 194:             // AFTER : PREV <----------> NEXT (shortRecordId)

- 205:             // BEFORE: .. C_ID2 <- C_ID1 <--------- HEAD <-> ... [ID]
+ 205:             // BEFORE: .. C_ID2 <- C_ID1 <--------- HEAD <-> ... [ID] (shortRecordId)

- 206:             // AFTER1: .. C_ID2 <- C_ID1 <- [ID] <- HEAD <-> ...
+ 206:             // AFTER1: .. C_ID2 <- C_ID1 <- [ID] <- HEAD <-> ... (shortRecordId)

- 213:                 // BEFORE: HEAD <--------- HEAD <-> ... [ID]
+ 213:                 // BEFORE: HEAD <--------- HEAD <-> ... [ID] (shortRecordId)

- 214:                 // AFTER1: HEAD <- [ID] <- HEAD <-> ...
+ 214:                 // AFTER1: HEAD <- [ID] <- HEAD <-> ... (shortRecordId)

- 219:             //SR may be cancelled, but there might tied to an active short order
+ 219:             //SR may be cancelled, but there might be an active short order tied to it

File: contracts/libraries/LibVault.sol

- 78:         // If no short records, yield goes to treasury
+ 78:         // If no short records, yield goes to treasuryF

Low Issues

	Issue	Instances
L-1	`abi.encodePacked()` should not be used with dynamic types when passing the result to a hash function such as `keccak256()`	2
L-2	Empty Function Body - Consider commenting why	2
L-3	Unsafe ERC20 operation(s)	5

[L-1] `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

Instances (2):

File: contracts/bridges/BridgeReth.sol

25:         RETH_TYPEHASH = keccak256(abi.encodePacked("contract.address", "rocketTokenRETH"));

27:             keccak256(abi.encodePacked("contract.address", "rocketDepositPool"));

[L-2] Empty Function Body - Consider commenting why

Instances (2):

File: contracts/bridges/BridgeReth.sol

37:     receive() external payable {}

File: contracts/facets/ERC721Facet.sol

286:     function tokenURI(uint256 id) public view virtual returns (string memory) {}

[L-3] Unsafe ERC20 operation(s)

Instances (5):

File: contracts/bridges/BridgeReth.sol

64:         rocketETHToken.transferFrom(from, address(this), amount);

94:         rocketETHToken.transfer(to, rethValue);

File: contracts/bridges/BridgeSteth.sol

26:         steth.approve(

66:         steth.transferFrom(from, address(this), amount);

87:         steth.transfer(to, amount);

cyfrin / 2023-09-ditto Goto Github PK

2023-09-ditto's People

Contributors

Stargazers

Watchers

Forkers

2023-09-ditto's Issues

More Docs/Known Complexity/Optimizations

Known Issues

Known Considerations from previous stablecoin Codehawk report

4nalyzer known issues

Report

Issues

Gas Optimizations

[GAS-1] Using bools for storage incurs overhead

[GAS-2] Cache array length outside of loop

[GAS-3] Use Custom Errors

[GAS-4] Don't initialize variables with default value

[GAS-5] Functions guaranteed to revert when called by normal users can be marked payable

[GAS-6] ++i costs less gas than i++, especially when it's used in for-loops (--i/i-- too)

[GAS-7] Use != 0 instead of > 0 for unsigned integer comparison

Non Critical Issues

[NC-1] Return values of approve() not checked

[NC-2] Constants should be defined rather than using magic numbers

[NC-3] Typos

Low Issues

[L-1] abi.encodePacked() should not be used with dynamic types when passing the result to a hash function such as keccak256()

[L-2] Empty Function Body - Consider commenting why

[L-3] Unsafe ERC20 operation(s)

Recommend Projects

Recommend Topics

Recommend Org

[GAS-5] Functions guaranteed to revert when called by normal users can be marked `payable`

[GAS-6] `++i` costs less gas than `i++`, especially when it's used in `for`-loops (`--i`/`i--` too)

[NC-1] Return values of `approve()` not checked

[L-1] `abi.encodePacked()` should not be used with dynamic types when passing the result to a hash function such as `keccak256()`