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
183 changes: 183 additions & 0 deletions l1-contracts/src/core/Rollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,189 @@ contract Rollup is Leonidas, IRollup, ITestRollup {
);
}

/**
* @notice Submit a proof for an epoch in the pending chain
*
* @dev Will emit `L2ProofVerified` if the proof is valid
*
* @dev Will throw if:
* - The block number is past the pending chain
* - The last archive root of the header does not match the archive root of parent block
* - The archive root of the header does not match the archive root of the proposed block
* - The proof is invalid
*
* @dev We provide the `_archive` and `_blockHash` even if it could be read from storage itself because it allow for
* better error messages. Without passing it, we would just have a proof verification failure.
*
* @param _epochSize - The size of the epoch (to be promoted to a constant)
* @param _args - Array of public inputs to the proof (previousArchive, endArchive, previousBlockHash, endBlockHash, endTimestamp, outHash, proverId)
* @param _fees - Array of recipient-value pairs with fees to be distributed for the epoch
* @param _aggregationObject - The aggregation object for the proof
* @param _proof - The proof to verify
*/
function submitEpochRootProof(
uint256 _epochSize,
bytes32[7] calldata _args,
bytes32[64] calldata _fees,
bytes calldata _aggregationObject,
bytes calldata _proof
) external override(IRollup) {
uint256 previousBlockNumber = tips.provenBlockNumber;
uint256 endBlockNumber = previousBlockNumber + _epochSize;

bytes32[] memory publicInputs =
getEpochProofPublicInputs(_epochSize, _args, _fees, _aggregationObject);

if (!verifier.verify(_proof, publicInputs)) {
revert Errors.Rollup__InvalidProof();
}

tips.provenBlockNumber = endBlockNumber;

for (uint256 i = 0; i < 32; i++) {
address coinbase = address(uint160(uint256(publicInputs[9 + i * 2])));
uint256 fees = uint256(publicInputs[10 + i * 2]);

if (coinbase != address(0) && fees > 0) {
// @note This will currently fail if there are insufficient funds in the bridge
// which WILL happen for the old version after an upgrade where the bridge follow.
// Consider allowing a failure. See #7938.
FEE_JUICE_PORTAL.distributeFees(coinbase, fees);
}
}

emit L2ProofVerified(endBlockNumber, _args[6]);
}

