AI Arena - K42's results

In AI Arena you train an AI character to battle in a platform fighting game. Imagine a cross between Pokรฉmon and Super Smash Bros, but the characters are AIs, and you can train them to learn almost any skill in preparation for battle.

General Information

Platform: Code4rena

Start Date: 09/02/2024

Pot Size: $60,500 USDC

Total HM: 17

Participants: 283

Period: 12 days

Judge:

Id: 328

League: ETH

AI Arena

Findings Distribution

Researcher Performance

Rank: 22/283

Findings: 2

Award: $262.57

๐ŸŒŸ Selected for report: 0

๐Ÿš€ Solo Findings: 0

Awards

242.4952 USDC - $242.50

Labels

bug
G (Gas Optimization)
grade-a
high quality report
sponsor acknowledged
G-31

External Links

Gas Optimization Report for AI-Arena by K42

Possible Optimization in AiArenaHelper.sol

Possible Optimization =

  • Issue: Storing attribute probabilities for each generation in a nested mapping introduces significant gas costs when updating these probabilities.
  • Optimization: Use a single-level mapping with a composite key combining generation and attribute name. This reduces the depth of mapping access and minimizes gas costs associated with storage operations.

Here is the optimized code snippet:

// Before Optimization
mapping(uint256 => mapping(string => uint8[])) public attributeProbabilities;

// After Optimization
mapping(bytes32 => uint8[]) public attributeProbabilitiesOptimized;

function _generateCompositeKey(uint256 generation, string memory attribute) private pure returns (bytes32) {
    return keccak256(abi.encodePacked(generation, attribute));
}

function addAttributeProbabilitiesOptimized(uint256 generation, uint8[][] memory probabilities) public {
    require(msg.sender == _ownerAddress, "Only owner can add probabilities");
    require(probabilities.length == attributes.length, "Invalid probabilities length");

    for (uint8 i = 0; i < attributes.length; i++) {
        bytes32 key = _generateCompositeKey(generation, attributes[i]);
        attributeProbabilitiesOptimized[key] = probabilities[i];
    }
}
  • Estimated gas saved = This change can significantly reduce gas costs by minimizing the depth of mapping access and the number of storage slots needed. The exact savings depend on the frequency and size of updates but expect reductions in the range of 20-30% for transactions involving these mappings.

Possible Optimizations in FighterFarm.sol

Possible Optimization 1 =

  • Issue: Frequent updates to state variables like numTrained and totalNumTrained can be gas-intensive.
  • Optimization: Batch updates to these variables or use events to log changes instead of storing every incremental change in state variables.

Here is the optimized code snippet:

// Before optimization
function updateModel(uint256 tokenId, string calldata modelHash, string calldata modelType) external {
    require(msg.sender == ownerOf(tokenId));
    fighters[tokenId].modelHash = modelHash;
    fighters[tokenId].modelType = modelType;
    numTrained[tokenId] += 1;
    totalNumTrained += 1;
}

// After optimization
function updateModel(uint256 tokenId, string calldata modelHash, string calldata modelType) external {
    require(msg.sender == ownerOf(tokenId));
    fighters[tokenId].modelHash = modelHash;
    fighters[tokenId].modelType = modelType;
    emit ModelUpdated(tokenId, modelHash, modelType); // Use an event instead of modifying state
}
  • Estimated gas saved = This change can significantly reduce gas costs associated with storage operations. The exact savings depend on the frequency of updateModel calls and the current gas prices.

Possible Optimization 2 =

  • Issue: The claimFighters() function performs multiple state changes within a loop, which can be gas-intensive.
  • Optimization: Batch state changes outside the loop to minimize the number of state updates. Specifically, update nftsClaimed after the loop to reduce the gas cost associated with state changes.

Here is the optimized code:

function claimFighters(
    uint8[2] calldata numToMint,
    bytes calldata signature,
    string[] calldata modelHashes,
    string[] calldata modelTypes
) external {
    // Existing validation logic remains unchanged
    uint16 totalToMint = uint16(numToMint[0] + numToMint[1]);
    // Perform minting operations first without updating nftsClaimed
    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)]
        );
    }
    // Batch update nftsClaimed after all minting operations
    nftsClaimed[msg.sender][0] += numToMint[0];
    nftsClaimed[msg.sender][1] += numToMint[1];
}
  • Estimated gas saved = This optimization can significantly reduce gas costs by minimizing the number of state changes, especially for bulk minting operations. The exact savings depend on the number of fighters being claimed but expect to save thousands of gas per transaction.

Possible Optimizations in GameItems.sol

Possible Optimization 1 =

  • Issue: The current implementation checks and updates the dailyAllowanceReplenishTime and allowanceRemaining for each mint operation, which can be optimized.
  • Optimization: Implement a single check at the beginning of the mint() function to update the daily allowance for all items at once if the replenish time has passed, instead of checking and updating it for each item individually.

Here is the optimized code:

function mint(uint256 tokenId, uint256 quantity) external {
    // Check and replenish daily allowance at the start
    if (dailyAllowanceReplenishTime[msg.sender][tokenId] <= block.timestamp) {
        _replenishDailyAllowance(tokenId);
    }
    // Continue with existing mint logic...
}
  • Estimated gas saved = This optimization reduces the gas cost by minimizing redundant checks and state updates, especially beneficial for users minting multiple items in a short period. The exact savings would depend on the frequency of mint operations and the number of items being minted.

