From 99d549eb4400f110d9c4d439c847e990d8b977ec Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Fri, 24 Jan 2025 12:27:43 +0000 Subject: [PATCH 1/4] Don't publish a block if we failed to create the block proposal --- .../src/sequencer/sequencer.test.ts | 31 +++++++++++++++++++ .../src/sequencer/sequencer.ts | 8 +++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index cd3debc369ef..5e0f67379351 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -491,6 +491,18 @@ describe('sequencer', () => { expect(publisher.proposeL2Block).not.toHaveBeenCalled(); }); + it('does not publish a block that received no attestations', async () => { + const tx = makeTx(); + mockPendingTxs([tx]); + block = await makeBlock([tx]); + + validatorClient.createBlockProposal.mockResolvedValue(undefined); + + await sequencer.doRealWork(); + + expect(publisher.proposeL2Block).not.toHaveBeenCalled(); + }); + describe('proof quotes', () => { let tx: Tx; let txHash: TxHash; @@ -587,6 +599,25 @@ describe('sequencer', () => { expect(publisher.proposeL2Block).not.toHaveBeenCalled(); }); + it('submits a valid proof quote if building a block proposal fails', async () => { + const blockNumber = epochDuration + 1; + await setupForBlockNumber(blockNumber); + + const proofQuote = mockEpochProofQuote(); + + p2p.getEpochProofQuotes.mockResolvedValue([proofQuote]); + publisher.validateProofQuote.mockImplementation((x: EpochProofQuote) => Promise.resolve(x)); + + // The previous epoch can be claimed + publisher.getClaimableEpoch.mockImplementation(() => Promise.resolve(currentEpoch - 1n)); + + validatorClient.createBlockProposal.mockResolvedValue(undefined); + + await sequencer.doRealWork(); + expect(publisher.claimEpochProofRight).toHaveBeenCalledWith(proofQuote); + expect(publisher.proposeL2Block).not.toHaveBeenCalled(); + }); + it('does not claim the epoch previous to the first', async () => { const blockNumber = 1; await setupForBlockNumber(blockNumber); diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index 6388bf22086e..c5a2bf059b7f 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -300,6 +300,9 @@ export class Sequencer { await this.buildBlockAndAttemptToPublish(pendingTxs, proposalHeader); } catch (err) { this.log.error(`Error assembling block`, err, { blockNumber: newBlockNumber, slot }); + + // If the block failed to build, we might still want to claim the proving rights + await this.claimEpochProofRightIfAvailable(slot); } this.setState(SequencerState.IDLE, 0n); } @@ -627,8 +630,9 @@ export class Sequencer { this.log.debug('Creating block proposal for validators'); const proposal = await this.validatorClient.createBlockProposal(block.header, block.archive.root, txHashes); if (!proposal) { - this.log.warn(`Failed to create block proposal, skipping collecting attestations`); - return undefined; + const msg = `Failed to create block proposal`; + this.log.error(msg); + throw new Error(msg); } this.log.debug('Broadcasting block proposal to validators'); From d6bbe7a4ea538ef6922e2beaf89724562a5fa0ec Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Fri, 24 Jan 2025 12:30:27 +0000 Subject: [PATCH 2/4] Test rename --- yarn-project/sequencer-client/src/sequencer/sequencer.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts index 5e0f67379351..f3c0a67cfcb4 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.test.ts @@ -491,7 +491,7 @@ describe('sequencer', () => { expect(publisher.proposeL2Block).not.toHaveBeenCalled(); }); - it('does not publish a block that received no attestations', async () => { + it('does not publish a block if the block proposal failed', async () => { const tx = makeTx(); mockPendingTxs([tx]); block = await makeBlock([tx]); From ccc9cbffb23352ba583a2d341eaa1fde8f2a87a1 Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Fri, 24 Jan 2025 12:39:21 +0000 Subject: [PATCH 3/4] Spelling --- yarn-project/simulator/src/public/public_processor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/simulator/src/public/public_processor.ts b/yarn-project/simulator/src/public/public_processor.ts index b3c1194c27ba..f95595699ac8 100644 --- a/yarn-project/simulator/src/public/public_processor.ts +++ b/yarn-project/simulator/src/public/public_processor.ts @@ -280,7 +280,7 @@ export class PublicProcessor implements Traceable { const rate = duration > 0 ? totalPublicGas.l2Gas / duration : 0; this.metrics.recordAllTxs(totalPublicGas, rate); - this.log.info(`Processed ${result.length} succesful txs and ${failed.length} txs in ${duration}ms`, { + this.log.info(`Processed ${result.length} successful txs and ${failed.length} txs in ${duration}s`, { duration, rate, totalPublicGas, From 4f65182c35b76638912175e339835bbe06f061e9 Mon Sep 17 00:00:00 2001 From: PhilWindle Date: Fri, 24 Jan 2025 12:40:57 +0000 Subject: [PATCH 4/4] Remove redundant log --- yarn-project/sequencer-client/src/sequencer/sequencer.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/yarn-project/sequencer-client/src/sequencer/sequencer.ts b/yarn-project/sequencer-client/src/sequencer/sequencer.ts index c5a2bf059b7f..d1300a73f44a 100644 --- a/yarn-project/sequencer-client/src/sequencer/sequencer.ts +++ b/yarn-project/sequencer-client/src/sequencer/sequencer.ts @@ -631,7 +631,6 @@ export class Sequencer { const proposal = await this.validatorClient.createBlockProposal(block.header, block.archive.root, txHashes); if (!proposal) { const msg = `Failed to create block proposal`; - this.log.error(msg); throw new Error(msg); }