/**
* @notice Returns the computed public inputs for the given epoch proof.
*
* @dev Useful for debugging and testing. Allows submitter to compare their
* own public inputs used for generating the proof vs the ones assembled
* by this contract when verifying it.
*
* @param _epochSize - The size of the epoch (to be promoted to a constant)
* @param _args - Array of public inputs to the proof (previousArchive, endArchive, previousBlockHash, endBlockHash, endTimestamp, outHash, proverId)
* @param _fees - Array of recipient-value pairs with fees to be distributed for the epoch
* @param _aggregationObject - The aggregation object for the proof
*/
function getEpochProofPublicInputs(
uint256 _epochSize,
bytes32[7] calldata _args,
bytes32[64] calldata _fees,
bytes calldata _aggregationObject
) public view returns (bytes32[] memory) {
uint256 previousBlockNumber = tips.provenBlockNumber;
uint256 endBlockNumber = previousBlockNumber + _epochSize;

// Args are defined as an array because Solidity complains with "stack too deep" otherwise
// 0 bytes32 _previousArchive,
// 1 bytes32 _endArchive,
// 2 bytes32 _previousBlockHash,
Copy link
Contributor

Choose a reason for hiding this comment

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

On these block hashes, I am still not fully getting why they touch L1. When a block is proposed they are stored, but not check in any way and here we use them to ensure that the proof was made with the same blockhash but seems like that would already be there from just the archives identifying the blocks. If we were computing it from the header on contract would make more sense.

Note, this is not really an issue specifically for this pr, but more general on the blockhash stored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree. I think we could remove them from the public inputs altogether: checking the archive root should be equivalent to checking the block hash. But I don't want to introduce changes to those circuits now.

// 3 bytes32 _endBlockHash,
// 4 bytes32 _endTimestamp,
// 5 bytes32 _outHash,
// 6 bytes32 _proverId,

// TODO(#7373): Public inputs are not fully verified

{
// We do it this way to provide better error messages than passing along the storage values
bytes32 expectedPreviousArchive = blocks[previousBlockNumber].archive;
if (expectedPreviousArchive != _args[0]) {
revert Errors.Rollup__InvalidPreviousArchive(expectedPreviousArchive, _args[0]);
}

bytes32 expectedEndArchive = blocks[endBlockNumber].archive;
if (expectedEndArchive != _args[1]) {
revert Errors.Rollup__InvalidArchive(expectedEndArchive, _args[1]);
}

bytes32 expectedPreviousBlockHash = blocks[previousBlockNumber].blockHash;
if (expectedPreviousBlockHash != _args[2]) {
revert Errors.Rollup__InvalidPreviousBlockHash(expectedPreviousBlockHash, _args[2]);
}

bytes32 expectedEndBlockHash = blocks[endBlockNumber].blockHash;
if (expectedEndBlockHash != _args[3]) {
revert Errors.Rollup__InvalidBlockHash(expectedEndBlockHash, _args[3]);
}
}

bytes32[] memory publicInputs = new bytes32[](
Constants.ROOT_ROLLUP_PUBLIC_INPUTS_LENGTH + Constants.AGGREGATION_OBJECT_LENGTH
);

// Structure of the root rollup public inputs we need to reassemble:
//
// struct RootRollupPublicInputs {
// previous_archive: AppendOnlyTreeSnapshot,
// end_archive: AppendOnlyTreeSnapshot,
// previous_block_hash: Field,
// end_block_hash: Field,
// end_timestamp: u64,
// end_block_number: Field,
// out_hash: Field,
// fees: [FeeRecipient; 32],
// vk_tree_root: Field,
// prover_id: Field
// }

// previous_archive.root: the previous archive tree root
publicInputs[0] = _args[0];

// previous_archive.next_available_leaf_index: the previous archive next available index
// normally this should be equal to the block number (since leaves are 0-indexed and blocks 1-indexed)
// but in yarn-project/merkle-tree/src/new_tree.ts we prefill the tree so that block N is in leaf N
publicInputs[1] = bytes32(previousBlockNumber + 1);

// end_archive.root: the new archive tree root
publicInputs[2] = _args[1];

// end_archive.next_available_leaf_index: the new archive next available index
publicInputs[3] = bytes32(endBlockNumber + 1);

// previous_block_hash: the block hash just preceding this epoch
publicInputs[4] = _args[2];

// end_block_hash: the last block hash in the epoch
publicInputs[5] = _args[3];

// end_timestamp: the timestamp of the last block in the epoch
publicInputs[6] = _args[4];
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we providing this as an input? Given the end block number, we can read the slotnumber for it and compute the timestamp. This might get slightly "odd" with the based fallback, but those should be proven individually anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know timestamps were derived directly from slots, I thought the sequencer had freedom to choose the timestamp within a range as in Ethereum (or in Ethereum pre-merge at least, not sure if it changed in PoS). But yeah, we should be able to remove this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We are deriving them directly from the slots, there is a getTimestampForSlot or something to that name which can be used for it.


// end_block_number: last block number in the epoch
publicInputs[7] = bytes32(endBlockNumber);

// out_hash: root of this epoch's l2 to l1 message tree
publicInputs[8] = _args[5];
Copy link
Contributor

Choose a reason for hiding this comment

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

This one is not clear to me, why are we providing this value? Also, it can be chosen freely, so if it just matching something in the circuit we don't need it is a public value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it can be removed as well.


// fees[9-40]: array of recipient-value pairs
for (uint256 i = 0; i < 64; i++) {
publicInputs[9 + i] = _fees[i];
}

// vk_tree_root
publicInputs[41] = vkTreeRoot;

// prover_id: id of current epoch's prover
publicInputs[42] = _args[6];
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of interest here, what is the prover id, an address? Can you point me at a definition somewhere 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can be anything, even a string. We're just using it to identify the provers in the context of integrations. My guess is we'll eventually replace this with some identifier that is also used during coordination and escrows, possibly an L1 address.


// the block proof is recursive, which means it comes with an aggregation object
// this snippet copies it into the public inputs needed for verification
// it also guards against empty _aggregationObject used with mocked proofs
uint256 aggregationLength = _aggregationObject.length / 32;
for (uint256 i = 0; i < Constants.AGGREGATION_OBJECT_LENGTH && i < aggregationLength; i++) {
bytes32 part;
assembly {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you could do a larger CALLDATACOPY and put it directly into place without the loop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mind if I push it for later, being just a gas optimization?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, is fine 👍

part := calldataload(add(_aggregationObject.offset, mul(i, 32)))
}
publicInputs[i + 43] = part;
}

return publicInputs;
}