Possible Optimization 2 =

  • Issue: The _replenishDailyAllowance() function updates allowanceRemaining and dailyAllowanceReplenishTime separately for each token ID.
  • Optimization: Implement a mechanism to batch these updates for multiple token IDs in a single transaction when possible.

Here is the optimized code snippet:

function _replenishDailyAllowances(uint256[] memory tokenIds) private {
    for (uint i = 0; i < tokenIds.length; i++) {
        uint256 tokenId = tokenIds[i];
        allowanceRemaining[msg.sender][tokenId] = allGameItemAttributes[tokenId].dailyAllowance;
        dailyAllowanceReplenishTime[msg.sender][tokenId] = uint32(block.timestamp + 1 days);
    }
}
  • Estimated gas saved = This optimization can reduce the gas cost when multiple tokens' daily allowances need replenishing, especially if called in contexts where batch processing is feasible. Savings vary based on the number of tokens processed.

Possible Optimization 3 =

  • Issue: Setting token URIs individually for each item can be gas-intensive, especially when adding multiple items.
  • Optimization: Allow batch setting of token URIs for multiple items in a single transaction to reduce gas costs associated with multiple state updates.

Here is the optimized code:

function setTokenURIs(uint256[] calldata tokenIds, string[] calldata uris) external {
    require(isAdmin[msg.sender], "Only admin can set URIs");
    require(tokenIds.length == uris.length, "TokenIds and URIs length mismatch");
    for (uint256 i = 0; i < tokenIds.length; i++) {
        _tokenURIs[tokenIds[i]] = uris[i];
    }
}
  • Estimated gas saved = This change significantly reduces gas costs when setting URIs for multiple items, as it consolidates state changes into a single transaction. Savings scale with the number of URIs being set simultaneously.

Possible Optimizations in MergingPool.sol

Possible Optimization 1 =

  • Issue: The addPoints() function updates fighterPoints and totalPoints for each token ID individually, which could be optimized for scenarios where multiple points need to be added across different token IDs.
  • Optimization: Allow batch processing of points addition to minimize the gas cost associated with multiple transactions and state updates.

Here is the optimized code snippet:

function addPointsBatch(uint256[] calldata tokenIds, uint256[] calldata pointsToAdd) external {
    require(msg.sender == _rankedBattleAddress, "Only Ranked Battle can add points");
    require(tokenIds.length == pointsToAdd.length, "Array length mismatch");

    for (uint256 i = 0; i < tokenIds.length; i++) {
        fighterPoints[tokenIds[i]] += pointsToAdd[i];
        totalPoints += pointsToAdd[i];
        emit PointsAdded(tokenIds[i], pointsToAdd[i]);
    }
}
  • Estimated gas saved = This optimization reduces the gas cost by consolidating state changes into fewer transactions. The savings would be particularly noticeable when adding points to multiple fighters in a single operation, with potential savings scaling with the number of updates performed in each batch.

Possible Optimization 2 =

  • Issue: The claimRewards() function iterates through all unclaimed rounds for a user, potentially leading to inefficient gas usage if a user has multiple rounds of unclaimed rewards.
  • Optimization: Implement a batch processing mechanism that aggregates claims across multiple rounds into a single operation where possible.

Here is the optimized code:

function claimRewardsBatch(
    uint32[] calldata roundIndices,
    string[] calldata modelURIs, 
    string[] calldata modelTypes,
    uint256[2][] calldata customAttributes
) external {
    require(roundIndices.length > 0, "No rounds specified");
    uint32 claimCount = 0;
    for (uint32 i = 0; i < roundIndices.length; i++) {
        uint32 round = roundIndices[i];
        require(round < roundId, "Round does not exist");
        require(isWinner(msg.sender, round), "Not a winner for round");
        _fighterFarmInstance.mintFromMergingPool(
            msg.sender,
            modelURIs[claimCount],
            modelTypes[claimCount],
            customAttributes[claimCount]
        );
        claimCount++;
        numRoundsClaimed[msg.sender]++;
    }
    if (claimCount > 0) {
        emit Claimed(msg.sender, claimCount);
    }
}
  • Estimated gas saved = This approach can significantly reduce gas costs for users claiming multiple rewards by consolidating state updates and reducing the number of transactions required. The savings will be more substantial for users with rewards spread across multiple rounds.

Possible Optimizations in Neuron.sol

Possible Optimization 1 =

  • Issue: The setupAirdrop() function iterates through recipients to set allowances individually, which can be gas-intensive for large airdrops.
  • Optimization: Implement a batch processing mechanism to set allowances for multiple recipients in a single transaction.

Here is the optimized code snippet:

function setupAirdropBatch(address[] calldata recipients, uint256[] calldata amounts) external {
    require(isAdmin[msg.sender], "Only admin can setup airdrops");
    require(recipients.length == amounts.length, "Mismatch between recipients and amounts");

    for (uint256 i = 0; i < recipients.length; i++) {
        _approve(treasuryAddress, recipients[i], amounts[i]);
    }
}
  • Estimated gas saved = This optimization could significantly reduce gas costs by minimizing the number of transactions required for large airdrops. The exact savings would depend on the size of the airdrop.

