1
2
3
4
5
6
7
8
9
10
11

function claimGovFees() public {
address[] memory assets = bondNFT.getAssets();

for (uint i=0; i < assets.length; i++) {
    uint balanceBefore = IERC20(assets[i]).balanceOf(address(this));
    IGovNFT(govNFT).claim(assets[i]);
    uint balanceAfter = IERC20(assets[i]).balanceOf(address(this));
    IERC20(assets[i]).approve(address(bondNFT), type(uint256).max);
    bondNFT.distribute(assets[i], balanceAfter - balanceBefore);
    }
}

1

uint constant private DAY = 24 * 60 * 60;

1
2
3

(uint80 roundId, int256 assetChainlinkPriceInt, , uint256 updatedAt, uint80 answeredInRound) = IPrice(_chainlinkFeed).latestRoundData();
require(answeredInRound >= roundId, "price is stale");
require(updatedAt > 0, "round is incomplete");

• Use the safe variant and ERC721.mint
• Add an event for critical parameter changes
• Declare interfaces on separate files
• Constants should be upper case
• Replace constant private with private constant

When minting and burning tokens,the ERC1155Enumerable implementation does not correctly update the following states:

• uint256[] private _allTokens;
• mapping(uint256 => uint256) private _allTokensIndex;
• mapping(address => uint256) internal _currentIndex;

In particular:

• the _allTokens array length (and therefore the totalSupply()) always increases (never decreases)
• the _allTokensIndex[id] always increases
• the _curentIndex[from] always increases

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24

contract HelperERC1155 is ERC1155Enumerable, ERC1155Holder {
    constructor() ERC1155("Test") {
    }
    function mint(uint256 id, uint256 amount) external {
        _mint(msg.sender, id, amount, bytes(""));
    }
    function burn(uint256 id, uint256 amount) external {
        _burn(msg.sender, id, amount);
    }
    function currentIndex(address owner) external view returns (uint256) {
        return _currentIndex[owner];
    }
    function allTokensIndex(uint256 id) external view returns (uint256) {
        return _allTokensIndex[id];
    }
    function allTokens(uint256 idx) external view returns (uint256) {
        return _allTokens[idx];
    }
    function idTotalSupply(uint256 id) external view returns (uint256) {
        return _idTotalSupply[id];
    }
}

1
2

-   pool.long0ProtocolFees = 0;
+   delete pool.long0ProtocolFees;

1
2
3
4
5
6
7
8
9
10

+    require(block.timestamp <= deadline, "KYCRegistry: signature expired");

    require(
    !kycState[kycRequirementGroup][user],
    "KYCRegistry: user already verified"
    );
-    require(block.timestamp <= deadline, "KYCRegistry: signature expired");
    bytes32 structHash = keccak256(
    abi.encode(_APPROVAL_TYPEHASH, kycRequirementGroup, user, deadline)
    );

1
2
3
4
5
6
7
8

-    for (uint256 i = 0; i < exCallData.length; ++i) {

+    for (uint256 i = 0; i < exCallData.length;) {
+      unchecked {
+        ++i;
+      }
    }
}

1
2
3
4
5

-    doTransferOut(admin, reduceAmount);
+    doTransferOut(payable(msg.sender), reduceAmount);
-    emit ReservesReduced(admin, reduceAmount, totalReservesNew);
+    emit ReservesReduced(msg.sender, reduceAmount, totalReservesNew);

1
2
3
4
5
6

-    if (fTokenToUnderlyingPrice[fToken] != 0) {
-      return fTokenToUnderlyingPrice[fToken];
+    uint256 fToken = fTokenToUnderlyingPrice[fToken];
+    if(fToken != 0) {
+      return fToken;   
    }

1
2

-  address public owner;
+  address public immutable owner;

1
2

-  mapping(address => bool) public allowedAsset;
+  mapping(address => uint256) public allowedAsset;

Recommendation:
import {contract1 , contract2} from "filename.sol";

1
2
3
4
5
6
7
8

function buy(uint256 _amount) external payable {
        ...
-       for (uint48 x = sale_.currentId + 1; x <= newId; x++) {
+       for (uint48 x = sale_.currentId; x < newId; ++x) {
            nft.mint(msg.sender, x);
        }
        ...
}

• Multiple access to mapping/array should use local variable cache
• Duplicated require should be modifier or function
• Use custom error rather than revert()/require()
• Use calldata instead of memory for read-only variable
• require()/revert() string longer than 32 bytes cost extra gas
• Using Openzeppelin Ownable2Step.sol is gas efficient
• Using UniswapV3 mulDiv function is gas-optimized
• Use nested if and, avoid multiple check combinations
• Avoid using state variable in emit (130 gas)
• Instead of cache a whole object ,try cache single Attributes
• Using int32 for time
• Don’t use _msgSender() if not supporting EIP-2771
• Using > 0 costs more gas than != 0 when used on a uint in a require() statement(version>0.8.13)
• Using bools for storage incurs overhead
• .length should not be looked up in every loop of a for-loop
• Using calldata instead of memory for read-only arguments in external functions saves gas
• Splitting require() statements that use && saves gas
• Use a more recent version of solidity
• require()/revert() strings longer than 32 bytes cost extra gas
• += costs more gas than = + for state variables
• Using storage instead of memory for structs/arrays saves gas