code-423n4 / 2024-05-olas-findings

12 stars 3 forks source link

Loss of incentives if total weight in an epoch is zero #61

Open howlbot-integration[bot] opened 2 months ago

howlbot-integration[bot] commented 2 months ago

Lines of code

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/Dispenser.sol#L911

Vulnerability details

Impact

If during an epoch, the total weight is zero, then the staking incentives related to that epoch will be lost, while it is expected that they will be refunded to tokenomics inflation.

Proof of Concept

During claiming the staking incentives through claimStakingIncentives, the function calculateStakingIncentives is called to calculate the totalStakingIncentive and totalReturnAmount which means the total staking incentives that should be distributed to staking instances and total incentives that should be refunded to tokenomics inflation.

    function claimStakingIncentives(
        uint256 numClaimedEpochs,
        uint256 chainId,
        bytes32 stakingTarget,
        bytes memory bridgePayload
    ) external payable {
        //.....

        (uint256 stakingIncentive, uint256 returnAmount, uint256 lastClaimedEpoch, bytes32 nomineeHash) =
            calculateStakingIncentives(numClaimedEpochs, chainId, stakingTarget, bridgingDecimals);

        //....

        if (returnAmount > 0) {
            ITokenomics(tokenomics).refundFromStaking(returnAmount);
        }

        //....

    }

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/Dispenser.sol#L953

In this function, the staking weight and total weight for each epoch is calculated. If available staking incentives is higher than total weight, then the extra part (diff) will be refunded to tokenomics inflation.

            uint256 availableStakingAmount = stakingPoint.stakingIncentive;

            uint256 stakingDiff;
            if (availableStakingAmount > totalWeightSum) {
                stakingDiff = availableStakingAmount - totalWeightSum;
                availableStakingAmount = totalWeightSum;
            }

            //.....

            returnAmount = (stakingDiff * stakingWeight) / 1e18;

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/Dispenser.sol#L903

Then, the rest of the available staking incentives will be distributed to the staking instance based on the staking weight assigned to it.

stakingIncentive = (availableStakingAmount * stakingWeight) / 1e18;

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/Dispenser.sol#L916

The issue is that if the total weight for an epoch is equal to zero, no staking incentives will be refunded to tokenomics inflation. While, it is expected that unused/extra staking incentives be returned to tokenomics inflation. The reason is that when totalWeightSum is zero (obviously the stakingWeight will be zero either), the following if-clause will be executed.

    if (stakingWeight < uint256(stakingPoint.minStakingWeight) * 1e14) {
        // If vote weighting staking weight is lower than the defined threshold - return the staking incentive
        returnAmount = ((stakingDiff + availableStakingAmount) * stakingWeight) / 1e18;
    } else {
        //.....
    }

https://github.com/code-423n4/2024-05-olas/blob/main/tokenomics/contracts/Dispenser.sol#L911

Since stakingWeight is zero, the returnAmount would be zero either. As a result returnAmount = 0, no incentives related to this epoch will be refunded to tokenomics inflation, and they will be lost.

Tests

Test1 (simple case):

In the following test case, a nominee is added, but no one has voted for it. So, the total weight in epoch 2 is zero. After the checkpoint (epoch 2 is ended, and now we are in epoch 3), the function calculateStakingIncentives is called to calculate the amount of incentives related to epoch 2 that are assigned to this nominee, as well as the amount of incentives that are extra and should be refunded. It returns totalStakingIncentive = 0 as expected (because no one had voted for it), but totalReturnAmount = 0 which is not expected. By calling (await tokenomics.mapEpochStakingPoints(2)).stakingIncentive, it shows that the available staking incentives in epoch 2 was equal to 259644210229512049587306, so there are incentives available in this epoch, but they are not refunded to tokenomics inflation, thus they are lost.

        it("loss of incentives if the total weight sum is zero", async () => {
            // Take a snapshot of the current state of the blockchain
            const snapshot = await helpers.takeSnapshot();

            // Set staking fraction to 100%
            await tokenomics.changeIncentiveFractions(0, 0, 0, 0, 0, 100);
            // Changing staking parameters
            await tokenomics.changeStakingParams(50, 10);

            // Checkpoint to apply changes
            await helpers.time.increase(epochLen);
            await tokenomics.checkpoint();

            // Unpause the dispenser
            await dispenser.setPauseState(0);

            // Add a staking instance as a nominee
            await vw.addNominee(stakingInstance.address, chainId);

            // Checkpoint to apply changes
            await helpers.time.increase(epochLen);
            await tokenomics.checkpoint();

            const stakingTarget = convertAddressToBytes32(stakingInstance.address);
            // Calculate staking incentives
            const stakingIncentives = await dispenser.callStatic.calculateStakingIncentives(numClaimedEpochs, chainId,
                stakingTarget, bridgingDecimals);

            // this shows the total staking incentives and return amount in epoch 2
            console.log("total staking incentive: ", await stakingIncentives.totalStakingIncentive);
            console.log("total return amount: ", await stakingIncentives.totalReturnAmount);

            // 2 is the epoch that the total weight sum is zero
            console.log("available staking amount: ", (await tokenomics.mapEpochStakingPoints(2)).stakingIncentive);

            // Restore to the state of the snapshot
            await snapshot.restore();
        });