Possible Optimization 2 =

  • Issue: The mint() function checks for the max supply cap with each mint operation, which is necessary but can be optimized.
  • Optimization: Pre-calculate the remaining supply before minting to reduce the cost of arithmetic operations and condition checks within the mint() function.

Here is the optimized code:

function mintOptimized(address to, uint256 amount) public {
    require(hasRole(MINTER_ROLE, msg.sender), "Must have minter role");
    uint256 remainingSupply = MAX_SUPPLY - totalSupply();
    require(amount <= remainingSupply, "Exceeds max supply");

    _mint(to, amount);
}
  • Estimated gas saved = This change potentially reduces the computational overhead by simplifying the supply cap check, especially if the totalSupply() function involves non-trivial computation or state access. Savings are minor per transaction but could add up over many transactions.

Possible Optimizations in RankedBattle.sol

Possible Optimization 1 =

  • Issue: Updating battle records individually for each fighter can be gas-intensive, especially after a large number of battles.
  • Optimization: Implement a batch update mechanism for battle records. This approach can consolidate multiple transaction calls into a single call, reducing the overall gas cost.

Here is the optimized code snippet:

function updateBattleRecordsBatch(uint256[] calldata tokenIds, uint8[] calldata battleResults) external {
    require(isAdmin[msg.sender], "Only admin can update records");
    require(tokenIds.length == battleResults.length, "Mismatch between token IDs and results");
    
    for (uint256 i = 0; i < tokenIds.length; i++) {
        _updateRecord(tokenIds[i], battleResults[i]);
    }
}
  • Estimated gas saved = This optimization can significantly reduce gas costs when updating records for multiple fighters, especially after large-scale events or battles. Proper validation of inputs (e.g., ensuring tokenIds and battleResults arrays are of equal length and contain valid data) is crucial. The require(isAdmin[msg.sender], "Only admin can update records"); check is essential to prevent unauthorized access to this function.

Possible Optimization 2 =

  • Issue: Individual claims for NRN rewards can lead to higher transaction costs for users.
  • Optimization: Allow users to claim NRN rewards for multiple rounds in a single transaction. This can be achieved by modifying the claimNRN() function to process claims across multiple rounds.

Here is the optimized code:

function claimNRNMultiround() external {
    uint256 totalClaimableNRN = 0;
    for (uint32 round = numRoundsClaimed[msg.sender]; round < roundId; round++) {
        uint256 nrnDistribution = rankedNrnDistribution[round];
        uint256 points = accumulatedPointsPerAddress[msg.sender][round];
        totalClaimableNRN += (points * nrnDistribution) / totalAccumulatedPoints[round];
        numRoundsClaimed[msg.sender] = round + 1;
    }
    
    if (totalClaimableNRN > 0) {
        _neuronInstance.mint(msg.sender, totalClaimableNRN);
        emit Claimed(msg.sender, totalClaimableNRN);
    }
}
  • Estimated gas saved = This optimization significantly reduces the gas cost for users claiming rewards from multiple rounds, as it aggregates multiple transactions into one. Ensure the the calculation of totalClaimableNRN is accurate and cannot be manipulated. The loop should correctly iterate over the rounds the user has participated in, and the calculations must accurately reflect the user's earned rewards without allowing for double-claiming or skipping rounds. The contract must also handle the potential for overflow in totalClaimableNRN and ensure that the numRoundsClaimed[msg.sender] is updated atomically to reflect the new state post-claim.

Possible Optimizations in StakeAtRisk.sol

Possible Optimization 1 =

  • Issue: The reclaimNRN() function transfers NRN tokens individually for each reclaim operation, which can be gas-intensive when multiple reclaims occur in a short period.
  • Optimization: Implement a batch processing mechanism for reclaiming NRN tokens. This would allow multiple reclaims to be processed in a single transaction, reducing the gas cost per reclaim.

Here is the optimized code snippet:

function reclaimNRNBatch(uint256[] calldata nrnToReclaimAmounts, uint256[] calldata fighterIds, address[] calldata fighterOwners) external {
    require(msg.sender == _rankedBattleAddress, "Call must be from RankedBattle contract");
    require(nrnToReclaimAmounts.length == fighterIds.length && fighterIds.length == fighterOwners.length, "Array length mismatch");

    for (uint256 i = 0; i < fighterIds.length; i++) {
        uint256 fighterId = fighterIds[i];
        uint256 nrnToReclaim = nrnToReclaimAmounts[i];
        address fighterOwner = fighterOwners[i];

        require(stakeAtRisk[roundId][fighterId] >= nrnToReclaim, "Insufficient stake at risk");

        stakeAtRisk[roundId][fighterId] -= nrnToReclaim;
        totalStakeAtRisk[roundId] -= nrnToReclaim;
        amountLost[fighterOwner] -= nrnToReclaim;

        emit ReclaimedStake(fighterId, nrnToReclaim);
    }

    bool success = _neuronInstance.transfer(_rankedBattleAddress, sum(nrnToReclaimAmounts));
    require(success, "NRN transfer failed");
}
  • Estimated gas saved = This optimization reduces the gas cost by minimizing the number of transactions required to reclaim NRN tokens for multiple fighters. The exact savings depend on the number of reclaims processed in each batch.

Possible Optimization 2 =

  • Issue: The updateAtRiskRecords() function updates the stake at risk for each fighter individually, which can lead to higher gas costs when updating multiple records.
  • Optimization: Allow batch updates to the stake at risk records. This would consolidate multiple updates into a single transaction, reducing the overall gas cost.

