From a53c26139e2d02de75faa761209cdb09d0a1b94e Mon Sep 17 00:00:00 2001 From: PhilWindle <60546371+PhilWindle@users.noreply.github.com> Date: Wed, 10 Jan 2024 13:14:39 +0000 Subject: [PATCH] feat: Verify state hash is correct before publishing to L1 (#3915) This PR changes the block publisher to verify the block's start hash is consistent with that on chain before publishing. It also removes the redundant test around checking the balance of the fee distributor which was inherited from aztec connect. Closes [this](https://github.com/AztecProtocol/aztec-packages/issues/1600) issue. # Checklist: Remove the checklist to signal you've completed it. Enable auto-merge if the PR is ready to merge. - [ ] If the pull request requires a cryptography review (e.g. cryptographic algorithm implementations) I have added the 'crypto' tag. - [ ] I have reviewed my diff in github, line by line and removed unexpected formatting changes, testing logs, or commented-out code. - [ ] Every change is related to the PR description. - [ ] I have [linked](https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue) this pull request to relevant issues (if any exist). --- .../src/publisher/l1-publisher.test.ts | 12 +++-- .../src/publisher/l1-publisher.ts | 49 ++++++++++--------- .../src/publisher/viem-tx-sender.ts | 5 ++ 3 files changed, 39 insertions(+), 27 deletions(-) diff --git a/yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts b/yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts index 7ffe7a97a5f..5c460c5d64c 100644 --- a/yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts +++ b/yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts @@ -24,6 +24,7 @@ describe('L1Publisher', () => { txReceipt = { transactionHash: txHash, status: true } as MinimalTransactionReceipt; txSender.sendProcessTx.mockResolvedValueOnce(txHash); txSender.getTransactionReceipt.mockResolvedValueOnce(txReceipt); + txSender.getCurrentStateHash.mockResolvedValue(l2Block.getStartStateHash()); publisher = new L1Publisher(txSender, { l1BlockPublishRetryIntervalMS: 1 }); }); @@ -36,6 +37,13 @@ describe('L1Publisher', () => { expect(txSender.getTransactionReceipt).toHaveBeenCalledWith(txHash); }); + it('does not publish if start hash is different to expected', async () => { + txSender.getCurrentStateHash.mockResolvedValueOnce(L2Block.random(43).getStartStateHash()); + const result = await publisher.processL2Block(l2Block); + expect(result).toBe(false); + expect(txSender.sendProcessTx).not.toHaveBeenCalled(); + }); + it('does not retry if sending a tx fails', async () => { txSender.sendProcessTx.mockReset().mockRejectedValueOnce(new Error()).mockResolvedValueOnce(txHash); @@ -72,8 +80,4 @@ describe('L1Publisher', () => { expect(result).toEqual(false); expect(txSender.getTransactionReceipt).not.toHaveBeenCalled(); }); - - it.skip('waits for fee distributor balance', () => {}); - - it.skip('fails if contract is changed underfoot', () => {}); }); diff --git a/yarn-project/sequencer-client/src/publisher/l1-publisher.ts b/yarn-project/sequencer-client/src/publisher/l1-publisher.ts index b87a473c888..0cc6d697ec3 100644 --- a/yarn-project/sequencer-client/src/publisher/l1-publisher.ts +++ b/yarn-project/sequencer-client/src/publisher/l1-publisher.ts @@ -73,6 +73,12 @@ export interface L1PublisherTxSender { * @param txHash - Hash of the tx to look for. */ getTransactionStats(txHash: string): Promise; + + /** + * Returns the current state hash. + * @returns The current state hash of the rollup contract. + */ + getCurrentStateHash(): Promise; } /** @@ -125,12 +131,13 @@ export class L1Publisher implements L2BlockReceiver { public async processL2Block(l2BlockData: L2Block): Promise { const proof = Buffer.alloc(0); const txData = { proof, inputs: l2BlockData.toBufferWithLogs() }; + const startStateHash = l2BlockData.getStartStateHash(); while (!this.interrupted) { - if (!(await this.checkFeeDistributorBalance())) { - this.log(`Fee distributor ETH balance too low, awaiting top up...`); - await this.sleepOrInterrupted(); - continue; + // TODO: Remove this block number check, it's here because we don't currently have proper genesis state on the contract + if (l2BlockData.number != 1 && !(await this.checkStartStateHash(startStateHash))) { + this.log(`Detected different state hash prior to publishing rollup, aborting publish...`); + break; } const txHash = await this.sendProcessTx(txData); @@ -157,8 +164,8 @@ export class L1Publisher implements L2BlockReceiver { } // Check if someone else incremented the block number - if (!(await this.checkNextL2BlockNum(l2BlockData.number))) { - this.log('Publish failed. Contract changed underfoot.'); + if (!(await this.checkStartStateHash(startStateHash))) { + this.log('Publish failed. Detected different state hash.'); break; } @@ -180,12 +187,6 @@ export class L1Publisher implements L2BlockReceiver { public async processNewContractData(l2BlockNum: number, l2BlockHash: Buffer, contractData: ExtendedContractData[]) { let _contractData: ExtendedContractData[] = []; while (!this.interrupted) { - if (!(await this.checkFeeDistributorBalance())) { - this.log(`Fee distributor ETH balance too low, awaiting top up...`); - await this.sleepOrInterrupted(); - continue; - } - const arr = _contractData.length ? _contractData : contractData; const txHashes = await this.sendEmitNewContractDataTx(l2BlockNum, l2BlockHash, arr); if (!txHashes) { @@ -235,17 +236,19 @@ export class L1Publisher implements L2BlockReceiver { this.interrupted = false; } - // TODO: Check fee distributor has at least 0.5 ETH. - // Related to https://github.com/AztecProtocol/aztec-packages/issues/1588 - // eslint-disable-next-line require-await - private async checkFeeDistributorBalance(): Promise { - return true; - } - - // TODO: Fail if blockchainStatus.nextBlockNum > thisBlockNum. - // Related to https://github.com/AztecProtocol/aztec-packages/issues/1588 - private checkNextL2BlockNum(_thisBlockNum: number): Promise { - return Promise.resolve(true); + /** + * Verifies that the given value of start state hash equals that on the rollup contract + * @param startStateHash - The start state hash of the block we wish to publish. + * @returns Boolean indicating if the hashes are equal. + */ + private async checkStartStateHash(startStateHash: Buffer): Promise { + const fromChain = await this.txSender.getCurrentStateHash(); + const areSame = startStateHash.equals(fromChain); + if (!areSame) { + this.log(`CONTRACT STATE HASH: ${fromChain.toString('hex')}`); + this.log(`NEW BLOCK STATE HASH: ${startStateHash.toString('hex')}`); + } + return areSame; } private async sendProcessTx(encodedData: L1ProcessArgs): Promise { diff --git a/yarn-project/sequencer-client/src/publisher/viem-tx-sender.ts b/yarn-project/sequencer-client/src/publisher/viem-tx-sender.ts index ee3a984170e..fc4db14df53 100644 --- a/yarn-project/sequencer-client/src/publisher/viem-tx-sender.ts +++ b/yarn-project/sequencer-client/src/publisher/viem-tx-sender.ts @@ -75,6 +75,11 @@ export class ViemTxSender implements L1PublisherTxSender { }); } + async getCurrentStateHash(): Promise { + const stateHash = await this.rollupContract.read.rollupStateHash(); + return Buffer.from(stateHash.replace('0x', ''), 'hex'); + } + async getTransactionStats(txHash: string): Promise { const tx = await this.publicClient.getTransaction({ hash: txHash as Hex }); if (!tx) {