obront - Batch validation logic is ordered differently to specification

[github-actions]

obront

low

Batch validation logic is ordered differently to specification

Summary

Optimism L2 block derivation has a long list of validation steps performed on incoming blocks. They are described in the spec page, but the code implements the validations in a different order than the spec lays out.

Vulnerability Detail

There is an error in the order which is described in the spec versus the one in the implementation.

According to the specs, the following check comes first:

batch.timestamp > batch_origin.time + max_sequencer_drift -> drop: i.e. a batch that does not adopt the next L1 within time will be dropped, in favor of an empty batch that can advance the L1 origin. This enforces the max L2 timestamp rule.

After it comes this check:

batch.timestamp < batch_origin.time -> drop: enforce the min L2 timestamp rule.

However, in code in batches.go executes in reverse:

if batch.Batch.Timestamp < batchOrigin.Time {
	log.Warn("batch timestamp is less than L1 origin timestamp", "l2_timestamp", batch.Batch.Timestamp, "l1_timestamp", batchOrigin.Time, "origin", batchOrigin.ID())
	return BatchDrop
}
// If we ran out of sequencer time drift, then we drop the batch and produce an empty batch instead,
// as the sequencer is not allowed to include anything past this point without moving to the next epoch.
if max := batchOrigin.Time + cfg.MaxSequencerDrift; batch.Batch.Timestamp > max {
	log.Warn("batch exceeded sequencer time drift, sequencer must adopt new L1 origin to include transactions again", "max_time", max)
	return BatchDrop
}

In practice this should not lead to any issues because both are drop rules and are right next to each other.

Impact

The code does not align with the ordering laid out in the specification.

Code Snippet

https://github.com/ethereum-optimism/optimism/blob/407f97b9d13448b766624995ec824d3059d4d4f6/op-node/rollup/derive/batches.go#L98

Tool used

Manual Review

Recommendation

Change the code so that it is in line with the specification.