Merge pull request #663 from liquity/hints-workaround

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);