Output is:

  DispenserStakingIncentives
    Staking incentives
total staking incentive:  BigNumber { value: "0" }
total return amount:  BigNumber { value: "0" }
available staking amount:  BigNumber { value: "259644210229512049587306" }
      ✓ loss of incentives if the total weight sum is zero

Test2 (more realistic case):

In the following test case, a nominee is added and 1 vote is allocated to it at epoch 2. So, at week 2844, the nominee's weight is nonzero.

Then, one epoch length (equal to 10 days) is passed, and the checkpoint is called. Then, 0 vote is allocated to the nominee, so the nominee's weight will be zero at week 2845.

Again, one epoch is passed, and the checkpoint is called (now we are at epoch 4). Then, checkpointNominee is called to update the nominee's weight and total sum. It shows that the nominee's relative weight at week 2844 is %100, and its relative weight at week 2845 is %0, as expected.

Then, calculateStakingIncentives is called to calculate the nominees incentives at epochs 2 and 3. It shows that totalStakingIncentive allocated to the nominee is equal to 50, and totalReturnAmount is equal to 86548378823584027017230. While, available staking incentives in epoch 2 and 3 are 86548378823584027017280 and 86548178481042039748640, respectively.

It was expected that totalStakingIncentive + totalReturnAmount be equal to sum of available staking incentives at epoch 2 and 3, but it shows that it is equal to staking incentives at epoch 2 only. In other words, since the total weight at epoch 3 is zero, the staking incentives available at that epoch is neither allocated to the nominee nor returned to the tokenomics inflation. So, they are lost.

/*global describe, context, beforeEach, it*/

const { expect } = require("chai");
const { ethers } = require("hardhat");
const helpers = require("@nomicfoundation/hardhat-network-helpers");
const { hrtime } = require("process");

