DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Valid

Updating the maxFundingVelocity should update the funding rate as well

Summary

maxFundingVelocity of a market can be freely updated by the admins, but the problem is that since funding rate and the elapsed time from last time that the funding was updated are also dependent on it, any update without calling PerpMarket::updateFunding will lead to stepwise jumps in the rates and will either incentivize longs (shorts pay funding fees - funding rate -ve, skew +ve), or shorts (longs pay funding fees - funding rate +ve, skew -ve) because lastFundingTime is not set to block.timestamp:

function updateFunding(Data storage self, SD59x18 fundingRate, SD59x18 fundingFeePerUnit) internal {
self.lastFundingRate = fundingRate.intoInt256();
self.lastFundingFeePerUnit = fundingFeePerUnit.intoInt256();
self.lastFundingTime = block.timestamp;
}

Vulnerability Details

  1. Admin updates the maxFundingVelocity to any reasonable amount, it’s important to note that admin has nothing to do in this situation.

  2. maxFundingVelocity is updated without invoking PerpMarket::updateFunding.

  3. Order is created and in order to get the latest funding rate, PerpMarket::getCurrentFundingRate is invoked:

PerpMarket.sol

function getCurrentFundingRate(Data storage self) internal view returns (SD59x18) {
return sd59x18(self.lastFundingRate).add(
getCurrentFundingVelocity(self).mul(getProportionalElapsedSinceLastFunding(self).intoSD59x18())
);
}
/// @notice Returns the current funding velocity of the given market.
/// @param self The PerpMarket storage pointer.
function getCurrentFundingVelocity(Data storage self) internal view returns (SD59x18) {
SD59x18 maxFundingVelocity = sd59x18(uint256(self.configuration.maxFundingVelocity).toInt256());
SD59x18 skewScale = sd59x18(uint256(self.configuration.skewScale).toInt256());
SD59x18 skew = sd59x18(self.skew);
if (skewScale.isZero()) {
return SD59x18_ZERO;
}
SD59x18 proportionalSkew = skew.div(skewScale);
SD59x18 proportionalSkewBounded = Math.min(Math.max(unary(SD_UNIT), proportionalSkew), SD_UNIT);
return proportionalSkewBounded.mul(maxFundingVelocity);
}

Let’s say the getProportionalElapsedSinceLastFunding is 86400 seconds (1 day) and then we see that the maxFundingVelocity directly influences the result from getCurrentFundingVelocity, and assumes that this velocity is set at the beginning of the previous epoch, while it is recently updated and should be applied from current time.

  1. As we saw the velocity is not what should be, which will either incentivize or disincentivize the trader depending on the sign of the new funding rate, but in all matters will not be the expected funding rate for this current period.

As stated by Zaros team, in the beginning these variables will be more frequently modified due to the early stage of the project and the off-chain calculation that are performed for maximising the profitability for given market. Another scenario which will require more frequent updates of the velocity is for memecoins - highly volatile tokens

Here is a POC, that first opens some positions and warp some time in order for the funding rates to be accumulated, then we update the maxFundingVelocity with uint256.max to demonstrate that numbers won’t be equal without even touching the system, but in reality it will be smaller amount, and lastly prints both old and new funding rates:

