Summary
The received market fee is split between protocol and LPs who stake their shares.
The problem is the market's wethRewardPerVaultShare is incorrectly calculated and the same amount is allocated to all vaults who delegated credit to market.
Vulnerability Details
LP who contributed with liquidity to a vault can stake their vault tokens to receive marketFee in the form of weth.
The weth marketFee is received and processed by receiveMarketFee ->_handleWethRewardDistribution -> market.receiveWethReward
Or it's converted to weth by convertAccumulatedFeesToWeth -> _handleWethRewardDistribution -> receiveWethReward if the fee was paid in a different token than weth.
function receiveWethReward(
Data storage self,
address asset,
UD60x18 receivedProtocolWethRewardX18,
UD60x18 receivedVaultsWethRewardX18
)
internal
{
if (asset != address(0)) {
self.receivedFees.remove(asset);
}
self.availableProtocolWethReward =
ud60x18(self.availableProtocolWethReward).add(receivedProtocolWethRewardX18).intoUint128();
@> self.wethRewardPerVaultShare =
ud60x18(self.wethRewardPerVaultShare).add(receivedVaultsWethRewardX18).intoUint128();
}
The vault vaultTotalWethRewardChangeX18 is calculated as the sum of all wethRewardChangeX18 returned by market.getVaultAccumulatedValues
function _recalculateConnectedMarketsState(
Data storage self,
uint128[] memory connectedMarketsIdsCache,
bool shouldRehydrateCache
)
private
returns (
uint128[] memory rehydratedConnectedMarketsIdsCache,
SD59x18 vaultTotalRealizedDebtChangeUsdX18,
SD59x18 vaultTotalUnrealizedDebtChangeUsdX18,
UD60x18 vaultTotalUsdcCreditChangeX18,
UD60x18 vaultTotalWethRewardChangeX18
)
{
...
(
ctx.realizedDebtChangeUsdX18,
ctx.unrealizedDebtChangeUsdX18,
ctx.usdcCreditChangeX18,
@> ctx.wethRewardChangeX18
@> ) = market.getVaultAccumulatedValues(
ud60x18(creditDelegation.valueUsd),
sd59x18(creditDelegation.lastVaultDistributedRealizedDebtUsdPerShare),
sd59x18(creditDelegation.lastVaultDistributedUnrealizedDebtUsdPerShare),
ud60x18(creditDelegation.lastVaultDistributedUsdcCreditPerShare),
ud60x18(creditDelegation.lastVaultDistributedWethRewardPerShare)
);
...
@> vaultTotalWethRewardChangeX18 = vaultTotalWethRewardChangeX18.add(ctx.wethRewardChangeX18);
...
The returned reward delta, wethRewardChangeX18, is the difference between the rewards stored in market's wethRewardPerVaultShare (updated by receiveWethReward as shown earlier) and the last value stored in creditDelegation.
function getVaultAccumulatedValues(
Data storage self,
UD60x18 vaultDelegatedCreditUsdX18,
SD59x18 lastVaultDistributedRealizedDebtUsdPerShareX18,
SD59x18 lastVaultDistributedUnrealizedDebtUsdPerShareX18,
UD60x18 lastVaultDistributedUsdcCreditPerShareX18,
UD60x18 lastVaultDistributedWethRewardPerShareX18
)
internal
view
returns (
SD59x18 realizedDebtChangeUsdX18,
SD59x18 unrealizedDebtChangeUsdX18,
UD60x18 usdcCreditChangeX18,
UD60x18 wethRewardChangeX18
)
{
@> wethRewardChangeX18 = ud60x18(self.wethRewardPerVaultShare).sub(lastVaultDistributedWethRewardPerShareX18);
After this sum is calculated, the sum,vaultTotalWethRewardChangeX18, is passed to self.wethRewardDistribution.distributeValue to increase the vault's value per share.
The issue is that market's rewards aren't distributed to vaults based on the amount of credit each vault delegated to the market. The Same amount of reward is distributed to all connected vaults. Instead, the fair share of reward should be distributed to each vault.
function receiveWethReward(
Data storage self,
address asset,
UD60x18 receivedProtocolWethRewardX18,
UD60x18 receivedVaultsWethRewardX18
)
internal
{
if (asset != address(0)) {
self.receivedFees.remove(asset);
}
self.availableProtocolWethReward =
ud60x18(self.availableProtocolWethReward).add(receivedProtocolWethRewardX18).intoUint128();
@> self.wethRewardPerVaultShare =
ud60x18(self.wethRewardPerVaultShare).add(receivedVaultsWethRewardX18).intoUint128();
}
Impact
Not all users who stakes their vault shares will be rewarded.
If market-making engine has weth from other sources than market fee (eg. if weth is deposited as credit when a trader is liquidated/ settles negative PnL, etc), stakers can withdraw rewards intended for other purposes. LP get less rewards than they should.
Tools Used
Recommendations
Consider the following changes:
function receiveWethReward(
Data storage self,
address asset,
UD60x18 receivedProtocolWethRewardX18,
UD60x18 receivedVaultsWethRewardX18
)
internal
{
...
// increment the all time weth reward storage
- self.wethRewardPerVaultShare =
- ud60x18(self.wethRewardPerVaultShare).add(receivedVaultsWethRewardX18).intoUint128();
+ self.wethRewardPerVaultShare =
+ ud60x18(self.wethRewardPerVaultShare).add(receivedVaultsWethRewardX18.div(getTotalDelegatedCreditUsd(self)).intoUint128();
}
function getVaultAccumulatedValues(
Data storage self,
UD60x18 vaultDelegatedCreditUsdX18,
SD59x18 lastVaultDistributedRealizedDebtUsdPerShareX18,
SD59x18 lastVaultDistributedUnrealizedDebtUsdPerShareX18,
UD60x18 lastVaultDistributedUsdcCreditPerShareX18,
UD60x18 lastVaultDistributedWethRewardPerShareX18
)
internal
view
returns (
SD59x18 realizedDebtChangeUsdX18,
SD59x18 unrealizedDebtChangeUsdX18,
UD60x18 usdcCreditChangeX18,
UD60x18 wethRewardChangeX18
)
{
// calculate the vault's share of the total delegated credit, from 0 to 1
UD60x18 vaultCreditShareX18 = vaultDelegatedCreditUsdX18.div(getTotalDelegatedCreditUsd(self));
// calculate the vault's value changes since its last accumulation
// note: if the last distributed value is zero, we assume it's the first time the vault is accumulating
// values, thus, it needs to return zero changes
realizedDebtChangeUsdX18 = !lastVaultDistributedRealizedDebtUsdPerShareX18.isZero()
? sd59x18(self.realizedDebtUsdPerVaultShare).sub(lastVaultDistributedRealizedDebtUsdPerShareX18).mul(
vaultCreditShareX18.intoSD59x18()
)
: SD59x18_ZERO;
...
// TODO: fix the vaultCreditShareX18 flow to multiply by `wethRewardChangeX18`
- wethRewardChangeX18 = ud60x18(self.wethRewardPerVaultShare).sub(lastVaultDistributedWethRewardPerShareX18);
+ wethRewardChangeX18 = !lastVaultDistributedWethRewardPerShareX18.isZero()
+ ? ud60x18(self.wethRewardPerVaultShare).sub(lastVaultDistributedWethRewardPerShareX18).mul(
+ vaultCreditShareX18
+ )
+ : UD60x18_ZERO;
}
