Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
72 changes: 36 additions & 36 deletions l1-contracts/partial_epoch_proof_gas_report.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,66 +8,66 @@
"functions": {
"gasReportSubmit16Checkpoints((uint256,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32,address),(bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,address,bytes32,(uint128,uint128),uint256,uint256)[],(bytes,bytes),bytes,bytes))": {
"calls": 1,
"min": 1298507,
"mean": 1298507,
"median": 1298507,
"max": 1298507
"min": 1298643,
"mean": 1298643,
"median": 1298643,
"max": 1298643
},
"gasReportSubmit16CheckpointsWithTwoOverrides((uint256,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32,address),(bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,address,bytes32,(uint128,uint128),uint256,uint256)[],(bytes,bytes),bytes,bytes))": {
"calls": 1,
"min": 1463518,
"mean": 1463518,
"median": 1463518,
"max": 1463518
"min": 1463654,
"mean": 1463654,
"median": 1463654,
"max": 1463654
},
"gasReportSubmit1Checkpoint((uint256,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32,address),(bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,address,bytes32,(uint128,uint128),uint256,uint256)[],(bytes,bytes),bytes,bytes))": {
"calls": 1,
"min": 659719,
"mean": 659719,
"median": 659719,
"max": 659719
"min": 659855,
"mean": 659855,
"median": 659855,
"max": 659855
},
"gasReportSubmit1CheckpointWithTwoOverrides((uint256,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32,address),(bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,address,bytes32,(uint128,uint128),uint256,uint256)[],(bytes,bytes),bytes,bytes))": {
"calls": 1,
"min": 686472,
"mean": 686472,
"median": 686472,
"max": 686472
"min": 686608,
"mean": 686608,
"median": 686608,
"max": 686608
},
"gasReportSubmit32Checkpoints((uint256,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32,address),(bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,address,bytes32,(uint128,uint128),uint256,uint256)[],(bytes,bytes),bytes,bytes))": {
"calls": 1,
"min": 1836386,
"mean": 1836386,
"median": 1836386,
"max": 1836386
"min": 1836522,
"mean": 1836522,
"median": 1836522,
"max": 1836522
},
"gasReportSubmit32CheckpointsWithTwoOverrides((uint256,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32,address),(bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,address,bytes32,(uint128,uint128),uint256,uint256)[],(bytes,bytes),bytes,bytes))": {
"calls": 1,
"min": 2094127,
"mean": 2094127,
"median": 2094127,
"max": 2094127
"min": 2094263,
"mean": 2094263,
"median": 2094263,
"max": 2094263
},
"gasReportSubmit8Checkpoints((uint256,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32,address),(bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,address,bytes32,(uint128,uint128),uint256,uint256)[],(bytes,bytes),bytes,bytes))": {
"calls": 1,
"min": 983949,
"mean": 983949,
"median": 983949,
"max": 983949
"min": 984085,
"mean": 984085,
"median": 984085,
"max": 984085
},
"gasReportSubmit8CheckpointsWithTwoOverrides((uint256,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32,address),(bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,address,bytes32,(uint128,uint128),uint256,uint256)[],(bytes,bytes),bytes,bytes))": {
"calls": 1,
"min": 1080528,
"mean": 1080528,
"median": 1080528,
"max": 1080528
"min": 1080664,
"mean": 1080664,
"median": 1080664,
"max": 1080664
},
"gasReportSubmit8MoreCheckpoints((uint256,uint256,(bytes32,bytes32,bytes32,bytes32,bytes32,address),(bytes32,bytes32,bytes32,bytes32,bytes32,uint256,uint256,address,bytes32,(uint128,uint128),uint256,uint256)[],(bytes,bytes),bytes,bytes))": {
"calls": 1,
"min": 1010042,
"mean": 1010042,
"median": 1010042,
"max": 1010042
"min": 974194,
"mean": 974194,
"median": 974194,
"max": 974194
}
}
}
Expand Down
18 changes: 9 additions & 9 deletions l1-contracts/partial_epoch_proof_gas_report.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

| Proof submission | Gas |
|---|---:|
| 1 Checkpoint | 659,719 |
| 1 Checkpoint With Two Overrides | 686,472 |
| 8 Checkpoints | 983,949 |
| 8 Checkpoints With Two Overrides | 1,080,528 |
| 8 More Checkpoints | 1,010,042 |
| 16 Checkpoints | 1,298,507 |
| 16 Checkpoints With Two Overrides | 1,463,518 |
| 32 Checkpoints | 1,836,386 |
| 32 Checkpoints With Two Overrides | 2,094,127 |
| 1 Checkpoint | 659,855 |
| 1 Checkpoint With Two Overrides | 686,608 |
| 8 Checkpoints | 984,085 |
| 8 Checkpoints With Two Overrides | 1,080,664 |
| 8 More Checkpoints | 974,194 |
| 16 Checkpoints | 1,298,643 |
| 16 Checkpoints With Two Overrides | 1,463,654 |
| 32 Checkpoints | 1,836,522 |
| 32 Checkpoints With Two Overrides | 2,094,263 |

