Part 2
Zaros
PerpetualsDEXFoundrySolidity
70,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

FeeConversionKeeper::performUpkeep May Exceed Gas Limit Due to Vault Updates

i_atiq

Summary

Chainlink Keepers have a gas limit of 5 million per upkeep [Doc]. FeeConversionKeeper::performUpkeep calls convertAccumulatedFeesToWeth, which calls _handleWethRewardDistribution. At the end of _handleWethRewardDistribution, it calls Vault.recalculateVaultsCreditCapacity. If too many vaults are connected to a market, _handleWethRewardDistribution updates all their states. This can cause an out-of-gas error, leading to failed upkeep and unprocessed rewards.

Vulnerability Details

Vault.recalculateVaultsCreditCapacity(market.getConnectedVaultsIds()) updates all connected vaults. If the number of vaults is large, the update cost exceeds the 5M gas limit. In worst case like when _performMultiDexSwap is triggered for asset conversion, the gas limit will be hit. If the transaction fails, the fees remain unconverted, causing liquidity inefficiencies. If a market has too many vault connected to it, chainlink keeper can never update the reward distribution. As a result stakers and protocol will lose fees and rewards

Poc

Paste the following code in test/integration/external/chainlink/keepers/fee-conversion/performUpkeep/performUpkeep.t.sol and run: forge test --mt testFuzz_GivenCallPerformUpkeepFunctionGasReport -vv

function testFuzz_GivenCallPerformUpkeepFunctionGasReport(
/* uint256 marketId,
uint256 amount,
uint256 minFeeDistributionValueUsd */
)
external
givenInitializeContract
{
uint256 marketId = 1;
uint256 amount = 100e18;
uint256 minFeeDistributionValueUsd = 1e18;
PerpMarketCreditConfig memory fuzzPerpMarketCreditConfig = getFuzzPerpMarketCreditConfig(marketId);
​
minFeeDistributionValueUsd = bound({
x: minFeeDistributionValueUsd,
min: 1,
max: convertUd60x18ToTokenAmount(address(usdc), USDC_DEPOSIT_CAP_X18)
});
​
amount = bound({
x: amount,
min: minFeeDistributionValueUsd,
max: convertUd60x18ToTokenAmount(address(usdc), USDC_DEPOSIT_CAP_X18)
});
deal({ token: address(usdc), to: address(fuzzPerpMarketCreditConfig.engine), give: amount });
​
changePrank({ msgSender: users.owner.account });
​
configureFeeConversionKeeper(1, uint128(minFeeDistributionValueUsd));
​
FeeConversionKeeper(feeConversionKeeper).setForwarder(users.keepersForwarder.account);
​
changePrank({ msgSender: address(fuzzPerpMarketCreditConfig.engine) });
​
marketMakingEngine.receiveMarketFee(fuzzPerpMarketCreditConfig.marketId, address(usdc), amount);
​
changePrank({ msgSender: users.keepersForwarder.account });
​
uint128[] memory marketIds = new uint128[](1);
address[] memory assets = new address[](1);
​
marketIds[0] = fuzzPerpMarketCreditConfig.marketId;
assets[0] = address(usdc);
​
bytes memory performData = abi.encode(marketIds, assets);
​
uint256 expectedTokenAmount = uniswapV3Adapter.getExpectedOutput(address(usdc), address(wEth), amount);
uint256 expectedAmount = uniswapV3Adapter.calculateAmountOutMin(expectedTokenAmount);
​
uint256 gasBefore = gasleft();
​
// it should emit {LogConvertAccumulatedFeesToWeth} event
vm.expectEmit();
emit FeeDistributionBranch.LogConvertAccumulatedFeesToWeth(expectedAmount);
​
FeeConversionKeeper(feeConversionKeeper).performUpkeep(performData);
uint256 gasAfter = gasleft();
​
console.log("--------------------------------------------------------");
console.log("Gas cost: ", gasBefore - gasAfter);
console.log("--------------------------------------------------------");
}

The gas cost for this is 3.6 million. If connected vault increase, keeper won't be able to call performUpkeep successfully. Or if the recalculateVaultsCreditCapacity has to update all the states in each vault due to changes in all markets, it will hit the gas limit and fail

Impact

  • Rewards are not distributed if performUpkeep git gas limit

  • Markets with too many vaults cannot efficiently update rewards

  • Users and protocols lose expected earnings due to gas constraints.

Recommendation

Create seperate offchain keeper which will call convertAccumulatedFeesToWeth periodcally. it can spend as much as gas needed, solving the problem when too much gas is needed for upkeep

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

FeeConversionKeeper::performUpkeep May Exceed Gas Limit Due to Vault Updates

Appeal created

bigsam Auditor
10 months ago
i_atiq Submitter
10 months ago
inallhonesty Lead Judge
10 months ago
inallhonesty Lead Judge 9 months ago
Submission Judgement Published
Validated
Assigned finding tags:

FeeConversionKeeper::performUpkeep May Exceed Gas Limit Due to Vault Updates

Support

FAQs

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

Give us feedback!