describe("PoC", function () {
    const AddressZero = ethers.constants.AddressZero;
    const initialMint = "1000000000000000000000000"; // 1_000_000
    const epochLen = 10 * 24 * 60 * 60;
    const retainer = "0x" + "0".repeat(24) + "5".repeat(40);
    const maxNumClaimingEpochs = 10;
    const maxNumStakingTargets = 100;
    const chainId = 31337;
    const maxVoteWeight = 10000;
    const defaultWeight = 1000;
    const oneOLASBalance = ethers.utils.parseEther("1");
    const oneYear = 365 * 86400;
    const oneWeek = 7 * 86400;
    const numClaimedEpochs = 1;
    const bridgingDecimals = 18;

    let olas;
    let ve;
    let vw;
    let signers;
    let deployer;
    let treasury;
    let dispenser;

    function convertAddressToBytes32(account) {
        return ("0x" + "0".repeat(24) + account.slice(2)).toLowerCase();
    }
    function getWeek(ts) {
        return Math.floor(ts / oneWeek);
    }

    beforeEach(async function () {
        const OLAS = await ethers.getContractFactory("OLAS");
        olas = await OLAS.deploy();
        await olas.deployed();

        signers = await ethers.getSigners();
        deployer = signers[0];
        await olas.mint(deployer.address, initialMint);

        const VE = await ethers.getContractFactory("veOLAS");
        ve = await VE.deploy(olas.address, "Voting Escrow OLAS", "veOLAS");
        await ve.deployed();

        const VoteWeighting = await ethers.getContractFactory("VoteWeighting");
        vw = await VoteWeighting.deploy(ve.address);
        await vw.deployed();

        const MockStakingProxy = await ethers.getContractFactory("MockStakingProxy");
        stakingInstance = await MockStakingProxy.deploy(olas.address);
        await stakingInstance.deployed();

        const MockStakingFactory = await ethers.getContractFactory("MockStakingFactory");
        stakingProxyFactory = await MockStakingFactory.deploy();
        await stakingProxyFactory.deployed();

        // Add a default implementation mocked as a proxy address itself
        await stakingProxyFactory.addImplementation(stakingInstance.address, stakingInstance.address);

        const Dispenser = await ethers.getContractFactory("Dispenser");
        dispenser = await Dispenser.deploy(olas.address, deployer.address, deployer.address, deployer.address,
            retainer, maxNumClaimingEpochs, maxNumStakingTargets);
        await dispenser.deployed();

        await vw.changeDispenser(dispenser.address);

        const Treasury = await ethers.getContractFactory("Treasury");
        treasury = await Treasury.deploy(olas.address, deployer.address, deployer.address, dispenser.address);
        await treasury.deployed();

        // Update for the correct treasury contract
        await dispenser.changeManagers(AddressZero, treasury.address, AddressZero);

        const tokenomicsFactory = await ethers.getContractFactory("Tokenomics");
        // Deploy master tokenomics contract
        const tokenomicsMaster = await tokenomicsFactory.deploy();
        await tokenomicsMaster.deployed();

        const proxyData = tokenomicsMaster.interface.encodeFunctionData("initializeTokenomics",
            [olas.address, treasury.address, deployer.address, dispenser.address, deployer.address, epochLen,
            deployer.address, deployer.address, deployer.address, AddressZero]);
        // Deploy tokenomics proxy based on the needed tokenomics initialization
        const TokenomicsProxy = await ethers.getContractFactory("TokenomicsProxy");
        const tokenomicsProxy = await TokenomicsProxy.deploy(tokenomicsMaster.address, proxyData);
        await tokenomicsProxy.deployed();

        // Get the tokenomics proxy contract
        tokenomics = await ethers.getContractAt("Tokenomics", tokenomicsProxy.address);

        // Change the tokenomics and treasury addresses in the dispenser to correct ones
        await dispenser.changeManagers(tokenomics.address, treasury.address, vw.address);

        // Update tokenomics address in treasury
        await treasury.changeManagers(tokenomics.address, AddressZero, AddressZero);

        // Give treasury the minter role
        await olas.changeMinter(treasury.address);

    });

    it("Loss of incentives", async () => {
        // Take a snapshot of the current state of the blockchain
        const snapshot = await helpers.takeSnapshot();

        // Lock one OLAS into veOLAS
        await olas.approve(ve.address, oneOLASBalance);
        await ve.createLock(oneOLASBalance, oneYear);

        // Set staking fraction to 100%
        await tokenomics.changeIncentiveFractions(0, 0, 0, 0, 0, 100);
        // Changing staking parameters
        await tokenomics.changeStakingParams(50, 10);

        // Checkpoint to apply changes
        await helpers.time.increase(epochLen);
        await tokenomics.checkpoint();

        console.log("Nominee added and 1 vote is allocated at epoch: ", await tokenomics.epochCounter()); // 2

        // Unpause the dispenser
        await dispenser.setPauseState(0);

        // Add a staking instance as a nominee
        await vw.addNomineeEVM(stakingInstance.address, chainId);

        const stakingTarget = convertAddressToBytes32(stakingInstance.address);
        const nomineeHash = await vw.getNomineeHash(stakingTarget, chainId);

        // Vote for the nominee
        await vw.voteForNomineeWeights(stakingTarget, chainId, 1); // 1 vote is allocated

        const timeWeightWhenVotedNonzero = getWeek(await vw.timeWeight(nomineeHash));
        console.log("nominee time weight in week when 1 vote is allocated: ", timeWeightWhenVotedNonzero);

        // Checkpoint to apply changes
        await helpers.time.increase(epochLen);
        await tokenomics.checkpoint();

        console.log("Zero vote is allocated to the nominee at epoch: ", await tokenomics.epochCounter()); // 3

        await vw.voteForNomineeWeights(stakingTarget, chainId, 0); // zero vote is allocated

        console.log("nominee time weight in week when zero vote is allocated: ", getWeek(await vw.timeWeight(nomineeHash)));

        await helpers.time.increase(epochLen);
        await tokenomics.checkpoint();

        await vw.checkpointNominee(stakingTarget, chainId); // to update the nominee's relative weight and total sum

        console.log("Relative weight and total sum at week 2844: ", (await vw.nomineeRelativeWeight(stakingTarget, chainId, timeWeightWhenVotedNonzero * oneWeek)));
        console.log("Relative weight and total sum at week 2845: ", (await vw.nomineeRelativeWeight(stakingTarget, chainId, (timeWeightWhenVotedNonzero + 1) * oneWeek)));

        // Calculate staking incentives
        const stakingIncentives = await dispenser.callStatic.calculateStakingIncentives(10, chainId,
            stakingTarget, bridgingDecimals);

        console.log("total staking incentive: ", await stakingIncentives.totalStakingIncentive);
        console.log("total return amount: ", await stakingIncentives.totalReturnAmount);

        console.log("available staking incentives at epoch 2: ", (await tokenomics.mapEpochStakingPoints(2)).stakingIncentive);
        console.log("available staking incentives at epoch 3: ", (await tokenomics.mapEpochStakingPoints(3)).stakingIncentive);

        // Restore to the state of the snapshot
        await snapshot.restore();
    });
});