_Uses the mock epoch proof verifier._
48 changes: 34 additions & 14 deletions l1-contracts/src/core/libraries/rollup/EpochProofLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -122,11 +122,17 @@ library EpochProofLib {
STFLib.prune();
}

(Epoch endEpoch, Epoch currentEpoch) = assertAcceptable(_args.start, _args.end);
(Epoch endEpoch, Epoch currentEpoch, uint256 provenBeforeSubmission) = assertAcceptable(_args.start, _args.end);
uint256 firstHeaderToVerify;
if (provenBeforeSubmission >= _args.start) {
uint256 provenPrefixLength = provenBeforeSubmission - _args.start + 1;
uint256 accountedPrefixLength = RewardLib.getLongestProvenLength(endEpoch);
Comment on lines +128 to +129

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When would these two differ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This covers when proofs for epoch N land in epoch N + 1.

Assuming N = 1, epoch 1 covers checkpoints 1..32, epoch 2 covers checkpoints 33..64

We could receive proofs for epoch 1 during epoch 2's lifetime while epoch 2 is partial proven. Let's assume we've proven the first 3 checkpoints of epoch 2 when another epoch 1 proof lands:

// uint256 provenPrefixLength = provenBeforeSubmission - _args.start + 1;
uint256 provenProfixLength = 35 - 1 + 1; // proven tip is 35 (ie after the first three checkpoints in epoch 2), args.start is 1 because that's the start of epoch 1's range

// uint256 accountedPrefixLength = RewardLib.getLongestProvenLength(endEpoch);
uint256 accountedPrrefixLength = 32; // we're strictly looking at the epoch to be proven, it's fully proven otherwise we couldn't accept proofs for epoch 2 so this'll return that all 32 checkpoints have had their rewards accounted for.

// firstHeaderToVerify = provenPrefixLength < accountedPrefixLength ? provenPrefixLength : accountedPrefixLength;
firstHeaderToVerify = 35 < 32 ? 35 : 32; // ie 32

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

But is there any scenario where accountedPrefixLength is greater than provenPrefixLength? Can't we just use RewardLib.getLongestProvenLength(endEpoch)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Astra here (the AI assistant), posting this explanation at Santiago's request; this is my analysis, not Santiago writing.

@alexghr Your example makes sense: the global proven tip can include checkpoints from the next epoch, so provenPrefixLength can exceed this epoch's accountedPrefixLength. Following up on Santiago's question, though, can the reverse ever happen? The min only changes the result relative to using accountedPrefixLength directly if accountedPrefixLength > provenPrefixLength.

For a smaller example, suppose epoch A contains checkpoints 1–4 and epoch B contains 5–8. Each row below is a successive submission; the counters are their values before that submission:

Submission Global proven tip Accounted length for the submitted epoch Headers needing hash verification
Prove A: 1–2 0 0 1–2
Extend A: 1–4 2 2 3–4
Prove B: 5–6 4 0 5–6
Another prover submits A: 1–4, while still within its submission window 6 4 None

In the last row, the current calculation gives:

provenPrefixLength = 6 - 1 + 1; // 6
accountedPrefixLength = 4;
firstHeaderToVerify = min(6, 4); // 4

The 6 includes checkpoints from B, whereas the value we need is simply how many checkpoints in A were already proven and accounted for.

From the production state transitions I reviewed, the invariant appears to be:

  • An epoch's longestProvenLength is extended only through successful proof submission.
  • That submission also advances the global proven tip to at least the submitted end checkpoint. Both updates commit atomically; a revert during reward handling rolls back the tip update too.
  • Pruning does not roll back the proven tip.

Therefore, whenever the epoch has an accounted prefix, the global proven tip must already cover that prefix. If the tip is before the epoch's start, its accounted length should be zero. I don't see a reachable state through normal completed submissions where accounting gets ahead of proving.

Could we consequently replace the tuple/conditional/min block with:

Epoch endEpoch = assertAcceptable(_args.start, _args.end);
uint256 firstHeaderToVerify = RewardLib.getLongestProvenLength(endEpoch);

That would also let assertAcceptable return just endEpoch; its internal proven-tip acceptance check would remain unchanged.

A shorter repeat proof is also worth considering: if A already has accounted length 4 and someone submits A: 1–2, starting verification at index 4 simply skips the loop, since both submitted headers were already verified. The existing header-count validation remains in place.

Is there a production path that breaks this invariant? If not, using the epoch-local accounted length directly seems equivalent and easier to follow.

firstHeaderToVerify = provenPrefixLength < accountedPrefixLength ? provenPrefixLength : accountedPrefixLength;
}

