Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Use last zombie trove first in a redemption sequence #426

Merged
merged 2 commits into from
Sep 19, 2024
Merged

fix: Use last zombie trove first in a redemption sequence #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6397d7
fix: Use last zombie trove first in a redemption sequence
bingen Sep 16, 2024
4c91464
chore: the author's barely-disguised OCD
danielattilasimon Sep 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 14 additions & 14 deletions contracts/src/BorrowerOperations.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio
error BatchInterestRateChangePeriodNotPassed();
error TroveNotOpen();
error TroveNotActive();
error TroveNotUnredeemable();
error TroveNotZombie();
error TroveOpen();
error UpfrontFeeTooHigh();
error BelowCriticalThreshold();
Expand Down Expand Up @@ -470,7 +470,7 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio
_adjustTrove(troveManagerCached, _troveId, troveChange, _maxUpfrontFee);
}

function adjustUnredeemableTrove(
function adjustZombieTrove(
uint256 _troveId,
uint256 _collChange,
bool _isCollIncrease,
Expand All @@ -481,7 +481,7 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio
uint256 _maxUpfrontFee
) external override {
ITroveManager troveManagerCached = troveManager;
_requireTroveIsUnredeemable(troveManagerCached, _troveId);
_requireTroveIsZombie(troveManagerCached, _troveId);

TroveChange memory troveChange;
_initTroveChange(troveChange, _collChange, _isCollIncrease, _boldChange, _isDebtIncrease);
Expand Down Expand Up @@ -650,8 +650,8 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio
}
}

// Make sure the Trove doesn't end up unredeemable
// Now the max repayment is capped to stay above MIN_DEBT, so this only applies to adjustUnredeemableTrove
// Make sure the Trove doesn't end up zombie
// Now the max repayment is capped to stay above MIN_DEBT, so this only applies to adjustZombieTrove
_requireAtLeastMinDebt(vars.newDebt);

vars.newICR = LiquityMath._computeCR(vars.newColl, vars.newDebt, vars.price);
Expand Down Expand Up @@ -787,8 +787,8 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio
);
activePool.mintAggInterestAndAccountForTroveChange(change, batchManager);