Here is the optimized code:

function updateAtRiskRecordsBatch(uint256[] calldata nrnToPlaceAtRiskAmounts, uint256[] calldata fighterIds, address[] calldata fighterOwners) external {
    require(msg.sender == _rankedBattleAddress, "Call must be from RankedBattle contract");
    require(nrnToPlaceAtRiskAmounts.length == fighterIds.length && fighterIds.length == fighterOwners.length, "Array length mismatch");

    uint256 totalRiskIncrease = 0;

    for (uint256 i = 0; i < fighterIds.length; i++) {
        uint256 fighterId = fighterIds[i];
        uint256 nrnToPlaceAtRisk = nrnToPlaceAtRiskAmounts[i];
        address fighterOwner = fighterOwners[i];

        stakeAtRisk[roundId][fighterId] += nrnToPlaceAtRisk;
        totalRiskIncrease += nrnToPlaceAtRisk;
        amountLost[fighterOwner] += nrnToPlaceAtRisk;

        emit IncreasedStakeAtRisk(fighterId, nrnToPlaceAtRisk);
    }

    totalStakeAtRisk[roundId] += totalRiskIncrease;
}
  • Estimated gas saved = This optimization significantly reduces the gas cost when updating stake at risk records for multiple fighters in a single operation. Savings scale with the number of records updated simultaneously.

Possible Optimization in VoltageManager.sol

Possible Optimization =

  • Issue: The useVoltageBattery() and spendVoltage() functions operate on a per-user basis, which could be inefficient if multiple users need their voltage replenished simultaneously.
  • Optimization: Implement a function that allows batch processing of voltage replenishment. This can be particularly useful for admin tasks or automated processes that manage voltage for multiple users at once.

Here is the optimized code snippet:

function replenishVoltageBatch(address[] calldata owners) external {
    require(isAdmin[msg.sender], "Only admin can replenish voltage");
    for (uint i = 0; i < owners.length; i++) {
        if (playerData[owners[i]].voltageReplenishTime <= block.timestamp) {
            playerData[owners[i]].voltage = 100;
            playerData[owners[i]].voltageReplenishTime = uint32(block.timestamp + 1 days);
            emit VoltageRemaining(owners[i], 100);
        }
    }
}
  • Estimated gas saved = While the savings per transaction might not be significant, this approach reduces the total number of transactions required for voltage management across multiple users, leading to overall gas savings.

#0 - raymondfam

2024-02-25T22:32:02Z

28 non-generic optimized code refactoring

#1 - c4-pre-sort

2024-02-25T22:32:22Z

raymondfam marked the issue as high quality report

#2 - c4-sponsor

2024-03-04T01:31:14Z

brandinho (sponsor) acknowledged

#3 - brandinho

2024-03-04T01:33:50Z

We might implement some of these optimizations, but most of the batch ones are not applicable for how we want to structure transactions with our project.

#4 - c4-judge

2024-03-19T07:45:10Z

HickupHH3 marked the issue as grade-a

Advanced Analysis Report for Ai-Arena by K42

Overview
  • The Ai-Arena project introduces a unique, on-chain NFT game that leverages the ERC-721 and ERC-1155 standards for a game involving NFT characters and in-game items.
Understanding the Ecosystem:
  • The ecosystem is a well-orchestrated play-to-earn model leveraging ERC721 and ERC1155 tokens for characters and items, respectively, and ERC20 tokens for in-game currency. It is supported by a range of solidity smart contracts that manage the game's logic, including battles, NFT staking, and NFT trading.
Codebase Quality Analysis:

1. Key Data Structures and Libraries in all contracts

  • The use of OpenZeppelin libraries for ERC standards, AccessControl, and security measures is a positive sign. As well as the use of solmate for gas efficent fixed point math usage, that is awesome. However, the game's data and the in-game logic are tightly coupled across the contracts, which could be a risk for upgradability and long term maintainability.

2. Use of Modifiers and Access Control in all contracts

  • The use of onlyOwner and isAdmin access controls is a concern, but at first glance well implemented. The game's power is too centralized however as a result, and the use of a single account to manage the game's control can be a single point of failure.

3. Use of State Variables in all contracts

  • The public and private state variables across the ecosystem can be a major risk, if creative attack vectors are used. It is always a good idea to review the scoping of these variables to ensure they are only accessible to the right entities.

4. Use of Events and Logging in all contracts

  • The event usage is well-implemented, with a lot of the game's main state changes being emitted.
Codebase Analysis:

AiArenaHelper.sol Contract

Functions and Risks

transferOwnership(address newOwnerAddress)
  • Specific Risk: Centralization Risk. This function allows the contract owner to transfer ownership, which could lead to a single point of failure if the owner's private key is compromised.
  • Recommendation: Implement a multi-signature mechanism for critical functions like this to enhance security and distribute trust.
addAttributeDivisor(uint8[] memory attributeDivisors)
  • Specific Risk: Input Validation Risk. Without proper validation, there's a risk that the input array might not match the expected length, potentially leading to incorrect configuration of attribute divisors.
  • Recommendation: Ensure rigorous validation of the input array length to match the expected number of attributes, preventing misconfiguration.