// Rehash the supplied headers against storage once, here: the public-input assembly below reads the fee
// recipient/value out of them and relies on this call having run.
verifyHeaders(_args.start, _args.end, _args.headers);
// The skipped calldata prefix is untrusted, but rewards have already consumed it and proof verification binds its
// fee data to the canonical checkpoint headers. We only verify new headers since the last proof
verifyHeaders(_args.start, _args.end, _args.headers, firstHeaderToVerify);

// Verify attestations for the last checkpoint in the epoch
// -> This serves as training wheels for the public part of the system (proving systems used in public and AVM)
Expand Down Expand Up @@ -178,8 +184,8 @@ library EpochProofLib {
*
* @dev The fee recipient/value public inputs are sourced from the supplied headers, so this entry point rehashes
* them against storage before assembling: an off-chain caller must not walk away with public inputs built from
* unverified fee fields and only discover the mismatch when the on-chain proof reverts. The submit path verifies
* the headers up front and assembles via computeEpochProofPublicInputs to avoid rehashing them twice.
* unverified fee fields and only discover the mismatch when the on-chain proof reverts. The submit path separately
* validates headers that have not already been proven and accounted for.
*
* @param _start - The start of the epoch (inclusive)
* @param _end - The end of the epoch (inclusive)
Expand All @@ -196,7 +202,7 @@ library EpochProofLib {
bytes calldata _blobPublicInputs,
RollupConfig memory _config
) internal view returns (bytes32[] memory) {
verifyHeaders(_start, _end, _headers);
verifyHeaders(_start, _end, _headers, 0);
return computeEpochProofPublicInputs(_start, _end, _args, _headers, _blobPublicInputs, _config);
}

Expand Down Expand Up @@ -416,19 +422,25 @@ library EpochProofLib {
}

/**
* @notice Rehashes each provided checkpoint header and requires it to match the stored header hash
* @notice Rehashes a suffix of the provided checkpoint headers and requires it to match the stored header hashes
*
* @param _start The first checkpoint number in the epoch (inclusive)
* @param _end The last checkpoint number in the epoch (inclusive)
* @param _headers The proposed headers for each checkpoint in [_start, _end]
* @param _firstHeaderToVerify The index of the first header that has not already been proven and accounted for
*/
function verifyHeaders(uint256 _start, uint256 _end, ProposedHeader[] calldata _headers) private view {
function verifyHeaders(
uint256 _start,
uint256 _end,
ProposedHeader[] calldata _headers,
uint256 _firstHeaderToVerify
) private view {
uint256 numCheckpoints = _end - _start + 1;
require(
_headers.length == numCheckpoints, Errors.Rollup__InvalidCheckpointHeaderCount(numCheckpoints, _headers.length)
);

for (uint256 i = 0; i < numCheckpoints; i++) {
for (uint256 i = _firstHeaderToVerify; i < numCheckpoints; i++) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the whole patch.

bytes32 expectedHeaderHash = STFLib.getHeaderHash(_start + i);
bytes32 providedHeaderHash = ProposedHeaderLib.hash(_headers[i]);
require(
Expand Down Expand Up @@ -460,8 +472,13 @@ library EpochProofLib {
* @param _end The last checkpoint number in the epoch (inclusive)
* @return endEpoch The epoch number that the proof covers
* @return currentEpoch The epoch at the time the proof is submitted
* @return provenBeforeSubmission The proven checkpoint number observed while checking the submission
*/
function assertAcceptable(uint256 _start, uint256 _end) private view returns (Epoch endEpoch, Epoch currentEpoch) {
function assertAcceptable(uint256 _start, uint256 _end)
private
view
returns (Epoch endEpoch, Epoch currentEpoch, uint256 provenBeforeSubmission)
{
RollupStore storage rollupStore = STFLib.getStorage();

Epoch startEpoch = STFLib.getEpochForCheckpoint(_start);
Expand All @@ -486,14 +503,17 @@ library EpochProofLib {
bool isStartOfEpoch = _start == 1 || parentEpoch <= startEpoch - Epoch.wrap(1);
require(isStartOfEpoch, Errors.Rollup__StartIsNotFirstCheckpointOfEpoch());

bool isStartBuildingOnProven = _start - 1 <= rollupStore.tips.getProven();
provenBeforeSubmission = rollupStore.tips.getProven();
bool isStartBuildingOnProven = _start - 1 <= provenBeforeSubmission;
require(isStartBuildingOnProven, Errors.Rollup__StartIsNotBuildingOnProven());

bool claimedNumCheckpointsInEpoch = _end - _start + 1 <= Constants.MAX_CHECKPOINTS_PER_EPOCH;
require(
claimedNumCheckpointsInEpoch,
Errors.Rollup__TooManyCheckpointsInEpoch(Constants.MAX_CHECKPOINTS_PER_EPOCH, _end - _start)
);

return (endEpoch, currentEpoch, provenBeforeSubmission);
}

/**
Expand Down Expand Up @@ -534,8 +554,8 @@ library EpochProofLib {
* 2. Assembling the public inputs for the root rollup circuit
* 3. Verifying the validity proof against the assembled public inputs using the configured verifier
*
* @dev Assumes the caller has already verified the supplied checkpoint headers against storage, so assembly skips
* rehashing them.
* @dev Assumes the caller has completed the submit path's required header checks, so assembly does not rehash
* headers.
*
* @dev Errors Thrown:
* - Rollup__InvalidBlobProof: Batched blob proof verification failed
Expand Down
4 changes: 4 additions & 0 deletions l1-contracts/src/core/libraries/rollup/RewardLib.sol
Original file line number Diff line number Diff line change
Expand Up @@ -327,6 +327,10 @@ library RewardLib {
return getStorage().epochRewards[_epoch].rewards;
}

function getLongestProvenLength(Epoch _epoch) internal view returns (uint256) {
return getStorage().epochRewards[_epoch].longestProvenLength;
}

function getHasSubmitted(Epoch _epoch, uint256 _length, address _prover) internal view returns (bool) {
return getStorage().epochRewards[_epoch].subEpoch[_length].shares[_prover] > 0;
}
Expand Down
73 changes: 73 additions & 0 deletions l1-contracts/test/Rollup.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,79 @@ contract RollupTest is RollupBase {
assertEq(outbox.getRootData(Epoch.wrap(0), 2), outHash2, "Root at K=2 should be outHash2");
}

function testLongerEpochProofAllowsModifiedPreviouslyProvenHeader() public setUpFor("mixed_checkpoint_1") {
_proposeCheckpoint("mixed_checkpoint_1", 1);
_proposeCheckpoint("mixed_checkpoint_2", 2);

DecoderBase.Data memory checkpoint1Data = load("mixed_checkpoint_1").checkpoint;
DecoderBase.Data memory checkpoint2Data = load("mixed_checkpoint_2").checkpoint;
CheckpointLog memory checkpoint = rollup.getCheckpoint(0);

_submitEpochProof(
1,
1,
checkpoint.archive,
checkpoint1Data.archive,
checkpoint1Data.batchedBlobInputs,
checkpoint1Data.header.outHash
);

address modifiedCoinbase = makeAddr("modifiedCoinbase");

// even though his header was tampered with the next _submitEpochProof call will succeed (with a MockVerifier):
// the correct header for slot 1 was correct when the previous proof was sent
// the fact that it is now bogus does no matter because its rewards will not be processed again
// with a RealVerifier the proof will fail because the header's hash is sent as a public input
proposedHeaders[1].coinbase = modifiedCoinbase;

_submitEpochProof(
1,
2,
checkpoint.archive,
checkpoint2Data.archive,
checkpoint2Data.batchedBlobInputs,
checkpoint2Data.header.outHash
);

assertEq(rollup.getProvenCheckpointNumber(), 2);
assertEq(rollup.getSequencerRewards(modifiedCoinbase), 0);
}

function testLongerEpochProofRejectsModifiedNewHeader() public setUpFor("mixed_checkpoint_1") {
_proposeCheckpoint("mixed_checkpoint_1", 1);
_proposeCheckpoint("mixed_checkpoint_2", 2);

DecoderBase.Data memory checkpoint1Data = load("mixed_checkpoint_1").checkpoint;
DecoderBase.Data memory checkpoint2Data = load("mixed_checkpoint_2").checkpoint;
CheckpointLog memory checkpoint = rollup.getCheckpoint(0);

_submitEpochProof(
1,
1,
checkpoint.archive,
checkpoint1Data.archive,
checkpoint1Data.batchedBlobInputs,
checkpoint1Data.header.outHash
);

bytes32 expectedHeaderHash = ProposedHeaderLib.hash(proposedHeaders[2]);
// send bogus header
proposedHeaders[2].accumulatedFees += 1;
bytes32 providedHeaderHash = ProposedHeaderLib.hash(proposedHeaders[2]);

vm.expectRevert(
abi.encodeWithSelector(Errors.Rollup__InvalidCheckpointHeader.selector, expectedHeaderHash, providedHeaderHash)
);
_submitEpochProof(
1,
2,
checkpoint.archive,
checkpoint2Data.archive,
checkpoint2Data.batchedBlobInputs,
checkpoint2Data.header.outHash
);
}

// getEpochProofPublicInputs is the view that the prover-publisher calls off-chain to validate its inputs before
// submitting. Because the fee recipient/value public inputs are taken from the supplied headers, the header check
// must run here too - not only on the submit path - so a mismatch is caught before publishing rather than reverting
Expand Down
Loading