Submission Details
Severity:
Long traders are charged higher fees than shorts traders
Long traders are charged higher fees than short traders
High
Summary
Zaros charges long traders higher order fees than short traders irrespective of whether they are taker or maker orders.
Vulnerability Details
Zaro is a perp market that allows traders to go long or short on an asset. Users create market orders and the keeper executes them. All orders are charged order fees and settlement fees. Order fees are based on trade bias i.e. if the order is taker or maker order. Takers are orders that push the market skew farther from zero, while maker pushes it closer to zero.
Like most exchanges, takers are charged higher fees. And being a taker/maker on Zaros is not about whether you are going long or short an asset, but the relationship between the market skew and the direction of your trade. Trades that push the skew further from zero are charged takerFees and trades that push the skew closer to zero are charged makerFees. Logically, the first trade in any market, or when the skew is zero, is charged takerFees.
The issue is that Zaros charges long traders higher fees compared to short traders irrespective of whether they are maker or taker orders.
Proof of Code
The PoC below shows the cost of long and short transactions under the same market conditions.
The market is a new market with every state at the default
sizeDeltaof the orders in both scenarios are the sameThe prices of the market are the same.
Basically, everything but the direction of the trade is the same, and yet the log shows that long traders are unfairly charged higher fees
How to run the PoC
Create a new test file named
SubmissionPoC.t.solintest/folder of the contest repoCopy the code below and paste into the file.
Run the commands above the test functions in your CLI to run the test.
Note that the mock price feed makes no distinction between the ask and bid price when encoding the verifiedPriceData, and the protocol does not adjust the prices when it is decoded, further proving that the imbalance fee charges are coming from the protocol's logic.
File to copy:
<details>
// SPDX-License-Identifier: UNLICENSED
// Zaros dependencies
import { Base_Test } from "test/Base.t.sol";
import { Errors } from "@zaros/utils/Errors.sol";
import { ERC20, IERC20 } from "@openzeppelin/token/ERC20/ERC20.sol";
import {console2} from "forge-std/console2.sol";
import { MockERC20 } from "test/mocks/MockERC20.sol";
import { MockPriceFeed } from "test/mocks/MockPriceFeed.sol";
import { Users, User, MockPriceAdapters } from "test/utils/Types.sol";
import { UpgradeBranch } from "@zaros/tree-proxy/branches/UpgradeBranch.sol";
import { LookupBranch } from "@zaros/tree-proxy/branches/LookupBranch.sol";
import { GlobalConfigurationBranch } from "@zaros/perpetuals/branches/GlobalConfigurationBranch.sol";
import { LiquidationBranch } from "@zaros/perpetuals/branches/LiquidationBranch.sol";
import { OrderBranch } from "@zaros/perpetuals/branches/OrderBranch.sol";
import { PerpMarketBranch } from "@zaros/perpetuals/branches/PerpMarketBranch.sol";
import { TradingAccountBranch } from "@zaros/perpetuals/branches/TradingAccountBranch.sol";
import { SettlementBranch } from "@zaros/perpetuals/branches/SettlementBranch.sol";
import { MarginCollateralConfiguration } from "@zaros/perpetuals/leaves/MarginCollateralConfiguration.sol";
import { Position } from "@zaros/perpetuals/leaves/Position.sol";
import { MarketOrder } from "@zaros/perpetuals/leaves/MarketOrder.sol";
import { SettlementConfiguration } from "@zaros/perpetuals/leaves/SettlementConfiguration.sol";
import { TradingAccountBranchTestnet } from "testnet/branches/TradingAccountBranchTestnet.sol";
import { GlobalConfigurationHarness } from "test/harnesses/perpetuals/leaves/GlobalConfigurationHarness.sol";
import { MarginCollateralConfigurationHarness } from "test/harnesses/perpetuals/leaves/MarginCollateralConfigurationHarness.sol";
import { MarketConfigurationHarness } from "test/harnesses/perpetuals/leaves/MarketConfigurationHarness.sol";
import { MarketOrderHarness } from "test/harnesses/perpetuals/leaves/MarketOrderHarness.sol";
import { PerpMarketHarness } from "test/harnesses/perpetuals/leaves/PerpMarketHarness.sol";
import { PositionHarness } from "test/harnesses/perpetuals/leaves/PositionHarness.sol";
import { SettlementConfigurationHarness } from "test/harnesses/perpetuals/leaves/SettlementConfigurationHarness.sol";
import { TradingAccountHarness } from "test/harnesses/perpetuals/leaves/TradingAccountHarness.sol";
import {LiquidationKeeper} from "@zaros/external/chainlink/keepers/liquidation/LiquidationKeeper.sol";
import {MarketOrderKeeper} from "@zaros/external/chainlink/keepers/market-order/MarketOrderKeeper.sol";
// PRB Math dependencies
import { UD60x18, ud60x18, ZERO as UD60x18_ZERO, convert as ud60x18Convert } from "@prb-math/UD60x18.sol";
import {SD59x18,sd59x18, unary, UNIT as SD_UNIT, ZERO as SD59x18_ZERO, convert as sd59x18Convert} from "@prb-math/SD59x18.sol";
contract BugPoC is Base_Test {
///////////// SETUP //////////////
address[] internal USERS;
MockERC20[] internal EXTERNAL_TOKENS;
address internal BOB;
address internal ALICE;
address internal defaultUser; // defaultUser
address USDC;
address wBTC;
address weETH;
uint256 internal constant INITIAL_USDC_BALANCE = 1_000_000;
uint256 internal constant INITIAL_WEETH_BALANCE = 100_000;
uint256 internal constant INITIAL_WBTC_BALANCE = 10_000;
MockPriceFeed btcPriceFeed;
MockPriceFeed ethPriceFeed;
function setUp() public override {
Base_Test.setUp();
changePrank({ msgSender: users.owner.account });
configureSystemParameters();
createPerpMarkets();
EXTERNAL_TOKENS = new MockERC20[](3);
EXTERNAL_TOKENS[0] = usdc;
EXTERNAL_TOKENS[1] = weEth;
EXTERNAL_TOKENS[2] = wBtc;
USDC = address(usdc);
wBTC = address(wBtc);
weETH = address(weEth);
btcPriceFeed = MockPriceFeed(marketsConfig[1].priceAdapter);
ethPriceFeed = MockPriceFeed(marketsConfig[2].priceAdapter);
BOB = users.naruto.account;
ALICE = users.sasuke.account;
USERS = new address[](2);
USERS[0] = BOB;
USERS[1] = ALICE;
_topUpUsers();
vm.stopPrank();
}
function _topUpUsers() internal {
address user;
for (uint8 i = 0; i < USERS.length; i++) {
user = USERS[i];
EXTERNAL_TOKENS[0].mint(user, INITIAL_USDC_BALANCE * (10 ** EXTERNAL_TOKENS[0].decimals()));
EXTERNAL_TOKENS[1].mint(user, INITIAL_WEETH_BALANCE * (10 ** EXTERNAL_TOKENS[0].decimals()));
EXTERNAL_TOKENS[2].mint(user, INITIAL_WBTC_BALANCE * (10 ** EXTERNAL_TOKENS[0].decimals()));
}
}
modifier updateMockPriceFeedBtcEth(uint256 _btcNewPrice, uint256 _ethNewPrice) {
updateMockPriceFeed(1, _btcNewPrice);
updateMockPriceFeed(2, _ethNewPrice);
_;
}
function getPriceUint(MockPriceFeed priceFeed) internal view returns (uint256) {
(, int256 answer,,,) = priceFeed.latestRoundData();
return uint256(answer);
}
function performLiquidation(uint128 nextAccountId) internal {
LiquidationKeeper liquidationKeeper_ = LiquidationKeeper(liquidationKeeper);
uint256 checkLowerBound = 0;
uint256 checkUpperBound = uint256(nextAccountId);
uint256 performLowerBound = 0;
uint256 performUpperBound = checkUpperBound + 1;
if (checkUpperBound < 1) return;
bytes memory data = abi.encode(checkLowerBound, checkUpperBound, performLowerBound, performUpperBound);
(bool upkeepNeeded, bytes memory performData) = liquidationKeeper_.checkUpkeep(data);
if (upkeepNeeded) {
vm.startPrank(users.keepersForwarder.account);
liquidationKeeper_.performUpkeep(performData);
vm.stopPrank();
}
}
struct TestHelper {
uint256 oldBtcPrice;
uint256 oldEthPrice;
uint256 btcNewPrice;
uint256 ethNewPrice;
uint256 price;
bytes32 streamId;
uint256 priceBtcChange;
uint256 priceEthChange;
OrderBranch.CreateMarketOrderParams params;
}
function _performOrderExecution(uint256 marketId, uint128 tradingAccountId, bool isIncrease) internal {
TestHelper memory helper;
helper.oldBtcPrice = getPriceUint(btcPriceFeed);
helper.oldEthPrice = getPriceUint(ethPriceFeed);
helper.btcNewPrice;
helper.ethNewPrice;
helper.price;
helper.streamId = marketsConfig[marketId].streamId;
helper.priceBtcChange = helper.oldBtcPrice % 100e18;
helper.priceEthChange = helper.oldEthPrice % 10e18;
if(isIncrease) {
helper.btcNewPrice = helper.oldBtcPrice + helper.priceBtcChange;
helper.ethNewPrice = helper.oldEthPrice + helper.priceEthChange;
} else {
helper.btcNewPrice = helper.oldBtcPrice - helper.priceBtcChange;
helper.ethNewPrice = helper.oldEthPrice - helper.priceEthChange;
}
btcPriceFeed.updateMockPrice(helper.btcNewPrice);
ethPriceFeed.updateMockPrice(helper.ethNewPrice);
address marketOrderKeeper_ = marketOrderKeepers[marketId];
MarketOrderKeeper marketOrderKeeper = MarketOrderKeeper(marketOrderKeeper_);
if (marketId == 1) {
helper.price = getPriceUint(btcPriceFeed);
} else {
helper.price = getPriceUint(ethPriceFeed);
}
bytes memory mockSignedReport = getMockedSignedReport(helper.streamId, helper.price);
vm.startPrank(users.owner.account);
marketOrderKeeper.setForwarder(users.keepersForwarder.account);
vm.stopPrank();
bytes memory performData = abi.encode(mockSignedReport, abi.encode(tradingAccountId));
vm.startPrank(users.keepersForwarder.account);
marketOrderKeeper.performUpkeep(performData);
vm.stopPrank();
}
function tradingAccountBranch_createTradingAccount(
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) returns(uint128 _tradingAccountId){
return perpsEngine.createTradingAccount(bytes(""), false) ;
}
/// depositMargin
function tradingAccountBranch_depositMargin(
uint128 _tradingAccountId,
address collateralType,
uint256 _amount,
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) {
perpsEngine.depositMargin(_tradingAccountId, collateralType, _amount);
}
/// withdrawMargin
function tradingAccountBranch_withdrawMargin(
uint128 _tradingAccountId,
address collateralType,
uint256 _amount,
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) {
perpsEngine.withdrawMargin(_tradingAccountId, collateralType, _amount);
}
/// createMarketOrder
function orderBranch_createMarketOrder(
uint128 _tradingAccountId,
bool _buyBtc,
int128 sizeDelta,
bool isIncrease,
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) {
TestHelper memory helper;
helper.params.tradingAccountId = _tradingAccountId;
helper.params.marketId = _buyBtc ? 1 : 2;
helper.params.sizeDelta = sizeDelta;
perpsEngine.createMarketOrder(helper.params);
_performOrderExecution(helper.params.marketId, helper.params.tradingAccountId, isIncrease);
}
///////////// TEST //////////////
////////// Imbalance Fee charge ////////////
// forge test --match-test "testMarginBalanceAfterLongTrade"
// Long traders are charged more fees than short traders for exactly thesame market conditions and taker/maker actions
function testMarginBalanceAfterLongTrade() public {
vm.warp(1 hours);
vm.startPrank(BOB);
tradingAccountBranch_createTradingAccount(30_000e18, 2_000e18);
vm.startPrank(BOB);
tradingAccountBranch_depositMargin(1, USDC, 10000e6, 100_000e18, 10_000e18); // deposit 10000e6 USDC
// cache depositor margin balance before
(SD59x18 before_marginBalanceUsdX18, UD60x18 before_initialMarginUsdX18, UD60x18 before_maintenanceMarginUsdX18,) = perpsEngine.getAccountMarginBreakdown(1);
// create a long position
vm.warp(1 hours);
vm.startPrank(BOB);
orderBranch_createMarketOrder(1, true, 1e18, true, 100_000e18, 10_000e18); // create Market order of 1e18 BTC @ 100_000e18 USDC : 10x leverage
// cache depositor margin balance after
(SD59x18 after_marginBalanceUsdX18, UD60x18 after_initialMarginUsdX18, UD60x18 after_maintenanceMarginUsdX18,) = perpsEngine.getAccountMarginBreakdown(1);
// log the difference between before and after margin balance
console2.log("before_marginBalanceUsdX18: ", (before_marginBalanceUsdX18.intoUD60x18()).intoUint256());
console2.log("after_marginBalanceUsdX18: ", (after_marginBalanceUsdX18.intoUD60x18()).intoUint256());
console2.log("diff_marginBalanceUsdX18: ", ((before_marginBalanceUsdX18 - after_marginBalanceUsdX18).intoUD60x18()).intoUint256());
console2.log("before_initialMarginUsdX18: ", before_initialMarginUsdX18.intoUint256());
console2.log("after_initialMarginUsdX18: ", after_initialMarginUsdX18.intoUint256());
// Logs:
// before_marginBalanceUsdX18: 10000000000000000000000
// after_marginBalanceUsdX18: 9917999996000000000000
// diff_marginBalanceUsdX18: 82000004000000000000
// before_initialMarginUsdX18: 0
// after_initialMarginUsdX18: 1000000050000000000000
}
// forge test --match-test "testMarginBalanceAfterShortTrade" -vvv
function testMarginBalanceAfterShortTrade() public {
vm.warp(1 hours);
vm.startPrank(BOB);
tradingAccountBranch_createTradingAccount(30_000e18, 2_000e18);
vm.startPrank(BOB);
tradingAccountBranch_depositMargin(1, USDC, 10000e6, 100_000e18, 10_000e18); // deposit 10000e6 USDC
// cache depositor margin balance before
(SD59x18 before_marginBalanceUsdX18, UD60x18 before_initialMarginUsdX18, UD60x18 before_maintenanceMarginUsdX18,) = perpsEngine.getAccountMarginBreakdown(1);
// create a long position
vm.warp(1 hours);
vm.startPrank(BOB);
orderBranch_createMarketOrder(1, true, -1e18, true, 100_000e18, 10_000e18); // create Market order of 1e18 BTC @ 100_000e18 USDC : 10x leverage
// cache depositor margin balance after
(SD59x18 after_marginBalanceUsdX18, UD60x18 after_initialMarginUsdX18, UD60x18 after_maintenanceMarginUsdX18,) = perpsEngine.getAccountMarginBreakdown(1);
// log the difference between before and after margin balance
console2.log("before_marginBalanceUsdX18: ", (before_marginBalanceUsdX18.intoUD60x18()).intoUint256());
console2.log("after_marginBalanceUsdX18: ", (after_marginBalanceUsdX18.intoUD60x18()).intoUint256());
console2.log("diff_marginBalanceUsdX18: ", ((before_marginBalanceUsdX18 - after_marginBalanceUsdX18).intoUD60x18()).intoUint256());
console2.log("before_initialMarginUsdX18: ", before_initialMarginUsdX18.intoUint256());
console2.log("after_initialMarginUsdX18: ", after_initialMarginUsdX18.intoUint256());
// Logs:
// before_marginBalanceUsdX18: 10000000000000000000000
// after_marginBalanceUsdX18: 9918000004000000000000
// diff_marginBalanceUsdX18: 81999996000000000000
// before_initialMarginUsdX18: 0
// after_initialMarginUsdX18: 999999950000000000000
}
}
</details>
Impact
Long traders are subjected to higher operation costs than their counterpart
Tools Used
Foundry
Recommendations
Review the fee calculation logic. The ratio of fee/reward for opposing but equally important actions such as long and short taker/maker orders should be thesame.
Updates
Prize pool breakdown
Total prize
60,000 USDC
nSLOC
2,87821 USDC / LOC
50,000 USDC
5,500 USDC
4,500 USDC
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.