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

refactor: eliminate recordedDebtSum (G) from ActivePool #150

Merged
merged 4 commits into from
May 6, 2024
Merged

refactor: eliminate recordedDebtSum (G) from ActivePool #150

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a695d4
refactor: eliminate `recordedDebtSum` (G) from ActivePool
danielattilasimon May 2, 2024
a6894f9
Merge remote-tracking branch 'origin/main' into activepool-small-cleanup
danielattilasimon May 3, 2024
a8acdee
Merge remote-tracking branch 'origin/main' into activepool-small-cleanup
danielattilasimon May 3, 2024
b5f4b95
Merge remote-tracking branch 'origin/main' into activepool-small-cleanup
danielattilasimon May 6, 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
67 changes: 17 additions & 50 deletions contracts/src/ActivePool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,6 @@ contract ActivePool is Ownable, CheckContract, IActivePool {

uint256 internal ETHBalance; // deposited ether tracker

// Sum of individual recorded Trove debts. Updated only at individual Trove operations.
// "G" in the spec.
uint256 internal recordedDebtSum;

// Aggregate recorded debt tracker. Updated whenever a Trove's debt is touched AND whenever the aggregate pending interest is minted.
// "D" in the spec.
uint256 public aggRecordedDebt;
Expand Down Expand Up @@ -118,10 +114,6 @@ contract ActivePool is Ownable, CheckContract, IActivePool {
return ETHBalance;
}

function getRecordedDebtSum() external view override returns (uint256) {
return recordedDebtSum;
}

function calcPendingAggInterest() public view returns (uint256) {
return aggWeightedDebtSum * (block.timestamp - lastAggUpdateTime) / SECONDS_IN_ONE_YEAR / 1e18;
}
Expand Down Expand Up @@ -179,35 +171,6 @@ contract ActivePool is Ownable, CheckContract, IActivePool {
emit ActivePoolETHBalanceUpdated(newETHBalance);
}

function increaseRecordedDebtSum(uint256 _amount) external {
_requireCallerIsTroveManager();
_changeRecordedDebtSum(_amount, 0);
}

// TODO: remove this once we implement interest minting in redemptions
function decreaseRecordedDebtSum(uint256 _amount) external {
_requireCallerIsTroveManager();
_changeRecordedDebtSum(0, _amount);
}

function _changeRecordedDebtSum(uint256 _recordedDebtIncrease, uint256 _recordedDebtDecrease) internal {
// Do the arithmetic in 2 steps here to avoid overflow from the decrease
uint256 newRecordedDebtSum = recordedDebtSum + _recordedDebtIncrease; // 1 SLOAD
newRecordedDebtSum -= _recordedDebtDecrease;
recordedDebtSum = newRecordedDebtSum; // 1 SSTORE
emit ActivePoolBoldDebtUpdated(newRecordedDebtSum);
}

function _changeAggWeightedDebtSum(
uint256 _newTroveWeightedRecordedTroveDebt,
uint256 _oldWeightedRecordedTroveDebt
) internal {
// Do the arithmetic in 2 steps here to avoid overflow from the decrease
uint256 newAggWeightedDebtSum = aggWeightedDebtSum + _newTroveWeightedRecordedTroveDebt; // 1 SLOAD
newAggWeightedDebtSum -= _oldWeightedRecordedTroveDebt;
aggWeightedDebtSum = newAggWeightedDebtSum; // 1 SSTORE
}

// --- Aggregate interest operations ---

// This function is called inside all state-changing user ops: borrower ops, liquidations, redemptions and SP deposits/withdrawals.
Expand All @@ -217,40 +180,44 @@ contract ActivePool is Ownable, CheckContract, IActivePool {
// It does *not* include the Trove's individual accrued interest - this gets accounted for in the aggregate accrued interest.
// The net Trove debt change could be positive or negative in a repayment (depending on whether its redistribution gain or repayment amount is larger),
// so this function accepts both the increase and the decrease to avoid using (and converting to/from) signed ints.
function mintAggInterest(
function mintAggInterestAndAccountForTroveChange(
uint256 _troveDebtIncrease,
uint256 _troveDebtDecrease,
uint256 recordedSumIncrease,
uint256 recordedSumDecrease,
uint256 newWeightedRecordedTroveDebt,
uint256 oldWeightedRecordedTroveDebt
uint256 _newWeightedRecordedTroveDebt,
uint256 _oldWeightedRecordedTroveDebt
) external {
_requireCallerIsBOorTroveM();

// Do the arithmetic in 2 steps here to avoid overflow from the decrease
uint256 newAggRecordedDebt = _mintAggInterestNoTroveChange() + _troveDebtIncrease; // 1 SLOAD
uint256 newAggRecordedDebt = aggRecordedDebt; // 1 SLOAD
newAggRecordedDebt += _mintAggInterest();
newAggRecordedDebt += _troveDebtIncrease;
newAggRecordedDebt -= _troveDebtDecrease;
aggRecordedDebt = newAggRecordedDebt; // 1 SSTORE

// assert(aggRecordedDebt >= 0) // This should never be negative. If all redistribution gians and all aggregate interest was applied
// and all Trove debts were repaid, it should become 0.

_changeRecordedDebtSum(recordedSumIncrease, recordedSumDecrease);
_changeAggWeightedDebtSum(newWeightedRecordedTroveDebt, oldWeightedRecordedTroveDebt);
// Do the arithmetic in 2 steps here to avoid overflow from the decrease
uint256 newAggWeightedDebtSum = aggWeightedDebtSum; // 1 SLOAD
newAggWeightedDebtSum += _newWeightedRecordedTroveDebt;
newAggWeightedDebtSum -= _oldWeightedRecordedTroveDebt;
aggWeightedDebtSum = newAggWeightedDebtSum; // 1 SSTORE
}

function mintAggInterestNoTroveChange() external override {
function mintAggInterest() external override {
_requireCallerIsSP();
aggRecordedDebt = _mintAggInterestNoTroveChange();
aggRecordedDebt += _mintAggInterest();
}

function _mintAggInterestNoTroveChange() internal returns (uint256) {
uint256 aggInterest = calcPendingAggInterest();
function _mintAggInterest() internal returns (uint256 aggInterest) {
aggInterest = calcPendingAggInterest();

// Mint the new BOLD interest to a mock interest router that would split it and send it onward to SP, LP staking, etc.
// TODO: implement interest routing and SP Bold reward tracking
if (aggInterest > 0) boldToken.mint(address(interestRouter), aggInterest);

lastAggUpdateTime = block.timestamp;
return aggRecordedDebt + aggInterest;
}

// --- 'require' functions ---
Expand Down
28 changes: 8 additions & 20 deletions contracts/src/BorrowerOperations.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,6 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe
uint256 annualInterestRate;
uint256 troveDebtIncrease;
uint256 troveDebtDecrease;
uint256 recordedDebtIncrease;
uint256 recordedDebtDecrease;
}

struct LocalVariables_openTrove {
Expand Down Expand Up @@ -204,8 +202,8 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe
// --- Effects & interactions ---

uint256 weightedRecordedTroveDebt = vars.compositeDebt * _annualInterestRate;
contractsCache.activePool.mintAggInterest(
vars.compositeDebt, 0, vars.compositeDebt, 0, weightedRecordedTroveDebt, 0
contractsCache.activePool.mintAggInterestAndAccountForTroveChange(
vars.compositeDebt, 0, weightedRecordedTroveDebt, 0
);

// Set the stored Trove properties and mint the NFT
Expand Down Expand Up @@ -425,21 +423,15 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe
if (_isDebtIncrease) {
// Increase Trove debt by the drawn debt + redist. gain
vars.troveDebtIncrease = _boldChange + vars.redistDebtGain;
vars.recordedDebtIncrease = _boldChange + vars.accruedTroveInterest;
} else {
// Increase Trove debt by redist. gain and decrease by the repaid debt
vars.troveDebtIncrease = vars.redistDebtGain;
vars.troveDebtDecrease = _boldChange;

vars.recordedDebtIncrease = vars.accruedTroveInterest;
vars.recordedDebtDecrease = _boldChange;
}

activePool.mintAggInterest(
activePool.mintAggInterestAndAccountForTroveChange(
vars.troveDebtIncrease,
vars.troveDebtDecrease,
vars.recordedDebtIncrease,
vars.recordedDebtDecrease,
vars.newWeightedTroveDebt,
vars.initialWeightedRecordedTroveDebt
);
Expand Down Expand Up @@ -475,7 +467,7 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe
(
uint256 entireTroveDebt,
uint256 entireTroveColl,
uint256 debtRedistGain,
, // debtRedistGain
, // ETHredist gain
uint256 accruedTroveInterest
) = contractsCache.troveManager.getEntireDebtAndColl(_troveId);
Expand All @@ -496,13 +488,9 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe
// Remove the Trove's initial recorded debt plus its accrued interest from ActivePool.aggRecordedDebt,
// but *don't* remove the redistribution gains, since these were not yet incorporated into the sum.
uint256 troveDebtDecrease = initialRecordedTroveDebt + accruedTroveInterest;
// Remove only the Trove's latest recorded debt (inc. redist. gains) from the recorded debt tracker,
// i.e. exclude the accrued interest since it has not been added.
// TODO: If/when redist. gains are gas-optimized, exclude them from here too.
uint256 recordedDebtSumDecrease = initialRecordedTroveDebt + debtRedistGain;

contractsCache.activePool.mintAggInterest(
0, troveDebtDecrease, 0, recordedDebtSumDecrease, 0, initialWeightedRecordedTroveDebt
contractsCache.activePool.mintAggInterestAndAccountForTroveChange(
0, troveDebtDecrease, 0, initialWeightedRecordedTroveDebt
);

contractsCache.troveManager.removeStake(_troveId);
Expand Down Expand Up @@ -674,8 +662,8 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe
// Add only the Trove's accrued interest to the recorded debt tracker since we have already applied redist. gains.
// No debt is issued/repaid, so the net Trove debt change is purely the redistribution gain
// TODO: also include redist. gains here in the recordedSumIncrease arg if we gas-optimize them
_activePool.mintAggInterest(
redistDebtGain, 0, accruedTroveInterest, 0, newWeightedTroveDebt, initialWeightedRecordedTroveDebt
_activePool.mintAggInterestAndAccountForTroveChange(
redistDebtGain, 0, newWeightedTroveDebt, initialWeightedRecordedTroveDebt
);

return entireTroveDebt;
Expand Down
13 changes: 4 additions & 9 deletions contracts/src/Interfaces/IActivePool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,20 @@ interface IActivePool {
) external;

function getETHBalance() external view returns (uint256);
function getRecordedDebtSum() external view returns (uint256);
function getTotalActiveDebt() external view returns (uint256);
function lastAggUpdateTime() external view returns (uint256);
function aggRecordedDebt() external view returns (uint256);
function aggWeightedDebtSum() external view returns (uint256);
function calcPendingAggInterest() external view returns (uint256);

function mintAggInterest(
function mintAggInterest() external;
function mintAggInterestAndAccountForTroveChange(
uint256 _troveDebtIncrease,
uint256 _troveDebtDecrease,
uint256 recordedSumIncrease,
uint256 recordedSumDecrease,
uint256 newWeightedRecordedTroveDebt,
uint256 oldWeightedRecordedTroveDebt
uint256 _newWeightedRecordedTroveDebt,
uint256 _oldWeightedRecordedTroveDebt
) external;

function mintAggInterestNoTroveChange() external;
function increaseRecordedDebtSum(uint256 _amount) external;
function decreaseRecordedDebtSum(uint256 _amount) external;
function sendETH(address _account, uint256 _amount) external;
function sendETHToDefaultPool(uint256 _amount) external;
function receiveETH(uint256 _amount) external;
Expand Down
4 changes: 2 additions & 2 deletions contracts/src/StabilityPool.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
function provideToSP(uint256 _amount) external override {
_requireNonZeroAmount(_amount);

activePool.mintAggInterestNoTroveChange();
activePool.mintAggInterest();

uint256 initialDeposit = deposits[msg.sender].initialValue;

Expand Down Expand Up @@ -314,7 +314,7 @@ contract StabilityPool is LiquityBase, Ownable, CheckContract, IStabilityPool {
uint256 initialDeposit = deposits[msg.sender].initialValue;
_requireUserHasDeposit(initialDeposit);

activePool.mintAggInterestNoTroveChange();
activePool.mintAggInterest();

uint256 depositorETHGain = getDepositorETHGain(msg.sender);

Expand Down
Loading