Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Liquidation process is inefficient as it should always have order of accounts to function

SBSecurity

Summary

Liquidation process is inefficient and malicious users can force it to use a lot more gas in order to process the liquidations, since it always relies on sequential order and does not allow passing certain trading account ids to be liquidated.

Vulnerability Details

Since liquidation keeper can’t process liquidation for given set of accounts it is forced to use sequential order and iterate over all the passed ids, potentially leading to higher gas consumptions. Imagine the situation that current trading account id is 100000, and more activity is expected from accounts with greater ids. Trader with account id = 1, constantly opens risky trades with small amount that become liquidateable in a short period of time. In this situation keeper will be forced to perform liquidation at least twice, for the account id = 1 and the rest, but if there are more accounts between 1 and 10000, due to the calculations performed in the checkLiquidatableAccounts keeper will run out of gas, as the checkUpkeep is allowed to consume up to 5 million. - https://docs.chain.link/chainlink-automation/overview/supported-networks#arbitrum-mainnet

Impact

Keeper will be forced to perform multiple liquidations for small number of accounts, since it does not allow passing set of accounts:

function checkUpkeep(bytes calldata checkData)
external
view
returns (bool upkeepNeeded, bytes memory performData)
{
(uint256 checkLowerBound, uint256 checkUpperBound, uint256 performLowerBound, uint256 performUpperBound) =
abi.decode(checkData, (uint256, uint256, uint256, uint256));
​
if (checkLowerBound >= checkUpperBound || performLowerBound >= performUpperBound) {
revert Errors.InvalidBounds();
}
​
IPerpsEngine perpsEngine = _getLiquidationKeeperStorage().perpsEngine;
uint128[] memory liquidatableAccountsIds =
perpsEngine.checkLiquidatableAccounts(checkLowerBound, checkUpperBound);
uint128[] memory accountsToBeLiquidated;
​
if (liquidatableAccountsIds.length == 0 || liquidatableAccountsIds.length <= performLowerBound) {
performData = abi.encode(accountsToBeLiquidated);
​
return (upkeepNeeded, performData);
}
​
uint256 boundsDelta = performUpperBound - performLowerBound;
uint256 performLength =
boundsDelta > liquidatableAccountsIds.length ? liquidatableAccountsIds.length : boundsDelta;
​
accountsToBeLiquidated = new uint128[](performLength);
​
for (uint256 i; i < performLength; i++) {
uint256 accountIdIndexAtLiquidatableAccounts = performLowerBound + i;
if (accountIdIndexAtLiquidatableAccounts >= liquidatableAccountsIds.length) {
break;
}
​
accountsToBeLiquidated[i] = liquidatableAccountsIds[accountIdIndexAtLiquidatableAccounts];
if (!upkeepNeeded && liquidatableAccountsIds[accountIdIndexAtLiquidatableAccounts] != 0) {
upkeepNeeded = true;
}
}
​
bytes memory extraData = abi.encode(accountsToBeLiquidated, address(this));
​
return (upkeepNeeded, extraData);
}

Tools Used

Manual Review

Recommendations

Allow checkUpkeep to receive set of account ids.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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