Skip to content

Commit

Permalink
Merge pull request #689 from Badger-Finance/feat/coll-and-debt-shares…
Browse files Browse the repository at this point in the history 
…-refactor

getCollAndDebtShares Cleanup
  • Loading branch information
@dapp-whisperer
dapp-whisperer authored Oct 13, 2023
2 parents a94f0f9 + d2a7452 commit 7adf7a4
Show file tree
Hide file tree
Showing 36 changed files with 268 additions and 248 deletions.
2 changes: 1 addition & 1 deletion README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -621,7 +621,7 @@ Hints allow cheaper CDP operations for the user, at the expense of a slightly lo
const EBTCRepayment = toBN(toWei('230')) // borrower wants to repay 230 eBTC
// Get CDP's current debt and coll
const {0: debt, 1: coll} = await cdpManager.getDebtAndCollShares(borrower)
const {0: debt, 1: coll} = await cdpManager.getSyncedDebtAndCollShares(borrower)
const newDebt = debt.sub(EBTCRepayment)
const newColl = coll.add(collIncrease)
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts/contracts/ActivePool.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ contract ActivePool is IActivePool, ERC3156FlashLender, ReentrancyGuard, BaseMat
uint256 internal systemCollShares; // deposited collateral tracker
uint256 internal systemDebt;
uint256 internal feeRecipientCollShares; // coll shares claimable by fee recipient
ICollateralToken public collateral;
ICollateralToken public immutable collateral;

// --- Contract setters ---

Expand Down
207 changes: 110 additions & 97 deletions packages/contracts/contracts/BorrowerOperations.sol
View file

Large diffs are not rendered by default.

