Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Users can loose accumulated rewards because `Distribution::accumulateActor` doesn't store already accumulated rewards

alexczm

Summary

Distribution::accumulateActor() do not accumulate user's rewards. Staking/ unstaking vault shares will clear already accumulated rewards.

Vulnerability Details

Users have the possibility to stake their vault shares to accumulate fee distributions from the market making engine.
They can stake by calling VaultRouterBranch::stake. The problem is that accumulateActor function doesn't save the amount of rewards user accumulated.

function accumulateActor(Data storage self, bytes32 actorId) internal returns (SD59x18 valueChange) {
Actor storage actor = self.actor[actorId];
return _updateLastValuePerShare(self, actor, ud60x18(actor.shares));
}
​
function _updateLastValuePerShare(
Data storage self,
Actor storage actor,
UD60x18 newActorShares
)
private
returns (SD59x18 valueChange)
{
valueChange = _getActorValueChange(self, actor); // @audit valueChange reward is lost
​
​
actor.lastValuePerShare = newActorShares.eq(UD60x18_ZERO) ? int256(0) : self.valuePerShare;
}
​
function _getActorValueChange(
Data storage self,
Actor storage actor
)
private
view
returns (SD59x18 valueChange)
{
SD59x18 deltaValuePerShare = sd59x18(self.valuePerShare).sub(sd59x18(actor.lastValuePerShare));
valueChange = deltaValuePerShare.mul(ud60x18(actor.shares).intoSD59x18());
}

Let's consider the following scenario:

  • user deposit asset in vault and stake half of received shares; his 'actor.lastValuePerShareis set toself.valuePerShare`

  • vault receive rewards and distributeValue() is called; vault's self.valuePerShare is updated considering the received value

  • user stake again. accumulateActor is called. user's lastValuePerShare is set to self.valuePerShare.
    accumulateActor returns the valueChange (rewards accumulated until that moment), but this reward is lost.

  • user call FeeDistributionBranch::claimFees. FUnction reverts because getActorValueChange returns 0 amount.

Coded PoC:
Add the following in a .t.sol file in test/integration/market-making. Execute the test with forge test --mt test_stakingRewards_doNotAccumulate -vvv.
The claimFees action from the test will revert with NoFeesToClaim error.

pragma solidity 0.8.25;
// External dependencies
import { ERC20, IERC20 } from "@openzeppelin/token/ERC20/ERC20.sol";
// Zaros dependencies test
import { Base_Test } from "test/Base.t.sol";
// Zaros dependencies source
import { Errors } from "@zaros/utils/Errors.sol";
import { Vault } from "@zaros/market-making/leaves/Vault.sol";
import { IDexAdapter } from "@zaros/utils/interfaces/IDexAdapter.sol";
// foundry dependencies
import {console} from "forge-std/console.sol";
​
contract StakingRewards_doNotAccumulate_Test is Base_Test {
​
VaultConfig wBtcVaultConfig;
VaultConfig wStEthVaultConfig;
​
PerpMarketCreditConfig wBtcMarket;
PerpMarketCreditConfig arbMarket;
​
​
function setUp() public virtual override {
Base_Test.setUp();
changePrank({ msgSender: users.owner.account });
createVaults(marketMakingEngine, INITIAL_VAULT_ID, FINAL_VAULT_ID, true, address(perpsEngine));
configureMarkets();
changePrank({ msgSender: users.naruto.account });
// connect 2 vaults and 2 markets
_localSetup();
}
​
function test_stakingRewards_doNotAccumulate() public {
console.log("=== test execution started ===");
address naruto = users.naruto.account;
address weth = marketMakingEngine.workaround_getWethAddress();
uint128 narutoDeposit = 1e8;
deal({ token: address(wBtc), to: naruto, give: narutoDeposit });
​
// naruto deposit 1wBTC and stake half his shares
_prank(naruto);
IERC20(wBtcVaultConfig.asset).approve(address(marketMakingEngine), narutoDeposit);
marketMakingEngine.deposit(wBtcVaultConfig.vaultId, narutoDeposit, 0, "", false);
// stake
uint256 vaultShares = IERC20(wBtcVaultConfig.indexToken).balanceOf(naruto);
IERC20(wBtcVaultConfig.indexToken).approve(address(marketMakingEngine), vaultShares);
marketMakingEngine.stake(wBtcVaultConfig.vaultId, uint128(vaultShares / 2));
​
// sent WETH market fees from PerpsEngine -> MarketEngine
deal({ token: weth, to: address(perpsEngine), give: 0.2 * 1e18 });
_prank(address(perpsEngine));
IERC20(weth).approve(address(marketMakingEngine), 0.2 * 1e18);
marketMakingEngine.receiveMarketFee(
wBtcMarket.marketId,
weth,
0.01 * 1e18
);
// stake the other half of the shares
_prank(naruto);
marketMakingEngine.stake(wBtcVaultConfig.vaultId, uint128(vaultShares / 2));
​
_prank(naruto);
​
// the following call should not revert
vm.expectRevert(Errors.NoFeesToClaim.selector);
marketMakingEngine.claimFees(wBtcVaultConfig.vaultId);
}
​
// helper functions
function _localSetup() internal {
// connect 1 vault and 2 markets
wBtcVaultConfig = vaultsConfig[WBTC_CORE_VAULT_ID];
wStEthVaultConfig = vaultsConfig[WSTETH_CORE_VAULT_ID];
uint256[] memory vaultIds = new uint256[]();
vaultIds[0] = wBtcVaultConfig.vaultId;
//vaultIds[1] = wStEthVaultConfig.vaultId;
​
wBtcMarket = perpMarketsCreditConfig[BTC_PERP_MARKET_CREDIT_CONFIG_ID];
arbMarket = perpMarketsCreditConfig[ARB_PERP_MARKET_CREDIT_CONFIG_ID];
uint256[] memory marketIds = new uint256[]();
marketIds[0] = wBtcMarket.marketId;
marketIds[1] = arbMarket.marketId;
​
_prank(users.owner.account );
marketMakingEngine.connectVaultsAndMarkets(marketIds, vaultIds);
}
​
function _prank(address account) internal {
// to avoid spamming "changePrank is deprecated." more than necessary
vm.stopPrank();
vm.startPrank(account);
}
}
​

Impact

Users will loose acumulated rewards.

Tools Used

Recommendations

Add an accumulator variable in Actor structure to store the accumulated rewards.getActorValueChange() should return the accumulated value plus the _getActorValueChange().

Updates

Lead Judging Commences

inallhonesty Lead Judge 6 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Inside VaultRouterBranch if you stake wait some time then stake again makes you lose the rewards.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.