import { GlobalConfigurationBranch } from "@zaros/perpetuals/branches/GlobalConfigurationBranch.sol"
forge test --match-test test_funding_velocity_applied_retroactively -vv
file: FillMarketOrder_Integration_Test
function test_funding_velocity_applied_retroactively() public {
Test_GivenTheMarginBalanceUsdIsOverTheMaintenanceMarginUsdRequired_Context memory ctx;
ctx.marketId = BTC_USD_MARKET_ID;
ctx.marginValueUsd = 100_000e18;
deal({ token: address(usdz), to: users.naruto.account, give: ctx.marginValueUsd });
// Config first fill order
ctx.fuzzMarketConfig = getFuzzMarketConfig(ctx.marketId);
ctx.marketOrderKeeper = marketOrderKeepers[ctx.fuzzMarketConfig.marketId];
ctx.tradingAccountId = createAccountAndDeposit(ctx.marginValueUsd, address(usdz));
ctx.firstOrderSizeDelta = 10e18;
(,,, ctx.firstOrderFeeUsdX18,,) = perpsEngine.simulateTrade(
ctx.tradingAccountId,
ctx.fuzzMarketConfig.marketId,
SettlementConfiguration.MARKET_ORDER_CONFIGURATION_ID,
ctx.firstOrderSizeDelta
);
ctx.firstFillPriceX18 = perpsEngine.getMarkPrice(
ctx.fuzzMarketConfig.marketId, ctx.fuzzMarketConfig.mockUsdPrice, ctx.firstOrderSizeDelta
);
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: ctx.tradingAccountId,
marketId: ctx.fuzzMarketConfig.marketId,
sizeDelta: ctx.firstOrderSizeDelta
})
);
ctx.firstOrderExpectedPnl = int256(0);
ctx.firstMockSignedReport =
getMockedSignedReport(ctx.fuzzMarketConfig.streamId, ctx.fuzzMarketConfig.mockUsdPrice);
changePrank({ msgSender: ctx.marketOrderKeeper });
// it should transfer the pnl and fees
expectCallToTransfer(usdz, feeRecipients.settlementFeeRecipient, DEFAULT_SETTLEMENT_FEE);
expectCallToTransfer(usdz, feeRecipients.orderFeeRecipient, ctx.firstOrderFeeUsdX18.intoUint256());
perpsEngine.fillMarketOrder(ctx.tradingAccountId, ctx.fuzzMarketConfig.marketId, ctx.firstMockSignedReport);
changePrank({ msgSender: users.naruto.account });
// if changed this to "/10" instead of "/11" naruto would be liquidatable,
// so this is just on the verge of being liquidated
uint256 updatedPrice = MOCK_BTC_USD_PRICE - MOCK_BTC_USD_PRICE / 11;
updateMockPriceFeed(BTC_USD_MARKET_ID, updatedPrice);
// reduce the position with negative size delta
ctx.secondOrderSizeDelta = -(ctx.firstOrderSizeDelta - ctx.firstOrderSizeDelta / 2);
ctx.fuzzMarketConfig.mockUsdPrice = updatedPrice;
ctx.secondFillPriceX18 = perpsEngine.getMarkPrice(
ctx.fuzzMarketConfig.marketId, ctx.fuzzMarketConfig.mockUsdPrice, ctx.secondOrderSizeDelta
);
(,,, ctx.secondOrderFeeUsdX18,,) = perpsEngine.simulateTrade(
ctx.tradingAccountId,
ctx.fuzzMarketConfig.marketId,
SettlementConfiguration.MARKET_ORDER_CONFIGURATION_ID,
ctx.secondOrderSizeDelta
);
//------------------------------After order simulations--------------------------
vm.warp(block.timestamp + 86_400);
//TODO pre-update
SD59x18 oldFundingRate = perpsEngine.getFundingRate(ctx.fuzzMarketConfig.marketId);
//TODO previous market config
(
string memory name,
string memory symbol,
uint128 initialMarginRateX18,
uint128 maintenanceMarginRateX18,
uint128 maxOpenInterest,
uint128 maxSkew,
uint128 minTradeSizeX18,
uint256 skewScale,
) = perpsEngine.getPerpMarketConfiguration(BTC_USD_MARKET_ID);
//TODO update max funding velocity (other params are mocked values, just reuse the old ones)
changePrank({ msgSender: users.owner.account });
GlobalConfigurationBranch.UpdatePerpMarketConfigurationParams memory params;
params.name = name;
params.symbol = symbol;
params.priceAdapter = address(1);
params.maintenanceMarginRateX18 = maintenanceMarginRateX18;
params.maxOpenInterest = maxOpenInterest;
params.maxSkew = maxSkew;
params.initialMarginRateX18 = initialMarginRateX18;
params.skewScale = skewScale;
params.minTradeSizeX18 = minTradeSizeX18;
params.priceFeedHeartbeatSeconds = 1;
//TODO here we change the MFV to really big number to see the difference:
params.maxFundingVelocity = type(uint128).max;
perpsEngine.updatePerpMarketConfiguration(BTC_USD_MARKET_ID, params);
SD59x18 updatedFundingRate = perpsEngine.getFundingRate(ctx.fuzzMarketConfig.marketId);
console.log("old:", oldFundingRate.intoUint256());
console.log("current:", updatedFundingRate.intoUint256());
assertNotEq(oldFundingRate.intoUint256(), updatedFundingRate.intoUint256());
}

Impact

Stepwise jumps in the funding rates when maxFundingVelocity is update, since updateFunding is not called and system wrongly assumes that velocity is changed at time T = lastFundingTime, while it should be from the time that updatePerpMarketConfiguration is called.

Tools Used

Manual Review

Recommendations

add updateFunding in GlobalConfigurationBranch::updatePerpMarketConfiguration when maxFundingVelocity is being modified.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

When calling updatePerpMarketConfiguration, the fundingRate and the fundingFeePerUnit must be updated if the value for scewScale or maxFundingVelocity is changed

Support

FAQs

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