bunker.finance contest - 0x4non's results

Critical function dont emit event

The critical function that change oracle admin doesnt emit any event; https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/CNftPriceOracle.sol#L54

Consider to add an event or if the admin is meant to never change remove function and add an immutable modifier to the admin variable.

Gas optimization

PriceOracleImplementation.sol

Use immutable on L10;

    address public cEtherAddress;

to

    address immutable public cEtherAddress;

CNftPriceOracle.sol

Use unchecked on loops; https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/CNftPriceOracle.sol#L66-L73

        for (uint256 i = 0; i < cNfts.length;) {
            address underlying = cNfts[i].underlying();
            require(
                underlyingNftxTokenAddress[underlying] == address(0),
                "CNftPriceOracle: Cannot overwrite existing address mappings."
            );
            underlyingNftxTokenAddress[underlying] = nftxTokens[i];
            unchecked{ i++; }
        }

ERC1155Enumerable.sol

Uso unchecked on loop and safe adds and minus, since ERC1155 from openzepellin validates the amounts in: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155/ERC1155.sol#L218 https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC1155/ERC1155.sol#L176 we could consider this safe.

https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/ERC1155Enumerable.sol#L51-L68

        for (uint256 i; i < ids.length;) {
            uint256 amount = amounts[i];
            if (amount == 0) {
                continue;
            }

            uint256 id = ids[i];
            if (from == address(0)) {
                unchecked { totalSupply += amount; }
            } else if (balanceOf(from, id) == amount) {
                fromTokens.remove(id);
            }

            if (to == address(0)) {
                unchecked { totalSupply -= amount; }
            } else if (balanceOf(to, id) == 0) {
                toTokens.add(id);
            }
            unchecked { i++; }
        }

CNft.sol

Consider moving this line to the beginning of the function: https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L48 Change

        require(tokenIds.length == amounts.length, "CNFT: id/amounts length mismatch");

to

        uint256 length = tokenIds.length;
        require(length == amounts.length, "CNFT: id/amounts length mismatch");

Same issue in https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L96 Change

        require(seizeIds.length == seizeAmounts.length, "CNFT: id/amounts length mismatch");

To

        uint256 length = seizeIds.length;
        require(length == seizeAmounts.length, "CNFT: id/amounts length mismatch");

Same issue in https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/CNft.sol#L118 Change

        require(tokenIds.length == amounts.length, "CNFT: id/amounts length mismatch");

to

        uint256 length = tokenIds.length;
        require(length == amounts.length, "CNFT: id/amounts length mismatch");

Use unchecked to increas i in loop. You could save gas using the pattern to increment the lopp variable using unchecked in; #L50 #L62 #L72 #L98 #L122 #L145 #L151 #L176

Pattern;

for (uint256 i = 0; i < length;) {
    // implementation
    unckecked{ i++; }
}

UniswapV2PriceOracle.sol

Uso unchecked on increments https://github.com/bunkerfinance/bunker-protocol/blob/752126094691e7457d08fc62a6a5006df59bd2fe/contracts/Oracles/UniswapV2PriceOracle.sol#L42-L45

Consider changing;

        for (uint256 i = 0; i < pairs.length; ++i) {
            if (update(pairs[i])) {
                ++numberUpdated;
            }
        }

For

        for (uint256 i = 0; i < pairs.length;) {
            if (update(pairs[i])) {
                unchecked{ numberUpdated++; }
            }
            unchecked{ i++; }
        }