Submission Details
Severity:
CreditDelegation values will always be 0 until market fee is received
Summary
CreditDelegation values will always be 0 until market fee is received and the values are not incorrectly accounted.
Vulnerability Details
The vault's accumulated debt, credit and reward changes are updated in recalculateVaultsCreditCapacity(), which retrieves the changes from getVaultAccumulatedValues().
Vault::_recalculateConnectedMarketsState():
(
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)
);
As can be seen, the passed arguments to getVaultAccumulatedValues() are CreditDelegation values, which are updated later.
Vault::_recalculateConnectedMarketsState():
// update the last distributed debt, credit and reward values to the vault's credit delegation to the
// given market id, in order to keep next calculations consistent
creditDelegation.updateVaultLastDistributedValues(
sd59x18(market.realizedDebtUsdPerVaultShare),
sd59x18(market.unrealizedDebtUsdPerVaultShare),
ud60x18(market.usdcCreditPerVaultShare),
ud60x18(market.wethRewardPerVaultShare)
);
In getVaultAccumulatedValues(), if the last distributed value is zero, protocol assumes it's the first time the vault is accumulating
values, thus, it needs to return zero changes.
Market::getVaultAccumulatedValues():
// 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;
unrealizedDebtChangeUsdX18 = !lastVaultDistributedUnrealizedDebtUsdPerShareX18.isZero()
? sd59x18(self.unrealizedDebtUsdPerVaultShare).sub(lastVaultDistributedUnrealizedDebtUsdPerShareX18).mul(
vaultCreditShareX18.intoSD59x18()
)
@> : SD59x18_ZERO;
usdcCreditChangeX18 = !lastVaultDistributedUsdcCreditPerShareX18.isZero()
? ud60x18(self.usdcCreditPerVaultShare).sub(lastVaultDistributedUsdcCreditPerShareX18).mul(
vaultCreditShareX18
)
@> : UD60x18_ZERO;
// TODO: fix the vaultCreditShareX18 flow to multiply by `wethRewardChangeX18`
wethRewardChangeX18 = ud60x18(self.wethRewardPerVaultShare).sub(lastVaultDistributedWethRewardPerShareX18);
This is problematic as protocol updates CreditDelegation values when not all of the values are not 0.
Vault::_recalculateConnectedMarketsState():
// if there's been no change in any of the returned values, we can iterate to the next
// market id
if (
ctx.realizedDebtChangeUsdX18.isZero() && ctx.unrealizedDebtChangeUsdX18.isZero()
&& ctx.usdcCreditChangeX18.isZero() && ctx.wethRewardChangeX18.isZero()
) {
continue;
}
This means that even if there are multple credit deposits from registered engines, the CreditDelegation values are always 0, until there are market fees received, which makes wethRewardChangeX18 non-zero.
Please run the coded POC:
// SPDX-License-Identifier: MIT
import { Test, console } from "forge-std/Test.sol";
import "./Base.t.sol";
import { Errors } from "@zaros/utils/Errors.sol";
import { Vault } from "@zaros/market-making/leaves/Vault.sol";
import { Collateral } from "@zaros/market-making/leaves/Collateral.sol";
contract AuditTest is Base_Test {
address owner = makeAddr("Owner");
address feeRecipient = makeAddr("FeeRecipient");
function setUp() public override {
// Perps Engine Set Up
bool isTestnet = false;
address[] memory branches = deployPerpsEngineBranches(isTestnet);
bytes4[][] memory branchesSelectors = getPerpsEngineBranchesSelectors(isTestnet);
RootProxy.BranchUpgrade[] memory branchUpgrades =
getBranchUpgrades(branches, branchesSelectors, RootProxy.BranchUpgradeAction.Add);
address[] memory initializables = getInitializables(branches);
bytes[] memory initializePayloads = getInitializePayloads(owner);
branchUpgrades = deployPerpsEngineHarnesses(branchUpgrades);
RootProxy.InitParams memory initParams = RootProxy.InitParams({
initBranches: branchUpgrades,
initializables: initializables,
initializePayloads: initializePayloads
});
perpsEngine = IPerpsEngine(address(new MockEngine(initParams)));
// Market Making Engine Set Up
address[] memory mmBranches = deployMarketMakingEngineBranches();
bytes4[][] memory mmBranchesSelectors = getMarketMakerBranchesSelectors();
RootProxy.BranchUpgrade[] memory mmBranchUpgrades =
getBranchUpgrades(mmBranches, mmBranchesSelectors, RootProxy.BranchUpgradeAction.Add);
RootProxy.BranchUpgrade[] memory mmbranchUpgrades = new RootProxy.BranchUpgrade[](mmBranchUpgrades.length);
for (uint256 i; i < mmbranchUpgrades.length; i++) {
mmbranchUpgrades[i] = mmBranchUpgrades[i];
}
initializables = getInitializables(mmBranches);
initializePayloads = getInitializePayloads(owner);
RootProxy.InitParams memory mmEngineInitParams = RootProxy.InitParams({
initBranches: mmbranchUpgrades,
initializables: initializables,
initializePayloads: initializePayloads
});
marketMakingEngine = IMarketMakingEngine(address(new MarketMakingEngine(mmEngineInitParams)));
vm.startPrank(owner);
// Configure Collaterals
uint256[2] memory marginCollateralIdsRange;
marginCollateralIdsRange[0] = INITIAL_MARGIN_COLLATERAL_ID;
marginCollateralIdsRange[1] = FINAL_MARGIN_COLLATERAL_ID;
mockSequencerUptimeFeed = address(new MockSequencerUptimeFeed(0));
configureMarginCollaterals(
IPerpsEngine(perpsEngine), marginCollateralIdsRange, true, mockSequencerUptimeFeed, owner
);
usdc = MockERC20(marginCollaterals[USDC_MARGIN_COLLATERAL_ID].marginCollateralAddress);
usdToken = MockUsdToken(marginCollaterals[USD_TOKEN_MARGIN_COLLATERAL_ID].marginCollateralAddress);
weEth = MockERC20(marginCollaterals[WEETH_MARGIN_COLLATERAL_ID].marginCollateralAddress);
wstEth = MockERC20(marginCollaterals[WSTETH_MARGIN_COLLATERAL_ID].marginCollateralAddress);
wEth = MockERC20(marginCollaterals[WETH_MARGIN_COLLATERAL_ID].marginCollateralAddress);
wBtc = MockERC20(marginCollaterals[WBTC_MARGIN_COLLATERAL_ID].marginCollateralAddress);
marketMakingEngine.configureCollateral(
address(usdc),
marginCollaterals[USDC_MARGIN_COLLATERAL_ID].priceAdapter,
MOCK_PERP_CREDIT_CONFIG_DEBT_CREDIT_RATIO,
true,
marginCollaterals[USDC_MARGIN_COLLATERAL_ID].tokenDecimals
);
marketMakingEngine.configureCollateral(
address(usdToken),
marginCollaterals[USD_TOKEN_MARGIN_COLLATERAL_ID].priceAdapter,
MOCK_PERP_CREDIT_CONFIG_DEBT_CREDIT_RATIO,
true,
marginCollaterals[USD_TOKEN_MARGIN_COLLATERAL_ID].tokenDecimals
);
marketMakingEngine.configureCollateral(
address(wEth),
marginCollaterals[WETH_MARGIN_COLLATERAL_ID].priceAdapter,
MOCK_PERP_CREDIT_CONFIG_DEBT_CREDIT_RATIO,
true,
marginCollaterals[WETH_MARGIN_COLLATERAL_ID].tokenDecimals
);
marketMakingEngine.configureCollateral(
address(wBtc),
marginCollaterals[WBTC_MARGIN_COLLATERAL_ID].priceAdapter,
MOCK_PERP_CREDIT_CONFIG_DEBT_CREDIT_RATIO,
true,
marginCollaterals[WBTC_MARGIN_COLLATERAL_ID].tokenDecimals
);
// Set the wETH address
marketMakingEngine.setWeth(address(wEth));
// Set the USDC address
marketMakingEngine.setUsdc(address(usdc));
// Configure Engine
marketMakingEngine.configureEngine(address(perpsEngine), address(usdToken), true);
// Configure Fee Recipient
marketMakingEngine.configureVaultDepositAndRedeemFeeRecipient(feeRecipient);
// Create Zlp Vaults
uint256[2] memory vaultsIdsRange;
vaultsIdsRange[0] = INITIAL_VAULT_ID;
vaultsIdsRange[1] = FINAL_VAULT_ID;
setupVaultsConfig();
createZlpVaults(address(marketMakingEngine), owner, vaultsIdsRange);
// Setup Perp Markets
bool isTest = true;
setupPerpMarketsCreditConfig(isTest, address(perpsEngine), address(usdToken));
vm.stopPrank();
}
function testAudit_CreditDelegationValues() public {
vm.startPrank(owner);
// Create USDC Vault
uint128 vaultId = USDC_CORE_VAULT_ID;
address indexToken = address(zlpVaults[vaultsConfig[vaultId].asset][vaultsConfig[vaultId].vaultType]);
ZlpVault zlpVault = ZlpVault(indexToken);
{
Collateral.Data memory collateral = Collateral.Data({
creditRatio: vaultsConfig[vaultId].creditRatio,
priceAdapter: vaultsConfig[vaultId].priceAdapter,
asset: vaultsConfig[vaultId].asset,
isEnabled: vaultsConfig[vaultId].isEnabled,
decimals: vaultsConfig[vaultId].decimals
});
Vault.CreateParams memory vaultCreatParams = Vault.CreateParams({
depositFee: vaultsConfig[vaultId].depositFee,
redeemFee: vaultsConfig[vaultId].redeemFee,
vaultId: vaultsConfig[vaultId].vaultId,
depositCap: vaultsConfig[vaultId].depositCap,
withdrawalDelay: vaultsConfig[vaultId].withdrawalDelay,
indexToken: indexToken,
engine: address(perpsEngine),
collateral: collateral
});
marketMakingEngine.createVault(vaultCreatParams);
Vault.UpdateParams memory vaultUpdateParams = Vault.UpdateParams({
vaultId: vaultId,
depositCap: vaultCreatParams.depositCap,
withdrawalDelay: vaultCreatParams.withdrawalDelay,
isLive: true,
lockedCreditRatio: 0
});
marketMakingEngine.updateVaultConfiguration(vaultUpdateParams);
}
// Deposit into Vault
{
uint256 depositAmount = 100e6;
usdc.mint(owner, depositAmount);
usdc.approve(address(marketMakingEngine), depositAmount);
marketMakingEngine.deposit(vaultsConfig[vaultId].vaultId, uint128(depositAmount), 0, "", false);
}
// Configure Market
uint128 marketId = perpMarketsCreditConfig[ARB_PERP_MARKET_CREDIT_CONFIG_ID].marketId;
{
uint128 marketCreditConfigId = ARB_PERP_MARKET_CREDIT_CONFIG_ID;
marketMakingEngine.configureMarket(
address(perpsEngine),
perpMarketsCreditConfig[marketCreditConfigId].marketId,
perpMarketsCreditConfig[marketCreditConfigId].autoDeleverageStartThreshold,
perpMarketsCreditConfig[marketCreditConfigId].autoDeleverageEndThreshold,
perpMarketsCreditConfig[marketCreditConfigId].autoDeleverageExpoentZ
);
marketMakingEngine.unpauseMarket(marketId);
// Connect Vaults and Market
uint256[] memory marketIds = new uint256[](1);
marketIds[0] = marketId;
uint256[] memory vaultIds = new uint256[](1);
vaultIds[0] = vaultsConfig[vaultId].vaultId;
marketMakingEngine.connectVaultsAndMarkets(marketIds, vaultIds);
marketMakingEngine.updateVaultCreditCapacity(vaultId);
}
vm.stopPrank();
// Deposit 100 USDC Credit from Perps Engine
vm.startPrank(address(perpsEngine));
usdc.mint(address(perpsEngine), 100e6);
usdc.approve(address(marketMakingEngine), 100e6);
marketMakingEngine.depositCreditForMarket(marketId, address(usdc), 100e6);
marketMakingEngine.updateVaultCreditCapacity(vaultId);
vm.stopPrank();
// Deposit 100 USDC Credit from Perps Engine again
vm.startPrank(address(perpsEngine));
usdc.mint(address(perpsEngine), 100e6);
usdc.approve(address(marketMakingEngine), 100e6);
marketMakingEngine.depositCreditForMarket(marketId, address(usdc), 100e6);
marketMakingEngine.updateVaultCreditCapacity(vaultId);
vm.stopPrank();
// Vault's depositedUsdc remains 0 even if there are 2 credit deposits
(,,, uint128 depositedUsdc,,) = marketMakingEngine.getVaultData(vaultId);
assertEq(depositedUsdc, 0);
// Receive Market Fee
// After Market Fee is received, creditDelegation's values are updated
wEth.mint(address(perpsEngine), 1 ether);
vm.startPrank(address(perpsEngine));
wEth.approve(address(marketMakingEngine), 1 ether);
marketMakingEngine.receiveMarketFee(marketId, address(wEth), 1 ether);
vm.stopPrank();
// Deposit 100 USDC Credit from Perps Engine again
vm.startPrank(address(perpsEngine));
usdc.mint(address(perpsEngine), 100e6);
usdc.approve(address(marketMakingEngine), 100e6);
marketMakingEngine.depositCreditForMarket(marketId, address(usdc), 100e6);
vm.stopPrank();
// Because Credit Delegation values are updated, the accumulate changes from market can only be stored
marketMakingEngine.updateVaultCreditCapacity(vaultId);
// Only by then depositedUsdc is updated
(,,, depositedUsdc,,) = marketMakingEngine.getVaultData(vaultId);
assertTrue(depositedUsdc > 0);
}
}
Impact
CreditDelegation values are not correctly updated.
Tools Used
Manual Review
Recommendations
It is recommended to store vault's accumualted values changes when CreditDelegation values are zero, so that CreditDelegation values can be updated later.
Updates
Lead Judging Commences
inallhonesty 4 months ago
Submission Judgement Published Assigned finding tags:
Vault and market state updates are incorrectly skipped when lastVaultDistributed values are zero, requiring WETH fees before accounting starts working
Prize pool breakdown
Total prize
70,000 USDC
nSLOC
3,25821 USDC / LOC
66,500
3,500
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.