fix: work around huge gas cost when reducing ICR of bottom Trove

packages/lib-ethers/src/PopulatableEthersLiquity.ts

@@ -693,7 +693,8 @@ export class PopulatableEthersLiquity
  }
 
  private async _findHintsForNominalCollateralRatio(
- nominalCollateralRatio: Decimal
+ nominalCollateralRatio: Decimal,
+ ownAddress?: string
  ): Promise<[string, string]> {
  const { sortedTroves, hintHelpers } = _getContracts(this._readable.connection);
  const numberOfTroves = await this._readable.getNumberOfTroves();
@@ -733,21 +734,40 @@ export class PopulatableEthersLiquity
 
  const { hintAddress } = results.reduce((a, b) => (a.diff.lt(b.diff) ? a : b));
 
- const [prev, next] = await sortedTroves.findInsertPosition(
+ let [prev, next] = await sortedTroves.findInsertPosition(
  nominalCollateralRatio.hex,
  hintAddress,
  hintAddress
  );
 
- return prev === AddressZero ? [next, next] : next === AddressZero ? [prev, prev] : [prev, next];
+ if (ownAddress) {
+ // In the case of reinsertion, the address of the Trove being reinserted is not a usable hint,
+ // because it is deleted from the list before the reinsertion.
+ // "Jump over" the Trove to get the proper hint.
+ if (prev === ownAddress) {
+ prev = await sortedTroves.getPrev(prev);
+ } else if (next === ownAddress) {
+ next = await sortedTroves.getNext(next);
+ }
+ }
+
+ // Don't use `address(0)` as hint as it can result in huge gas cost.
+ // (See https://github.com/liquity/dev/issues/600).
+ if (prev === AddressZero) {
+ prev = next;
+ } else if (next === AddressZero) {
+ next = prev;
+ }
+
+ return [prev, next];
  }
 
- private async _findHints(trove: Trove): Promise<[string, string]> {
+ private async _findHints(trove: Trove, ownAddress?: string): Promise<[string, string]> {
  if (trove instanceof TroveWithPendingRedistribution) {
  throw new Error("Rewards must be applied to this Trove");
  }
 
- return this._findHintsForNominalCollateralRatio(trove._nominalCollateralRatio);
+ return this._findHintsForNominalCollateralRatio(trove._nominalCollateralRatio, ownAddress);
  }
 
  private async _findRedemptionHints(
@@ -775,7 +795,10 @@ export class PopulatableEthersLiquity
  partialRedemptionLowerHint
  ] = partialRedemptionHintNICR.isZero()
  ? [AddressZero, AddressZero]
- : await this._findHintsForNominalCollateralRatio(decimalify(partialRedemptionHintNICR));
+ : await this._findHintsForNominalCollateralRatio(
+ decimalify(partialRedemptionHintNICR)
+ // XXX: if we knew the partially redeemed Trove's address, we'd pass it here
+ );
 
  return [
  decimalify(truncatedLUSDamount),
@@ -942,7 +965,7 @@ export class PopulatableEthersLiquity
 
  const currentBorrowingRate = decayBorrowingRate(0);
  const adjustedTrove = trove.adjust(normalizedParams, currentBorrowingRate);
- const hints = await this._findHints(adjustedTrove);
+ const hints = await this._findHints(adjustedTrove, address);
 
  const {
  maxBorrowingRate,
@@ -1139,7 +1162,7 @@ export class PopulatableEthersLiquity
  await stabilityPool.estimateAndPopulate.withdrawETHGainToTrove(
  { ...overrides },
  compose(addGasForPotentialListTraversal, addGasForLQTYIssuance),
- ...(await this._findHints(finalTrove))
+ ...(await this._findHints(finalTrove, address))
  )
  );
  }

packages/lib-ethers/test/Liquity.test.ts

@@ -988,6 +988,92 @@ describe("EthersLiquity", () => {
  });
  });
 
