51124 sc high validator would loss commission fee if the rewards token are removed

Submitted on Jul 31st 2025 at 10:45:48 UTC by @pks271 for Attackathon | Plume Network

• Report ID: #51124

• Report Type: Smart Contract

• Report severity: High

• Target: https://github.com/immunefi-team/attackathon-plume-network/blob/main/plume/src/facets/ValidatorFacet.sol

• Impacts:

  • Temporary freezing of funds for at least 24 hours

Description

Bug Description

When stake/unstake/claim including RewardsFacet#removeRewardToken actions are called, the $.validatorAccruedCommission[validatorId][token] += commissionDeltaForValidator inside PlumeRewardLogic#updateRewardPerTokenForValidator() function will be updated. This parameter tracks the history of validators' commission fees and validators can call ValidatorFacet#requestCommissionClaim/finalizeCommissionClaim to claim their commission fee.

When REWARD_MANAGER_ROLE calls RewardsFacet#removeRewardToken to remove the reward token, the $.validatorAccruedCommission[validatorId][token] is updated first, then $.isRewardToken[token] is set to false. The problem is that requestCommissionClaim will revert because it uses _validateIsToken to check whether the reward token exists; if not, it reverts.

Relevant code excerpts:

function removeRewardToken(
        address token
) external onlyRole(PlumeRoles.REWARD_MANAGER_ROLE) {
    PlumeStakingStorage.Layout storage $ = PlumeStakingStorage.layout();
    if (!$.isRewardToken[token]) {
        revert TokenDoesNotExist(token);
    }

    // Find the index of the token in the array
    uint256 tokenIndex = _getTokenIndex(token);

    // Store removal timestamp to prevent future accrual
    uint256 removalTimestamp = block.timestamp;
    $.tokenRemovalTimestamps[token] = removalTimestamp;

    // Update validators (bounded by number of validators, not users)
    // @audit - update the reward per token for all validators
    for (uint256 i = 0; i < $.validatorIds.length; i++) {
        uint16 validatorId = $.validatorIds[i];

        // Final update to current time to settle all rewards up to this point
        PlumeRewardLogic.updateRewardPerTokenForValidator($, token, validatorId);

        // Create a final checkpoint with a rate of 0 to stop further accrual definitively.
        PlumeRewardLogic.createRewardRateCheckpoint($, token, validatorId, 0);
    }

    // Set rate to 0 to prevent future accrual. This is now redundant but harmless.
    $.rewardRates[token] = 0;
    // DO NOT delete global checkpoints. Historical data is needed for claims.
    // delete $.rewardRateCheckpoints[token];

    // Update the array
    $.rewardTokens[tokenIndex] = $.rewardTokens[$.rewardTokens.length - 1];
    $.rewardTokens.pop();
    // Update the mapping
    // @audit - set `$.isRewardToken[token]` to false
    $.isRewardToken[token] = false;

    delete $.maxRewardRates[token];
    emit RewardTokenRemoved(token);
}

The _validateIsToken modifier currently reverts when isRewardToken[token] is false:

modifier _validateIsToken(
        address token
) {
    if (!PlumeStakingStorage.layout().isRewardToken[token]) {
        revert TokenDoesNotExist(token);
    }
    _;
}

requestCommissionClaim uses this modifier, which causes it to revert for tokens that were valid reward tokens but were later removed after commission accrual:

function requestCommissionClaim(
        uint16 validatorId,
        address token
    )
    external
    onlyValidatorAdmin(validatorId)
    nonReentrant
    _validateValidatorExists(validatorId)
    // @audit - revert here
    _validateIsToken(token)
    {
        // skip
    }

Impact

• Validators who accrued commission for a token that is later removed will be unable to request/finalize claims for that accrued commission because _validateIsToken will revert if the token has been removed.

• This can temporarily freeze validator funds (commission) for at least 24 hours (or until the token is re-added or the contract is changed).

Recommendation

Consider adding a historical commission check to the token validation so that tokens that are no longer active but have accrued commission can still be claimed. For example:

modifier _validateIsToken(address token, uint16 validatorId) {
    if (!$.isRewardToken[token]) {
        bool hasAccruedCommission = false;
        if ($.validatorAccruedCommission[validatorId][token] > 0) {
            hasAccruedCommission = true;
        }
        if (!hasAccruedCommission) {
            revert TokenNotRegistered(token);
        }
    }
}

This approach allows validators to claim previously accrued commission for tokens that have been removed while still preventing operations for tokens that were never registered.

Proof of Concept

Insert the following test case into PlumeStakingDiamond.t.sol:

function testCommissionLostAfterRewardTokenRemoval() public {
    console2.log("--- Test: testCommissionLostAfterRewardTokenRemoval START ---");
    
    // Setup: Add reward token and set reward rate
    uint256 rewardRate = 1e18; // 1 PUSD per second
    vm.startPrank(admin);
    address[] memory tokens = new address[](1);
    tokens[0] = address(pUSD);
    uint256[] memory rates = new uint256[](1);
    rates[0] = rewardRate;
    RewardsFacet(address(diamondProxy)).setRewardRates(tokens, rates);
    RewardsFacet(address(diamondProxy)).setTreasury(address(treasury));
    
    // Ensure treasury has enough PUSD
    uint256 treasuryAmount = 1000 ether;
    pUSD.transfer(address(treasury), treasuryAmount);
    vm.stopPrank();

    // Setup validator with commission
    vm.startPrank(validatorAdmin);
    uint256 commissionRate = 10e16; // 10%
    ValidatorFacet(address(diamondProxy)).setValidatorCommission(
        DEFAULT_VALIDATOR_ID,
        commissionRate
    );
    vm.stopPrank();

    // User stakes to generate rewards and commission
    uint256 stakeAmount = 10 ether;
    vm.startPrank(user1);
    StakingFacet(address(diamondProxy)).stake{value: stakeAmount}(
        DEFAULT_VALIDATOR_ID
    );
    vm.stopPrank();

    // Move time forward to accrue rewards and commission
    vm.roll(block.number + 100);
    vm.warp(block.timestamp + 100);

    // Trigger reward updates to ensure commission is accrued
    vm.startPrank(user1);
    StakingFacet(address(diamondProxy)).unstake(DEFAULT_VALIDATOR_ID, 1); // Unstake 1 wei to trigger update
    vm.stopPrank();

    // Verify commission has accrued
    uint256 accruedCommission = ValidatorFacet(address(diamondProxy))
        .getAccruedCommission(DEFAULT_VALIDATOR_ID, address(pUSD));
    assertEq(accruedCommission, 100*1e18, "Commission should have accrued");

    // Now remove the reward token
    vm.startPrank(admin);
    RewardsFacet(address(diamondProxy)).removeRewardToken(address(pUSD));
    vm.stopPrank();

    // Verify token is no longer a reward token
    bool isRewardToken = RewardsFacet(address(diamondProxy)).isRewardToken(address(pUSD));
    assertEq(isRewardToken, false, "Token should no longer be a reward token");

    // Try to request commission claim - this should fail due to _validateIsToken check
    vm.startPrank(validatorAdmin);
    
    // This should revert because _validateIsToken will check isRewardToken[token] which is now false
    vm.expectRevert(); // Expect revert due to TokenNotRegistered error
    ValidatorFacet(address(diamondProxy)).requestCommissionClaim(
        DEFAULT_VALIDATOR_ID,
        address(pUSD)
    );
    vm.stopPrank();
}