fix(hub-pool): Enable data worker to ensure that roots are not redundantly relayed to SpokePools

contracts/HubPool.sol

@@ -193,6 +193,7 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
         address indexed proposer
     );
     event RootBundleExecuted(
+        uint256 groupIndex,
         uint256 indexed leafId,
         uint256 indexed chainId,
         address[] l1Token,
@@ -593,6 +594,8 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
      * @dev In some cases, will instruct spokePool to send funds back to L1.
      * @notice Deletes the published root bundle if this is the last leaf to be executed in the root bundle.
      * @param chainId ChainId number of the target spoke pool on which the bundle is executed.
+     * @param groupIndex If set to 0, then relay roots to SpokePool via cross chain bridge. Used by off-chain validator
+     * to organize leafs with the same chain ID and also set which leaves should result in relayed messages.
      * @param bundleLpFees Array representing the total LP fee amount per token in this bundle for all bundled relays.
      * @param netSendAmounts Array representing the amount of tokens to send to the SpokePool on the target chainId.
      * @param runningBalances Array used to track any unsent tokens that are not included in the netSendAmounts.
@@ -603,6 +606,7 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
 
     function executeRootBundle(
         uint256 chainId,
+        uint256 groupIndex,
         uint256[] memory bundleLpFees,
         int256[] memory netSendAmounts,
         int256[] memory runningBalances,
@@ -621,6 +625,7 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
                 rootBundleProposal.poolRebalanceRoot,
                 PoolRebalanceLeaf({
                     chainId: chainId,
+                    groupIndex: groupIndex,
                     bundleLpFees: bundleLpFees,
                     netSendAmounts: netSendAmounts,
                     runningBalances: runningBalances,
@@ -658,27 +663,39 @@ contract HubPool is HubPoolInterface, Testable, Lockable, MultiCaller, Ownable {
             bundleLpFees
         );
 
-        // Relay root bundles to spoke pool on destination chain by
-        // performing delegatecall to use the adapter's code with this contract's context.
-        (bool success, ) = adapter.delegatecall(
-            abi.encodeWithSignature(
-                "relayMessage(address,bytes)",
-                spokePool, // target. This should be the spokePool on the L2.
+        // Check bool used by data worker to prevent relaying redundant roots to SpokePool.
+        if (groupIndex == 0) {
+            // Relay root bundles to spoke pool on destination chain by
+            // performing delegatecall to use the adapter's code with this contract's context.
+            (bool success, ) = adapter.delegatecall(
                 abi.encodeWithSignature(
-                    "relayRootBundle(bytes32,bytes32)",
-                    rootBundleProposal.relayerRefundRoot,
-                    rootBundleProposal.slowRelayRoot
-                ) // message
-            )
-        );
-        require(success, "delegatecall failed");
+                    "relayMessage(address,bytes)",
+                    spokePool, // target. This should be the spokePool on the L2.
+                    abi.encodeWithSignature(
+                        "relayRootBundle(bytes32,bytes32)",
+                        rootBundleProposal.relayerRefundRoot,
+                        rootBundleProposal.slowRelayRoot
+                    ) // message
+                )
+            );
+            require(success, "delegatecall failed");
+        }
 
         // Transfer the bondAmount to back to the proposer, if this the last executed leaf. Only sending this once all
         // leafs have been executed acts to force the data worker to execute all bundles or they wont receive their bond.
         if (rootBundleProposal.unclaimedPoolRebalanceLeafCount == 0)
             bondToken.safeTransfer(rootBundleProposal.proposer, bondAmount);
 
-        emit RootBundleExecuted(leafId, chainId, l1Tokens, bundleLpFees, netSendAmounts, runningBalances, msg.sender);
+        emit RootBundleExecuted(
+            groupIndex,
+            leafId,
+            chainId,
+            l1Tokens,
+            bundleLpFees,
+            netSendAmounts,
+            runningBalances,
+            msg.sender
+        );
     }
 
     /**

contracts/HubPoolInterface.sol

@@ -26,6 +26,13 @@ interface HubPoolInterface {
         // A positive number indicates that the HubPool owes the SpokePool funds. A negative number indicates that the
         // SpokePool owes the HubPool funds. See the comment above for the dynamics of this and netSendAmounts
         int256[] runningBalances;
+        // Used by data worker to mark which leaves should relay roots to SpokePools, and to otherwise organize leaves.
+        // For example, each leaf should contain all the rebalance information for a single chain, but in the case where
+        // the list of l1Tokens is very large such that they all can't fit into a single leaf that can be executed under
+        // the block gas limit, then the data worker can use this groupIndex to organize them. Any leaves with
+        // a groupIndex equal to 0 will relay roots to the SpokePool, so the data worker should ensure that only one
+        // leaf for a specific chainId should have a groupIndex equal to 0.
+        uint256 groupIndex;
         // Used as the index in the bitmap to track whether this leaf has been executed or not.
         uint8 leafId;
         // The following arrays are required to be the same length. They are parallel arrays for the given chainId and
@@ -92,6 +99,7 @@ interface HubPoolInterface {
 
     function executeRootBundle(
         uint256 chainId,
+        uint256 groupIndex,
         uint256[] memory bundleLpFees,
         int256[] memory netSendAmounts,
         int256[] memory runningBalances,

scripts/buildSampleTree.ts

@@ -43,6 +43,7 @@ async function main() {
         bundleLpFees: [toBNWei(0.1)],
         netSendAmounts: [toBNWei(POOL_REBALANCE_NET_SEND_AMOUNT)],
         runningBalances: [toWei(0)],
+        groupIndex: toBN(0),
         leafId: toBN(i),
         l1Tokens: [L1_TOKEN],
       });

test/HubPool.ExecuteRootBundle.ts

@@ -14,11 +14,12 @@ async function constructSimpleTree() {
   const wethToSendToL2 = toBNWei(100);
   const daiToSend = toBNWei(1000);
   const leafs = buildPoolRebalanceLeafs(
-    [consts.repaymentChainId], // repayment chain. In this test we only want to send one token to one chain.
-    [[weth.address, dai.address]], // l1Token. We will only be sending WETH and DAI to the associated repayment chain.
-    [[toBNWei(1), toBNWei(10)]], // bundleLpFees. Set to 1 ETH and 10 DAI respectively to attribute to the LPs.
-    [[wethToSendToL2, daiToSend]], // netSendAmounts. Set to 100 ETH and 1000 DAI as the amount to send from L1->L2.
-    [[wethToSendToL2, daiToSend]] // runningBalances. Set to 100 ETH and 1000 DAI.
+    [consts.repaymentChainId, consts.repaymentChainId], // repayment chain.
+    [[weth.address, dai.address], []], // l1Token. We will only be sending WETH and DAI to the associated repayment chain.
+    [[toBNWei(1), toBNWei(10)], []], // bundleLpFees. Set to 1 ETH and 10 DAI respectively to attribute to the LPs.
+    [[wethToSendToL2, daiToSend], []], // netSendAmounts. Set to 100 ETH and 1000 DAI as the amount to send from L1->L2.
+    [[wethToSendToL2, daiToSend], []], // runningBalances. Set to 100 ETH and 1000 DAI.
+    [0, 1] // groupIndex. Second leaf should not relay roots to spoke pool.
   );
   const tree = await buildPoolRebalanceLeafTree(leafs);
 
@@ -45,19 +46,24 @@ describe("HubPool Root Bundle Execution", function () {
     const { wethToSendToL2, daiToSend, leafs, tree } = await constructSimpleTree();
 
     await hubPool.connect(dataWorker).proposeRootBundle(
-      [3117], // bundleEvaluationBlockNumbers used by bots to construct bundles. Length must equal the number of leafs.
-      1, // poolRebalanceLeafCount. There is exactly one leaf in the bundle (just sending WETH to one address).
+      [3117, 3118], // bundleEvaluationBlockNumbers used by bots to construct bundles. Length must equal the number of leafs.
+      2, // poolRebalanceLeafCount.
       tree.getHexRoot(), // poolRebalanceRoot. Generated from the merkle tree constructed before.
       consts.mockRelayerRefundRoot, // Not relevant for this test.
       consts.mockSlowRelayRoot // Not relevant for this test.
     );
 
-    // Advance time so the request can be executed and execute the request.
+    // Advance time so the request can be executed and execute first leaf.
     await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness + 1);
-    await hubPool.connect(dataWorker).executeRootBundle(...Object.values(leafs[0]), tree.getHexProof(leafs[0]));
-
-    // Balances should have updated as expected.
-    expect(await weth.balanceOf(hubPool.address)).to.equal(consts.amountToLp.sub(wethToSendToL2));
+    expect(
+      await hubPool.connect(dataWorker).executeRootBundle(...Object.values(leafs[0]), tree.getHexProof(leafs[0]))
+    ).to.emit(hubPool, "RootBundleExecuted");
+
+    // Balances should have updated as expected. Note that pool should still have bond remaining since a leaf
+    // is unexecuted.
+    expect(await weth.balanceOf(hubPool.address)).to.equal(
+      consts.amountToLp.sub(wethToSendToL2).add(consts.bondAmount.add(consts.finalFee))
+    );
     expect(await weth.balanceOf(await mockAdapter.bridge())).to.equal(wethToSendToL2);
     expect(await dai.balanceOf(hubPool.address)).to.equal(consts.amountToLp.mul(10).sub(daiToSend));
     expect(await dai.balanceOf(await mockAdapter.bridge())).to.equal(daiToSend);
@@ -91,19 +97,64 @@ describe("HubPool Root Bundle Execution", function () {
     expect(relayTokensEvents[1].args?.to).to.equal(mockSpoke.address);
 
     // Check the leaf count was decremented correctly.
-    expect((await hubPool.rootBundleProposal()).unclaimedPoolRebalanceLeafCount).to.equal(0);
+    expect((await hubPool.rootBundleProposal()).unclaimedPoolRebalanceLeafCount).to.equal(1);
   });
 
-  it("Reverts if spoke pool not set for chain ID", async function () {
+  it("Executing two leaves with the same chain ID does not relay root bundle to spoke pool twice", async function () {
     const { leafs, tree } = await constructSimpleTree();
 
-    await hubPool.connect(dataWorker).proposeRootBundle(
-      [3117], // bundleEvaluationBlockNumbers used by bots to construct bundles. Length must equal the number of leafs.
-      1, // poolRebalanceLeafCount. There is exactly one leaf in the bundle (just sending WETH to one address).
-      tree.getHexRoot(), // poolRebalanceRoot. Generated from the merkle tree constructed before.
-      consts.mockRelayerRefundRoot, // Not relevant for this test.
-      consts.mockSlowRelayRoot // Not relevant for this test.
+    await hubPool
+      .connect(dataWorker)
+      .proposeRootBundle([3117, 3118], 2, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
+
+    // Advance time so the request can be executed and execute two leaves with same chain ID.
+    await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness + 1);
+    await hubPool.connect(dataWorker).executeRootBundle(...Object.values(leafs[0]), tree.getHexProof(leafs[0]));
+    await hubPool.connect(dataWorker).executeRootBundle(...Object.values(leafs[1]), tree.getHexProof(leafs[1]));
+
+    // Since the mock adapter is delegatecalled, when querying, its address should be the hubPool address.
+    const mockAdapterAtHubPool = mockAdapter.attach(hubPool.address);
+
+    // Check the mockAdapter was called with the correct arguments for each method. The event counts should be identical
+    // to the above test.
+    const relayMessageEvents = await mockAdapterAtHubPool.queryFilter(
+      mockAdapterAtHubPool.filters.RelayMessageCalled()
     );
+    expect(relayMessageEvents.length).to.equal(7); // Exactly seven message send from L1->L2. 6 for each whitelist route
+    // and 1 for the initiateRelayerRefund.
+    expect(relayMessageEvents[relayMessageEvents.length - 1].args?.target).to.equal(mockSpoke.address);
+    expect(relayMessageEvents[relayMessageEvents.length - 1].args?.message).to.equal(
+      mockSpoke.interface.encodeFunctionData("relayRootBundle", [
+        consts.mockRelayerRefundRoot,
+        consts.mockSlowRelayRoot,
+      ])
+    );
+  });
+
+  it("Executing all leaves returns bond to proposer", async function () {
+    const { leafs, tree } = await constructSimpleTree();
+
+    await hubPool
+      .connect(dataWorker)
+      .proposeRootBundle([3117, 3118], 2, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
+
+    // Advance time so the request can be executed and execute both leaves.
+    await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness + 1);
+    await hubPool.connect(dataWorker).executeRootBundle(...Object.values(leafs[0]), tree.getHexProof(leafs[0]));
+
+    // Second execution sends bond back to data worker.
+    const bondAmount = consts.bondAmount.add(consts.finalFee);
+    expect(
+      await hubPool.connect(dataWorker).executeRootBundle(...Object.values(leafs[1]), tree.getHexProof(leafs[1]))
+    ).to.changeTokenBalances(weth, [dataWorker, hubPool], [bondAmount, bondAmount.mul(-1)]);
+  });
+
+  it("Reverts if spoke pool not set for chain ID", async function () {
+    const { leafs, tree } = await constructSimpleTree();
+
+    await hubPool
+      .connect(dataWorker)
+      .proposeRootBundle([3117, 3118], 2, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
 
     // Set spoke pool to address 0x0
     await hubPool.setCrossChainContracts(consts.repaymentChainId, mockAdapter.address, ZERO_ADDRESS);
@@ -119,7 +170,7 @@ describe("HubPool Root Bundle Execution", function () {
     const { leafs, tree } = await constructSimpleTree();
     await hubPool
       .connect(dataWorker)
-      .proposeRootBundle([3117], 1, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
+      .proposeRootBundle([3117, 3118], 2, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
 
     // Set time 10 seconds before expiration. Should revert.
     await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness - 10);
@@ -137,7 +188,7 @@ describe("HubPool Root Bundle Execution", function () {
     const { leafs, tree } = await constructSimpleTree();
     await hubPool
       .connect(dataWorker)
-      .proposeRootBundle([3117], 1, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
+      .proposeRootBundle([3117, 3118], 2, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
     await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness + 1);
 
     // Take the valid root but change some element within it, such as the chainId. This will change the hash of the leaf
@@ -152,7 +203,7 @@ describe("HubPool Root Bundle Execution", function () {
     const { leafs, tree } = await constructSimpleTree();
     await hubPool
       .connect(dataWorker)
-      .proposeRootBundle([3117], 1, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
+      .proposeRootBundle([3117, 3118], 2, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
     await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness + 1);
 
     // First claim should be fine. Second claim should be reverted as you cant double claim a leaf.
@@ -165,13 +216,9 @@ describe("HubPool Root Bundle Execution", function () {
   it("Cannot execute while paused", async function () {
     const { leafs, tree } = await constructSimpleTree();
 
-    await hubPool.connect(dataWorker).proposeRootBundle(
-      [3117], // bundleEvaluationBlockNumbers used by bots to construct bundles. Length must equal the number of leafs.
-      1, // poolRebalanceLeafCount. There is exactly one leaf in the bundle (just sending WETH to one address).
-      tree.getHexRoot(), // poolRebalanceRoot. Generated from the merkle tree constructed before.
-      consts.mockRelayerRefundRoot, // Not relevant for this test.
-      consts.mockSlowRelayRoot // Not relevant for this test.
-    );
+    await hubPool
+      .connect(dataWorker)
+      .proposeRootBundle([3117, 3118], 2, tree.getHexRoot(), consts.mockRelayerRefundRoot, consts.mockSlowRelayRoot);
 
     // Advance time so the request can be executed and execute the request.
     await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness + 1);

test/MerkleLib.Proofs.ts

@@ -33,6 +33,7 @@ describe("MerkleLib Proofs", async function () {
         bundleLpFees,
         netSendAmounts,
         runningBalances,
+        groupIndex: BigNumber.from(0),
       });
     }
 

test/MerkleLib.utils.ts

@@ -4,6 +4,7 @@ import { MerkleTree } from "../utils/MerkleTree";
 import { RelayData } from "./fixtures/SpokePool.Fixture";
 export interface PoolRebalanceLeaf {
   chainId: BigNumber;
+  groupIndex: BigNumber;
   bundleLpFees: BigNumber[];
   netSendAmounts: BigNumber[];
   runningBalances: BigNumber[];
@@ -71,13 +72,15 @@ export function buildPoolRebalanceLeafs(
   l1Tokens: string[][],
   bundleLpFees: BigNumber[][],
   netSendAmounts: BigNumber[][],
-  runningBalances: BigNumber[][]
+  runningBalances: BigNumber[][],
+  groupIndex: number[]
 ): PoolRebalanceLeaf[] {
   return Array(destinationChainIds.length)
     .fill(0)
     .map((_, i) => {
       return {
         chainId: BigNumber.from(destinationChainIds[i]),
+        groupIndex: BigNumber.from(groupIndex[i]),
         bundleLpFees: bundleLpFees[i],
         netSendAmounts: netSendAmounts[i],
         runningBalances: runningBalances[i],
@@ -109,7 +112,8 @@ export async function constructSingleChainTree(token: string, scalingSize = 1, r
     [[token]], // l1Token. We will only be sending 1 token to one chain.
     [[realizedLpFees]], // bundleLpFees.
     [[tokensSendToL2]], // netSendAmounts.
-    [[tokensSendToL2]] // runningBalances.
+    [[tokensSendToL2]], // runningBalances.
+    [0] // groupIndex
   );
   const tree = await buildPoolRebalanceLeafTree(leafs);
 

test/gas-analytics/HubPool.RootExecution.ts

@@ -46,7 +46,8 @@ async function constructSimpleTree(_destinationChainIds: number[], _l1Tokens: Co
     _l1TokenAddresses,
     _bundleLpFeeAmounts,
     _netSendAmounts, // netSendAmounts.
-    _netSendAmounts // runningBalances.
+    _netSendAmounts, // runningBalances.
+    Array(REFUND_CHAIN_COUNT).fill(0) // relayToSpokePool
   );
   const tree = await buildPoolRebalanceLeafTree(leaves);
 
@@ -203,7 +204,10 @@ describe("Gas Analytics: HubPool Root Bundle Execution", function () {
       // Advance time so the request can be executed and execute the request.
       await timer.setCurrentTime(Number(await timer.getCurrentTime()) + consts.refundProposalLiveness + 1);
       const multicallData = leaves.map((leaf) => {
-        return hubPool.interface.encodeFunctionData("executeRootBundle", [leaf, tree.getHexProof(leaf)]);
+        return hubPool.interface.encodeFunctionData("executeRootBundle", [
+          ...Object.values(leaf),
+          tree.getHexProof(leaf),
+        ]);
       });
 
       const receipt = await (await hubPool.connect(dataWorker).multicall(multicallData)).wait();