createPhysicalAttributes(uint256 dna, uint8 generation, uint8 iconsType, bool dendroidBool)
  • Specific Risk: Deterministic Output Risk. Given the deterministic nature of blockchain, the output of this function is predictable if the inputs are known, potentially allowing gaming of the system to achieve desired attributes.
  • Recommendation: Introduce additional randomness or unpredictability in the attribute generation process to mitigate predictability and ensure fairness.
addAttributeProbabilities(uint256 generation, uint8[][] memory probabilities)
  • Specific Risk: Unauthorized Access Risk. This function allows the owner to set probabilities for attributes, which could be misused if the owner account is compromised, affecting the game's balance and fairness.
  • Recommendation: Limit access to this function with role-based access controls (RBAC) and consider a decentralized governance model for making such critical changes.
deleteAttributeProbabilities(uint8 generation)
  • Specific Risk: Data Loss Risk. This function enables the deletion of attribute probabilities for a generation, which could unintentionally remove essential data for the game's mechanics.
  • Recommendation: Implement a safeguard mechanism, such as requiring confirmation through a multi-signature process or a time-lock delay to prevent accidental or malicious deletions.
getAttributeProbabilities(uint256 generation, string memory attribute)
  • Specific Risk: Transparency vs. Strategy Risk. While providing transparency, revealing attribute probabilities could influence player strategies in a way that reduces the game's competitive nature.
  • Recommendation: Balance transparency with gameplay integrity by carefully considering what information is made public and when.
dnaToIndex(uint256 generation, uint256 rarityRank, string memory attribute)
  • Specific Risk: Algorithm Exploit Risk. The method for converting DNA to attribute index based on rarity rank could be exploited if the underlying algorithm or its inputs are predictable.
  • Recommendation: Review and possibly enhance the algorithm for converting DNA to attribute indices to ensure it remains unpredictable and fair.

FighterFarm.sol Contract

Functions and Risks

transferOwnership(address newOwnerAddress)
  • Specific Risk: Potential for a single point of control, where the contract's ownership could be transferred to a malicious address if the private key of the current owner is compromised.
  • Recommendation: Implement a multi-signature requirement or a decentralized governance mechanism for ownership-related and other sensitive functions to mitigate the potential for misuse.
incrementGeneration(uint8 fighterType)
  • Specific Risk: The operation can unilaterally change the game's economy or balance by the owner, potentially introducing new generations without community consent, which could affect the rarity and value of the NFTs.
  • Recommendation: Consider a DAO or a community voting mechanism for any action that could have a significant effect on the game's balance or the NFTs' value to ensure the community's support for the change.
addStaker(address newStaker)
  • Specific Risk: The owner has the power to arbitrarily add new stakers, which could be a vector for centralization and favoritism, potentially compromising the staking mechanism's fairness.
  • Recommendation: Implement a transparent, permissionless strategy for adding stakers, possibly through a staking contract that allows any user to participate under predefined and automatic conditions.
instantiateAIArenaHelperContract(address aiArenaHelperAddress)
  • Specific Risk: The process of updating or setting a new AI Arena Helper contract poses a security risk if the new address is not properly audited, potentially compromising the game's integrity.
  • Recommendation: Any changes to the AI Arena Helper or other critical game mechanics should be done through a time-locked, community-reviewed process to ensure the new code's trustworthiness.
claimFighters(...)
  • Specific Risk: The process of claiming fighters could be exploited if the input validation is not secure, potentially allowing bad actors to mint more NFTs than intended or to others' detriment.
  • Recommendation: Ensure that the number of NFTs a player can claim is capped and that inputs are validated against a whitelist or a cryptographic proof (like signatures) to prevent over-claiming or fraudulent minting.
redeemMintPass(...)
  • Specific Risk: The process of redeeming mint passes and burning them for NFTs could be a vector for economic imbalance if the mint pass system is exploited or if the burning mechanism fails.
  • Recommendation: Ensure the mint pass redemption and burn logic is airtight, with fail-safes and pre-checks to prevent double spending or the minting of NFTs without a valid mint pass.
updateFighterStaking(uint256 tokenId, bool stakingStatus)
  • Specific Risk: The process of updating a fighter's staking status could be a vector for staking-related exploits, such as staking the same NFT in multiple pools or withdrawing it without the right to do so.
  • Recommendation: Secure the process with a time-locked or escrow-like mechanism to prevent the same NFT from being used in multiple systems or being unstaked too quickly, and ensure the process is in line with the staking pool's game theory.
setTokenURI(uint256 tokenId, string calldata newTokenURI)
  • Specific Risk: The process of updating the token URI could be exploited to redirect the metadata to fraudulent or inappropriate data, affecting the NFT's visual representation and value.
  • Recommendation: Restrict the process of updating the token URI to a set of pre-approved or on-chain generated URIs to prevent misuse, and ensure the data's integrity and appropriateness.
updateModel(uint256 tokenId, string calldata modelHash, string calldata modelType)
  • Specific Risk: The action of updating the model for a given token could be exploited to change the NFT's in-game character or model, affecting the game's balance or the NFT's collectible value.
  • Recommendation: Any action that could alter the NFT's in-game value or balance should be community-reviewed or -proposed, with changes being as transparent and consensual as possible.

FighterOps.sol Contract

Functions and Risks

