WIP: improve(InventoryClient): Align allocation percentage computatio…

…ns between Rebalancer and Relayer

As part of a larger inventory client cleanup/refactor I noticed that there is possibly a bug in how we compute "allocation" percentages when (1) determining repayment chain for fills and (2) computing possible rebalances.

For (2), we subtract the "token shortfall" from the numerator but in (2) we subtract it from both the numerator and the denominator. This is inconsistent and I don't think it makes sense.

A shortfall represents a lack of tokens on a chain suggesting that we need that amount of tokens to fill a deposit. The rebalancer (i.e. function (2) above) it designed to rebalance inventory from L1 to L2 to fulfill these shortfalls. Therefore, it makes sense to me that in (2) the shortfall is subtracted from the numerator to compute the allocation percentage on the L2.

The cumulative balance technically is the total inventory across all chains. This should intuitively include all outstanding transfers between L1 and L2 because this is still part of the inventory, its just in-flight between chains.

A shortfall will lead to an outstanding transfer but it will also lead to a subtraction in the total inventory once the transfer finalizes and part of that new L2 balance is used to make a fill.

However, if the shortfall is subtracted from the denominator in the rebalancer/(2) function, then the amount of tokens bridged over to the L2 will be insufficient to fulfill the shortfall.

This PR makes the allocation % computation consistent between (1) and (2) by removing the shortfall subtraction from the denominator in (2).

src/clients/InventoryClient.ts

@@ -473,7 +473,6 @@ export class InventoryClient {
       const chainShortfall = this.tokenClient.getShortfallTotalRequirement(_chain, repaymentToken);
       const chainVirtualBalance = this.getBalanceOnChain(_chain, l1Token, repaymentToken);
       const chainVirtualBalanceWithShortfall = chainVirtualBalance.sub(chainShortfall);
-      let cumulativeVirtualBalanceWithShortfall = cumulativeVirtualBalance.sub(chainShortfall);
       // @dev No need to factor in outputAmount when computing origin chain balance since funds only leave relayer
       // on destination chain
       // @dev Do not subtract outputAmount from virtual balance if output token and input token are not equivalent.
@@ -491,14 +490,13 @@ export class InventoryClient {
       );
       // To correctly compute the allocation % for this destination chain, we need to add all upcoming refunds for the
       // equivalents of l1Token on all chains.
-      cumulativeVirtualBalanceWithShortfall = cumulativeVirtualBalanceWithShortfall.add(cumulativeRefunds);
-      const cumulativeVirtualBalanceWithShortfallPostRelay = cumulativeVirtualBalanceWithShortfall.sub(outputAmount);
+      const cumulativeVirtualBalancePostRelay = cumulativeVirtualBalance.add(cumulativeRefunds).sub(outputAmount);
 
       // Compute what the balance will be on the target chain, considering this relay and the finalization of the
       // transfers that are currently flowing through the canonical bridge.
       const expectedPostRelayAllocation = chainVirtualBalanceWithShortfallPostRelay
         .mul(this.scalar)
-        .div(cumulativeVirtualBalanceWithShortfallPostRelay);
+        .div(cumulativeVirtualBalancePostRelay);
 
       // Consider configured buffer for target to allow relayer to support slight overages.
       const tokenConfig = this.getTokenConfig(l1Token, _chain, repaymentToken);
@@ -522,8 +520,7 @@ export class InventoryClient {
           chainVirtualBalanceWithShortfall,
           chainVirtualBalanceWithShortfallPostRelay,
           cumulativeVirtualBalance,
-          cumulativeVirtualBalanceWithShortfall,
-          cumulativeVirtualBalanceWithShortfallPostRelay,
+          cumulativeVirtualBalancePostRelay,
           thresholdPct,
           expectedPostRelayAllocation,
           chainsToEvaluate,
@@ -864,7 +861,10 @@ export class InventoryClient {
               this.crossChainTransferClient
                 .getOutstandingCrossChainTransferAmount(this.relayer, chainId, l1Token, l2Token)
                 .toString()
-            )}.\n`;
+            )}.` +
+            ` This chain has a shortfall of ${formatter(
+              this.tokenClient.getShortfallTotalRequirement(chainId, l2Token).toString()
+            )} ${symbol}.\n`;
         }
       }
 

test/InventoryClient.RefundChain.ts

