Skip to content

Commit

Permalink
fix: Don't publish a block if we failed to create the block proposal (#…
Browse files Browse the repository at this point in the history
…11475)

If we fail to build a block proposal, we shouldn't attempt to publish
the block as it will fail.
  • Loading branch information
PhilWindle authored Jan 24, 2025
1 parent 5018c94 commit f589c90
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 3 deletions.
31 changes: 31 additions & 0 deletions yarn-project/sequencer-client/src/sequencer/sequencer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -491,6 +491,18 @@ describe('sequencer', () => {
expect(publisher.proposeL2Block).not.toHaveBeenCalled();
});

it('does not publish a block if the block proposal failed', 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;
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 5 additions & 2 deletions yarn-project/sequencer-client/src/sequencer/sequencer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -627,8 +630,8 @@ 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`;
throw new Error(msg);
}

this.log.debug('Broadcasting block proposal to validators');
Expand Down
2 changes: 1 addition & 1 deletion yarn-project/simulator/src/public/public_processor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit f589c90

Please sign in to comment.