```legacyReward(v2)``` and ```totalAssetsDelta(v3)`` are different in usage

[hats-bug-reporter[bot]]

Github username: @0xmahdirostami Submission hash (on-chain): 0x3c02012856136163c770d89a10302ccda3ec21d6cd30adf8e58a5b7b1a47edc0 Severity: high

Description: Description\ EthGenesisVault splits reward between v2 and v3 in the following way:

    uint256 legacyPrincipal = _rewardEthToken.totalAssets() - _rewardEthToken.totalPenalty();

    // calculate total principal
    uint256 totalPrincipal = _totalAssets + legacyPrincipal;
    if (totalAssetsDelta < 0) {
      // calculate and update penalty for legacy pool
      int256 legacyPenalty = SafeCast.toInt256(
        Math.mulDiv(uint256(-totalAssetsDelta), legacyPrincipal, totalPrincipal)
      );
      _rewardEthToken.updateTotalRewards(-legacyPenalty);
      // deduct penalty from total assets delta
      totalAssetsDelta += legacyPenalty;
    } else {
      // calculate and update reward for legacy pool
      int256 legacyReward = SafeCast.toInt256(
        Math.mulDiv(uint256(totalAssetsDelta), legacyPrincipal, totalPrincipal)
      );
      _rewardEthToken.updateTotalRewards(legacyReward);
      // deduct reward from total assets delta
      totalAssetsDelta -= legacyReward;
    }

As seen above, in updateState() if totalAssetsDelta > 0, for v2, _rewardEthToken will be increased and for v3, in _processTotalAssetsDelta(), _totalAssets will be increased, so in the next updateState() both legacyPrincipal(v2) and _totalAssets(v3) will be increased, But the usage of _rewardEthToken(v2) and totalAssetsDelta(V3) are different, rewards in v3 could be used to gain more rewards but rewards in v2 doesn't uesd for gaining more rewards

Scenario\

• _totalAssets is X amount, legacyPrincipal is X amount
• legacyReward will be 5% of X and totalAssetsDelta will be 5%
• totalAssetsDelta will be used by validators or ExitQueue (used by vault)
• legacyReward isn't used by the vault
• But both legacyPrincipal and _totalAsset will be increased by 5% of X.
• Actually, the legacyPrincipal shouldn't increase and it should remain at X amount.

Attachments

1. Proof of Concept (PoC) File

2. Revised Code File (Optional)

   Don't consider rewardETH(v2) for splitting rewards.

[tsudmi]

If the legacyPrincipal is not increased, users that are still on StakeWise v2 won't see any rewards. We want StakeWise V2 and V3 work in parallel, so we need to make sure the users still get rewards.

[0xmahdirostami]

@tsudmi

But rewards in V3 are used by Vault, and rewards in V2 aren't used by Vault, it is fairness for V3 users.

-    uint256 legacyPrincipal = _rewardEthToken.totalAssets() - _rewardEthToken.totalPenalty() ;
+   uint256 legacyPrincipal = _rewardEthToken.totalAssets() - _rewardEthToken.totalPenalty() - _rewardEthToken.totalRewards();
OR
+  uint256 legacyPrincipal = _rewardEthToken.totalStaked() - _rewardEthToken.totalPenalty()

    // calculate total principal
    uint256 totalPrincipal = _totalAssets + legacyPrincipal;
    if (totalAssetsDelta < 0) {
      // calculate and update penalty for legacy pool
      int256 legacyPenalty = SafeCast.toInt256(
        Math.mulDiv(uint256(-totalAssetsDelta), legacyPrincipal, totalPrincipal)
      );
      _rewardEthToken.updateTotalRewards(-legacyPenalty);
      // deduct penalty from total assets delta
      totalAssetsDelta += legacyPenalty;
    } else {
      // calculate and update reward for legacy pool
      int256 legacyReward = SafeCast.toInt256(
        Math.mulDiv(uint256(totalAssetsDelta), legacyPrincipal, totalPrincipal)
      );
      _rewardEthToken.updateTotalRewards(legacyReward);
      // deduct reward from total assets delta
      totalAssetsDelta -= legacyReward;
    }

In this way, rewards will be increased for v2 but not calculated in legacyPrincipal, As those rewards aren't used for gaining more rewards, they don't affect splitting.

[tsudmi]

Hey @0xmahdirostami these rewards are still restaked and earn rewards (even in RewardEthToken form). So users in V2 must earn from them as well.

[0xmahdirostami]

@

Hey @0xmahdirostami these rewards are still restaked and earn rewards (even in RewardEthToken form). So users in V2 must earn from them as well.

@tsudmi v2 is a dual token and v3 is a single token, the difference is in v2, users' rewards aren't originally used for gaining more rewards. It's optional, users could reinvest or not. So those rewards couldn't be used for calculation, as it's fairness for v3 users.

Scenario

• User A deposits 32 ETH to v3
• User B deposits 32 ETH to v2
• After some time 1 ETH reward will be gained for users A and B
• User A's reward, is used by Vault BUT User B's reward isn't used by Vault (unless he decides to reinvest it)
• In the current implementation, both users gain the same reward in the next update state.

[tsudmi]

Hi @0xmahdirostami once v3 is live the deposits will stop on v2, so "User B deposits 32 ETH to v2" won't go through. The rewards of all the validators in v2 and v3 are controlled by v3, so they will be reinvested. In v2 it would be more correct for user to receive sETH2, but because the change is too big we've decided to keep rETH2.