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

`swapFee` and `baseFee` will have negligible values in `StabilityBranch`

brgltd

Summary

baseFee and swapFee will have very small, almost insignificant values.

Vulnerability Details

StabilityBranch.getFeesForAssetsAmountOut will compute the baseFee and swapFee which will be used to compute the fees which will be distributed in StabilityBranch.fulfillSwap.

Let's start with baseFee:

// convert the base fee in usd to the asset amount to be charged
baseFeeX18 = ud60x18(tokenSwapData.baseFeeUsd).div(priceX18);

Running testFuzz_WhenSlippageCheckPassesAndThePremiumOrDiscountIsNotZero, the baseFeeUsd is equal to 1 (baseFeeUsd being an admin controlled value), We can observe that, if baseFeeUsd is equal to 1, the baseFeeX18 will be equal to zero for practically any price.

And for swapFee:

// calculates the swap fee portion rounding up
swapFeeX18 = Math.divUp(
assetsAmountOutX18.mul(ud60x18(tokenSwapData.swapSettlementFeeBps)),
ud60x18Convert(Constants.BPS_DENOMINATOR)
);

If we ran testFuzz_WhenSlippageCheckPassesAndThePremiumOrDiscountIsNotZero we've can observe that swapFeesX18 will also be zero.

If we provide a more concrete example, by converting a test to have fixed values (no fuzz), we can observe swapFeesX18 will be still be zero. For example we can run the following test inside

If we remove the fuzz inputs and test a function with static vaultConfig, we can confirm baseFee and swapFee will still be zero. For example we can run the following test inside ./test/integration/market-making/stability-branch/fulfillSwap/fulfillSwap.t.sol

struct Test_SwapAndBaseFeesAreZero {
// Use `vaultConfig` instead of `fuzzVaultConfig`.
VaultConfig vaultConfig;
// Remaining fields are the same as `TestFuzz_WhenSlippageCheckPassesAndThePremiumOrDiscountIsNotZero_Context`.
uint256 oneAsset;
PerpMarketCreditConfig fuzzPerpMarketCreditConfig;
IMockEngine engine;
uint128 minAmountOut;
bytes priceData;
address usdTokenSwapKeeper;
UsdTokenSwapConfig.SwapRequest request;
uint128 requestId;
UD60x18 amountOut;
uint256 amountOutAfterFee;
UD60x18 baseFeeX18;
UD60x18 swapFeeX18;
UD60x18 protocolSwapFee;
uint256 protocolReward;
}
​
function testSwapAndBaseFeesAreZero() external {
Test_SwapAndBaseFeesAreZero memory ctx;
​
// Will use weWETH.
uint256 vaultId = 1;
uint256 marketId = 1;
​
bool useCredit = true;
​
ctx.vaultConfig = vaultsConfig[vaultId];
​
ctx.oneAsset = 10 ** ctx.vaultConfig.decimals;
​
changePrank({ msgSender: users.owner.account });
marketMakingEngine.configureUsdTokenSwapConfig(1, 30, type(uint96).max);
​
changePrank({ msgSender: users.naruto.account });
​
// Even if we try different values for `vaultAssetsBalance` the fees will still be zero.
uint256 vaultAssetsBalance = ctx.vaultConfig.depositCap;
uint256 vaultDebtAbsUsd = vaultAssetsBalance;
​
deal({
token: address(ctx.vaultConfig.asset),
to: ctx.vaultConfig.indexToken,
give: vaultAssetsBalance
});
​
// Even if we try different values for `vaultAssetsBalance` the fees will still be zero.
uint256 swapAmount = vaultAssetsBalance / 1e9;
​
deal({ token: address(usdToken), to: users.naruto.account, give: swapAmount });
​
ctx.fuzzPerpMarketCreditConfig = getFuzzPerpMarketCreditConfig(marketId);
ctx.engine = IMockEngine(perpMarketsCreditConfig[ctx.fuzzPerpMarketCreditConfig.marketId].engine);
ctx.engine.setUnrealizedDebt(useCredit ? -int256(vaultDebtAbsUsd) : int256(vaultDebtAbsUsd));
​
ctx.minAmountOut = 0;
UD60x18 priceUsdX18 = IPriceAdapter(vaultsConfig[ctx.vaultConfig.vaultId].priceAdapter).getPrice();
​
ctx.priceData = getMockedSignedReport(ctx.vaultConfig.streamId, priceUsdX18.intoUint256());
ctx.usdTokenSwapKeeper = usdTokenSwapKeepers[ctx.vaultConfig.asset];
​
ctx.amountOut =
marketMakingEngine.getAmountOfAssetOut(ctx.vaultConfig.vaultId, ud60x18(swapAmount), priceUsdX18);
​
vm.assume(ctx.amountOut.intoUint256() > 0);
​
initiateUsdSwap(uint128(ctx.vaultConfig.vaultId), swapAmount, ctx.minAmountOut);
​
(ctx.baseFeeX18, ctx.swapFeeX18) = marketMakingEngine.getFeesForAssetsAmountOut(ctx.amountOut, priceUsdX18);
​
// Confirm fees are zero currently.
assertEq(ctx.baseFeeX18.intoUint256(), 0);
assertEq(ctx.swapFeeX18.intoUint256(), 0);
}

Impact

The protocol fee calculations today for StabilityBranch.getFeesForAssetsAmountOut will result in zero (0 can be obtained by testFuzz_WhenSlippageCheckPassesAndThePremiumOrDiscountIsNotZero or the static test added above), or 1 wei (1 wei can be obtained by running testFuzz_WhenSlippageCheckPassesAndThePremiumOrDiscountIsZero).

We believe the fees should have a non negligible value, otherwise there's no reason for them to be computed.

Tools Used

Manual review.

Recommendations

Update StabilityBranch.getFeesForAssetsAmountOut so that the baseFee and swapFee contain values bigger than 0 or 1 wei.

Updates

Lead Judging Commences

inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!