The result is:

   PoC
Nominee added and 1 vote is allocated at epoch:  2
nominee time weight in week when 1 vote is allocated:  2844
Zero vote is allocated to the nominee at epoch:  3
nominee time weight in week when zero vote is allocated:  2845
Relative weight and total sum at week 2844:  [
  BigNumber { value: "1000000000000000000" },
  BigNumber { value: "23493126988800" },
  relativeWeight: BigNumber { value: "1000000000000000000" },
  totalSum: BigNumber { value: "23493126988800" }
]
Relative weight and total sum at week 2845:  [
  BigNumber { value: "0" },
  BigNumber { value: "0" },
  relativeWeight: BigNumber { value: "0" },
  totalSum: BigNumber { value: "0" }
]
total staking incentive:  BigNumber { value: "50" }
total return amount:  BigNumber { value: "86548378823584027017230" }
available staking incentives at epoch 2:  BigNumber { value: "86548378823584027017280" }
available staking incentives at epoch 3:  BigNumber { value: "86548178481042039748640" }
    ✓ Loss of incentives

Tools Used

Recommended Mitigation Steps

It is recommended to handle the case when total weight is zero. Moreover, it is necessary to record which epoch had zero total weight, so if its staking incentives are refunded, it should not be refunded again.

    mapping(uint256 => bool) public zeroWeighEpochRefunded;

    function calculateStakingIncentives(
        uint256 numClaimedEpochs,
        uint256 chainId,
        bytes32 stakingTarget,
        uint256 bridgingDecimals
    ) public returns (
        uint256 totalStakingIncentive,
        uint256 totalReturnAmount,
        uint256 lastClaimedEpoch,
        bytes32 nomineeHash
    ) {
        //.....
        for (uint256 j = firstClaimedEpoch; j < lastClaimedEpoch; ++j) {
            //....

            if(zeroWeighEpochRefunded[j]){
                // this epoch has zero total weight and all its staking incentives are already refunded to tokenomics inflation
                continue;
            }

            if (totalWeightSum == 0) {

                returnAmount = stakingPoint.stakingIncentive;
                zeroWeighEpochRefunded[j] = true;

            } else if (stakingWeight < uint256(stakingPoint.minStakingWeight) * 1e14) {
                //....
            } else {
                //....
            }

            //....
        }
        //.....
    }

Assessed type

Context

c4-sponsor commented 2 months ago

kupermind (sponsor) confirmed

c4-judge commented 2 months ago

0xA5DF changed the severity to 2 (Med Risk)

c4-judge commented 2 months ago

0xA5DF marked the issue as satisfactory

c4-judge commented 2 months ago

0xA5DF marked the issue as selected for report

0xA5DF commented 2 months ago

Marking as med since:

vsharma4394 commented 1 month ago

Fixed