19 changes: 9 additions & 10 deletions packages/contracts/contracts/CdpManager.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,12 @@ contract CdpManager is CdpManagerStorage, ICdpManager, Proxy {
// Repurposing this struct here to avoid stack too deep.
CdpDebtAndCollShares memory _oldDebtAndColl = CdpDebtAndCollShares(
Cdps[_redeemColFromCdp.cdpId].debt,
Cdps[_redeemColFromCdp.cdpId].coll,
0
Cdps[_redeemColFromCdp.cdpId].coll
);

// Decrease the debt and collateral of the current Cdp according to the EBTC lot and corresponding ETH to send
uint256 newDebt = _oldDebtAndColl.entireDebt - singleRedemption.debtToRedeem;
uint256 newColl = _oldDebtAndColl.entireColl - singleRedemption.collSharesDrawn;
uint256 newDebt = _oldDebtAndColl.debt - singleRedemption.debtToRedeem;
uint256 newColl = _oldDebtAndColl.collShares - singleRedemption.collSharesDrawn;

if (newDebt == 0) {
// No debt remains, close CDP
Expand All @@ -185,8 +184,8 @@ contract CdpManager is CdpManagerStorage, ICdpManager, Proxy {
_redeemColFromCdp.cdpId,
_borrower,
msg.sender,
_oldDebtAndColl.entireDebt,
_oldDebtAndColl.entireColl,
_oldDebtAndColl.debt,
_oldDebtAndColl.collShares,
0,
0,
0,
Expand All @@ -205,7 +204,7 @@ contract CdpManager is CdpManagerStorage, ICdpManager, Proxy {
*/
if (
newNICR != _redeemColFromCdp.partialRedemptionHintNICR ||
collateral.getPooledEthByShares(newColl) < MIN_NET_COLL
collateral.getPooledEthByShares(newColl) < MIN_NET_STETH_BALANCE
) {
singleRedemption.cancelledPartial = true;
return singleRedemption;
Expand All @@ -226,8 +225,8 @@ contract CdpManager is CdpManagerStorage, ICdpManager, Proxy {
_redeemColFromCdp.cdpId,
ISortedCdps(sortedCdps).getOwnerAddress(_redeemColFromCdp.cdpId),
msg.sender,
_oldDebtAndColl.entireDebt,
_oldDebtAndColl.entireColl,
_oldDebtAndColl.debt,
_oldDebtAndColl.collShares,
newDebt,
newColl,
Cdps[_redeemColFromCdp.cdpId].stake,
Expand Down Expand Up @@ -514,7 +513,7 @@ contract CdpManager is CdpManagerStorage, ICdpManager, Proxy {
}

function syncAccounting(bytes32 _cdpId) external override {
// _requireCallerIsBorrowerOperations(); /// @audit Please check this and let us know if opening this creates issues | TODO: See Stermi Partial Liq
// _requireCallerIsBorrowerOperations(); /// @audit Opening can cause invalid reordering of Cdps due to changing values without reInserting into sortedCdps
return _syncAccounting(_cdpId);
}

Expand Down
19 changes: 8 additions & 11 deletions packages/contracts/contracts/CdpManagerStorage.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,7 @@ contract CdpManagerStorage is EbtcBase, ReentrancyGuard, ICdpManagerData, AuthNo
/// @notice Return the nominal collateral ratio (ICR) of a given Cdp, without the price.
/// @dev Takes a cdp's pending coll and debt rewards from redistributions into account.
function getNominalICR(bytes32 _cdpId) external view returns (uint256) {
(uint256 currentEBTCDebt, uint256 currentCollShares, ) = getDebtAndCollShares(_cdpId);
(uint256 currentEBTCDebt, uint256 currentCollShares) = getSyncedDebtAndCollShares(_cdpId);

uint256 NICR = EbtcMath._computeNominalCR(currentCollShares, currentEBTCDebt);
return NICR;
Expand All @@ -668,7 +668,7 @@ contract CdpManagerStorage is EbtcBase, ReentrancyGuard, ICdpManagerData, AuthNo
// Return the current collateral ratio (ICR) of a given Cdp.
//Takes a cdp's pending coll and debt rewards from redistributions into account.
function getICR(bytes32 _cdpId, uint256 _price) public view returns (uint256) {
(uint256 currentEBTCDebt, uint256 currentCollShares, ) = getDebtAndCollShares(_cdpId);
(uint256 currentEBTCDebt, uint256 currentCollShares) = getSyncedDebtAndCollShares(_cdpId);
uint256 ICR = _calculateCR(currentCollShares, currentEBTCDebt, _price);
return ICR;
}
Expand Down Expand Up @@ -696,28 +696,25 @@ contract CdpManagerStorage is EbtcBase, ReentrancyGuard, ICdpManagerData, AuthNo
}

// Return the Cdps entire debt and coll struct
function _getDebtAndCollShares(
function _getSyncedDebtAndCollShares(
bytes32 _cdpId
) internal view returns (CdpDebtAndCollShares memory) {
(uint256 entireDebt, uint256 entireColl, uint256 pendingDebtReward) = getDebtAndCollShares(
_cdpId
);
return CdpDebtAndCollShares(entireDebt, entireColl, pendingDebtReward);
(uint256 entireDebt, uint256 entireColl) = getSyncedDebtAndCollShares(_cdpId);
return CdpDebtAndCollShares(entireDebt, entireColl);
}

// Return the Cdps entire debt and coll, including pending rewards from redistributions and collateral reduction from split fee.
/// @notice pending rewards are included in the debt and coll totals returned.
function getDebtAndCollShares(
function getSyncedDebtAndCollShares(
bytes32 _cdpId
) public view returns (uint256 debt, uint256 coll, uint256 pendingEBTCDebtReward) {
(uint256 _newColl, uint256 _newDebt, , uint256 _pendingDebt) = _calcSyncedAccounting(
) public view returns (uint256 debt, uint256 coll) {
(uint256 _newColl, uint256 _newDebt, , ) = _calcSyncedAccounting(
_cdpId,
cdpStEthFeePerUnitIndex[_cdpId],
systemStEthFeePerUnitIndex
);
coll = _newColl;
debt = _newDebt;
pendingEBTCDebtReward = _pendingDebt;
}

/// @dev calculate pending global state change to be applied:
Expand Down
6 changes: 3 additions & 3 deletions packages/contracts/contracts/Dependencies/EbtcBase.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ contract EbtcBase is BaseMath, IEbtcBase {
uint256 public constant LIQUIDATOR_REWARD = 2e17;

// Minimum amount of stETH collateral a CDP must have
uint256 public constant MIN_NET_COLL = 2e18;
uint256 public constant MIN_NET_STETH_BALANCE = 2e18;

uint256 public constant PERCENT_DIVISOR = 200; // dividing by 200 yields 0.5%

Expand All @@ -53,8 +53,8 @@ contract EbtcBase is BaseMath, IEbtcBase {

// --- Gas compensation functions ---

function _getNetColl(uint256 _coll) internal pure returns (uint256) {
return _coll - LIQUIDATOR_REWARD;
function _calcNetStEthBalance(uint256 _stEthBalance) internal pure returns (uint256) {
return _stEthBalance - LIQUIDATOR_REWARD;
}

/**
Expand Down
7 changes: 5 additions & 2 deletions packages/contracts/contracts/HintHelpers.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ contract HintHelpers is EbtcBase {

// If the partial redemption would leave the CDP with less than the minimum allowed coll, bail out of partial redemption and return only the fully redeemable
// TODO: This seems to return the original coll? why?
if (collateral.getPooledEthByShares(partialRedemptionNewColl) < MIN_NET_COLL) {
if (
collateral.getPooledEthByShares(partialRedemptionNewColl) <
MIN_NET_STETH_BALANCE
) {
partialRedemptionHintNICR = 0; //reset to 0 as there is no partial redemption in this case
partialRedemptionNewColl = 0;
vars.remainingEbtcToRedeem = _cachedEbtcToRedeem;
Expand Down Expand Up @@ -148,7 +151,7 @@ contract HintHelpers is EbtcBase {
_oldIndex
);
} else {
(, newCollShare, ) = cdpManager.getDebtAndCollShares(vars.currentCdpId);
(, newCollShare) = cdpManager.getSyncedDebtAndCollShares(vars.currentCdpId);
}

vars.remainingEbtcToRedeem = vars.remainingEbtcToRedeem - maxRedeemableEBTC;
Expand Down
9 changes: 4 additions & 5 deletions packages/contracts/contracts/Interfaces/ICdpManagerData.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,8 @@ interface ICdpManagerData is IRecoveryModeGracePeriod {
**/

struct CdpDebtAndCollShares {
uint256 entireDebt;
uint256 entireColl;
uint256 pendingDebtReward;
uint256 debt;
uint256 collShares;
}

struct LiquidationLocals {
Expand Down Expand Up @@ -255,9 +254,9 @@ interface ICdpManagerData is IRecoveryModeGracePeriod {

function hasPendingRedistributedDebt(bytes32 _cdpId) external view returns (bool);

function getDebtAndCollShares(
function getSyncedDebtAndCollShares(
bytes32 _cdpId
) external view returns (uint256 debt, uint256 coll, uint256 pendingEBTCDebtReward);
) external view returns (uint256 debt, uint256 collShares);

function canLiquidateRecoveryMode(uint256 icr, uint256 tcr) external view returns (bool);
}
19 changes: 10 additions & 9 deletions packages/contracts/contracts/LiquidationLibrary.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ contract LiquidationLibrary is CdpManagerStorage {
// without emmiting events
function _closeCdpByLiquidation(bytes32 _cdpId) private returns (uint256, uint256, uint256) {
// calculate entire debt to repay
(uint256 entireDebt, uint256 entireColl, ) = getDebtAndCollShares(_cdpId);
(uint256 entireDebt, uint256 entireColl) = getSyncedDebtAndCollShares(_cdpId);

// housekeeping after liquidation by closing the CDP
uint256 _liquidatorReward = Cdps[_cdpId].liquidatorRewardShares;
Expand All @@ -392,16 +392,16 @@ contract LiquidationLibrary is CdpManagerStorage {
uint256 _partialDebt = _partialState.partialAmount;

// calculate entire debt to repay
CdpDebtAndCollShares memory _debtAndColl = _getDebtAndCollShares(_cdpId);
_requirePartialLiqDebtSize(_partialDebt, _debtAndColl.entireDebt, _partialState.price);
uint256 newDebt = _debtAndColl.entireDebt - _partialDebt;
CdpDebtAndCollShares memory _debtAndColl = _getSyncedDebtAndCollShares(_cdpId);
_requirePartialLiqDebtSize(_partialDebt, _debtAndColl.debt, _partialState.price);
uint256 newDebt = _debtAndColl.debt - _partialDebt;

// credit to https://arxiv.org/pdf/2212.07306.pdf for details
(uint256 _partialColl, uint256 newColl, ) = _calculatePartialLiquidationSurplusAndCap(
_partialState.ICR,
_partialState.price,
_partialDebt,
_debtAndColl.entireColl
_debtAndColl.collShares
);

// early return: if new collateral is zero, we have a full liqudiation
Expand All @@ -420,8 +420,8 @@ contract LiquidationLibrary is CdpManagerStorage {
_reInsertPartialLiquidation(
_partialState,
EbtcMath._computeNominalCR(newColl, newDebt),
_debtAndColl.entireDebt,
_debtAndColl.entireColl
_debtAndColl.debt,
_debtAndColl.collShares
);
uint _debtToColl = (_partialDebt * DECIMAL_PRECISION) / _partialState.price;
uint _cappedColl = collateral.getPooledEthByShares(_partialColl);
Expand Down Expand Up @@ -891,14 +891,15 @@ contract LiquidationLibrary is CdpManagerStorage {
uint256 _price
) internal view {
require(
(_partialDebt + _convertDebtDenominationToBtc(MIN_NET_COLL, _price)) <= _entireDebt,
(_partialDebt + _convertDebtDenominationToBtc(MIN_NET_STETH_BALANCE, _price)) <=
_entireDebt,
"LiquidationLibrary: Partial debt liquidated must be less than total debt"
);
}

function _requirePartialLiqCollSize(uint256 _entireColl) internal pure {
require(
_entireColl >= MIN_NET_COLL,
_entireColl >= MIN_NET_STETH_BALANCE,
"LiquidationLibrary: Coll remaining in partially liquidated CDP must be >= minimum"
);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts/contracts/TestContracts/invariants/BeforeAfter.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ abstract contract BeforeAfter is BaseStorageVariables {
function _before(bytes32 _cdpId) internal {
vars.priceBefore = priceFeedMock.fetchPrice();

(uint256 debtBefore, , ) = cdpManager.getDebtAndCollShares(_cdpId);
(uint256 debtBefore, ) = cdpManager.getSyncedDebtAndCollShares(_cdpId);

vars.nicrBefore = _cdpId != bytes32(0) ? crLens.quoteRealNICR(_cdpId) : 0;
vars.icrBefore = _cdpId != bytes32(0) ? cdpManager.getICR(_cdpId, vars.priceBefore) : 0;
Expand Down
8 changes: 5 additions & 3 deletions packages/contracts/contracts/TestContracts/invariants/Properties.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ abstract contract Properties is BeforeAfter, PropertiesDescriptions, Asserts, Pr
uint256 _cdpCount = cdpManager.getActiveCdpsCount();
uint256 _sum;
for (uint256 i = 0; i < _cdpCount; ++i) {
(, uint256 _coll, ) = cdpManager.getDebtAndCollShares(cdpManager.CdpIds(i));
(, uint256 _coll) = cdpManager.getSyncedDebtAndCollShares(cdpManager.CdpIds(i));
_sum += _coll;
}
uint256 _activeColl = activePool.getSystemCollShares();
Expand All @@ -64,7 +64,7 @@ abstract contract Properties is BeforeAfter, PropertiesDescriptions, Asserts, Pr
uint256 _cdpCount = cdpManager.getActiveCdpsCount();
uint256 _sum;
for (uint256 i = 0; i < _cdpCount; ++i) {
(uint256 _debt, , ) = cdpManager.getDebtAndCollShares(cdpManager.CdpIds(i));
(uint256 _debt, ) = cdpManager.getSyncedDebtAndCollShares(cdpManager.CdpIds(i));
_sum += _debt;
}

Expand Down Expand Up @@ -296,7 +296,9 @@ abstract contract Properties is BeforeAfter, PropertiesDescriptions, Asserts, Pr
bytes32 currentCdp = sortedCdps.getFirst();
uint256 cdpsBalance;
while (currentCdp != bytes32(0)) {
(uint256 entireDebt, , ) = cdpManager.getDebtAndCollShares(currentCdp);
(uint256 entireDebt, uint256 entireColl) = cdpManager.getSyncedDebtAndCollShares(
currentCdp
);
cdpsBalance += entireDebt;
currentCdp = sortedCdps.getNext(currentCdp);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/contracts/contracts/TestContracts/invariants/Simulator.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ contract Simulator {
bytes32 currentCdp = sortedCdps.getFirst();
while (currentCdp != bytes32(0) && sortedCdps.getSize() > 1) {
Actor actor = Actor(payable(sortedCdps.getOwnerAddress(currentCdp)));
(uint256 entireDebt, , ) = cdpManager.getDebtAndCollShares(currentCdp);
(uint256 entireDebt, ) = cdpManager.getSyncedDebtAndCollShares(currentCdp);

(success, ) = actor.proxy(
address(borrowerOperations),
Expand Down
Loading

0 comments on commit 7adf7a4

Please sign in to comment.