roguereddwarf - Governance should always be able to make liquidations profitable

[sherlock-admin2]

roguereddwarf

medium

Governance should always be able to make liquidations profitable

Summary

In cases where liquidation doesn't include a swap callback, i.e. shouldSwap=false the only incentive for liquidators is the ante amount that is deposited by the borrowers inside the Borrrower contract. Each pool is created with the DEFAULT_ANTE value, and the governance has the ability to step in and increase the ante up to its maximum value of CONSTRAINT_ANTE_MAX (0.1 ether). Based on the previous gas prices even the CONSTRAINT_ANTE_MAX value is not enough to make liquidations profitable. This introduces the risk of bad debt in the system since Borrowers wouldn't get liquidated because liquidators would have to pay instead of getting paid to liquidate.

Vulnerability Detail

When liquidation doesn't include a swap callback, the liquidator gets the ante amount deposited by the borrower in the Borrower contract. Borrowers can open up to 3 Uniswap positions per Borrowing contract.

Majority of the gas used during execution of liquidate function is due to:

• Reading prices from UniswapV3 pool
• Withdrawing liquidity from each position
• Repay the debt, i.e. transferring tokens.

The gas usage of the liquidate function can go up to 300k gas or more. If we do the math and consider a scenario of a highly congested network with gas prices of 400 gwei, the total gas cost of the liquidator is 300k * 400 gwei which equals 0.12 ether. This is higher than the CONSTRAINT_ANTE_MAX which is 0.1 ether and would result in liquidators paying instead of getting paid to liquidate. The consequence of this is extended periods of liquidations not happening and risk of bad debt accumulating in the system.

Impact

Periods of high volatility are usually accompanied by high gas prices and congested network. This is also the time when liquidations are mostly likely to occur.

Due to CONSTRAINT_ANTE_MAX not being enough to cover the gas costs of liquidators in such an environment there is a high risk of bad debt accumulating in the system.

Code Snippet

https://github.com/aloelabs/aloe-ii/blob/c71e7b0cfdec830b1f054486dfe9d58ce407c7a4/core/src/libraries/constants/Constants.sol#L77

Tool used

Manual Review

Recommendation

Increase the CONSTRAINT_ANTE_MAX to a value that is enough to cover the gas costs of liquidators during periods of extremely high gas prices.

CONSTRAINT_ANTE_MAX = 0.5 ether should be sufficient to cover worst case scenarios, and ensure that liquidations are profitable at some point during the 24-hour period, since IV calculation is based upon this time window.

Duplicate of #34

[sherlock-admin2]

2 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid, because this is the value intended to be used by the protocol team, the main goal is for the governor to not be able to set the param outside of limits. As for the value being too low, having gas price 400+ for 24 hours is highly unlikely, even when the gas spikes, it does come back to more reasonable levels time to time.

MohammedRizwan commented:

seems intended design

[panprog]

Escalate

I don't think this is dup of #34, because #34 is mainly about LIQUIDATION_INCENTIVE (the other settings are just additional consideration) with a different impact, different scenario and different fix.

This issue talks about insufficient liquidation incentive, which I think is more info/low, because 0.1 ether was chosen by developers and it's a balancing act - setting this to 0.5 ether per recommendation will make it much more costly to borrow and might turn users away. So I'd say that 0.1 ether is developers choice, and is not market-dependant nor volatility-dependant (even though gas might increase during volatility, but you can't base ante on volatility anyway, because it's deposited once and should work in all market circumstances). Maybe it's network-dependant (doesn't make sense to set 0.1 ether in L2 networks as it's too much), so maybe that should be immutable constructor setting.

Either way, not a dup of #34 even if it also talks about altering some settings constants.

[sherlock-admin2]

Escalate

I don't think this is dup of #34, because #34 is mainly about LIQUIDATION_INCENTIVE (the other settings are just additional consideration) with a different impact, different scenario and different fix.

This issue talks about insufficient liquidation incentive, which I think is more info/low, because 0.1 ether was chosen by developers and it's a balancing act - setting this to 0.5 ether per recommendation will make it much more costly to borrow and might turn users away. So I'd say that 0.1 ether is developers choice, and is not market-dependant nor volatility-dependant (even though gas might increase during volatility, but you can't base ante on volatility anyway, because it's deposited once and should work in all market circumstances). Maybe it's network-dependant (doesn't make sense to set 0.1 ether in L2 networks as it's too much), so maybe that should be immutable constructor setting.

