AI Arena - oualidpro's results

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L539-L545 https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/FighterFarm.sol#L338-L348

Vulnerability details

Impact

The Fighter NFT owner will lose his NRN tokens if he trade his Fighter NFT while it is staked for a ranked battle.

Proof of Concept

To be able to earn points, the Fighter NFT owner should stake some of its NRN tokens into the RankedBattle.sol using the stakeNRN() function. Moreover, this function insure that the Fighter NFT for which the NRN token have been staked is not transferable or tradable by changing its staked status (check bellow code):

    function stakeNRN(uint256 amount, uint256 tokenId) external {
        require(amount > 0, "Amount cannot be 0");
        require(_fighterFarmInstance.ownerOf(tokenId) == msg.sender, "Caller does not own fighter");
        require(_neuronInstance.balanceOf(msg.sender) >= amount, "Stake amount exceeds balance");
        require(hasUnstaked[tokenId][roundId] == false, "Cannot add stake after unstaking this round");

        _neuronInstance.approveStaker(msg.sender, address(this), amount);
        bool success = _neuronInstance.transferFrom(msg.sender, address(this), amount);
        if (success) {
            if (amountStaked[tokenId] == 0) {
                _fighterFarmInstance.updateFighterStaking(tokenId, true); //<== status change
            }
            amountStaked[tokenId] += amount;
            globalStakedAmount += amount;
            stakingFactor[tokenId] = _getStakingFactor(
                tokenId, 
                _stakeAtRiskInstance.getStakeAtRisk(tokenId)
            );
            _calculatedStakingFactor[tokenId][roundId] = true;
            emit Staked(msg.sender, amount);
        }
    }

by changing this status both transferFrom and safeTransferFrom functions in FighterFarm contract will not work due to the following check in there code:

    function _ableToTransfer(uint256 tokenId, address to) private view returns(bool) {
        return (
          _isApprovedOrOwner(msg.sender, tokenId) &&
          balanceOf(to) < MAX_FIGHTERS_ALLOWED &&
          !fighterStaked[tokenId] // <== stake status check
        );
    }

However, the FighterFarm contract is an ERC721. therefore, another function exists that allow transfer of tokens and that is not protected with the _ableToTransfer:

    function safeTransferFrom(
        address from,
        address to,
        uint256 tokenId,
        bytes memory data
    ) public virtual override {
        require(_isApprovedOrOwner(_msgSender(), tokenId), "ERC721: caller is not token owner nor approved");
        _safeTransfer(from, to, tokenId, data);
    }

Therefore, even if the Fighter NFT is staked, it will still be transferable using this function. Here is a foundry PoC:

function testStakeNRNExploit() public {
        address staker = vm.addr(3);
        address staker_helper = vm.addr(4);
        _mintFromMergingPool(staker);
        _fundUserWith4kNeuronByTreasury(staker);
        assertEq(_neuronContract.balanceOf(staker) >= 4_000 * 10 ** 18, true);
        assertEq(_fighterFarmContract.ownerOf(0), staker);
        assertEq(_neuronContract.hasRole(keccak256("STAKER_ROLE"), address(_rankedBattleContract)), true);
        vm.prank(staker);
        _rankedBattleContract.stakeNRN(3_000 * 10 ** 18, 0);
        assertEq(_fighterFarmContract.ownerOf(0), staker);
        console.log("Before staking: ");
        console.log(_fighterFarmContract.ownerOf(0));
        bytes memory data = "test";
        vm.prank(staker);
        _fighterFarmContract.safeTransferFrom(staker,staker_helper,0,data);
        assertEq(_fighterFarmContract.ownerOf(0), staker_helper);
        console.log("After staking: ");
        console.log(_fighterFarmContract.ownerOf(0));
        assertEq(_rankedBattleContract.amountStaked(0), 3_000 * 10 ** 18);
    }

Tools Used

Manual

Recommended Mitigation Steps

To fix this, you can either override all the transfer functions, or litteraly transfer the Fighter NFT from its owner to the RankedBattle contract when the NRN tokens are staked for a Battle.

Assessed type

