Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Verify state hash is correct before publishing to L1 #3915

Merged
merged 2 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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')}`);
}
Comment on lines +247 to +250
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were these logs just for debugging during dev, or are they meant to be there?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initially for debugging. But if someone does ever get an invalid state hash when attempting to publish it would probably be useful to have the info so I left them here.

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