Either way, not a dup of #34 even if it also talks about altering some settings constants.

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

[roguereddwarf]

@panprog I agree that these are not perfect dupes.

Hayden commented below your report #34: https://github.com/sherlock-audit/2023-10-aloe-judging/issues/34#issuecomment-1797916528

So according to Hayden both #34 and #83 ARE valid.

Both reports deal with how due to market conditions, liquidations may not always be profitable with given system parameters. I would be ok with having them as dupes or separating them into their own unique issues. But I don't think this should be info or low since your argument is wrong: because 0.1 ether was chosen by developers and it's a balancing act. The sponsor said this is an issue.

[roguereddwarf]

@panprog

I want to also address this specific objection: setting this to 0.5 ether per recommendation will make it much more costly to borrow and might turn users away

This is just the maximum value that the governance can set the ante to. If it's not needed that high, it can be lower.

[Trumpero]

Planning to reject the escalation and maintain this issue as a dup of #34, based the comments provided by @roguereddwarf above.

[panprog]

I've thought about this, and I believe this is valid and possibly connected to #34 in some way, but solution should be different - it's really hard to come up with any fixed ether value for liquidators, because it can be either too high or too low based on network congestion, but this value is fixed for the market and can't be changed when it's active as the users who have borrowed earlier will already have previous amount of ether. The best way to fix this one is probably to make liquidation fee depend on volatility and current gas prices, and pay it off user balance, not ether (because it's impossible to determine fair ETH amount for liquidators at the time user borrows). This will make it possible to make it profitable for liquidator in all circumstances, while ensuring that it's not overly profitable in all market conditions (both calm and highly volatile, congested or not). Such solution will combine this one and #34 and in this sense they probably can be considered dups.

[cvetanovv]

Planning to reject the escalation and maintain this issue as a dup of #34, based the comments provided by @roguereddwarf above.

I agree it should stay duplicated with #34

[Czar102]

I think both this and #34 are invalid because they comment on the design choices. The protocol makes some assumptions about the markets, gas prices, etc. and these considerations are out of scope.

[roguereddwarf]

I understand your "design choice" argument. But I don't think this really applies here and in #34.

The design choice is to implement parameters that should allow profitable liquidation in all real world market conditions.

The issue is not the design decision as such but the values of those parameters. They fail to implement the design choice, i.e, liquidations are not always profitable and cannot always be made profitable by governance due to restrictions on those parameters.

[roguereddwarf]

The argument that both reports do not describe intended design is supported by Hayden's comment that I referenced here: https://github.com/sherlock-audit/2023-10-aloe-judging/issues/83#issuecomment-1801362572

[Czar102]

In this issue, no matter how high you set the upper limit, there will always be a gas price for which the liquidation cost will be too large. And the protocol intended to introduce some bounds so that the governance doesn't have to be fully trusted.

[roguereddwarf]

Sure, for every upper bound there is a gas price that makes liquidations unprofitable. This submission shows however that the upper bound is too low based on historic data.

[Czar102]

Historic data can contradict those weak assumptions and it's a good informational issue. But it doesn't qualify for valid issues on Sherlock as Watsons are to implicitly assume that protocol parameters are sufficient with respect to external conditions.

[roguereddwarf]

But it doesn't qualify for valid issues on Sherlock as Watsons are to implicitly assume that protocol parameters are sufficient with respect to external conditions. I understand, thanks for the explanation.

[Czar102]

Result: Low Duplicate of #34

[sherlock-admin2]

Escalations have been resolved successfully!

Escalation status:

• panprog: accepted

[haydenshively]

Fixed in https://github.com/aloelabs/aloe-ii/pull/211

[roguereddwarf]

Mitigation Review:

While this issue is not considered a valid High / Medium as per Sherlock's rules, the protocol team still decided to fix it.

The CONSTRAINT_ANTE_MAX constant has been set to 0.5 ETH which, based on the reasoning in this report, should be sufficient to make liquidations profitable even in extreme situations.