// If the trove was unredeemable, and now it’s not anymore, put it back in the list
if (_checkTroveIsUnredeemable(troveManagerCached, _troveId) && trove.entireDebt >= MIN_DEBT) {
// If the trove was zombie, and now it’s not anymore, put it back in the list
if (_checkTroveIsZombie(troveManagerCached, _troveId) && trove.entireDebt >= MIN_DEBT) {
troveManagerCached.setTroveStatusToActive(_troveId);
_reInsertIntoSortedTroves(
_troveId, trove.annualInterestRate, _upperHint, _lowerHint, batchManager, batch.annualInterestRate
Expand Down Expand Up @@ -1304,7 +1304,7 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio

function _requireTroveIsOpen(ITroveManager _troveManager, uint256 _troveId) internal view {
ITroveManager.Status status = _troveManager.getTroveStatus(_troveId);
if (status != ITroveManager.Status.active && status != ITroveManager.Status.unredeemable) {
if (status != ITroveManager.Status.active && status != ITroveManager.Status.zombie) {
revert TroveNotOpen();
}
}
Expand All @@ -1316,20 +1316,20 @@ contract BorrowerOperations is LiquityBase, AddRemoveManagers, IBorrowerOperatio
}
}

function _requireTroveIsUnredeemable(ITroveManager _troveManager, uint256 _troveId) internal view {
if (!_checkTroveIsUnredeemable(_troveManager, _troveId)) {
revert TroveNotUnredeemable();
function _requireTroveIsZombie(ITroveManager _troveManager, uint256 _troveId) internal view {
if (!_checkTroveIsZombie(_troveManager, _troveId)) {
revert TroveNotZombie();
}
}

function _checkTroveIsUnredeemable(ITroveManager _troveManager, uint256 _troveId) internal view returns (bool) {
function _checkTroveIsZombie(ITroveManager _troveManager, uint256 _troveId) internal view returns (bool) {
ITroveManager.Status status = _troveManager.getTroveStatus(_troveId);
return status == ITroveManager.Status.unredeemable;
return status == ITroveManager.Status.zombie;
}

function _requireTroveIsNotOpen(ITroveManager _troveManager, uint256 _troveId) internal view {
ITroveManager.Status status = _troveManager.getTroveStatus(_troveId);
if (status == ITroveManager.Status.active || status == ITroveManager.Status.unredeemable) {
if (status == ITroveManager.Status.active || status == ITroveManager.Status.zombie) {
revert TroveOpen();
}
}
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/Interfaces/IBorrowerOperations.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ interface IBorrowerOperations is ILiquityBase, IAddRemoveManagers {
uint256 _maxUpfrontFee
) external;

function adjustUnredeemableTrove(
function adjustZombieTrove(
uint256 _troveId,
uint256 _collChange,
bool _isCollIncrease,
Expand Down
6 changes: 4 additions & 2 deletions contracts/src/Interfaces/ITroveManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface ITroveManager is ILiquityBase {
active,
closedByOwner,
closedByLiquidation,
unredeemable
zombie
}

function shutdownTime() external view returns (uint256);
Expand Down Expand Up @@ -53,6 +53,8 @@ interface ITroveManager is ILiquityBase {

function getCurrentICR(uint256 _troveId, uint256 _price) external view returns (uint256);

function lastZombieTroveId() external view returns (uint256);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"last" here refers to the last Trove that was redeemed to below MIN_DEBT right?


function batchLiquidateTroves(uint256[] calldata _troveArray) external;

function redeemCollateral(
Expand Down Expand Up @@ -88,7 +90,7 @@ interface ITroveManager is ILiquityBase {
uint256 _batchDebt
) external;

// Called from `adjustUnredeemableTrove()`
// Called from `adjustZombieTrove()`
function setTroveStatusToActive(uint256 _troveId) external;

function onAdjustTroveInterestRate(
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/NFTMetadata/MetadataNFT.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ contract MetadataNFT is IMetadataNFT {
if (status == ITroveManager.Status.active) return "Active";
if (status == ITroveManager.Status.closedByOwner) return "Closed";
if (status == ITroveManager.Status.closedByLiquidation) return "Liquidated";
if (status == ITroveManager.Status.unredeemable) return "Unredeemable";
if (status == ITroveManager.Status.zombie) return "Zombie";
return "";
}
}
55 changes: 45 additions & 10 deletions contracts/src/TroveManager.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {
// Array of all batch managers - used to fetch them off-chain
address[] public batchIds;

uint256 public lastZombieTroveId;

// Error trackers for the trove redistribution calculation
uint256 internal lastCollError_Redistribution;
uint256 internal lastBoldDebtError_Redistribution;
Expand Down Expand Up @@ -149,6 +151,7 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {
uint256 oldWeightedRecordedDebt;
uint256 newWeightedRecordedDebt;
uint256 newStake;
bool isZombieTrove;
LatestTroveData trove;
LatestBatchData batch;
}
Expand Down Expand Up @@ -452,7 +455,7 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {
}

function _isLiquidatableStatus(Status _status) internal pure returns (bool) {
return _status == Status.active || _status == Status.unredeemable;
return _status == Status.active || _status == Status.zombie;
}

function _batchLiquidateTroves(
Expand Down Expand Up @@ -663,14 +666,26 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {
bool isTroveInBatch = _singleRedemption.batchAddress != address(0);
uint256 newDebt = _applySingleRedemption(_defaultPool, _singleRedemption, isTroveInBatch);

// Make Trove unredeemable if it's tiny, in order to prevent griefing future (normal, sequential) redemptions
// Make Trove zombie if it's tiny (and it wasn’t already), in order to prevent griefing future (normal, sequential) redemptions
if (newDebt < MIN_DEBT) {
Troves[_singleRedemption.troveId].status = Status.unredeemable;
if (isTroveInBatch) {
sortedTroves.removeFromBatch(_singleRedemption.troveId);
} else {
sortedTroves.remove(_singleRedemption.troveId);
if (!_singleRedemption.isZombieTrove) {
Troves[_singleRedemption.troveId].status = Status.zombie;
if (isTroveInBatch) {
sortedTroves.removeFromBatch(_singleRedemption.troveId);
} else {
sortedTroves.remove(_singleRedemption.troveId);
}
// If it’s a partial redemption, let’s store a pointer to it so it’s used first in the next one
if (newDebt > 0) {
lastZombieTroveId = _singleRedemption.troveId;
}
} else if (newDebt == 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so newDebt == 0 implies this Trove was fully redeemed, therefore we unzombify it

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn’t say “unzombified”. It’s still zombie, but it doesn’t hold the pointer to be the first one in the next redemption sequence anymore.

// Reset last zombie trove pointer if the previous one was fully redeemed now
lastZombieTroveId = 0;
}
} else {
// Reset last zombie trove pointer if the previous one ended up above min debt
lastZombieTroveId = 0;
}
}

Expand Down Expand Up @@ -730,15 +745,27 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {
uint256 remainingBold = _boldamount;

SingleRedemptionValues memory singleRedemption;
singleRedemption.troveId = sortedTrovesCached.getLast();
// Let’s check if there’s a pending zombie trove from previous redemption
if (lastZombieTroveId != 0) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can never have a valid troveId == 0, right?

singleRedemption.troveId = lastZombieTroveId;
singleRedemption.isZombieTrove = true;
} else {
singleRedemption.troveId = sortedTrovesCached.getLast();
}
address lastBatchUpdatedInterest = address(0);

// Loop through the Troves starting from the one with lowest collateral ratio until _amount of Bold is exchanged for collateral
if (_maxIterations == 0) _maxIterations = type(uint256).max;
while (singleRedemption.troveId != 0 && remainingBold > 0 && _maxIterations > 0) {
_maxIterations--;
// Save the uint256 of the Trove preceding the current one
uint256 nextUserToCheck = sortedTrovesCached.getPrev(singleRedemption.troveId);
uint256 nextUserToCheck;
if (singleRedemption.isZombieTrove) {
nextUserToCheck = sortedTrovesCached.getLast();
} else {
nextUserToCheck = sortedTrovesCached.getPrev(singleRedemption.troveId);
}

// Skip if ICR < 100%, to make sure that redemptions always improve the CR of hit Troves
if (getCurrentICR(singleRedemption.troveId, _price) < _100pct) {
singleRedemption.troveId = nextUserToCheck;
Expand Down Expand Up @@ -769,6 +796,7 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {

remainingBold -= singleRedemption.boldLot;
singleRedemption.troveId = nextUserToCheck;
singleRedemption.isZombieTrove = false;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only need to do this if it was a zombieTrove. Though seems fine to just do it here at the end.

}

// We are removing this condition to prevent blocking redemptions
Expand Down Expand Up @@ -811,7 +839,7 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {
bool isTroveInBatch = _singleRedemption.batchAddress != address(0);
_applySingleRedemption(_defaultPool, _singleRedemption, isTroveInBatch);

// No need to make this Trove unredeemable if it has tiny debt, since:
// No need to make this Trove zombie if it has tiny debt, since:
// - This collateral branch has shut down and urgent redemptions are enabled
// - Urgent redemptions aren't sequential, so they can't be griefed by tiny Troves.
}
Expand Down Expand Up @@ -1284,6 +1312,9 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {
function setTroveStatusToActive(uint256 _troveId) external {
_requireCallerIsBorrowerOperations();
Troves[_troveId].status = Status.active;
if (lastZombieTroveId == _troveId) {
lastZombieTroveId = 0;
}
}

function onAdjustTroveInterestRate(
Expand Down Expand Up @@ -1436,6 +1467,8 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {
if (_batchAddress != address(0)) {
if (trove.status == Status.active) {
sortedTroves.removeFromBatch(_troveId);
} else if (trove.status == Status.zombie && lastZombieTroveId == _troveId) {
lastZombieTroveId = 0;
}

_removeTroveSharesFromBatch(
Expand All @@ -1450,6 +1483,8 @@ contract TroveManager is LiquityBase, ITroveManager, ITroveEvents {
} else {
if (trove.status == Status.active) {
sortedTroves.remove(_troveId);
} else if (trove.status == Status.zombie && lastZombieTroveId == _troveId) {
lastZombieTroveId = 0;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this logic twice? Could we do it once at the end?

}
}

Expand Down
4 changes: 2 additions & 2 deletions contracts/src/Zappers/GasCompZapper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ contract GasCompZapper is AddRemoveManagers {
_adjustTrovePost(_collChange, _isCollIncrease, _boldChange, _isDebtIncrease, receiver);
}

function adjustUnredeemableTroveWithRawETH(
function adjustZombieTroveWithRawETH(
uint256 _troveId,
uint256 _collChange,
bool _isCollIncrease,
Expand All @@ -166,7 +166,7 @@ contract GasCompZapper is AddRemoveManagers {
uint256 _maxUpfrontFee
) external {
address receiver = _adjustTrovePre(_troveId, _collChange, _isCollIncrease, _boldChange, _isDebtIncrease);
borrowerOperations.adjustUnredeemableTrove(
borrowerOperations.adjustZombieTrove(
_troveId, _collChange, _isCollIncrease, _boldChange, _isDebtIncrease, _upperHint, _lowerHint, _maxUpfrontFee
);
_adjustTrovePost(_collChange, _isCollIncrease, _boldChange, _isDebtIncrease, receiver);
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/Zappers/WETHZapper.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ contract WETHZapper is AddRemoveManagers {
_adjustTrovePost(_collChange, _isCollIncrease, _boldChange, _isDebtIncrease, receiver);
}

function adjustUnredeemableTroveWithRawETH(
function adjustZombieTroveWithRawETH(
uint256 _troveId,
uint256 _collChange,
bool _isCollIncrease,
Expand All @@ -154,7 +154,7 @@ contract WETHZapper is AddRemoveManagers {
uint256 _maxUpfrontFee
) external {
address payable receiver = _adjustTrovePre(_troveId, _collChange, _isCollIncrease, _boldChange, _isDebtIncrease);
borrowerOperations.adjustUnredeemableTrove(
borrowerOperations.adjustZombieTrove(
_troveId, _collChange, _isCollIncrease, _boldChange, _isDebtIncrease, _upperHint, _lowerHint, _maxUpfrontFee
);
_adjustTrovePost(_collChange, _isCollIncrease, _boldChange, _isDebtIncrease, receiver);
Expand Down
Loading