Other

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/f2952187a8afc44ee6adc28769657717b498b7d4/src/GameItems.sol#L291-L303

Vulnerability details

Impact

Non Transferable GameItems can be transfered

Proof of Concept

When a new GameItem is created, a bool variable called transferable is set to define if the GameItem is actually transferable or not. If the item is not tansferable, the first Owner will not be able to call the following function:

    function safeTransferFrom(
        address from, 
        address to, 
        uint256 tokenId,
        uint256 amount,
        bytes memory data
    ) 
        public 
        override(ERC1155)
    {
        require(allGameItemAttributes[tokenId].transferable);
        super.safeTransferFrom(from, to, tokenId, amount, data);
    }

because the require(allGameItemAttributes[tokenId].transferable); will revert.

However, the GameItems is an ERC1155 token, therefore another function exists that could be used to tranfer the non transferable item:

    function safeBatchTransferFrom(
        address from,
        address to,
        uint256[] memory ids,
        uint256[] memory amounts,
        bytes memory data
    ) public virtual override {
        require(
            from == _msgSender() || isApprovedForAll(from, _msgSender()),
            "ERC1155: caller is not token owner nor approved"
        );
        _safeBatchTransferFrom(from, to, ids, amounts, data);
    }

here is a Foundry PoC for this vulnerability:

    function testSafeTransferNonTransfererable() public {
        address user3 = vm.addr(3);
        address user4 = vm.addr(4);
        _fundUserWith4kNeuronByTreasury(_ownerAddress);
        _gameItemsContract.mint(0, 1);
        assertEq(_gameItemsContract.balanceOf(user3, 0), 0);
        _gameItemsContract.safeTransferFrom(_ownerAddress, user3, 0, 1, "");
        assertEq(_gameItemsContract.balanceOf(user3, 0), 1);
        vm.prank(_ownerAddress);
        _gameItemsContract.adjustTransferability(0, false);

        uint256[] memory items = new uint256[](1);
        items[0] = 0;
        uint256[] memory amounts = new uint256[](1);
        amounts[0] = 1;
        vm.prank(user3);
        _gameItemsContract.safeBatchTransferFrom(user3, user4, items, amounts, "");
        assertEq(_gameItemsContract.balanceOf(user4, 0), 1);
        assertEq(_gameItemsContract.balanceOf(user3, 0), 0);
    }

Tools Used

Manual

Recommended Mitigation Steps

Override all the transfer functions and protect it with the required check.

Assessed type

Other

Summary

Low Issues

Total: 6 instances over 4 issues

Non Critical Issues

Total: 31 instances over 17 issues

Low Issues

<a name="L-1"></a>[L-1] Unspecific compiler version pragma

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

2: pragma solidity >=0.8.0 <0.9.0;

Link to code

</details>

<a name="L-2"></a>[L-2] Forging new fighter types

According to the documentation the figther type should either be 0 or 1. However, no check is performed in this function to verify if the specified value is 0 or 1.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterFarm.sol

function incrementGeneration(uint8 fighterType) external returns (uint8) {
    require(msg.sender == _ownerAddress);
    generation[fighterType] += 1; //@audit The value of fighterType is not checked
    maxRerollsAllowed[fighterType] += 1;
    return generation[fighterType];
}

Link to code

</details>

<a name="L-3"></a>[L-3] Possible signature replay

If the protocole is deployed in another blockchain, a user would be able to replay the same signature to claim fighters on the second blockchain. This is possible because the chain id is not verified.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterFarm.sol

    function claimFighters(
        uint8[2] calldata numToMint,
        bytes calldata signature,
        string[] calldata modelHashes,
        string[] calldata modelTypes
    ) 
        external 
    {
        bytes32 msgHash = bytes32(keccak256(abi.encode(
            msg.sender, 
            numToMint[0], 
            numToMint[1],
            nftsClaimed[msg.sender][0],
            nftsClaimed[msg.sender][1]
        )));
        require(Verification.verify(msgHash, signature, _delegatedAddress));//@audit Possible signature replay.
        uint16 totalToMint = uint16(numToMint[0] + numToMint[1]);
        require(modelHashes.length == totalToMint && modelTypes.length == totalToMint);
        nftsClaimed[msg.sender][0] += numToMint[0];
        nftsClaimed[msg.sender][1] += numToMint[1];
        for (uint16 i = 0; i < totalToMint; i++) {
            _createNewFighter(
                msg.sender, 
                uint256(keccak256(abi.encode(msg.sender, fighters.length))),
                modelHashes[i], 
                modelTypes[i],
                i < numToMint[0] ? 0 : 1,
                0,
                [uint256(100), uint256(100)]
            );
        }
    }