+ // Test workarounds related to https://github.com/liquity/dev/issues/600
+ describe("Hints (adjustTrove)", () => {
+ let eightOtherUsers: Signer[];
+
+ before(async () => {
+ deployment = await deployLiquity(deployer);
+ eightOtherUsers = otherUsers.slice(0, 8);
+ liquity = await connectToDeployment(deployment, user);
+
+ await openTroves(eightOtherUsers, [
+ { depositCollateral: 30, borrowLUSD: 2000 }, // 0
+ { depositCollateral: 30, borrowLUSD: 2100 }, // 1
+ { depositCollateral: 30, borrowLUSD: 2200 }, // 2
+ { depositCollateral: 30, borrowLUSD: 2300 }, // 3
+ // Test 1: 30, 2400
+ { depositCollateral: 30, borrowLUSD: 2500 }, // 4
+ { depositCollateral: 30, borrowLUSD: 2600 }, // 5
+ { depositCollateral: 30, borrowLUSD: 2700 }, // 6
+ { depositCollateral: 30, borrowLUSD: 2800 } // 7
+ // Test 2: 30, 2900
+ // Test 2 (other): 30, 3000
+ // Test 3: 30, 3100 -> 3200
+ ]);
+ });
+
+ // Test 1
+ it("should not use extra gas when a Trove's position doesn't change", async () => {
+ const { newTrove: initialTrove } = await liquity.openTrove({
+ depositCollateral: 30,
+ borrowLUSD: 2400
+ });
+
+ // Maintain the same ICR / position in the list
+ const targetTrove = initialTrove.multiply(1.1);
+
+ const { rawReceipt } = await waitForSuccess(
+ liquity.send.adjustTrove(initialTrove.adjustTo(targetTrove))
+ );
+
+ const gasUsed = rawReceipt.gasUsed.toNumber();
+ expect(gasUsed).to.be.at.most(250000);
+ });
+
+ // Test 2
+ it("should not traverse the whole list when bottom Trove moves", async () => {
+ const bottomLiquity = await connectToDeployment(deployment, eightOtherUsers[7]);
+
+ const initialTrove = await liquity.getTrove();
+ const bottomTrove = await bottomLiquity.getTrove();
+
+ const targetTrove = Trove.create({ depositCollateral: 30, borrowLUSD: 2900 });
+ const interferingTrove = Trove.create({ depositCollateral: 30, borrowLUSD: 3000 });
+
+ const tx = await liquity.populate.adjustTrove(initialTrove.adjustTo(targetTrove));
+
+ // Suddenly: interference!
+ await bottomLiquity.adjustTrove(bottomTrove.adjustTo(interferingTrove), {}, { gasPrice: 0 });
+
+ const { rawReceipt } = await waitForSuccess(tx.send());
+
+ const gasUsed = rawReceipt.gasUsed.toNumber();
+ expect(gasUsed).to.be.at.most(310000);
+ });
+
+ // Test 3
+ it("should not traverse the whole list when lowering ICR of bottom Trove", async () => {
+ const initialTrove = await liquity.getTrove();
+
+ const targetTrove = [
+ Trove.create({ depositCollateral: 30, borrowLUSD: 3100 }),
+ Trove.create({ depositCollateral: 30, borrowLUSD: 3200 })
+ ];
+
+ await liquity.adjustTrove(initialTrove.adjustTo(targetTrove[0]));
+ // Now we are the bottom Trove
+
+ // Lower our ICR even more
+ const { rawReceipt } = await waitForSuccess(
+ liquity.send.adjustTrove(targetTrove[0].adjustTo(targetTrove[1]))
+ );
+
+ const gasUsed = rawReceipt.gasUsed.toNumber();
+ expect(gasUsed).to.be.at.most(240000);
+ });
+ });
+
  describe("Gas estimation", () => {
  const troveWithICRBetween = (a: Trove, b: Trove) => a.add(b).multiply(0.5);