fighterCreatedEmitter(uint256 id, uint256 weight, uint256 element, uint8 generation)
  • Specific Risk: Event Over-Reliance. The use of this event as the sole means of tracking fighter creation could lead to issues if event logs are not properly monitored or if they are inaccessible due to data pruning by Ethereum nodes.
  • Recommendation: Supplement the event-based tracking with state-based records for critical game mechanics. Consider maintaining a mapping or an array of created fighters for on-chain verification and data integrity.
getFighterAttributes(Fighter storage self)
  • Specific Risk: View Function Misuse. As a public view function, it exposes the full set of a fighter's attributes, which could be leveraged by players to gain an unfair edge in gameplay or for botting purposes.
  • Recommendation: Implement access control mechanisms to limit the exposure of sensitive data. Consider using hash-based or encrypted data for attributes that should remain hidden until certain gameplay conditions are met.
viewFighterInfo(Fighter storage self, address owner)
  • Specific Risk: Inadvertent Privacy Breach. This view function consolidates and exposes a lot of data about a fighter, including potentially player-identifiable data through the association of the owner address.
  • Recommendation: Ensure that the use of the owner parameter and the data returned by this method are in compliance with the intended game dynamics and privacy considerations. Evaluate the necessity of all the returned data and potentially restrict access to sensitive data.

GameItems.sol Contract

Functions and Risks

transferOwnership(address newOwnerAddress)
  • Specific Risk: Centralization of control introduces a high risk if the contract owner's management key is compromised, allowing a bad actor to unilaterally change the game's course.
  • Recommendation: Implement a decentralized decision-making mechanism, such as a DAO, to govern the game's future, or a multi-signature system for this and other sensitive operations.
adjustAdminAccess(address adminAddress, bool access)
  • Specific Risk: The power to grant or revoke admin status can be a vector for misuse, potentially allowing the unauthorized access to admin-only methods or the locking out of legitimate actors.
  • Recommendation: Transition to a transparent, community-validated system for role changes, such as a time-locked, off-chain voting strategy to limit the admin's absolute authority.
adjustTransferability(uint256 tokenId, bool transferable)
  • Specific Risk: The game's ability to toggle the transferability of in-game items can have a major, direct effect on the player economy, potentially rendering items illiquid without notice.
  • Recommendation: Token transferability should be a right of the token holder, not the system. If the game's logic requires the option to lock items, it should be a part of the in-game state, not the NFT's on-chain security properties.
mint(uint256 tokenId, uint256 quantity)
  • Specific Risk: The unbounded item minting can lead to the devaluation of in-game items, and the game's market can be flooded with items, making the in-game environment unbalanced.
  • Recommendation: Implement a cap on the number of items that can be minted and a deflationary system to ensure the game's market balance. The cap should be a decision of the game's governance model.
setAllowedBurningAddresses(address newBurningAddress)
  • Specific Risk: The process of allowing specific addresses to burn in-game items can be a vector for potential misuse, where an address could be authorized to burn items they shouldn't have access to.
  • Recommendation: Enforce a strict validation and a uditing trail for any changes to allowed burning addresses. Consider a DAO or a community vote for any new addresses to be added to this list, ensuring transparency and community support.
setTokenURI(uint256 tokenId, string memory _tokenURI)
  • Specific Risk: The process of updating the tokenURI dynamically could be exploited to redirect metadata to fraudulent or misleading data, affecting the in-game representation and the value of the NFTs.
  • Recommendation: Implement a secure, verifiable method for metadata updates, such as a DAO vote or a content hash check to ensure the integrity of the NFT metadata. Additionally, consider a review process for any content changes.
createGameItem(...)
  • Specific Risk: The process of creating new game items unilaterally by the admin can be a vector for economic imbalance, potentially saturating the market with new or unbalanced items.
  • Recommendation: Transition to a model where the community or a DAO has a say in the creation of new game items. This could be through a proposal and review process, ensuring that new items have the support of the game's community and are well-balanced.
burn(address account, uint256 tokenId, uint256 amount)
  • Specific Risk: The process of burning in-game items, if not properly managed, can be a vector for potential data loss or market manipulation, affecting the game's play-to-earn system.
  • Recommendation: Enforce a set of in-game preconditions that must be met before an item can be burned. Additionally, consider a review or a delay system for the burning of high-value or high-impact items to prevent hasty or emotional item destruction.
mint(uint256 tokenId, uint256 quantity)
  • Specific Risk: The minting process, especially for items with finite supply, needs to ensure that it doesn't exceed the predefined limits, which could lead to inflation and devaluation of existing items.
  • Recommendation: Strictly enforce the game's logic to check and adhere to the game's pre-established supply caps. Implement a supply management sub-system that transparently and accurately tracks and controls the game's in-game economy.
safeTransferFrom(...)
  • Specific Risk: The transfer function must ensure that the item's game logic, such as transferability and staking conditions, is respected, to prevent the exploitation of game mechanics.
  • Recommendation: Implement a pre-transfer check that verifies all the game's business logic is in a valid state for the transfer to occur. This could include staking conditions, cool-down periods, or other in-game business logic.

MergingPool.sol Contract

Functions and Risks

transferOwnership(address newOwnerAddress)
  • Specific Risk: Centralization of power in a single admin's hands can pose a major project governance and fund mismanagement threat.
  • Recommendation: Implement a more decentralized or multi-signature system for making high-stake project decisions, such as a DAO.
