claimNRN will DoS when MAX_SUPPLY is reached

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-02-ai-arena/blob/cd1a0e6d1b40168657d1aaee8223dc050e15f8cc/src/RankedBattle.sol#L294-L311

Vulnerability details

Impact

In the Neuron constructor, 500,000,000e18 NRNs are minted to the contributorAddress (half of totalSupply), and 200,000,000e18 NRNs to treasuryAddress, leaving 300,000,000e18 NRNs before reaching MAX_SUPPLY. Since we are minting new NRNs during each rounds, minting will accumulate through the rounds, and eventually, there will be a point where no one will be able to receive the rewards. This is due to the function claimNRN() that will revert. In addition, this could disincentivize players to stake because they will no longer receive any new NRNs.

Proof of Concept

Add the following lines at the end of the setup() function of RankedBattle.t.sol:

_neuronContract.addSpender(address(_gameItemsContract));
_gameItemsContract.instantiateNeuronContract(address(_neuronContract));
_gameItemsContract.createGameItem("Battery", "https://ipfs.io/ipfs/", true, true, 10_000, 1 * 10 ** 18, 10);     _gameItemsContract.setAllowedBurningAddresses(address(_voltageManagerContract));

Add the following test to RankedBattle.t.sol:

   function testClaimNRNDoS() public {
        address player = vm.addr(3);
        uint256 stakeAmount = 299_895_000 * 10 ** 18;
        uint256 newDistribution = 106_908;
        _neuronContract.addMinter(player);
        vm.prank(player);  
        _neuronContract.mint(player, stakeAmount);
        _mintFromMergingPool(player);
        vm.prank(player);

        // stake NRNs
        _rankedBattleContract.stakeNRN(stakeAmount, 0);
        vm.prank(_GAME_SERVER_ADDRESS);

        // player won the match
        _rankedBattleContract.updateBattleRecord(0, 0, 0, 1500, false);

        //set rankedNRNDistribution to 106_908 NRNs
        _rankedBattleContract.setRankedNrnDistribution(newDistribution);

        // set a new round
        _rankedBattleContract.setNewRound();

        // player claim NRN
        vm.prank(player);
        vm.expectRevert("Trying to mint more than the max supply");
        _rankedBattleContract.claimNRN();
    }

Tools Used

Manual review

Recommended Mitigation Steps

A potential solution would be to implement a burning mechanism within the claimNRN() function by calling the burnFrom() function with treasury and claimableNRN as arguments. The RankedBattle contract would need to have the necessary allowance set up for this, which can be done manually after deployment. The following burning mechanism could mitigate the risk of claimNRN() DoS:

if (claimableNRN > 0) {
            amountClaimed[msg.sender] += claimableNRN;
            if (claimableNRN >= (neuron.MAX_SUPPLY() - neuron.totalSupply()) {
                 neuron.burnFrom(treasury, claimableNRN)
            }
           _neuronInstance.mint(msg.sender, claimableNRN);
            emit Claimed(msg.sender, claimableNRN);
}

Assessed type

DoS

[c4-pre-sort]

raymondfam marked the issue as insufficient quality report

[c4-pre-sort]

raymondfam marked the issue as duplicate of #7

[c4-judge]

HickupHH3 changed the severity to QA (Quality Assurance)

[HickupHH3]

R

1391: 1R

1366: R

[c4-judge]

HickupHH3 marked the issue as grade-c

[0xvangrim]

@HickupHH3 I believe this issue is wrongly duplicated since it has nothing to do with "not being able to reach MAX_SUPPLY."

This specific issue concerns the claimNRN() getting DoS'ed when MAX_SUPPLY is reached. At the moment, NRNs will accumulate each round until they reach the MAX_SUPPLY. However, there exists no recourse for burning or inflating the token. This means that, at some point, it will no longer be possible to claim NRNs, which in continuation means that players will not stake any NRNs.

[HickupHH3]

if MAX_SUPPLY is reached, then FDV is achieved and no more NRN should be minted, so not being able to claim NRNs should be desired.

[kazantseff]

Hey @HickupHH3, I agree with @0xvangrim that this issue is wrongly duped under #7, the AI-112 group to which this issue belongs to is a totally different set of issues. As for validity concerns, I'd like to add some more context. The core idea of the game is that "humans are in the loop of the process to improve AI" as stated in the docs. There are 2 sides to that: The research competition and The gaming competition Both these sides fuel each other, but for the sake of the argument, I'm focusing only on the gaming part. If claiming of NRN were to be disabled, gamers won't be incentivized to compete in the game, which in turn will disincentivize them from wanting to upgrade their AI models, which will nullify the demand for NFT's, thus brining the collapse to the entire system. Also as I mentioned in my submission #1656 there is also an economic threat to the approach currently used, since even when MAX_SUPPLY is not reached, the value is literally leaking, since the game gives aways and mints something that does not exist. Given all of the above, I belive medium severity can be justified for the group of issues under AI-112 label (except for the issue #1971, it's different from the other 5 issues).

[HickupHH3]

firmly disagree, how the company plans to spread out and handle the case when MAX_SUPPLY (-1) is reached is up to them. right now it's speculating on the impact on a known issue.