settleUpnl function can be DOSed by other PartyBs/hedgersSource: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest-judging/issues/39
xiaoming90
settleUpnl function can be DOSed. Users will not be able to close their positions, which could lead to a loss to the users due to the inability to close their positions. In the report's example, PartyA wants to close its losing position to stop its account from incurring further losses due to unfavorable market conditions. However, since PartyA is unable to close its losing position, PartyA continues to incur more losses and might eventually be liquidated. Furthermore, trading is a time-sensitive activity. Thus, any blockage or delay would eventually lead to a loss of assets.
settleUpnl transaction from executing.No response
No response
Assume the following scenario:
PartyA wants to close its losing position to stop its account from incurring further losses due to unfavorable market conditions.
However, the close position transaction cannot go through due to an insufficient allocated balance on PartyA's account. This is a similar scenario described in the Symm IO v0.8.4 documentation.
In order for PartyB_1 to close PartyA's position, it first has to execute the settleUpnl function to settle PartyA's unrealized profit so that PartyA's allocated balance will be "refilled" with more funds. Once there are sufficient funds in PartyA's allocated balance, PartyB_1 can proceed to fulfill PartyA's closing request.
The only option to increase the PartyA's allocated balance is to close the PartyA's winning position with PartyB_2.
When the settleUpnl function is executed, it will verify the signature provided by Muon via the verifySettlement function below. Line 20 below shows that the encoded data in the signature includes the nonce of the PartyBs of the positions/quotes where the PnL is to be settled. In this case, PartyB_2's nonce will be included in the signature.
Assume that the time when PartyB_1 fetches the signature is T0. At T0, PartyB_2's nonce is 890. Thus, the signature is signed against encoded data where PartyB_2's nonce is 890.
At T1, PartyB_1 executes the settleUpnl function along with the signature. However, PartyB_2 could front-run the transaction and perform certain actions (e.g., open/close position, charge funding rate) that will increment its own nonce to 891. In this case, the nonce in PartyB_1's signature and PartyB_2's nonce will differ, and the settleUpnl transaction will revert. PartyB_2 could repeat this continuously DOS or block PartyB_1 attempts to settle UPNL and close PartyA's positions.
File: LibMuonSettlement.sol
12: function verifySettlement(SettlementSig memory settleSig, address partyA) internal view {
13: MuonStorage.Layout storage muonLayout = MuonStorage.layout();
14: // == SignatureCheck( ==
15: require(block.timestamp <= settleSig.timestamp + muonLayout.upnlValidTime, "LibMuon: Expired signature");
16: // == ) ==
17: bytes memory encodedData;
18: uint256[] memory nonces = new uint256[](settleSig.quotesSettlementsData.length);
19: for (uint8 i = 0; i < settleSig.quotesSettlementsData.length; i++) {
20: nonces[i] = AccountStorage.layout().partyBNonces[QuoteStorage.layout().quotes[settleSig.quotesSettlementsData[i].quoteId].partyB][partyA];
21: encodedData = abi.encodePacked(
22: encodedData, // Append the previously encoded data
23: settleSig.quotesSettlementsData[i].quoteId,
24: settleSig.quotesSettlementsData[i].currentPrice,
25: settleSig.quotesSettlementsData[i].partyBUpnlIndex
26: );
27: }
28: bytes32 hash = keccak256(
29: abi.encodePacked(
30: muonLayout.muonAppId,
31: settleSig.reqId,
32: address(this),
33: "verifySettlement",
34: nonces,
35: AccountStorage.layout().partyANonces[partyA],
..SNIP..
44: }Users will not be able to close their positions, which could lead to a loss to the users due to the inability to close their positions. In the above example, PartyA wants to close its losing position to stop its account from incurring further losses due to unfavorable market conditions. However, since PartyA is unable to close its losing position, PartyA continues to incur more losses and might eventually be liquidated. Furthermore, trading is a time-sensitive activity. Thus, any blockage or delay would eventually lead to a loss of assets.
No response
No response
Source: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest-judging/issues/40
The protocol has acknowledged this issue.
xiaoming90
The settleUpnl function does not include notPartyB modifier. As a result, unauthorized PartyB could settle PNL of other PartyBs and users in the system, causing disruption and breaking core protocol functionality. Unauthorized PartyB can prematurely settle PartyB's (victim) positions prematurely at times that are disadvantageous to PartyB, resulting in asset loss for them.
For example, in a highly volatile market, if PartyB’s positions temporarily incur a loss due to sudden market fluctuations, the Unauthorized PartyB could immediately settle these positions before the market has a chance to recover. This premature settlement forces PartyB to realize losses that might have otherwise been avoided if the positions had remained open.
settleUpnl function does not include notPartyB modifier.No response
No response
The settleUpnl function can only be accessed by PartyB, as per the comment below. However, the function is not guarded by the notPartyB modifier.
File: SettlementFacet.sol
16: /**
17: * @notice Allows Party B to settle the upnl of party A position for the specified quotes.
18: * @param settlementSig The data struct contains quoteIds and upnl of parties and market prices
19: * @param updatedPrices New prices to be set as openedPrice for the specified quotes.
20: * @param partyA Address of party A
21: */
22: function settleUpnl(
23: SettlementSig memory settlementSig,
24: uint256[] memory updatedPrices,
25: address partyA
26: ) external whenNotPartyBActionsPaused notLiquidatedPartyA(partyA) {
27: uint256[] memory newPartyBsAllocatedBalances = SettlementFacetImpl.settleUpnl(settlementSig, updatedPrices, partyA);
..SNIP..
35: }Instead, it depends on the quoteLayout.partyBOpenPositions[msg.sender][partyA].length > 0 at Line 31 below, which is not a reliable method to determine whether the caller is a valid PartyB. The reason is that it is possible that a PartyB (e.g., one that might be removed due to malicious activities) that has already been removed from the system still has residual open positions. In this case, the position's length check will pass, and the unauthorized PartyB could continue to settle the PNL of other PartyBs and users in the system.
File: LibSettlement.sol
15: function settleUpnl(
..SNIP..
30: require(
31: isForceClose || quoteLayout.partyBOpenPositions[msg.sender][partyA].length > 0,
32: "LibSettlement: Sender should have a position with partyA"
33: );Unauthorized PartyB could settle PNL of other PartyBs and users in the system, causing disruption and breaking core protocol functionality. Unauthorized PartyB can prematurely settle PartyB's (victim) positions prematurely at times that are disadvantageous to PartyB, resulting in asset loss for them.
For example, in a highly volatile market, if PartyB’s positions temporarily incur a loss due to sudden market fluctuations, the Unauthorized PartyB could immediately settle these positions before the market has a chance to recover. This premature settlement forces PartyB to realize losses that might have otherwise been avoided if the positions had remained open.
No response
Include the notPartyB modifier to the settleUpnl function.
MoonKnightDev
the check in the libSettlement.sol file prevents the scenario mentioned: https://github.com/SYMM-IO/protocol-core/blob/eac73bf1d97df96bcd5b19bcc972792ef96c70e1/contracts/libraries/LibSettlement.sol#L30
settleUpnl functionSource: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest-judging/issues/42
xiaoming90
PartyB can abuse the newly implemented settleUpnl function to increment the nonce to block PartyA's force close action.
SettlementSig signature with quotesSettlementsData and/or partyBs array being empty can be passed into settleUpnl functionNo response
No response
This report is similar to the report from the previous contest, but with a different attack vector.
Assume that PartyA requests to close a quote via requestToClosePosition function. If PartyB does not respond within the cooldown period, PartyA can call the PartyAFacetImpl.forceClosePosition to close the quote forcefully.
Within the forceClosePosition function:
verifyHighLowPrice function will verify that the signature is valid. The function will use the nonce of PartyA and PartyB to generate the hash. verifySettlement function verifies that the signature is valid. Similarly, it also relies on the nonce of PartyA and PartyB to generate the hash. File: ForceActionsFacetImpl.sol
51: function forceClosePosition(
52: uint256 quoteId,
53: HighLowPriceSig memory sig,
54: SettlementSig memory settlementSig,
55: uint256[] memory updatedPrices
56: ) internal returns (uint256 closePrice, bool isPartyBLiquidated, int256 upnlPartyB, uint256 partyBAllocatedBalance) {
..SNIP..
86: LibMuonForceActions.verifyHighLowPrice(sig, quote.partyB, quote.partyA, quote.symbolId);
87: if (updatedPrices.length > 0) {
88: LibMuonSettlement.verifySettlement(settlementSig, quote.partyA);
89: }Attack Path 1 - Incrementing PartyA's nonce
The following describes the attack path:
forceClosePosition transaction, and submits to the mempool to close the quote forcefully as PartyB did not respond to PartyA's close request for an extended period.settleUpnl function to increment its nonce to 201.There are several ways that PartyB can increment the PartyA's nonce via the settleUpnl function with no cost or minimum cost:
SettlementSig signature with quotesSettlementsData and/or partyBs array being empty. In this case, the code at Line 34 will increment PartyA's nonce, but the rest of the code that involves the for-loop will be bypassed since the array (settleSig.quotesSettlementsData.length = 0) is empty File: LibSettlement.sol
15: function settleUpnl(
16: SettlementSig memory settleSig,
..SNIP..
34: accountLayout.partyANonces[partyA] += 1;
..SNIP..
40: for (uint8 i = 0; i < settleSig.quotesSettlementsData.length; i++) {Attack Path 2 - Incrementing PartyB's nonce
The following describes the attack path:
forceClosePosition transaction, and submits to the mempool to close the quote forcefully as PartyB did not respond to PartyA's close request for an extended period.settleUpnl function to increment its nonce to 801.There are several ways that PartyB can increment its own nonce via the settleUpnl function with no cost or minimum cost:
This impact is the same as the impact of a report in the previous Symm IO contest. Thus, the risk rating should be aligned to Medium.
PartyB can take advantage of it against PartyA, making themselves always profitable. For instance:
forceClosePosition and call settleUpnl to increase PartyA's nonces. In this way, PartyA's forceClosePosition will inevitably revert because the nonces are incorrect.In addition, when PartyA cannot close its position promptly, it exposes PartyA to unnecessary market risks and potential losses and gives PartyB an unfair advantage by giving PartyB an opportunity to turn the table (e.g., loss -> profit).
No response
Consider implementing the following measures to mitigate the root causes:
SettlementSig signature with quotesSettlementsData and/or partyBs array being empty are rejected within settleUpnl functionSource: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest-judging/issues/46
The protocol has acknowledged this issue.
xiaoming90
PartyB can settle PartyA’s losing positions, even when it is unnecessary and only further decreases PartyA's allocated balance. This unnecessary and premature settling causes PartyA to realize losses at unfavorable market conditions and prices, without giving them the opportunity to react or potentially recover from the temporary downturn. As a result, Party A is unfairly exposed to significant financial harm, having their positions closed at the worst possible moments, leading to a loss of assets for them.
No response
No response
Per Symm IO v0.8.3 documentation, the purpose of the newly implemented settleUpnl function is as follows:
allows hedgers to settle a portion of the user's unrealized PNL, effectively converting it into realized PNL. This enables the hedger to fill the user's close requests, even when the user lacks sufficient allocated balance.
This new feature is intended to solve the problem where a hedger cannot fill PartyA's close request because PartyA does not have enough allocated balance, as described in the documentation's example. As shown in the documentation's example, only PartyA's winning positions (those with positive PnL) need to be settled. Once these winning positions are settled, PartyA's allocated balance will increase, and there will be sufficient allocated balance in PartyA's account to execute PartyA's close request for its losing positions.
With that in mind, it does not make sense to allow hedgers to settle the losing positions of PartyA under any circumstance because:
However, it was observed that the settleUpnl allows hedgers to settle the losing positions of PartyA.
Assume that PartyA's LONG position with symbol=ETH, quote.openedPrice = 3000 USDC, quote.quantity = 1, and data.currentPrice = 2000 USC. This position is losing position for PartyA as the ETH's price dropped after the position was opened. However, none of the price validation checks at Lines 57-67 below prevents PartyB from settling a losing position of PartyA.
File: LibSettlement.sol
15: function settleUpnl(
..SNIP..
57: if (quote.openedPrice > data.currentPrice) {
58: require(
59: updatedPrices[i] < quote.openedPrice && updatedPrices[i] >= data.currentPrice,
60: "LibSettlement: Updated price is out of range"
61: );
62: } else {
63: require(
64: updatedPrices[i] > quote.openedPrice && updatedPrices[i] <= data.currentPrice,
65: "LibSettlement: Updated price is out of range"
66: );
67: }
68: if (quote.positionType == PositionType.LONG) {
69: settleAmounts[data.partyBUpnlIndex] +=
70: ((int256(updatedPrices[i]) - int256(quote.openedPrice)) * int256(LibQuote.quoteOpenAmount(quote))) /
71: 1e18;
72: } else {
73: settleAmounts[data.partyBUpnlIndex] +=
74: ((int256(quote.openedPrice) - int256(updatedPrices[i])) * int256(LibQuote.quoteOpenAmount(quote))) /
75: 1e18;
76: }
77: quote.openedPrice = updatedPrices[i];PartyB can settle PartyA’s losing positions, even when it is unnecessary and only further decreases PartyA's allocated balance. This unnecessary and premature settling causes PartyA to realize losses at unfavorable market conditions and prices, without giving them the opportunity to react or potentially recover from the temporary downturn. As a result, Party A is unfairly exposed to significant financial harm, having their positions closed at the worst possible moments, leading to a loss of assets for them.
No response
Update the settleUpnl function to ensure that only positions/quotes that increase the target account's allocated balance can be settled. In this report's example, PartyB should only be allowed to settle PartyA's winning positions, as only this settlement will increase PartyA's allocated balance.
MoonKnightDev
There is no issue with settling positions that are in a loss. Settling a position means that the unrealized P&L (uPnL) is converted into realized P&L (PnL) for the parties involved, without affecting the trading rules. On the other hand, Party Bs may also need to realize their profits
Source: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest-judging/issues/47
The protocol has acknowledged this issue.
xiaoming90
An excessive number of user positions can be settled even if the shortfall allocated balance needed for fulfilling the close request is only a small amount. As a result, victim's losses will be realized prematurely at unfavorable market conditions and prices, without giving them the opportunity to react or potentially recover from the temporary downturn. As a result, the victim is unfairly exposed to significant financial harm, having their positions closed at the worst possible moments, leading to a loss of assets for them. In this scenario, the victims are Symm Hedger (Party B1), Capital Hedger (Party B2), and Rasa Hedger (Party B3) in the report.
No response
No response
Assuming a similar scenario mentioned in the Symm IO v0.8.3 documentation.
Bob (Party A) has five (5) open positions and zero allocated balance:
Bob wants to close Position 5, which has an unrealized loss of \$250. However, since Bob has zero allocated balance, he cannot cover this loss upon closing.
PerpsHub Hedger (Party B4) cannot fill Bob's close request for Position 5 because Bob doesn't have enough allocated balance to cover the \$250 loss. Even though Bob has an overall positive UPNL (\$750 net profit), his profits are unrealized and cannot be used to settle the loss directly.
Thus, the solution provided by the newly implemented settleUpnl feature is to realized the \$200 profit for Position 4 and \$50 profit for Position 3. Settling these PnLs will increase Bob's allocated balance by \$250. PerpsHub can now successfully fill the close request for Position 5, as Bob has sufficient allocated balance (\$250) to cover the loss (\$250).
However, the issue is that within the settleUpnl function, there is no limit on how many positions the hedger/PartyB (PerpsHub) can settle the user's positions. It can effectively settle all of the user's positions (Position 1-5) even if the shortfall allocated balance needed for fulfilling the close request is only a small amount (\$250 shortfall), resulting in an excessive amount of PnL being settled for no good reason. In this case, the hedger can go ahead and settle the entire $1000 profit instead of only the \$250 that is required.
File: LibSettlement.sol
15: function settleUpnl(
16: SettlementSig memory settleSig,
17: uint256[] memory updatedPrices,
18: address partyA,
19: bool isForceClose
20: ) internal returns (uint256[] memory newPartyBsAllocatedBalances) {
..SNIP..
40: for (uint8 i = 0; i < settleSig.quotesSettlementsData.length; i++) {
41: QuoteSettlementData memory data = settleSig.quotesSettlementsData[i];
42: Quote storage quote = quoteLayout.quotes[data.quoteId];
43: require(quote.partyA == partyA, "LibSettlement: PartyA is invalid");
..SNIP..
68: if (quote.positionType == PositionType.LONG) {
69: settleAmounts[data.partyBUpnlIndex] +=
70: ((int256(updatedPrices[i]) - int256(quote.openedPrice)) * int256(LibQuote.quoteOpenAmount(quote))) /
71: 1e18;
72: } else {
73: settleAmounts[data.partyBUpnlIndex] +=
74: ((int256(quote.openedPrice) - int256(updatedPrices[i])) * int256(LibQuote.quoteOpenAmount(quote))) /
75: 1e18;
76: }
77: quote.openedPrice = updatedPrices[i];
78: }An excessive number of user positions can be settled even if the shortfall allocated balance needed for fulfilling the close request is only a small amount. As a result, victim's losses will be realized prematurely at unfavorable market conditions and prices, without giving them the opportunity to react or potentially recover from the temporary downturn. As a result, the victim is unfairly exposed to significant financial harm, having their positions closed at the worst possible moments, leading to a loss of assets for them. In this scenario, the victims are Symm Hedger (Party B1), Capital Hedger (Party B2), and Rasa Hedger (Party B3) in the report.
No response
Consider limiting the maximum amount of profits that the hedgers can settle to the shortfall. In this example, the shortfall is \$250. Thus, the hedgers should not be allowed to settle more than \$250 to prevent excessive settlement of PartyA's PnL.
This issue also applied to the other side, where Party A needed to settle Party B's PnL during force close. Thus, the relevant mitigation also needs to be applied for this.
MoonKnightDev
There is no issue with settling more positions than needed. The settling of a position means that the unrealized P&L (uPnL) becomes realized P&L (PnL) for the parties involved, and it does not affect the trading rules.
Source: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest-judging/issues/50
The protocol has acknowledged this issue.
xiaoming90
Emergency close might be blocked due to insufficient PartyA's allocated balance. During urgent situations where emergency mode is activated, the positions need to be promptly closed to avoid negative events that could potentially lead to serious loss of funds (e.g. the protocol is compromised, and the attacker is planning to or has started draining funds from the protocols). However, if the emergency closure of positions is blocked or delayed due to the above-mentioned issue, it might lead to unrecoverable losses.
No response
No response
Assuming a similar scenario mentioned in the Symm IO v0.8.3 documentation.
Bob (Party A) has three open positions and zero allocated balance:
123PartyB wants to emergency close Bob's Position 3 via the emergencyClosePosition function, which has an unrealized loss of $250. However, since Bob has zero allocated balance, the loss in Position 3 cannot cover this loss upon closing, and an attempt to emergency close will revert.
File: PartyBPositionActionsFacetImpl.sol
099: function emergencyClosePosition(uint256 quoteId, PairUpnlAndPriceSig memory upnlSig) internal {
100: AccountStorage.Layout storage accountLayout = AccountStorage.layout();
101: Quote storage quote = QuoteStorage.layout().quotes[quoteId];
102: Symbol memory symbol = SymbolStorage.layout().symbols[quote.symbolId];
103: require(
104: GlobalAppStorage.layout().emergencyMode || GlobalAppStorage.layout().partyBEmergencyStatus[quote.partyB] || !symbol.isValid,
105: "PartyBFacet: Operation not allowed. Either emergency mode must be active, party B must be in emergency status, or the symbol must be delisted"
106: );
107: require(quote.quoteStatus == QuoteStatus.OPENED || quote.quoteStatus == QuoteStatus.CLOSE_PENDING, "PartyBFacet: Invalid state");
108: LibMuonPartyB.verifyPairUpnlAndPrice(upnlSig, quote.partyB, quote.partyA, quote.symbolId);
109: uint256 filledAmount = LibQuote.quoteOpenAmount(quote);
110: quote.quantityToClose = filledAmount;
111: quote.requestedClosePrice = upnlSig.price;
112: require(
113: LibAccount.partyAAvailableBalanceForLiquidation(upnlSig.upnlPartyA, accountLayout.allocatedBalances[quote.partyA], quote.partyA) >= 0,
114: "PartyBFacet: PartyA is insolvent"
115: );
116: require(
117: LibAccount.partyBAvailableBalanceForLiquidation(upnlSig.upnlPartyB, quote.partyB, quote.partyA) >= 0,
118: "PartyBFacet: PartyB should be solvent"
119: );
120: accountLayout.partyBNonces[quote.partyB][quote.partyA] += 1;
121: accountLayout.partyANonces[quote.partyA] += 1;
122: LibQuote.closeQuote(quote, filledAmount, upnlSig.price);
123: }During urgent situations where emergency mode is activated, the positions need to be promptly closed to avoid negative events that could potentially lead to serious loss of funds (e.g. the protocol is compromised, and the attacker is planning to or has started draining funds from the protocols). However, if the emergency closure of positions is blocked or delayed due to the above-mentioned issue, it might lead to unrecoverable losses.
No response
Consider implementing the settleUpnl function within the emergencyClosePosition function so that PartyB can proceed to settle PartyA's open positions to ensure that PartyA's allocated balance will have sufficient balance to allow the position to be emergency closed.
MoonKnightDev
The emergency mode is designed for situations when a symbol is delisted or when a party B wants to exit. They can still use the settle unrealized profit and loss (uPnL) function separately to add to the user's allocated balance.
Source: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest-judging/issues/52
The protocol has acknowledged this issue.
xiaoming90
Reserve vault results in unfairness in the liquidation process. PartyA using force close mechanism to liquidate a position will receive more liquidation fees compared to normal liquidators as they have access to the funds in the reserve vault.
This creates unfairness to the protocol's liquidation process and to the existing liquidators, as they will receive lesser liquidation fees than Alice (PartyA). Without a fair and effective liquidation process, the protocol's solvent will be at risk, as bad positions will not be liquidated in a timely manner, and some liquidators might not want to participate in the process due to unfairness in the system.
No response
No response
Assume that PartyB's reserve vault has 1000 USD. During force closing, if PartyB's account is underwater, even with the additional support from its reserve vault, PartyB's account will be liquidated, as per Lines 127-138 below.
In Lines 128 and 129, the protocol transfers all the existing funds (1000 USD) from PartyB's reserve vault to PartyB's allocated balance. Thus, before the liquidation process is executed at Line 137 below, the PartyB's allocated balance will increase by 1000 USD.
File: ForceActionsFacetImpl.sol
051: function forceClosePosition(
..SNIP..
112: require(partyAAvailableBalance >= 0, "PartyAFacet: PartyA will be insolvent");
113: if (partyBAvailableBalance >= 0) {
..SNIP..
118: } else if (partyBAvailableBalance + int256(reserveAmount) >= 0) {
..SNIP..
127: } else { // @audit-info Code block for liquidating position
128: accountLayout.reserveVault[quote.partyB] = 0;
129: accountLayout.partyBAllocatedBalances[quote.partyB][quote.partyA] += reserveAmount;
130: emit SharedEvents.BalanceChangePartyB(quote.partyB, quote.partyA, reserveAmount, SharedEvents.BalanceChangeType.REALIZED_PNL_IN);
131: int256 diff = (int256(quote.quantityToClose) * (int256(closePrice) - int256(sig.currentPrice))) / 1e18;
132: if (quote.positionType == PositionType.LONG) {
133: diff = diff * -1;
134: }
135: isPartyBLiquidated = true;
136: upnlPartyB = sig.upnlPartyB + diff;
137: LibLiquidation.liquidatePartyB(quote.partyB, quote.partyA, upnlPartyB, block.timestamp);
138: }As a result, when the LibLiquidation.liquidatePartyB is executed at Line 137 above, the computed availableBalance will be 1000 USD smaller when the liquidation process is triggered via the forceClosePosition function compared to the standard liquidation process that is triggered via LiquidationFacet.liquidatePartyB by the liquidators, as shown below. This can be proven via the formula within partyBAvailableBalanceForLiquidation function.
**Normal scenario without access to reserve vault's funds**
availableBalance = partyBAllocatedBalances - lockedValue + PnL
availableBalance = 1000 - 100 - 2000 = -1100
**Force close with access the reserve vault's funds => partyBAllocatedBalances will 1000 USDC higher (1000 => 2000)**
availableBalance = partyBAllocatedBalances - lockedValue + PnL
availableBalance = 2000 - 100 - 2000 = -100Assume Alice is entitled to execute the force close function against PartyB, while Bob is a normal liquidator that can only liquidate PartyB via the standard LiquidationFacet.liquidatePartyB function.
When Alice triggers the liquidation process, the availableBalance will be -100 instead of -1000. Thus, when computing the liquidation fee that the liquidator is entitled to at Line 39 below, the computed liquidation fee (remainingLf) will be higher. This means that if Alice triggered the liquidation process, she would receive more liquidation fees compared to Bob because she has access to funds within PartyB's reserve vault. In this case, Alice is the liquidator and will receive the liquidation fee.
As a result, this creates unfairness within the system and to the existing liquidators as they (e.g., Bob) will receive lesser liquidation fees compared to Alice.
File: LibLiquidation.sol
23: function liquidatePartyB(address partyB, address partyA, int256 upnlPartyB, uint256 timestamp) internal {
24: AccountStorage.Layout storage accountLayout = AccountStorage.layout();
25: MAStorage.Layout storage maLayout = MAStorage.layout();
26: QuoteStorage.Layout storage quoteLayout = QuoteStorage.layout();
27:
28: // Calculate available balance for liquidation
29: int256 availableBalance = LibAccount.partyBAvailableBalanceForLiquidation(upnlPartyB, partyB, partyA);
30:
31: // Ensure Party B is insolvent
32: require(availableBalance < 0, "LiquidationFacet: partyB is solvent");
..SNIP..
37: // Determine liquidator share and remaining locked funds
38: if (uint256(-availableBalance) < accountLayout.partyBLockedBalances[partyB][partyA].lf) {
39: remainingLf = accountLayout.partyBLockedBalances[partyB][partyA].lf - uint256(-availableBalance);
40: liquidatorShare = (remainingLf * maLayout.liquidatorShare) / 1e18;
41:
42: maLayout.partyBPositionLiquidatorsShare[partyB][partyA] =
43: (remainingLf - liquidatorShare) /
44: quoteLayout.partyBPositionsCount[partyB][partyA];
45: } else {
46: maLayout.partyBPositionLiquidatorsShare[partyB][partyA] = 0;
47: }This creates unfairness to the protocol's liquidation process and to the existing liquidators, as they will receive lesser liquidation fees than Alice. Without a fair and effective liquidation process, the protocol's solvent will be at risk, as bad positions will not be liquidated in a timely manner, and some liquidators might not want to participate in the process due to unfairness in the system.
No response
The liquidation process for Alice and existing liquidators should be consistent to ensure fairness. In the above case, consider either of the following solutions:
forceClosePosition function; ORLiquidationFacet.liquidatePartyB for consistency.MoonKnightDev
Normal liquidators can also call the settleAndForceClose functions.