adjustAdminAccess(address adminAddress, bool access)
  • Specific Risk: The power to unilaterally grant or revoke admin status can be a security and centralization issue, potentially allowing the abuse of the power by the project's owner.
  • Recommendation: Transition to a more transparent and permissionless process for conferring admin status, such as a time-locked, community-voted mechanism.
updateWinnersPerPeriod(uint256 newWinnersPerPeriodAmount)
  • Specific Risk: The process of changing the number of winners can be a vector for fund misallocation or manipulation, affecting the project's long-term success.
  • Recommendation: Automate the project's high-stake game mechanics, like the number of winners, through a set of in-game or on-chain oracles to ensure the game's invariability and predictability.
pickWinner(uint256[] calldata winners)
  • Specific Risk: The game's high-stake power to pick winners can be a major security and manipulation issue, affecting the project's game-theoretic and long-term community support.
  • Recommendation: Transition to a more on-chain, verifiable, and automatic process for determining winners, such as a VRF-based process, to ensure the game's invariability and predictability.
claimRewards(...)
  • Specific Risk: The project's action of claiming rewards can be a vector for contract manipulation or front-running, affecting the project's short-term and long-term support.
  • Recommendation: Secure the project's data flow and fund flow, especially for the game's in-game and on-chain oracles, to ensure the game's invariability and predictability.
addPoints(uint256 tokenId, uint256 points)
  • Specific Risk: The game's admin has the power to arbitrarily add points to fighters, which could be a vector for potential manipulation or unfair play, affecting the project's integrity.
  • Recommendation: Transition to a more on-chain, verifiable, and automatic point calculation and awarding process to ensure fairness and transparency in gameplay.
getFighterPoints(uint256 maxId)
  • Specific Risk: The ability to query the total number of points for a range of fighters can be a vector for privacy issues or market manipulation, affecting the game's long-term support.
  • Recommendation: Ensure the action of querying the game's in-game and on-chain oracles is in line with the game's business logic and invariability to ensure the project's invariability and predictability.

Neuron.sol Contract

Functions and Risks

transferOwnership(address newOwnerAddress)
  • Specific Risk: Centralization of power. This function allows the current owner to transfer ownership, which could be a security risk if the admin's control is compromised.
  • Recommendation: Implement a decentralized access control system or a multi-signature system for such sensitive functions to mitigate the single point of control risk.
addMinter(address newMinterAddress)
  • Specific Risk: Unlimited minting access. By adding new minters, there's a security risk if the minter's key is compromised, potentially leading to over-minting.
  • Recommendation: Limit the minter's power by imposing a hard cap on the number of tokens a minter can mint and by time-locking the minter's power to prevent abuse.
addStaker(address newStakerAddress)
  • Specific Risk: Concentration of staking management. A new staker can have too much control over the staking process, potentially compromising the staking model's integrity.
  • Recommendation: Implement a transparent and permissionless staking system that doesn't rely on a centralized power to manage or control the staking system.
addSpender(address newSpenderAddress)
  • Specific Risk: Mismanagement of spending power. The potential for a new spender to misuse their control over the network's resources, affecting the game's in-game balance or the game's network performance.
  • Recommendation: Enforce a cap on the total spendable tokens by any spender and a time-bound access to the power to prevent the risk of mismanagement.
adjustAdminAccess(address adminAddress, bool access)
  • Specific Risk: Admin power abuse. The action of unilaterally granting or revoking admin power can be a control and power abuse point.
  • Recommendation: Transition to a more transparent and permissionless power system, where the game's power is more decentralized or the game's major players have a voice in the action.
setupAirdrop(address[] calldata recipients, uint256[] calldata amounts)
  • Specific Risk: Economic imbalance. The game's project can be at risk of airdrop misuse, which can be a point of power abuse, potentially affecting the game's in-game balance.
  • Recommendation: Ensure the airdrop process is transparent and the game's project has a plan to prevent the misuse of airdrops, which can be a power abuse method.
claim(uint256 amount)
  • Specific Risk: Over-claiming. The system can be a risk of airdrop misuse, which can be a power abuse method, potentially affecting the project's game in-game balance.
  • Recommendation: Ensure the game's power is more decentralized or the game's major players have a voice in the process to prevent the misuse of airdrops.
mint(address to, uint256 amount)
  • Specific Risk: Inflation. Minting without strict controls can dilute the value of the token, affecting all token holders.
  • Recommendation: Enforce a hard cap on the number of tokens that can be minted to prevent the game's market from being flooded with new tokens, which could devalue the game's network.
burn(uint256 amount)
  • Specific Risk: Loss of value. The process of burning tokens can be a vector for power abuse, potentially affecting the project's market cap or the power of the project's power system.
  • Recommendation: Transition to a more transparent and power system, where the project's game power is more decentralized or the game's major players have a voice in the power abuse.
approveSpender(address account, uint256 amount)
  • Specific Risk: Misuse of spending authority. The system can be a risk of airdrop misuse, which can be a point of control and a point of risk for the project's project game.
  • Recommendation: Transition to a more transparent and permissionless game system, where the project's project game is more decentralized or the game's main players have a voice in the game's main game.
