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.

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

fix: Optimize calls to TroveManager on trove open #131

Merged
merged 1 commit into from
Apr 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions contracts/src/BorrowerOperations.sol
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,11 @@ contract BorrowerOperations is LiquityBase, Ownable, CheckContract, IBorrowerOpe
);

// Set the stored Trove properties and mint the NFT
vars.stake = contractsCache.troveManager.setTrovePropertiesOnOpen(
(vars.stake, vars.arrayIndex) = contractsCache.troveManager.setTrovePropertiesOnOpen(
_owner, troveId, _ETHAmount, vars.compositeDebt, _annualInterestRate
);

sortedTroves.insert(troveId, _annualInterestRate, _upperHint, _lowerHint);
vars.arrayIndex = contractsCache.troveManager.addTroveIdToArray(troveId);
emit TroveCreated(_owner, troveId, vars.arrayIndex);

// Pull ETH tokens from sender and move them to the Active Pool
Expand Down
4 changes: 1 addition & 3 deletions contracts/src/Interfaces/ITroveManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,6 @@ interface ITroveManager is IERC721, ILiquityBase {

function updateStakeAndTotalStakes(uint256 _troveId) external returns (uint256);

function addTroveIdToArray(uint256 _troveId) external returns (uint256 index);

function getPendingETHReward(uint256 _troveId) external view returns (uint256);

function getPendingBoldDebtReward(uint256 _troveId) external view returns (uint256);
Expand Down Expand Up @@ -105,7 +103,7 @@ interface ITroveManager is IERC721, ILiquityBase {
uint256 _coll,
uint256 _debt,
uint256 _annualInterestRate
) external returns (uint256);
) external returns (uint256, uint256);

function troveIsStale(uint256 _troveId) external view returns (bool);

Expand Down
11 changes: 6 additions & 5 deletions contracts/src/TroveManager.sol
Original file line number Diff line number Diff line change
Expand Up @@ -1312,9 +1312,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, CheckContract, ITroveMana
}

// Push the trove's id to the Trove list, and record the corresponding array index on the Trove struct
function addTroveIdToArray(uint256 _troveId) external override returns (uint256) {
_requireCallerIsBorrowerOperations();

function _addTroveIdToArray(uint256 _troveId) internal returns (uint256) {
/* Max array size is 2**128 - 1, i.e. ~3e30 troves. No risk of overflow, since troves have minimum Bold
debt of liquidation reserve plus MIN_NET_DEBT. 3e30 Bold dwarfs the value of all wealth in the world ( which is < 1e15 USD). */

Expand Down Expand Up @@ -1571,7 +1569,7 @@ contract TroveManager is ERC721, LiquityBase, Ownable, CheckContract, ITroveMana
uint256 _coll,
uint256 _debt,
uint256 _annualInterestRate
) external returns (uint256) {
) external returns (uint256, uint256) {
_requireCallerIsBorrowerOperations();
// TODO: optimize gas for writing to this struct
Troves[_troveId].status = Status.active;
Expand All @@ -1584,8 +1582,11 @@ contract TroveManager is ERC721, LiquityBase, Ownable, CheckContract, ITroveMana
// mint ERC721
_mint(_owner, _troveId);

uint256 index = _addTroveIdToArray(_troveId);

// Record the Trove's stake (for redistributions) and update the total stakes
return _updateStakeAndTotalStakes(_troveId);
uint256 stake = _updateStakeAndTotalStakes(_troveId);
return (stake, index);
}

function updateTroveDebtAndInterest(uint256 _troveId, uint256 _entireTroveDebt, uint256 _newAnnualInterestRate)
Expand Down
13 changes: 0 additions & 13 deletions contracts/test/AccessControlTest.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,19 +126,6 @@ contract(
}
});

// addTroveIdToArray
it("addTroveIdToArray(): reverts when called by an account that is not BorrowerOperations", async () => {
// Attempt call from alice
try {
const txAlice = await troveManager.addTroveIdToArray(th.addressToTroveId(bob), {
from: alice,
});
} catch (err) {
assert.include(err.message, "revert");
// assert.include(err.message, "Caller is not the BorrowerOperations contract")
}
});

// updateTroveColl
it("updateTroveColl(): reverts when called by an account that is not BorrowerOperations", async () => {
// Attempt call from alice
Expand Down