Merge pull request #131 from liquity/optimize_tm_calls_open_trove

fix: Optimize calls to TroveManager on trove open
liquity · Apr 26, 2024 · 3998fd0 · 3998fd0
2 parents c267c34 + 8a4e8ff
commit 3998fd0
Show file tree

Hide file tree

Showing 4 changed files with 8 additions and 23 deletions.
diff --git a/contracts/src/BorrowerOperations.sol b/contracts/src/BorrowerOperations.sol
@@ -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

diff --git a/contracts/src/Interfaces/ITroveManager.sol b/contracts/src/Interfaces/ITroveManager.sol
@@ -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);
@@ -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);
 

diff --git a/contracts/src/TroveManager.sol b/contracts/src/TroveManager.sol
@@ -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). */
 
@@ -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;
@@ -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)

diff --git a/contracts/test/AccessControlTest.js b/contracts/test/AccessControlTest.js
@@ -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