Petite Spruce Mammoth - Gas inefficiency in `settleAndForceClosePosition` results in higher transaction costs and reduced throughput for users interacting with the contract in `ForceActionsFacet.sol`

[sherlock-admin3]

Petite Spruce Mammoth

Low/Info

Gas inefficiency in settleAndForceClosePosition results in higher transaction costs and reduced throughput for users interacting with the contract in ForceActionsFacet.sol

Summary

Gas inefficiency in settleAndForceClosePosition in ForceActionsFacet.sol results in higher transaction costs and reduced throughput for users interacting with the contract.

Root Cause

In the settleAndForceClosePosition function from the ForceActionsFacet contract, there are potential gas inefficiencies caused by excessive memory allocations, redundant operations, and unnecessary state writes. Gas inefficiencies can result in higher transaction costs and reduced throughput for users interacting with the contract.

Below are some inefficiencies observed in the function:

Code segment 1: redundant events emissions In the settleAndForceClosePosition function, the same ForceClosePosition event is emitted twice for backward compatibility: https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/ForceActions/ForceActionsFacet.sol#L96-L97 Issue: Emitting the same event twice increases gas costs unnecessarily. Although backward compatibility is needed, emitting the event twice is inefficient and wasteful. Optimization: Instead of emitting two identical events, a conditional flag or versioning mechanism can be implemented to emit the appropriate event based on the caller's contract version.

Code segment 2: inefficient memory allocation for newPartyBsAllocatedBalances https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/ForceActions/ForceActionsFacet.sol#L84-L85 Issue: The array newPartyBsAllocatedBalances is allocated in memory with a size of 1, but only stores a single value. This results in unnecessary memory allocation and gas consumption. Optimization: Instead of allocating an array in memory for a single value, pass the value directly or avoid using an array if only one value is required.

Code segment 3: redundant updatedPrices processing https://github.com/sherlock-audit/2024-09-symmio-v0-8-4-update-contest/blob/main/protocol-core/contracts/facets/ForceActions/ForceActionsFacet.sol#L89-L95 Issue: If the updatedPrices array is large, this will increase memory and gas usage significantly, especially when processing large amounts of data. Additionally, this array may not be necessary for every call. Optimization: Ensure that the updatedPrices array is only passed and processed when absolutely necessary. If it's not needed for certain types of settlements, its inclusion should be avoided.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

1. The inefficiency will result in higher gas costs for every user calling the settleAndForceClosePosition function. Given that this function may be called frequently in a live system, the cumulative gas waste can be substantial.
2. Inefficient gas usage contributes to blockchain network congestion, as more gas is consumed to process each transaction. This can slow down the network for other users as well.
3. Higher transaction costs can disincentivize users from interacting with the contract.

PoC

No response

Mitigation

1. Remove the second ForceClosePosition event emission for backward compatibility. Alternatively, emit only one event depending on a version flag to distinguish between backward and forward-compatible contracts.

   // Use a conditional flag for backward compatibility
   if (backwardCompatibility) {
   emit ForceClosePosition(quoteId, quote.partyA, quote.partyB, filledAmount, closePrice, quote.quoteStatus);
   } else {
   emit ForceClosePosition(quoteId, quote.partyA, quote.partyB, filledAmount, closePrice);
   }

2. Directly pass values instead of using memory allocations for single values.

   uint256 partyBAllocatedBalance = 5000;  // Simplified balance
   emit SettleUpnl(settleSig, updatedPrices, msg.sender, AccountStorage.layout().allocatedBalances[msg.sender], [partyBAllocatedBalance]);

3. Ensure that arrays such as updatedPrices are only passed and processed when necessary, reducing unnecessary memory allocation and gas consumption.