Liquity v2 ~ Cantina Fixes Review

The last trove of a shut-down branch cannot be liquidated:

...
// If branch has not been shut down, or it's a liquidation,
// require at least 1 trove in the system
if (shutdownTime == 0 || closedStatus == Status.closedByLiquidation) {
	_requireMoreThanOneTroveInSystem(TroveIdsArrayLength);
}
...

This does not create any immediate issues, but restricting a debt-wiping operation during shutdown appears to offer no benefit to the system’s health, especially given that the primary goal during shutdown is to incentivize the rapid removal of the branch’s debt.

There are various TODO comments in the codebase. The team could consider removing them (deeming them as “won’t fix”), even if they are not implemented. The changes described do not affect the system’s functionality and/or serve as runtime assertions that should never be triggered during execution:

…
assert(_troveChange.debtIncrease > 0); // TODO: remove before deployment
…

…
assert(_newTroveDebt > 0); // TODO: remove before deployment
…

…
assert(_newTroveDebt > 0); // TODO: remove before deployment
…

…
//@Dedaub: the assertion is fine in this case, since there is no explicit check
//         in BorrowerOperations::adjustTroveInterestRate
assert(_newTroveDebt > 0); // TODO: remove before deployment
…

// TODO: remove this and set address in constructor, since we'll use CREATE2
function setAddresses(address _borrowOperationsAddress) external onlyOwner {
...

...
// TODO: pass it as param in functions, so we can reuse the same exchange for different branches
IERC20 public immutable collToken;
...

Both LeverageLSTZapper::openLeveragedTroveWithRawETH and LeverageWETHZapper::openLeveragedTroveWithRawETH could use _getTroveIndex(uint256) instead of _getTroveIndex(address _sender, uint256 _ownerIndex), since the prior uses msg.sender directly when calculating the troves owner index

function _getTroveIndex(address _sender, uint256 _ownerIndex) internal pure returns (uint256) {
	return uint256(keccak256(abi.encode(_sender, _ownerIndex)));
}

function _getTroveIndex(uint256 _ownerIndex) internal view returns (uint256) {
	return _getTroveIndex(msg.sender, _ownerIndex);
}

function openLeveragedTroveWithRawETH(OpenLeveragedTroveParams memory _params) external payable {
	require(msg.value == ETH_GAS_COMPENSATION, "LZ: Wrong ETH");
	require(
    	_params.batchManager == address(0) || _params.annualInterestRate == 0,
    	"LZ: Cannot choose interest if joining a batch"
	);

	...
	_params.ownerIndex = _getTroveIndex(msg.sender, _params.ownerIndex);
...

function openLeveragedTroveWithRawETH(OpenLeveragedTroveParams memory _params) external payable {
	require(msg.value == ETH_GAS_COMPENSATION, "LZ: Wrong ETH");
	require(
    	_params.batchManager == address(0) || _params.annualInterestRate == 0,
    	"LZ: Cannot choose interest if joining a batch"
	);

	...
	_params.ownerIndex = _getTroveIndex(msg.sender, _params.ownerIndex);
...

Pull request #897 introduces the ability to permissionlessly remove troves from a batch which has debt:shares ratio more than 1e9.

This is meant to serve as a way to:

• Remove zombie troves that have received a large redistributed debt, but cannot redeem the debt while being inside the batch
• Allow the rest of the healthy troves in the batch to mint additional debt shares

...
if (_kick) {
	_requireTroveIsOpen(vars.troveManager, _troveId);
} else {
	_requireTroveIsActive(vars.troveManager, _troveId);
	_requireCallerIsBorrower(_troveId);
	_requireValidAnnualInterestRate(_newAnnualInterestRate);
}

vars.batchManager = _requireIsInBatch(_troveId);
vars.trove = vars.troveManager.getLatestTroveData(_troveId);
vars.batch = vars.troveManager.getLatestBatchData(vars.batchManager);

if (_kick) {
	if (vars.batch.totalDebtShares * MAX_BATCH_SHARES_RATIO >= vars.batch.entireDebtWithoutRedistribution) {
    	revert BatchSharesRatioTooLow();
	}
	_newAnnualInterestRate = vars.batch.annualInterestRate;
}
...

An issue could arise if someone is able to:

• Join an existing batch
• Inflate the debt:share ratio of the batch
• Kick the rest of the troves out of the batch

To do this the griefer can utilize a flashloan, but they would have to pay for fees applicable (e.g., upfront fee for joining the batch) out of pocket. The grieferer wouldn’t gain anything out of this, this is mostly aimed at griefing other users.

In principle, step 2 cannot be performed by borrowing, since the debt:share ratio is preserved for debt shares minted through borrowing. The ratio starts at 1 and increases as interest accrues and batch management fees are added. However, it is theoretically possible to cause the ratio to change through borrowing by exploiting the fact that the number of minted debt shares is rounded down:

...
batchDebtSharesDelta = currentBatchDebtShares * debtIncrease / _batchDebt;
...

The team was already aware of this, but we are interested in producing a risk analysis of this.

In principle, a trove created with $d$ debt in a batch with $d_{batch}$ debt and $s_{batch}$ shares should retain the branch's share ratio.

Algebraically, that's easy to observe since:

$\frac{d_{batch} + d'}{s_{batch} + d' \frac{s_{batch}}{d_{batch}}} = \frac{d_{batch}}{s_{batch}}$

Since the operation rounds down because of integer division, the actual amount of new shares is

$\lfloor d' \frac{d_{batch}}{s_{batch}} \rfloor = d' \frac{d_{batch}}{s_{batch}} - \epsilon$

with $\epsilon \leq 1$

When accounting for the rounding error that gets created it can be shown that:

$\frac{d_{batch} + d'}{s_{batch} + d' \frac{s_{batch}}{d_{batch}} - \epsilon} > \frac{d_{batch}}{s_{batch}}$

But ultimately we would like to use $d'$ in order to inflate the ratio by $\delta$:

$\frac{d_{batch} + d'}{s_{batch} + d' \frac{s_{batch}}{d_{batch}} - \epsilon} = \frac{d_{batch}}{s_{batch}} + \delta \iff$

$\frac{d_{batch} + d'}{s_{batch} + d' \frac{s_{batch}}{d_{batch}} - \epsilon} = \frac{d_{batch} + \delta\cdot s_{batch}}{s_{batch}} \iff$

$\frac{d_{batch} + d'}{s_{batch} + d' \frac{s_{batch}}{d_{batch}} - \epsilon} = \frac{d_{batch} + \delta\cdot s_{batch}}{s_{batch}} \iff$

$s_{batch}d_{batch} + d's_{batch} = d_{batch}s_{batch} + d's_{batch} - \epsilon \cdot d_{batch} + \delta \cdot s_{batch}^2 + \frac{d's_{batch}^2\delta}{d_{batch}} - \epsilon \cdot \delta \cdot s_{batch} \iff$

$0 = \delta \cdot s_{batch}^2 + \frac{d's_{batch}^2\delta}{d_{batch}} - \epsilon(\delta \cdot s_{batch} + d_{batch}) \iff$

$\frac{d's_{batch}^2\delta}{d_{batch}} = \epsilon(\delta \cdot s_{batch} + d_{batch}) - \delta \cdot s_{batch}^2 \iff$

$d's_{batch}^2\delta = [ \epsilon(\delta \cdot s_{batch} + d_{batch}) - \delta \cdot s_{batch}^2 ] d_{batch}\iff$

$d' = [ \epsilon(\delta \cdot s_{batch} + d_{batch}) - \delta \cdot s_{batch}^2 ] \frac{d_{batch}}{s_{batch}^2\delta}$

In a typical branch, since the debt in the debt:shares ratio does not account for redistributions, it is not practical to assume that $\delta * s_{batch}^2 \sim d_{batch}$ for $\delta \geq 1$. This means one cannot inflate the ratio by $\delta$ in a single adjustment—unless we are dealing with batches that contain a very small number of debt shares.

In order to pull off step 2 in the above-mentioned attack, the griefer would have to:

• choose a small enough $\delta$ (i.e., inflate the ratio by a small number)
• carefully choose to create debt $d'$ in order to maximize $\epsilon$
• perform the above two parts multiple times.

The orchestration required, together with the gas cost and the fact that the griefer does not benefit directly, makes this griefing scenario highly unlikely to occur in practice. If the team wishes to address this without changing the rounding direction in the debt shares expression, they could consider allowing only the batch manager or the trove owner to kick a trove out of a branch.

The code is compiled with Solidity 0.8.24. Version 0.8.24, at the time of writing, has no known bugs.