Skip to content

Commit

Permalink
feat: Verify state hash is correct before publishing to L1 (#3915)
Browse files Browse the repository at this point in the history
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](#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).
  • Loading branch information
PhilWindle authored Jan 10, 2024
1 parent 1fd135c commit a53c261
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 27 deletions.
12 changes: 8 additions & 4 deletions yarn-project/sequencer-client/src/publisher/l1-publisher.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
});
Expand All @@ -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);

Expand Down Expand Up @@ -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', () => {});
});
49 changes: 26 additions & 23 deletions yarn-project/sequencer-client/src/publisher/l1-publisher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,12 @@ export interface L1PublisherTxSender {
* @param txHash - Hash of the tx to look for.
*/
getTransactionStats(txHash: string): Promise<TransactionStats | undefined>;

/**
* Returns the current state hash.
* @returns The current state hash of the rollup contract.
*/
getCurrentStateHash(): Promise<Buffer>;
}

/**
Expand Down Expand Up @@ -125,12 +131,13 @@ export class L1Publisher implements L2BlockReceiver {
public async processL2Block(l2BlockData: L2Block): Promise<boolean> {
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);
Expand All @@ -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;
}

Expand All @@ -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) {
Expand Down Expand Up @@ -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<boolean> {
return true;
}

// TODO: Fail if blockchainStatus.nextBlockNum > thisBlockNum.
// Related to https://github.com/AztecProtocol/aztec-packages/issues/1588
private checkNextL2BlockNum(_thisBlockNum: number): Promise<boolean> {
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<boolean> {
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<string | undefined> {
Expand Down
5 changes: 5 additions & 0 deletions yarn-project/sequencer-client/src/publisher/viem-tx-sender.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,11 @@ export class ViemTxSender implements L1PublisherTxSender {
});
}

async getCurrentStateHash(): Promise<Buffer> {
const stateHash = await this.rollupContract.read.rollupStateHash();
return Buffer.from(stateHash.replace('0x', ''), 'hex');
}

async getTransactionStats(txHash: string): Promise<TransactionStats | undefined> {
const tx = await this.publicClient.getTransaction({ hash: txHash as Hex });
if (!tx) {
Expand Down

0 comments on commit a53c261

Please sign in to comment.