improve(Dataworker): Remove 0-value refunds and 0-value empty-message slow fills

package.json

@@ -12,7 +12,7 @@
   "dependencies": {
     "@across-protocol/constants": "^3.1.25",
     "@across-protocol/contracts": "^3.0.19",
-    "@across-protocol/sdk": "^3.3.31",
+    "@across-protocol/sdk": "^3.3.32",
     "@arbitrum/sdk": "^4.0.2",
     "@consensys/linea-sdk": "^0.2.1",
     "@defi-wonderland/smock": "^2.3.5",

src/dataworker/DataworkerUtils.ts

@@ -9,6 +9,7 @@ import {
 import { CONTRACT_ADDRESSES } from "../common/ContractAddresses";
 import {
   PoolRebalanceLeaf,
+  Refund,
   RelayerRefundLeaf,
   RelayerRefundLeafWithGroup,
   RunningBalances,
@@ -160,10 +161,13 @@ export function _buildSlowRelayRoot(bundleSlowFillsV3: BundleSlowFills): {
   // Append V3 slow fills to the V2 leaf list
   Object.values(bundleSlowFillsV3).forEach((depositsForChain) => {
     Object.values(depositsForChain).forEach((deposits) => {
-      deposits.forEach((deposit) => {
-        const v3SlowFillLeaf = buildV3SlowFillLeaf(deposit, deposit.lpFeePct);
-        slowRelayLeaves.push(v3SlowFillLeaf);
-      });
+      // Do not create slow fill leaves where the amount to transfer would be 0 and the message is empty
+      deposits
+        .filter((deposit) => !utils.isZeroValueDeposit(deposit))
+        .forEach((deposit) => {
+          const v3SlowFillLeaf = buildV3SlowFillLeaf(deposit, deposit.lpFeePct);
+          slowRelayLeaves.push(v3SlowFillLeaf);
+        });
     });
   });
 
@@ -233,10 +237,6 @@ export function _buildRelayerRefundRoot(
   Object.entries(combinedRefunds).forEach(([_repaymentChainId, refundsForChain]) => {
     const repaymentChainId = Number(_repaymentChainId);
     Object.entries(refundsForChain).forEach(([l2TokenAddress, refunds]) => {
-      // We need to sort leaves deterministically so that the same root is always produced from the same loadData
-      // return value, so sort refund addresses by refund amount (descending) and then address (ascending).
-      const sortedRefundAddresses = sortRefundAddresses(refunds);
-
       const l1TokenCounterpart = clients.hubPoolClient.getL1TokenForL2TokenAtBlock(
         l2TokenAddress,
         repaymentChainId,
@@ -255,20 +255,8 @@ export function _buildRelayerRefundRoot(
         runningBalances[repaymentChainId][l1TokenCounterpart]
       );
 
-      // Create leaf for { repaymentChainId, L2TokenAddress }, split leaves into sub-leaves if there are too many
-      // refunds.
-      for (let i = 0; i < sortedRefundAddresses.length; i += maxRefundCount) {
-        relayerRefundLeaves.push({
-          groupIndex: i, // Will delete this group index after using it to sort leaves for the same chain ID and
-          // L2 token address
-          amountToReturn: i === 0 ? amountToReturn : bnZero,
-          chainId: repaymentChainId,
-          refundAmounts: sortedRefundAddresses.slice(i, i + maxRefundCount).map((address) => refunds[address]),
-          leafId: 0, // Will be updated before inserting into tree when we sort all leaves.
-          l2TokenAddress,
-          refundAddresses: sortedRefundAddresses.slice(i, i + maxRefundCount),
-        });
-      }
+      const _refundLeaves = _getRefundLeaves(refunds, amountToReturn, repaymentChainId, l2TokenAddress, maxRefundCount);
+      relayerRefundLeaves.push(..._refundLeaves);
     });
   });
 
@@ -325,6 +313,42 @@ export function _buildRelayerRefundRoot(
   };
 }
 
+export function _getRefundLeaves(
+  refunds: Refund,
+  amountToReturn: BigNumber,
+  repaymentChainId: number,
+  l2TokenAddress: string,
+  maxRefundCount: number
+): RelayerRefundLeafWithGroup[] {
+  const nonZeroRefunds = Object.fromEntries(Object.entries(refunds).filter(([, refundAmount]) => refundAmount.gt(0)));
+  // We need to sort leaves deterministically so that the same root is always produced from the same loadData
+  // return value, so sort refund addresses by refund amount (descending) and then address (ascending).
+  const sortedRefundAddresses = sortRefundAddresses(nonZeroRefunds);
+
+  const relayerRefundLeaves: RelayerRefundLeafWithGroup[] = [];
+
+  // Create leaf for { repaymentChainId, L2TokenAddress }, split leaves into sub-leaves if there are too many
+  // refunds.
+  for (let i = 0; i < sortedRefundAddresses.length; i += maxRefundCount) {
+    const newLeaf = {
+      groupIndex: i, // Will delete this group index after using it to sort leaves for the same chain ID and
+      // L2 token address
+      amountToReturn: i === 0 ? amountToReturn : bnZero,
+      chainId: repaymentChainId,
+      refundAmounts: sortedRefundAddresses.slice(i, i + maxRefundCount).map((address) => refunds[address]),
+      leafId: 0, // Will be updated before inserting into tree when we sort all leaves.
+      l2TokenAddress,
+      refundAddresses: sortedRefundAddresses.slice(i, i + maxRefundCount),
+    };
+    assert(
+      newLeaf.refundAmounts.length === newLeaf.refundAddresses.length,
+      "refund address and amount array lengths mismatch"
+    );
+    relayerRefundLeaves.push(newLeaf);
+  }
+  return relayerRefundLeaves;
+}
+
 /**
  * @notice Returns WETH and ETH token addresses for chain if defined, or throws an error if they're not
  * in the hardcoded dictionary.

test/DataworkerUtils.ts

@@ -0,0 +1,155 @@
+import { _buildSlowRelayRoot, _getRefundLeaves } from "../src/dataworker/DataworkerUtils";
+import { BundleSlowFills, DepositWithBlock } from "../src/interfaces";
+import { BigNumber, bnOne, bnZero, toBNWei, ZERO_ADDRESS } from "../src/utils";
+import { repaymentChainId } from "./constants";
+import { assert, expect, randomAddress } from "./utils";
+
+describe("RelayerRefund utils", function () {
+  it("Removes zero value refunds from relayer refund root", async function () {
+    const recipient1 = randomAddress();
+    const recipient2 = randomAddress();
+    const repaymentToken = randomAddress();
+    const maxRefundsPerLeaf = 2;
+    const result = _getRefundLeaves(
+      {
+        [recipient1]: bnZero,
+        [recipient2]: bnOne,
+      },
+      bnZero,
+      repaymentChainId,
+      repaymentToken,
+      maxRefundsPerLeaf
+    );
+    expect(result.length).to.equal(1);
+    expect(result[0].refundAddresses.length).to.equal(1);
+  });
+  it("No more than maxRefundsPerLeaf number of refunds in a leaf", async function () {
+    const recipient1 = randomAddress();
+    const recipient2 = randomAddress();
+    const repaymentToken = randomAddress();
+    const amountToReturn = bnOne;
+    const maxRefundsPerLeaf = 1;
+    const result = _getRefundLeaves(
+      {
+        [recipient1]: bnOne,
+        [recipient2]: bnOne,
+      },
+      amountToReturn,
+      repaymentChainId,
+      repaymentToken,
+      maxRefundsPerLeaf
+    );
+    expect(result.length).to.equal(2);
+    // Only the first leaf should have an amount to return.
+    expect(result[0].groupIndex).to.equal(0);
+    expect(result[0].amountToReturn).to.equal(amountToReturn);
+    expect(result[1].groupIndex).to.equal(1);
+    expect(result[1].amountToReturn).to.equal(0);
+  });
+  it("Sorts refunds by amount in descending order", async function () {
+    const recipient1 = randomAddress();
+    const recipient2 = randomAddress();
+    const repaymentToken = randomAddress();
+    const maxRefundsPerLeaf = 2;
+    const result = _getRefundLeaves(
+      {
+        [recipient1]: bnOne,
+        [recipient2]: bnOne.mul(2),
+      },
+      bnZero,
+      repaymentChainId,
+      repaymentToken,
+      maxRefundsPerLeaf
+    );
+    expect(result.length).to.equal(1);
+    expect(result[0].refundAddresses[0]).to.equal(recipient2);
+    expect(result[0].refundAddresses[1]).to.equal(recipient1);
+  });
+});
+
+describe("SlowFill utils", function () {
+  /**
+   * @notice Returns dummy slow fill leaf that you can insert into a BundleSlowFills object.
+   * @dev The leaf returned will not actually be executable so its good for testing functions
+   * that produce but do not execute merkle leaves.
+   * @param depositId This is used to sort slow fill leaves so allow caller to set.
+   * @param amountToFill This will be set to the deposit's inputAmount because the slow fill pays out
+   * inputAmount * (1 - lpFeePct).
+   * @param lpFeePct Amount to charge on the amountToFill.
+   * @param message 0-value, empty-message slow fills should be ignored by dataworker so allow caller
+   * to set this to non-empty to test logic.
+   * @param originChainId This is used to sort slow fill leaves so allow caller to set.
+   */
+
+  function createSlowFillLeaf(
+    depositId: number,
+    originChainId: number,
+    amountToFill: BigNumber,
+    message: string,
+    _lpFeePct: BigNumber
+  ): DepositWithBlock & { lpFeePct: BigNumber } {
+    assert(message.slice(0, 2) === "0x", "Need to specify message beginning with 0x");
+    const destinationChainId = originChainId + 1;
+    const deposit: DepositWithBlock = {
+      inputAmount: amountToFill,
+      inputToken: randomAddress(),
+      outputAmount: bnOne,
+      outputToken: randomAddress(),
+      depositor: randomAddress(),
+      depositId,
+      originChainId: 1,
+      recipient: randomAddress(),
+      exclusiveRelayer: ZERO_ADDRESS,
+      exclusivityDeadline: 0,
+      message,
+      destinationChainId,
+      fillDeadline: 0,
+      quoteBlockNumber: 0,
+      blockNumber: 0,
+      transactionHash: "",
+      logIndex: 0,
+      transactionIndex: 0,
+      quoteTimestamp: 0,
+      fromLiteChain: false,
+      toLiteChain: false,
+    };
+    return {
+      ...deposit,
+      lpFeePct: _lpFeePct,
+    };
+  }
+  it("Filters out 0-value empty-message slowfills", async function () {
+    const zeroValueSlowFillLeaf = createSlowFillLeaf(0, 1, bnZero, "0x", bnZero);
+    const oneWeiSlowFillLeaf = createSlowFillLeaf(1, 1, bnOne, "0x", bnZero);
+    const zeroValueNonEmptyMessageSlowFillLeaf = createSlowFillLeaf(2, 1, bnZero, "0x12", bnZero);
+    const bundleSlowFills: BundleSlowFills = {
+      [zeroValueSlowFillLeaf.destinationChainId]: {
+        [zeroValueSlowFillLeaf.outputToken]: [
+          zeroValueSlowFillLeaf,
+          oneWeiSlowFillLeaf,
+          zeroValueNonEmptyMessageSlowFillLeaf,
+        ],
+      },
+    };
+
+    // Should return two out of three leaves, sorted by deposit ID.
+    const { leaves } = _buildSlowRelayRoot(bundleSlowFills);
+    expect(leaves.length).to.equal(2);
+    expect(leaves[0].relayData.depositId).to.equal(1);
+    expect(leaves[1].relayData.depositId).to.equal(2);
+  });
+  it("Applies LP fee to input amount", async function () {
+    const slowFillLeaf = createSlowFillLeaf(0, 1, toBNWei("4"), "0x", toBNWei("0.25"));
+    const bundleSlowFills: BundleSlowFills = {
+      [slowFillLeaf.destinationChainId]: {
+        [slowFillLeaf.outputToken]: [slowFillLeaf],
+      },
+    };
+
+    // Should return two out of three leaves, sorted by deposit ID.
+    const { leaves } = _buildSlowRelayRoot(bundleSlowFills);
+    expect(leaves.length).to.equal(1);
+    // updatedOutputAmount should be equal to inputAmount * (1 - lpFee) so 4 * (1 - 0.25) = 3
+    expect(leaves[0].updatedOutputAmount).to.equal(toBNWei("3"));
+  });
+});

yarn.lock

@@ -58,10 +58,10 @@
     yargs "^17.7.2"
     zksync-web3 "^0.14.3"
 
-"@across-protocol/sdk@^3.3.31":
-  version "3.3.31"
-  resolved "https://registry.yarnpkg.com/@across-protocol/sdk/-/sdk-3.3.31.tgz#6fe622517962f84fa140184ec209529f755d9787"
-  integrity sha512-C7LGiNC+kKxPfimomlRIuKDBD2u7uH19YRiRJjLBbt0pPbl9L40ockXxYOs7+WSxgCjXnY03micljhNz2wvHyg==
+"@across-protocol/sdk@^3.3.32":
+  version "3.3.32"
+  resolved "https://registry.yarnpkg.com/@across-protocol/sdk/-/sdk-3.3.32.tgz#fa2428df5f9b6cb0392c46f742f11265efa4abb3"
+  integrity sha512-ADyZQeWxjGAreLoeVQYNiJN4zMmmJ7h6ItgbSjP2+JvZENPaH9t23xCegPIyI0oiVqLrOHOGCJ/yEdX6X3HqpQ==
   dependencies:
     "@across-protocol/across-token" "^1.0.0"
     "@across-protocol/constants" "^3.1.25"