burnFrom(address account, uint256 amount)
  • Specific Risk: The power to burn tokens from another account can be misused if the power is not carefully managed and audited.
  • Recommendation: Require a time-locked or a community-approved action for the game's main game to prevent the misuse of the game's main game, which can be a control and a risk for the game's project game.

RankedBattle.sol Contract

Functions and Risks

transferOwnership(address newOwnerAddress)
  • Specific Risk: Centralization of control. This action allows the admin to unilaterally change the contract's owner, which could lead to a single point of failure or misuse if the account is compromised.
  • Recommendation: Implement a decentralized access management model, such as a DAO or a multi-signature system, to manage the power to transfer ownership.
adjustAdminAccess(address adminAddress, bool access)
  • Specific Risk: Admin power misuse. Admins have the power to change the control of the game's logic, which could be a problem if the key is compromised.
  • Recommendation: Use a time-locked smart contract or a DAO to manage the project's high-stake game mechanics, such as power handover, to ensure the community's interest is prioritized.
setGameServerAddress(address gameServerAddress)
  • Specific Risk: Single point of trust. The admin can change the game server's control, which could be a problem if the account is compromised.
  • Recommendation: Automate the process of server rotation or game state management through a set of in-game or on-chain oracles to ensure the project's invariability and predictability.
stakeNRN(uint256 amount, uint256 tokenId)
  • Specific Risk: Stake manipulation. The admin can change the staking logic, which could be a security threat if the project's business logic is not well-audited.
  • Recommendation: Automate the staking control and cap the admin's role to a system of in-game or on-chain oracles to ensure the business logic's invariability and predictability.
unstakeNRN(uint256 amount, uint256 tokenId)
  • Specific Risk: Unstaking manipulation. The method of unstaking can be a security threat if the power is not well-audited.
  • Recommendation: Implement a set of in-game or on-chain oracles to ensure the business logic's invariability and predictability.
claimNRN()
  • Specific Risk: Claim manipulation. The process of claiming rewards could be a security risk if the business logic is not well-tested.
  • Recommendation: Use a series of on-chain data and a more secure, time-locked, and automated function to claim the game's in-game money to ensure the community's long-term interest.
updateBattleRecord(...)
  • Specific Risk: Battle outcome manipulation. The project's current game logic could be a problem if the ability to update the game's action is not in line with the business logic.
  • Recommendation: Transition to a more on-chain, real-time, and secure power model to prevent the problem of a business logic that is not in line with the business logic.
setRankedNrnDistribution(...)
  • Specific Risk: Inflation risk. The game's business logic could be a point of a game's action that is not in line with the market's role if the account is compromised.
  • Recommendation: Transition to a more on-chain, time-locked, and real-time price feed to ensure the market's game logic is in line with the project's main game logic.

StakeAtRisk.sol Contract

Functions and Risks

setNewRound(uint256 roundId_)
  • Specific Risk: Centralized Authority. This function, callable by the RankedBattle contract, can transition the staking game to a new phase, potentially affecting the risk and return profile of all players' strategies.
  • Recommendation: Ensure that the transition to a new game phase is a result of a community consensus or a set of in-game, on-chain, and pre-defined criteria to prevent manipulation.
reclaimNRN(uint256 nrnToReclaim, uint256 fighterId, address fighterOwner)
  • Specific Risk: Token Withdrawal. This method allows the RankedBattle contract to withdraw a particular number of tokens from the staking pool, which could be a security risk if the RankedBattle control is compromised.
  • Recommendation: Enforce a time-locked or a multi-confirmation system for any fund's outflow, and ensure that the RankedBattle's power is as decentralized as possible to prevent a single point of control.
updateAtRiskRecords(uint256 nrnToPlaceAtRisk, uint256 fighterId, address fighterOwner)
  • Specific Risk: Inadequate Validation. The system could be exploited to inappropriately increase the risk for a player's funds or to maliciously alter the staking power of a user's game character.
  • Recommendation: Ensure that the game's business logic is in a good position to prevent the system from being manipulated. Additionally, the project should have a self-managed and self-governed process to prevent the self-managed and self-governed project from being a self-managed and self-governed process.

VoltageManager.sol Contract

Functions and Risks

transferOwnership(address newOwnerAddress)
  • Specific Risk: Centralization of control. The process of transferring the entire control of the smart contract to a new address can be a security and mismanagement threat.
  • Recommendation: Implement a time-locked, multi-signature, or DAO-based system for any control-changing operations to ensure community or stakeholder consensus.
adjustAdminAccess(address adminAddress, bool access)
  • Specific Risk: Admin access mismanagement. The process of unilaterally granting or revoking admin status can be a control and manipulation issue.
  • Recommendation: Shift to a more transparent and user-managed power management process via dao structure.

General high level recommendation:
  • Use Tenderly and Defender for continued monitoring to prevent un-foreseen risks or actions.
Conclusion:
  • The Ai-Arena project showcases a well-architected foray into the world of on-chain NFT gaming. By leveraging the ERC-721 and ERC-1155 standards, it has created a game that is not only unique in its play-to-earn (P2E) dynamics but also in the deep integration of NFTs and in-game micro-economies for a more immersive and self-sustaining game world.

Time spent:

28 hours

#0 - c4-pre-sort

2024-02-25T20:35:40Z

raymondfam marked the issue as sufficient quality report

#1 - c4-judge

2024-03-19T08:11:18Z

HickupHH3 marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax ยฉ 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter