Update hash logic and add integration test

2026-01-08 23:28:21 -05:00 · 2023-05-02 17:55:57 +01:00
parent 4d170f73dd
commit 576e778855
7 changed files with 228 additions and 145 deletions
--- a/aggregator/src/app/BundleService.ts
+++ b/aggregator/src/app/BundleService.ts
@@ -8,6 +8,7 @@ import {
  ethers,
  Operation,
  Semaphore,
+  VerificationGatewayFactory,
 } from "../../deps.ts";

 import { IClock } from "../helpers/Clock.ts";
@@ -24,6 +25,7 @@ import BundleTable, { BundleRow } from "./BundleTable.ts";
 import plus from "./helpers/plus.ts";
 import AggregationStrategy from "./AggregationStrategy.ts";
 import nil from "../helpers/nil.ts";
+import ExplicitAny from "../helpers/ExplicitAny.ts";

 export type AddBundleResponse = { hash: string } | {
  failures: TransactionFailure[];
@@ -245,43 +247,41 @@ export default class BundleService {
  }

  async hashBundle(bundle: Bundle): Promise<string> {
-    const bundleSubHashes = await this.#hashSubBundles(bundle);
-    const concatenatedHashes = bundleSubHashes.join("");
-    return ethers.utils.keccak256(ethers.utils.toUtf8Bytes(concatenatedHashes));
-  }
-
-  async #hashSubBundles(bundle: Bundle): Promise<Array<string>> {
-    const bundlesWithoutSignature: Array<BundleWithoutSignature> =
-      bundle.operations.map((operation, index) => ({
-        senderPublicKeys: bundle.senderPublicKeys[index],
-        operations: {
-          nonce: operation.nonce,
-          actions: operation.actions,
-        },
-      }));
+    const operationsWithZeroGas = bundle.operations.map((operation) => {
+      return {
+        ...operation,
+        gas: BigNumber.from(0),
+      };
+    });
  
-    const serializedBundlesWithoutSignature = bundlesWithoutSignature.map(
-        bundleWithoutSignature => {
-          return JSON.stringify({
-            senderPublicKeys: bundleWithoutSignature.senderPublicKeys,
-            operations: bundleWithoutSignature.operations,
-          });
-        }
+    const verifyMethodName = "verify";
+    const bundleType = VerificationGatewayFactory.abi.find(
+      (entry) => "name" in entry && entry.name === verifyMethodName,
+    )?.inputs[0];
+  
+    const validatedBundle = {
+      ...bundle,
+      operations: operationsWithZeroGas,
+    };
+  
+    const encodedBundleWithZeroSignature = ethers.utils.defaultAbiCoder.encode(
+      [bundleType as ExplicitAny],
+      [
+        {
+          ...validatedBundle,
+          signature: [BigNumber.from(0), BigNumber.from(0)],
+        },
+      ],
    );
  
-    const hashes = await Promise.all(serializedBundlesWithoutSignature.map(async (serializedBundleWithoutSignature) => {
-      const bundleHash = ethers.utils.keccak256(
-        ethers.utils.toUtf8Bytes(serializedBundleWithoutSignature)
-      );
-      const chainId = (await this.ethereumService.provider.getNetwork()).chainId;
-      
-      const encoding = ethers.utils.defaultAbiCoder.encode(
-        ['bytes32', 'uint256'],
-        [bundleHash, chainId])
-      return ethers.utils.keccak256(encoding)
-    }));
-  
-    return hashes
+    const bundleHash = ethers.utils.keccak256(encodedBundleWithZeroSignature);
+    const chainId = (await this.ethereumService.provider.getNetwork()).chainId;
+
+    const bundleAndChainIdEncoding = ethers.utils.defaultAbiCoder.encode(
+      ["bytes32", "uint256"],
+      [bundleHash, chainId],
+    );
+    return ethers.utils.keccak256(bundleAndChainIdEncoding);
  }

  async runSubmission() {
--- a/aggregator/test/BundleService.test.ts
+++ b/aggregator/test/BundleService.test.ts
@@ -1,5 +1,5 @@
-import { assertBundleSucceeds, assertEquals, ethers, Operation } from "./deps.ts";
-
+import { BigNumber, Operation, VerificationGatewayFactory, assertBundleSucceeds, assertEquals, ethers } from "./deps.ts";
+import ExplicitAny from "../src/helpers/ExplicitAny.ts";
 import Fixture from "./helpers/Fixture.ts";

 Fixture.test("adds valid bundle", async (fx) => {
@@ -272,31 +272,40 @@ Fixture.test("hashes bundle with single operation", async (fx) => {
    ],
  });

-  const expectedSubBundleHashes = await Promise.all(bundle.operations.map(async (operation, index) => {
-    const bundlesWithoutSignature = {
-      senderPublicKeys: bundle.senderPublicKeys[index],
-        operations: {
-          nonce: operation.nonce,
-          actions: operation.actions,
-        },
-    }
+  const operationsWithZeroGas = bundle.operations.map((operation) => {
+    return {
+      ...operation,
+      gas: BigNumber.from(0),
+    };
+  });

-    const serializedBundle = JSON.stringify({
-      senderPublicKeys: bundlesWithoutSignature.senderPublicKeys,
-      operations: bundlesWithoutSignature.operations,
-    });
+  const bundleType = VerificationGatewayFactory.abi.find(
+    (entry) => "name" in entry && entry.name === "verify",
+  )?.inputs[0];

-    const bundleHash = ethers.utils.keccak256(ethers.utils.toUtf8Bytes(serializedBundle))
-    const chainId = (await bundleService.ethereumService.provider.getNetwork()).chainId;
+  const validatedBundle = {
+    ...bundle,
+    operations: operationsWithZeroGas,
+  };

-    const encoding = ethers.utils.defaultAbiCoder.encode(
-      ['bytes32', 'uint256'],
-      [bundleHash, chainId])
-    return ethers.utils.keccak256(encoding)
-  }));
+  const encodedBundleWithZeroSignature = ethers.utils.defaultAbiCoder.encode(
+    [bundleType as ExplicitAny],
+    [
+      {
+        ...validatedBundle,
+        signature: [BigNumber.from(0), BigNumber.from(0)],
+      },
+    ],
+  );

-  const concatenatedHashes = expectedSubBundleHashes.join("");
-  const expectedBundleHash = ethers.utils.keccak256(ethers.utils.toUtf8Bytes(concatenatedHashes));
+  const bundleHash = ethers.utils.keccak256(encodedBundleWithZeroSignature);
+  const chainId = (await bundleService.ethereumService.provider.getNetwork()).chainId;
+
+  const bundleAndChainIdEncoding = ethers.utils.defaultAbiCoder.encode(
+    ["bytes32", "uint256"],
+    [bundleHash, chainId],
+  );
+  const expectedBundleHash = ethers.utils.keccak256(bundleAndChainIdEncoding);
  
  const hash = await bundleService.hashBundle(bundle);
  
@@ -339,32 +348,40 @@ Fixture.test("hashes bundle with multiple operations", async (fx) => {
    }),
  ]);

-  const expectedSubBundleHashes = await Promise.all(bundle.operations.map(async (operation, index) => {
-    const bundlesWithoutSignature = {
-      senderPublicKeys: bundle.senderPublicKeys[index],
-      operations: {
-          nonce: operation.nonce,
-          actions: operation.actions,
-        },
-    }
+  const operationsWithZeroGas = bundle.operations.map((operation) => {
+    return {
+      ...operation,
+      gas: BigNumber.from(0),
+    };
+  });

-    const serializedBundle = JSON.stringify({
-      senderPublicKeys: bundlesWithoutSignature.senderPublicKeys,
-      operations: bundlesWithoutSignature.operations,
-    });
+  const bundleType = VerificationGatewayFactory.abi.find(
+    (entry) => "name" in entry && entry.name === "verify",
+  )?.inputs[0];

-    const bundleHash = ethers.utils.keccak256(ethers.utils.toUtf8Bytes(serializedBundle))
+  const validatedBundle = {
+    ...bundle,
+    operations: operationsWithZeroGas,
+  };

-    const chainId = (await bundleService.ethereumService.provider.getNetwork()).chainId;
-    const encoding = ethers.utils.defaultAbiCoder.encode(
-      ['bytes32', 'uint256'],
-      [bundleHash, chainId])
+  const encodedBundleWithZeroSignature = ethers.utils.defaultAbiCoder.encode(
+    [bundleType as ExplicitAny],
+    [
+      {
+        ...validatedBundle,
+        signature: [BigNumber.from(0), BigNumber.from(0)],
+      },
+    ],
+  );

-    return ethers.utils.keccak256(encoding)
-  }));
+  const bundleHash = ethers.utils.keccak256(encodedBundleWithZeroSignature);
+  const chainId = (await bundleService.ethereumService.provider.getNetwork()).chainId;

-  const concatenatedHashes = expectedSubBundleHashes.join("");
-  const expectedBundleHash = ethers.utils.keccak256(ethers.utils.toUtf8Bytes(concatenatedHashes));
+  const bundleAndChainIdEncoding = ethers.utils.defaultAbiCoder.encode(
+    ["bytes32", "uint256"],
+    [bundleHash, chainId],
+  );
+  const expectedBundleHash = ethers.utils.keccak256(bundleAndChainIdEncoding);

  const hash = await bundleService.hashBundle(bundle);

@@ -375,32 +392,40 @@ Fixture.test("hashes empty bundle", async (fx) => {
  const bundleService = fx.createBundleService();
  const bundle = fx.blsWalletSigner.aggregate([]);

-  const expectedSubBundleHashes = bundle.operations.map(async (operation, index) => {
-    const bundlesWithoutSignature = {
-      senderPublicKeys: bundle.senderPublicKeys[index],
-      operations: {
-          nonce: operation.nonce,
-          actions: operation.actions,
-        },
-    }
+  const operationsWithZeroGas = bundle.operations.map((operation) => {
+    return {
+      ...operation,
+      gas: BigNumber.from(0),
+    };
+  });

-    const serializedBundle = JSON.stringify({
-      senderPublicKeys: bundlesWithoutSignature.senderPublicKeys,
-      operations: bundlesWithoutSignature.operations,
-    });
-    
-    const bundleHash = ethers.utils.keccak256(ethers.utils.toUtf8Bytes(serializedBundle))
+  const bundleType = VerificationGatewayFactory.abi.find(
+    (entry) => "name" in entry && entry.name === "verify",
+  )?.inputs[0];

-    const chainId = (await bundleService.ethereumService.provider.getNetwork()).chainId;
-    const encoding = ethers.utils.defaultAbiCoder.encode(
-      ['bytes32', 'uint256'],
-      [bundleHash, chainId])
+  const validatedBundle = {
+    ...bundle,
+    operations: operationsWithZeroGas,
+  };

-    return ethers.utils.keccak256(encoding)
-  })
+  const encodedBundleWithZeroSignature = ethers.utils.defaultAbiCoder.encode(
+    [bundleType as ExplicitAny],
+    [
+      {
+        ...validatedBundle,
+        signature: [BigNumber.from(0), BigNumber.from(0)],
+      },
+    ],
+  );

-  const concatenatedHashes = expectedSubBundleHashes.join("");
-  const expectedBundleHash = ethers.utils.keccak256(ethers.utils.toUtf8Bytes(concatenatedHashes));
+  const bundleHash = ethers.utils.keccak256(encodedBundleWithZeroSignature);
+  const chainId = (await bundleService.ethereumService.provider.getNetwork()).chainId;
+
+  const bundleAndChainIdEncoding = ethers.utils.defaultAbiCoder.encode(
+    ["bytes32", "uint256"],
+    [bundleHash, chainId],
+  );
+  const expectedBundleHash = ethers.utils.keccak256(bundleAndChainIdEncoding);

  const hash = await bundleService.hashBundle(bundle);

--- a/aggregator/test/BundleServiceSubmitting.test.ts
+++ b/aggregator/test/BundleServiceSubmitting.test.ts
@@ -399,16 +399,16 @@ Fixture.test("Retrieves all sub bundles included in a submitted bundle from sing

  await bundleService.submissionTimer.trigger();
  await bundleService.waitForConfirmations();
-
-  const aggregateBundle = bundleService.lookupAggregateBundle(subBundleHashes[0]);
-  
  const firstSubBundle = bundleService.lookupBundle(subBundleHashes[0]);
  const secondSubBundle = bundleService.lookupBundle(subBundleHashes[1]);
  const thirdSubBundle = bundleService.lookupBundle(subBundleHashes[2]);

-  const orderedSubBundles = [firstSubBundle, secondSubBundle, thirdSubBundle].sort((a, b) => a!.id! - b!.id!);
+  const orderedSubBundles = [firstSubBundle, secondSubBundle, thirdSubBundle].sort((a, b) => a!.id - b!.id);

-  assertEquals(aggregateBundle?.[0], orderedSubBundles[0]);
-  assertEquals(aggregateBundle?.[1], orderedSubBundles[1]);
-  assertEquals(aggregateBundle?.[2], orderedSubBundles[2]);
+  for (const subBundleHash of subBundleHashes) {
+    const aggregateBundle = bundleService.lookupAggregateBundle(subBundleHash);
+    assertEquals(aggregateBundle?.[0], orderedSubBundles[0]);
+    assertEquals(aggregateBundle?.[1], orderedSubBundles[1]);
+    assertEquals(aggregateBundle?.[2], orderedSubBundles[2]);
+  }
 });
--- a/contracts/clients/src/BlsProvider.ts
+++ b/contracts/clients/src/BlsProvider.ts
@@ -343,7 +343,7 @@ export default class BlsProvider extends ethers.providers.JsonRpcProvider {
    return [
      ...actions,
      {
-        ethValue: fee,
+        ethValue: fee.toHexString(),
        contractAddress: this.aggregatorUtilitiesAddress,
        encodedFunction:
          aggregatorUtilitiesContract.interface.encodeFunctionData(
--- a/contracts/clients/src/helpers/hashBundle.ts
+++ b/contracts/clients/src/helpers/hashBundle.ts
@@ -1,50 +1,58 @@
-import { BigNumberish, ethers } from "ethers";
-import { Bundle, Operation } from "../signer";
+import { BigNumber, ethers } from "ethers";
+import { Bundle } from "../signer";
+import { VerificationGatewayFactory } from "../index";

-type BundleWithoutSignature = {
-  senderPublicKeys: [BigNumberish, BigNumberish, BigNumberish, BigNumberish];
-  operations: Omit<Operation, "gas">;
-};
-
-export function hashBundle(bundle: Bundle, chainId: number): string {
+/**
+ * Generates a deterministic hash of a bundle. Because the signature of the bundle could change, along with the gas property on operations,
+ * those values are set to 0 before hashing. This leads to a more consistent hash for variations of the same bundle.
+ *
+ * @remarks the hash output is senstive to the internal types of the bundle. For example, an identical bundle with a
+ * BigNumber value for one of the properties, vs the same bundle with a hex string value for one of the properties, will
+ * generate different hashes, even though the underlying value may be the same.
+ *
+ * @param bundle the signed bundle to generate the hash for
+ * @param chainId the chain id of the network the bundle is being submitted to
+ * @returns a deterministic hash of the bundle
+ */
+export default function hashBundle(bundle: Bundle, chainId: number): string {
  if (bundle.operations.length !== bundle.senderPublicKeys.length) {
    throw new Error(
      "number of operations does not match number of public keys",
    );
  }

-  const bundlesWithoutSignature: Array<BundleWithoutSignature> =
-    bundle.operations.map((operation, index) => ({
-      senderPublicKeys: bundle.senderPublicKeys[index],
-      operations: {
-        nonce: operation.nonce,
-        actions: operation.actions,
+  const operationsWithZeroGas = bundle.operations.map((operation) => {
+    return {
+      ...operation,
+      gas: BigNumber.from(0),
+    };
+  });
+
+  const verifyMethodName = "verify";
+  const bundleType = VerificationGatewayFactory.abi.find(
+    (entry) => "name" in entry && entry.name === verifyMethodName,
+  )?.inputs[0];
+
+  const validatedBundle = {
+    ...bundle,
+    operations: operationsWithZeroGas,
+  };
+
+  const encodedBundleWithZeroSignature = ethers.utils.defaultAbiCoder.encode(
+    [bundleType as any],
+    [
+      {
+        ...validatedBundle,
+        signature: [BigNumber.from(0), BigNumber.from(0)],
      },
-    }));
-
-  const serializedBundlesWithoutSignature = bundlesWithoutSignature.map(
-    (bundleWithoutSignature) => {
-      return JSON.stringify({
-        senderPublicKeys: bundleWithoutSignature.senderPublicKeys,
-        operations: bundleWithoutSignature.operations,
-      });
-    },
+    ],
  );

-  const bundleSubHashes = serializedBundlesWithoutSignature.map(
-    async (serializedBundleWithoutSignature) => {
-      const bundleHash = ethers.utils.keccak256(
-        ethers.utils.toUtf8Bytes(serializedBundleWithoutSignature),
-      );
+  const bundleHash = ethers.utils.keccak256(encodedBundleWithZeroSignature);

-      const encoding = ethers.utils.defaultAbiCoder.encode(
-        ["bytes32", "uint256"],
-        [bundleHash, chainId],
-      );
-      return ethers.utils.keccak256(encoding);
-    },
+  const encoding = ethers.utils.defaultAbiCoder.encode(
+    ["bytes32", "uint256"],
+    [bundleHash, chainId],
  );
-
-  const concatenatedHashes = bundleSubHashes.join("");
-  return ethers.utils.keccak256(ethers.utils.toUtf8Bytes(concatenatedHashes));
+  return ethers.utils.keccak256(encoding);
 }
--- a/contracts/clients/src/index.ts
+++ b/contracts/clients/src/index.ts
@@ -18,6 +18,8 @@ export {
  OperationResultError,
 } from "./OperationResults";

+export { default as hashBundle } from "./helpers/hashBundle";
+
 export {
  VerificationGateway__factory as VerificationGatewayFactory,
  AggregatorUtilities__factory as AggregatorUtilitiesFactory,
--- a/contracts/test-integration/BlsProvider.test.ts
+++ b/contracts/test-integration/BlsProvider.test.ts
@@ -1,6 +1,7 @@
 import chai, { expect } from "chai";
 import { BigNumber, ethers } from "ethers";
 import { formatEther, parseEther } from "ethers/lib/utils";
+import sinon from "sinon";

 import {
  BlsWalletWrapper,
@@ -9,6 +10,7 @@ import {
  BlsSigner,
  MockERC20Factory,
  NetworkConfig,
+  hashBundle,
 } from "../clients/src";
 import getNetworkConfig from "../shared/helpers/getNetworkConfig";

@@ -65,6 +67,7 @@ describe("BlsProvider", () => {

  afterEach(() => {
    chai.spy.restore();
+    sinon.restore();
  });

  it("calls a getter method on a contract using call()", async () => {
@@ -704,4 +707,49 @@ describe("BlsProvider", () => {
    );
    expect(feeData.gasPrice).to.deep.equal(expectedFeeData.gasPrice);
  });
+
+  it("should return a deterministic hash generated by the aggregator that can be replicated by the client module", async () => {
+    // Arrange
+    const transaction = {
+      to: ethers.Wallet.createRandom().address,
+      value: parseEther("1"),
+      from: await blsSigner.getAddress(),
+    };
+
+    const action = {
+      ethValue: transaction.value?.toHexString(),
+      contractAddress: transaction.to!.toString(),
+      encodedFunction: "0x",
+    };
+
+    // BlsWalletWrapper.getRandomBlsPrivateKey from "estimateGas" method results in slightly different
+    // fee estimates. This fake avoids this mismatch by stubbing a constant value.
+    sinon.replace(
+      BlsWalletWrapper,
+      "getRandomBlsPrivateKey",
+      sinon.fake.resolves(blsSigner.wallet.blsWalletSigner.privateKey),
+    );
+
+    const feeEstimate = await blsProvider.estimateGas(transaction);
+    const actionsWithSafeFee = blsProvider._addFeePaymentActionWithSafeFee(
+      [action],
+      feeEstimate,
+    );
+
+    const bundle = blsSigner.wallet.sign({
+      nonce: await blsSigner.wallet.Nonce(),
+      gas: 100000,
+      actions: [...actionsWithSafeFee],
+    });
+
+    // Act
+    const transactionResponse = await blsSigner.sendTransaction(transaction);
+
+    // Assert
+    const expectedTransactionHash = hashBundle(
+      bundle,
+      blsProvider.network.chainId,
+    );
+    expect(transactionResponse.hash).to.deep.equal(expectedTransactionHash);
+  });
 });