Link to code

</details>

<a name="L-4"></a>[L-4] _setupRole function is deprecated

For more details check the following link

Instances (3):

<details><summary> see instances </summary>

File: src/Neuron.sol

function addMinter(address newMinterAddress) external {
    require(msg.sender == _ownerAddress);
    _setupRole(MINTER_ROLE, newMinterAddress);
}

/// @notice Adds a new address to the staker role.
/// @dev Only the owner address is authorized to call this function.
/// @param newStakerAddress The address to be added as a staker
function addStaker(address newStakerAddress) external {
    require(msg.sender == _ownerAddress);
    _setupRole(STAKER_ROLE, newStakerAddress);
}

/// @notice Adds a new address to the spender role.
/// @dev Only the owner address is authorized to call this function.
/// @param newSpenderAddress The address to be added as a spender
function addSpender(address newSpenderAddress) external {
    require(msg.sender == _ownerAddress);
    _setupRole(SPENDER_ROLE, newSpenderAddress);
}

Link to code

</details>

Non Critical Issues

<a name="NC-1"></a>[NC-1] [NatSpec] Natspec @dev is missing from documentation comments (New Instances)

Instances (7):

<details><summary> see instances </summary>

File: src/FighterOps.sol

4: /// @title FighterOps library for managing fighters in the AI Arena game.
   /// @author ArenaX Labs Inc.
   /// @notice This library is used for creating and managing Fighter NFTs in the AI Arena game.
   library FighterOps {

13:     /// @notice Emitted when a new Fighter NFT is created.

25:     /// @notice Struct that defines a Fighter's physical attributes.

35:     /// @notice Struct that defines a Fighter NFT.

52:     /// @notice Emits a FighterCreated event.

64:     /// @notice Extracting the fighter attributes from the struct
        /// @param self Fighter struct
        /// @return Array of Fighter Attributes 

78:     /// @notice Gets all of the relevant fighter information 

Link to code

</details>

<a name="NC-2"></a>[NC-2] [NatSpec] Natspec @param is missing (New Instances)

Instances (5):

<details><summary> see instances </summary>

File: src/FighterOps.sol

13:     /// @notice Emitted when a new Fighter NFT is created.

25:     /// @notice Struct that defines a Fighter's physical attributes.

35:     /// @notice Struct that defines a Fighter NFT.

52:     /// @notice Emits a FighterCreated event.

78:     /// @notice Gets all of the relevant fighter information 

Link to code

</details>

<a name="NC-3"></a>[NC-3] Complex functions should include comments (New Instances)

Large and/or complex functions should include comments to make them easier to understand and reduce margin for error.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

79:     function viewFighterInfo(

Link to code

</details>

<a name="NC-4"></a>[NC-4] Contract does not follow the Solidity style guide's suggested layout ordering (New Instances)

The style guide says that, within a contract, the ordering should be 1) Type declarations, 2) State variables, 3) Events, 4) Modifiers, and 5) Functions, but the contract(s) below do not follow this ordering

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