@@ -202,11 +202,10 @@ describe("InventoryClient: Refund chain selection", async function () {
       // 3. chainVirtualBalanceWithShortfallPostRelay: virtual balance with shortfall minus the relay amount. 9.8-1.69=8.11.
       // 4. cumulativeVirtualBalance: total balance across all chains considering fund movement. funds moving over the bridge
       //    does not impact the balance; they are just "moving" so it should be 140-15+15=140
-      // 5. cumulativeVirtualBalanceWithShortfall: cumulative virtual balance minus the shortfall. 140-15+15=140-15=125.
-      // 6. cumulativeVirtualBalanceWithShortfallPostRelay: cumulative virtual balance with shortfall minus the relay amount
-      //    125-1.69=124.31. This is total funds considering the shortfall and the relay amount that is to be executed.
-      // 7. expectedPostRelayAllocation: the expected post relay allocation is the chainVirtualBalanceWithShortfallPostRelay
-      //    divided by the cumulativeVirtualBalanceWithShortfallPostRelay. 8.11/123.31 = 0.0657.
+      // 5. cumulativeVirtualBalancePostRelay: cumulative virtual balance plus upcoming refunds minus the relay amount
+      //    140-1.69=138.31. This is total funds considering the relay amount that is to be executed.
+      // 6. expectedPostRelayAllocation: the expected post relay allocation is the chainVirtualBalanceWithShortfallPostRelay
+      //    divided by the cumulativeVirtualBalancePostRelay. 8.11/138.31 = 0.0586.
       // This number is then used to decide on where funds should be allocated! If this number is above the threshold plus
       // the buffer then refund on L1. if it is below the threshold then refund on the target chain. As this number is
       // is below the buffer plus the threshold then the bot should refund on L2.
@@ -222,20 +221,18 @@ describe("InventoryClient: Refund chain selection", async function () {
       expect(lastSpyLogIncludes(spy, 'chainVirtualBalanceWithShortfall":"9800000000000000000"')).to.be.true; // 24.8-15=9.8
       expect(lastSpyLogIncludes(spy, 'chainVirtualBalanceWithShortfallPostRelay":"8110000000000000000"')).to.be.true; // 9.8-1.69=8.11
       expect(lastSpyLogIncludes(spy, 'cumulativeVirtualBalance":"140000000000000000000')).to.be.true; // 140-15+15=140
-      expect(lastSpyLogIncludes(spy, 'cumulativeVirtualBalanceWithShortfall":"125000000000000000000"')).to.be.true; // 140-15=125
-      expect(lastSpyLogIncludes(spy, 'cumulativeVirtualBalanceWithShortfallPostRelay":"123310000000000000000"')).to.be
-        .true; // 125-1.69=123.31
-      expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"65769199578298597')).to.be.true; // 8.11/123.31 = 0.0657
+      expect(lastSpyLogIncludes(spy, 'cumulativeVirtualBalancePostRelay":"138310000000000000000"')).to.be.true; // 125-1.69=123.31
+      expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"58636396500614561')).to.be.true; // 8.11/123.31 = 0.0657
 
       // Now consider if this small relay was larger to the point that we should be refunding on the L2. set it to 5 WETH.
       // Numerically we can shortcut some of the computations above to the following: chain virtual balance with shortfall
-      // post relay is 9.8 - 5 = 4.8. cumulative virtual balance with shortfall post relay is 125 - 5 = 120. Expected post
-      // relay allocation is 4.8/120 = 0.04. This is below the threshold of 0.05 so the bot should refund on the target.
+      // post relay is 9.8 - 5 = 4.8. cumulative virtual balance post relay is 140 - 5 = 135. Expected post
+      // relay allocation is 4.8/135 = 0.036. This is below the threshold of 0.05 so the bot should refund on the target.
       sampleDepositData.inputAmount = toWei(5);
       sampleDepositData.outputAmount = await computeOutputAmount(sampleDepositData);
       expect(await inventoryClient.determineRefundChainId(sampleDepositData)).to.equal(ARBITRUM);
       // Check only the final step in the computation.
-      expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"40000000000000000"')).to.be.true; // 4.8/120 = 0.04
+      expect(lastSpyLogIncludes(spy, 'expectedPostRelayAllocation":"35555555555555555"')).to.be.true; // 4.8/120 = 0.04
 
       // Consider that we manually send the relayer som funds while it's large transfer is currently in the bridge. This
       // is to validate that the module considers funds in transit correctly + dropping funds indirectly onto the L2 wallet.