/**
* @notice Check if msg.sender can propose at a given time
*
Expand Down
9 changes: 9 additions & 0 deletions l1-contracts/src/core/interfaces/IRollup.sol
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,16 @@ interface IRollup {
bytes calldata _proof
) external;

function submitEpochRootProof(
uint256 _epochSize,
bytes32[7] calldata _args,
bytes32[64] calldata _fees,
bytes calldata _aggregationObject,
bytes calldata _proof
) external;

function canProposeAtTime(uint256 _ts, bytes32 _archive) external view returns (uint256, uint256);

function validateHeader(
bytes calldata _header,
SignatureLib.Signature[] memory _signatures,
Expand Down
2 changes: 2 additions & 0 deletions l1-contracts/src/core/libraries/ConstantsGen.sol
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,8 @@ library Constants {
uint256 internal constant CONSTANT_ROLLUP_DATA_LENGTH = 12;
uint256 internal constant BASE_OR_MERGE_PUBLIC_INPUTS_LENGTH = 29;
uint256 internal constant BLOCK_ROOT_OR_BLOCK_MERGE_PUBLIC_INPUTS_LENGTH = 91;
uint256 internal constant FEE_RECIPIENT_LENGTH = 2;
uint256 internal constant ROOT_ROLLUP_PUBLIC_INPUTS_LENGTH = 75;
uint256 internal constant GET_NOTES_ORACLE_RETURN_LENGTH = 674;
uint256 internal constant NOTE_HASHES_NUM_BYTES_PER_BASE_ROLLUP = 2048;
uint256 internal constant NULLIFIERS_NUM_BYTES_PER_BASE_ROLLUP = 2048;
Expand Down
3 changes: 3 additions & 0 deletions l1-contracts/src/core/libraries/Errors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,13 @@ library Errors {
// Rollup
error Rollup__InsufficientBondAmount(uint256 minimum, uint256 provided); // 0xa165f276
error Rollup__InvalidArchive(bytes32 expected, bytes32 actual); // 0xb682a40e
error Rollup__InvalidBlockHash(bytes32 expected, bytes32 actual);
error Rollup__InvalidBlockNumber(uint256 expected, uint256 actual); // 0xe5edf847
error Rollup__InvalidChainId(uint256 expected, uint256 actual); // 0x37b5bc12
error Rollup__InvalidEpoch(uint256 expected, uint256 actual); // 0x3c6d65e6
error Rollup__InvalidInHash(bytes32 expected, bytes32 actual); // 0xcd6f4233
error Rollup__InvalidPreviousArchive(bytes32 expected, bytes32 actual); // 0xb682a40e
error Rollup__InvalidPreviousBlockHash(bytes32 expected, bytes32 actual);
error Rollup__InvalidProof(); // 0xa5b2ba17
error Rollup__InvalidProposedArchive(bytes32 expected, bytes32 actual); // 0x32532e73
error Rollup__InvalidTimestamp(uint256 expected, uint256 actual); // 0x3132e895
Expand Down
Loading