diff --git a/aggregator/src/app/BundleService.ts b/aggregator/src/app/BundleService.ts index b1771721..922dc711 100644 --- 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 { - const bundleSubHashes = await this.#hashSubBundles(bundle); - const concatenatedHashes = bundleSubHashes.join(""); - return ethers.utils.keccak256(ethers.utils.toUtf8Bytes(concatenatedHashes)); - } - - async #hashSubBundles(bundle: Bundle): Promise> { - const bundlesWithoutSignature: Array = - 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() { diff --git a/aggregator/test/BundleService.test.ts b/aggregator/test/BundleService.test.ts index 0f3e2191..14e3ddf9 100644 --- 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); diff --git a/aggregator/test/BundleServiceSubmitting.test.ts b/aggregator/test/BundleServiceSubmitting.test.ts index 767a0124..35be9e5d 100644 --- 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]); + } }); \ No newline at end of file diff --git a/contracts/clients/src/BlsProvider.ts b/contracts/clients/src/BlsProvider.ts index f7b12ace..f839b6b9 100644 --- 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( diff --git a/contracts/clients/src/helpers/hashBundle.ts b/contracts/clients/src/helpers/hashBundle.ts index 2235cd2b..6fd3a5f4 100644 --- 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; -}; - -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 = - 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); } diff --git a/contracts/clients/src/index.ts b/contracts/clients/src/index.ts index ef5d7372..9eed56e8 100644 --- 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, diff --git a/contracts/test-integration/BlsProvider.test.ts b/contracts/test-integration/BlsProvider.test.ts index c1a77e18..07073ec5 100644 --- 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); + }); });