//@audit the structure definition is misplaced
26:     struct FighterPhysicalAttributes {

Link to code

</details>

<a name="NC-5"></a>[NC-5] Control structures do not follow the Solidity Style Guide (New Instances)

See the control structures section of the Solidity Style Guide

Instances (2):

<details><summary> see instances </summary>

File: src/FighterOps.sol

53:     function fighterCreatedEmitter(

79:     function viewFighterInfo(

Link to code

</details>

<a name="NC-6"></a>[NC-6] Consider adding emergency-stop functionality (New Instances)

In the event of a security breach or any unforeseen emergency, swiftly suspending all protocol operations becomes crucial. Having a mechanism in place to halt all functions collectively, instead of pausing individual contracts separately, substantially enhances the efficiency of mitigating ongoing attacks or vulnerabilities. This not only quickens the response time to potential threats but also reduces operational stress during these critical periods. Therefore, consider integrating a 'circuit breaker' or 'emergency stop' function into the smart contract system architecture. Such a feature would provide the capability to suspend the entire protocol instantly, which could prove invaluable during a time-sensitive crisis management situation.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

7: library FighterOps {

Link to code

</details>

<a name="NC-7"></a>[NC-7] Events are missing sender information (New Instances)

When an action is triggered based on a user's action, not being able to filter based on who triggered the action makes event processing a lot more cumbersome. Including the msg.sender the events of these types of action will make events much more useful to end users, especially when msg.sender is not tx.origin.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

61:         emit FighterCreated(id, weight, element, generation);

Link to code

</details>

<a name="NC-8"></a>[NC-8] Use named return values (New Instances)

Using named returns makes the code more self-documenting, makes it easier to fill out NatSpec, and in some cases can save gas. The cases below are where there currently is at most one return statement, which is ideal for named returns.

Instances (2):

<details><summary> see instances </summary>

File: src/FighterOps.sol

67:     function getFighterAttributes(Fighter storage self) public view returns (uint256[6] memory) {

79:     function viewFighterInfo(
            Fighter storage self, 
            address owner
        ) 
            public 
            view 
            returns (
                address,

Link to code

</details>

<a name="NC-9"></a>[NC-9] Missing zero address check in functions with address parameters (New Instances)

Adding a zero address check for each address type parameter can prevent errors.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

//@audit owner,  are not checked
79:     function viewFighterInfo(
            Fighter storage self, 
            address owner
        ) 

Link to code

</details>

<a name="NC-10"></a>[NC-10] public functions not called by the contract should be declared external instead (New Instances)

Contracts are allowed to override their parents' functions and change the visibility from external to public.

Instances (2):

<details><summary> see instances </summary>

File: src/FighterOps.sol

53:     function fighterCreatedEmitter(

79:     function viewFighterInfo(

Link to code

</details>

<a name="NC-11"></a>[NC-11] NatSpec @return argument is missing (New Instances)

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

// @audit the @return is missing
@notice Gets all of the relevant fighter information 
79:     function viewFighterInfo(

Link to code

</details>

<a name="NC-12"></a>[NC-12] Uncommented fields in a struct (New Instances)

Consider adding comments for all the fields in a struct to improve the readability of the codebase.

Instances (2):

<details><summary> see instances </summary>

File: src/FighterOps.sol

//@audit Add explanational comments to the following items `head`, `eyes`, `mouth`, `body`, `hands`, `feet`, 
26:     struct FighterPhysicalAttributes {
            uint256 head;
            uint256 eyes;
            uint256 mouth;
            uint256 body;
            uint256 hands;
            uint256 feet;
        }

//@audit Add explanational comments to the following items `weight`, `element`, `physicalAttributes`, `id`, `modelHash`, `modelType`, `generation`, `iconsType`, `dendroidBool`, 
36:     struct Fighter {
            uint256 weight;
            uint256 element;
            FighterPhysicalAttributes physicalAttributes;
            uint256 id;
            string modelHash;
            string modelType;
            uint8 generation;
            uint8 iconsType;
            bool dendroidBool;
        }

Link to code

</details>

<a name="NC-13"></a>[NC-13] Event is missing indexed fields (New Instances)

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.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

14:     event FighterCreated(

Link to code

</details>

<a name="NC-14"></a>[NC-14] Missing upgradability functionality (New Instances)

At the begining of a project, there is always the need to modify of add something to the source code especialy if any vulnerability is discovered. Therefore, having such system is crusial at least at the first stages of the project

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

1: // SPDX-License-Identifier: MIT

Link to code

</details>

<a name="NC-15"></a>[NC-15] Use the latest Solidity version for better security (New Instances)

Using the latest solidity version will help avoid old compiler related vulnerabilities

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

2: pragma solidity >=0.8.0 <0.9.0;

Link to code

</details>

<a name="NC-16"></a>[NC-16] Consider using SMTChecker (New Instances)

The SMTChecker is a valuable tool for Solidity developers as it helps detect potential vulnerabilities and logical errors in the contract's code. By utilizing Satisfiability Modulo Theories (SMT) solvers, it can reason about the potential states a contract can be in, and therefore, identify conditions that could lead to undesirable behavior. This automatic formal verification can catch issues that might otherwise be missed in manual code reviews or standard testing, enhancing the overall contract's security and reliability.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

2: pragma solidity >=0.8.0 <0.9.0;

Link to code

</details>

<a name="NC-17"></a>[NC-17] Returning a struct instead of returning many variables is better (New Instances)

Returning a struct from a Solidity function instead of multiple variables offers several benefits, enhancing code clarity and efficiency. Structs allow for the grouping of related data into a single entity, making the function's return values more organized and easier to manage. This approach significantly improves readability, as it encapsulates the data logically, helping developers quickly understand the returned information's structure. Additionally, it simplifies function interfaces, reducing the potential for errors when handling multiple return values. By using structs, you can also easily extend or modify the returned data without altering the function signature, facilitating smoother updates and maintenance of your smart contract code.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

79:     function viewFighterInfo(
            Fighter storage self, 
            address owner
        ) 
            public 
            view 
            returns (
                address,
                uint256[6] memory,
                uint256,
                uint256,
                string memory,
                string memory,
                uint16
            )
        {
            return (
                owner,
                getFighterAttributes(self),
                self.weight,
                self.element,
                self.modelHash,
                self.modelType,
                self.generation
            );
        }

Link to code

</details>

Summary

Gas Optimizations

<a name="GAS-1"></a>[GAS-1] Avoid external call for event emiting to save gas

The _createNewFighter internal function perform an external call to fighterCreatedEmitter() in the FighterOps contract to simply emit an event. Therefore, to save gas you should copy that event into the FighterFarm and call it internaly to save 3301 gas

Instances (1):

<details><summary> see instances </summary>

File: /src/FighterFarm.sol

function _createNewFighter(
        address to, 
        uint256 dna, 
        string memory modelHash,
        string memory modelType, 
        uint8 fighterType,
        uint8 iconsType,
        uint256[2] memory customAttributes
    ) 
        private 
    {  
        require(balanceOf(to) < MAX_FIGHTERS_ALLOWED);
        uint256 element; 
        uint256 weight;
        uint256 newDna;
        if (customAttributes[0] == 100) {
            (element, weight, newDna) = _createFighterBase(dna, fighterType);
        }
        else {//@audit-info require more investigations (it seems like we can predict what kind of Fighter we can get in minting)
            element = customAttributes[0];
            weight = customAttributes[1];
            newDna = dna;
        }
        uint256 newId = fighters.length;

        bool dendroidBool = fighterType == 1;
        FighterOps.FighterPhysicalAttributes memory attrs = _aiArenaHelperInstance.createPhysicalAttributes(
            newDna,
            generation[fighterType],
            iconsType,
            dendroidBool
        );
        fighters.push(
            FighterOps.Fighter(
                weight,
                element,
                attrs,
                newId,
                modelHash,
                modelType,
                generation[fighterType],
                iconsType,
                dendroidBool
            )
        );
        _safeMint(to, newId);
        FighterOps.fighterCreatedEmitter(newId, weight, element, generation[fighterType]); //@audit GAS: you can copy the same event and call it directly in this function to
    }

Link to code

</details>

<a name="GAS-2"></a>[GAS-2] Remove redendente code to save gas

The value of attributeProbabilities is calculated two times when the contract is deployed for the first time in the constractor function. the first time when the addAttributeProbabilities() funtion is called, and the second time throught the for loop.

Instances (1):

<details><summary> see instances </summary>

File: src/AiArenaHelper.sol

constructor(uint8[][] memory probabilities) {
    _ownerAddress = msg.sender;

    // Initialize the probabilities for each attribute
    addAttributeProbabilities(0, probabilities);

    uint256 attributesLength = attributes.length;
    for (uint8 i = 0; i < attributesLength; i++) {
        attributeProbabilities[0][attributes[i]] = probabilities[i]; // @audit Gas: this line of code could be removed as it is already calculated before
        attributeToDnaDivisor[attributes[i]] = defaultAttributeDivisor[i];
    }
} 

Link to code

</details>

<a name="GAS-3"></a>[GAS-3] Optimize Deployment Size by Fine-tuning IPFS Hash

Optimizing the deployment size of a smart contract is vital to minimize gas costs, and one way to achieve this is by fine-tuning the IPFS hash appended by the Solidity compiler as metadata. This metadata, consisting of 53 bytes, increases the gas required for contract deployment by approximately 10,600 gas due to bytecode costs, and additionally, up to 848 gas due to calldata costs, depending on the proportion of zero and non-zero bytes. Utilize the --no-cbor-metadata compiler flag to prevent the compiler from appending metadata. However, this approach has a drawback as it can complicate the contract verification process on block explorers like Etherscan, potentially reducing transparency.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

7: library FighterOps {

Link to code

</details>

<a name="GAS-4"></a>[GAS-4] Use assembly to emit events

We can use assembly to emit events efficiently by utilizing scratch space and the free memory pointer. This will allow us to potentially avoid memory expansion costs. Note: In order to do this optimization safely, we will need to cache and restore the free memory pointer.

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

61:         emit FighterCreated(id, weight, element, generation);

Link to code

</details>

<a name="GAS-5"></a>[GAS-5] 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):

<details><summary> see instances </summary>

File: src/FighterOps.sol

//@audit change `dendroidBool` to `uint256`
36:     struct Fighter {
            uint256 weight;
            uint256 element;
            FighterPhysicalAttributes physicalAttributes;
            uint256 id;
            string modelHash;
            string modelType;
            uint8 generation;
            uint8 iconsType;
            bool dendroidBool;
        }

Link to code

File: src/FighterOps.sol

//@audit change `dendroidBool` to `uint256`
36:     struct Fighter {
            uint256 weight;
            uint256 element;
            FighterPhysicalAttributes physicalAttributes;
            uint256 id;
            string modelHash;
            string modelType;
            uint8 generation;
            uint8 iconsType;
            bool dendroidBool;
        }

Link to code

File: src/GameItems.sol

//@audit change `finiteSupply` to `uint256`
//@audit change `transferable` to `uint256`
35:     struct GameItemAttributes {
            string name;
            bool finiteSupply;
            bool transferable;
            uint256 itemsRemaining;
            uint256 itemPrice;
            uint256 dailyAllowance;
        }  

Link to code

</details>

<a name="GAS-6"></a>[GAS-6] Optimize names to save gas

public/external function names and public member variable names can be optimized to save gas. See this link for an example of how it works. Below are the interfaces/abstract contracts that can be optimized so that the most frequently-called functions use the least amount of gas possible during method lookup. Method IDs that have two leading zero bytes can save 128 gas each during deployment, and renaming functions to have lower method IDs will save 22 gas per call, per sorted position shifted

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

// @audit fighterCreatedEmitter(uint256,uint256,uint256,uint8) ==> fighterCreatedEmitter_M1y(uint256,uint256,uint256,uint8),00004d48
7: library FighterOps {

Link to code

</details>

<a name="GAS-7"></a>[GAS-7] Usage of uints/ints smaller than 32 bytes (256 bits) incurs overhead

When 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

Instances (1):

<details><summary> see instances </summary>

File: src/FighterOps.sol

//@audit `generation` is `uint8`
57:         uint8 generation

Link to code

</details>

<a name="GAS-8"></a>[GAS-8] State variable read in a loop (New Instances)

The state variable should be cached in a local variable rather than reading it on every iteration of the for-loop, which will replace each Gwarmaccess (100 gas) with a much cheaper stack read.

Instances (1):

<details><summary> see instances </summary>

File: src/MergingPool.sol

//@audit `roundId` is read in this loop
149:         for (uint32 currentRound = lowerBound; currentRound < roundId; currentRound++) {

Link to code

</details>