From 2927e93a757f2ace307f7166b8cc596e853cadbc Mon Sep 17 00:00:00 2001 From: Anna Carroll Date: Tue, 16 Apr 2024 16:00:24 -0500 Subject: [PATCH 1/5] redo natspec & error name --- src/Passage.sol | 188 ++++++++++++++++++++++++------------------------ 1 file changed, 94 insertions(+), 94 deletions(-) diff --git a/src/Passage.sol b/src/Passage.sol index 69b93f6..9131c5c 100644 --- a/src/Passage.sol +++ b/src/Passage.sol @@ -4,35 +4,35 @@ pragma solidity ^0.8.24; // import IERC20 from OpenZeppelin import {IERC20} from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; -// @notice A contract deployed to Host chain that allows tokens to enter the rollup, -// and enables Builders to fulfill requests to exchange tokens on the Rollup for tokens on the Host. +/// @notice A contract deployed to Host chain that allows tokens to enter the rollup, +/// and enables Builders to fulfill requests to exchange tokens on the Rollup for tokens on the Host. contract HostPassage { - // @notice Thrown when attempting to fulfill an exit order with a deadline that has passed. - error Expired(); + /// @notice Thrown when attempting to fulfill an exit order with a deadline that has passed. + error OrderExpired(); - // @notice Emitted when tokens enter the rollup. - // @param token - The address of the token entering the rollup. - // @param rollupRecipient - The recipient of the token on the rollup. - // @param amount - The amount of the token entering the rollup. + /// @notice Emitted when tokens enter the rollup. + /// @param token - The address of the token entering the rollup. + /// @param rollupRecipient - The recipient of the token on the rollup. + /// @param amount - The amount of the token entering the rollup. event Enter(address indexed token, address indexed rollupRecipient, uint256 amount); - // @notice Emitted when an exit order is fulfilled by the Builder. - // @param token - The address of the token transferred to the recipient. - // @param hostRecipient - The recipient of the token on host. - // @param amount - The amount of the token transferred to the recipient. + /// @notice Emitted when an exit order is fulfilled by the Builder. + /// @param token - The address of the token transferred to the recipient. + /// @param hostRecipient - The recipient of the token on host. + /// @param amount - The amount of the token transferred to the recipient. event ExitFilled(address indexed token, address indexed hostRecipient, uint256 amount); - // @notice Details of an exit order to be fulfilled by the Builder. - // @param token - The address of the token to be transferred to the recipient. - // If token is the zero address, the amount is native Ether. - // Corresponds to tokenOut_H in the RollupPassage contract. - // @param recipient - The recipient of the token on host. - // Corresponds to recipient_H in the RollupPassage contract. - // @param amount - The amount of the token to be transferred to the recipient. - // Corresponds to one or more amountOutMinimum_H in the RollupPassage contract. - // @param deadline - The deadline by which the exit order must be fulfilled. - // Corresponds to deadline in the RollupPassage contract. - // If the ExitOrder is a combination of multiple orders, the deadline SHOULD be the latest of all orders. + /// @notice Details of an exit order to be fulfilled by the Builder. + /// @param token - The address of the token to be transferred to the recipient. + /// If token is the zero address, the amount is native Ether. + /// Corresponds to tokenOut_H in the RollupPassage contract. + /// @param recipient - The recipient of the token on host. + /// Corresponds to recipient_H in the RollupPassage contract. + /// @param amount - The amount of the token to be transferred to the recipient. + /// Corresponds to one or more amountOutMinimum_H in the RollupPassage contract. + /// @param deadline - The deadline by which the exit order must be fulfilled. + /// Corresponds to deadline in the RollupPassage contract. + /// If the ExitOrder is a combination of multiple orders, the deadline SHOULD be the latest of all orders. struct ExitOrder { address token; address recipient; @@ -40,43 +40,43 @@ contract HostPassage { uint256 deadline; } - // @notice Allows native Ether to enter the rollup by being sent directly to the contract. + /// @notice Allows native Ether to enter the rollup by being sent directly to the contract. fallback() external payable { enter(msg.sender); } - // @notice Allows native Ether to enter the rollup by being sent directly to the contract. + /// @notice Allows native Ether to enter the rollup by being sent directly to the contract. receive() external payable { enter(msg.sender); } - // @notice Allows native Ether to enter the rollup. - // @dev Permanently burns the entire msg.value by locking it in this contract. - // @param rollupRecipient - The recipient of the Ether on the rollup. - // @custom:emits Enter indicatig the amount of Ether to mint on the rollup & its recipient. + /// @notice Allows native Ether to enter the rollup. + /// @dev Permanently burns the entire msg.value by locking it in this contract. + /// @param rollupRecipient - The recipient of the Ether on the rollup. + /// @custom:emits Enter indicatig the amount of Ether to mint on the rollup & its recipient. function enter(address rollupRecipient) public payable { emit Enter(address(0), rollupRecipient, msg.value); } - // @notice Fulfills exit orders by transferring tokenOut to the recipient - // @param orders The exit orders to fulfill - // @custom:emits ExitFilled for each exit order fulfilled. - // @dev Called by the Builder atomically with a transaction calling `submitBlock`. - // The user-submitted transactions initiating the ExitOrders on the rollup - // must be included by the Builder in the rollup block submitted via `submitBlock`. - // @dev The user transfers tokenIn on the rollup, and receives tokenOut on host. - // @dev The Builder receives tokenIn on the rollup, and transfers tokenOut to the user on host. - // @dev The rollup STF MUST NOT apply `submitExit` transactions to the rollup state - // UNLESS a corresponding ExitFilled event is emitted on host in the same block. - // @dev If the user submits multiple exit transactions for the same token in the same rollup block, - // the Builder may transfer the cumulative tokenOut to the user in a single ExitFilled event. - // The rollup STF will apply the user's exit transactions on the rollup up to the point that sum(tokenOut) is lte the ExitFilled amount. - // TODO: add option to fulfill ExitOrders with native ETH? or is it sufficient to only allow users to exit via WETH? + /// @notice Fulfills exit orders by transferring tokenOut to the recipient + /// @param orders The exit orders to fulfill + /// @custom:emits ExitFilled for each exit order fulfilled. + /// @dev Called by the Builder atomically with a transaction calling `submitBlock`. + /// The user-submitted transactions initiating the ExitOrders on the rollup + /// must be included by the Builder in the rollup block submitted via `submitBlock`. + /// @dev The user transfers tokenIn on the rollup, and receives tokenOut on host. + /// @dev The Builder receives tokenIn on the rollup, and transfers tokenOut to the user on host. + /// @dev The rollup STF MUST NOT apply `submitExit` transactions to the rollup state + /// UNLESS a corresponding ExitFilled event is emitted on host in the same block. + /// @dev If the user submits multiple exit transactions for the same token in the same rollup block, + /// the Builder may transfer the cumulative tokenOut to the user in a single ExitFilled event. + /// The rollup STF will apply the user's exit transactions on the rollup up to the point that sum(tokenOut) is lte the ExitFilled amount. + /// TODO: add option to fulfill ExitOrders with native ETH? or is it sufficient to only allow users to exit via WETH? function fulfillExits(ExitOrder[] calldata orders) external { for (uint256 i = 0; i < orders.length; i++) { ExitOrder memory order = orders[i]; // check that the deadline hasn't passed - if (block.timestamp >= order.deadline) revert Expired(); + if (block.timestamp > order.deadline) revert OrderExpired(); // transfer tokens to the recipient IERC20(order.token).transferFrom(msg.sender, order.recipient, order.amount); // emit @@ -85,13 +85,13 @@ contract HostPassage { } } -// A contract deployed to the Rollup that allows users to atomically exchange tokens on the Rollup for tokens on the Host. +/// @notice A contract deployed to the Rollup that allows users to atomically exchange tokens on the Rollup for tokens on the Host. contract RollupPassage { - // @notice Thrown when an exit tranaction is submitted with a deadline that has passed. - error Expired(); + /// @notice Thrown when an exit transaction is submitted with a deadline that has passed. + error OrderExpired(); - // @notice Emitted when an exit order is submitted & successfully processed, indicating it was also fulfilled on host. - // @dev See `submitExit` for parameter docs. + /// @notice Emitted when an exit order is submitted & successfully processed, indicating it was also fulfilled on host. + /// @dev See `submitExit` for parameter docs. event Exit( address indexed tokenIn_RU, address indexed tokenOut_H, @@ -101,31 +101,31 @@ contract RollupPassage { uint256 amountOutMinimum_H ); - // @notice Emitted when tokens or native Ether is swept from the contract. - // @dev Intended to improve visibility for Builders to ensure Sweep isn't called unexpectedly. - // Intentionally does not bother to emit which token(s) were swept, nor their amounts. + /// @notice Emitted when tokens or native Ether is swept from the contract. + /// @dev Intended to improve visibility for Builders to ensure Sweep isn't called unexpectedly. + /// Intentionally does not bother to emit which token(s) were swept, nor their amounts. event Sweep(address indexed recipient); - // @notice Expresses an intent to exit the rollup with ERC20s. - // @dev Exits are modeled as a swap between two tokens. - // tokenIn_RU is provided on the rollup; in exchange, - // tokenOut_H is expected to be received on host. - // Exits may "swap" native rollup Ether for host WETH - - // two assets that represent the same underlying token and should have roughly the same value - - // or they may be a more "true" swap of rollup USDC for host WETH. - // Fees paid to the Builders for fulfilling the exit orders - // can be included within the "exchange rate" between tokenIn and tokenOut. - // @dev The Builder claims the tokenIn_RU from the contract by submitting a transaction to `sweep` the tokens within the same block. - // @dev The Rollup STF MUST NOT apply `submitExit` transactions to the rollup state - // UNLESS a sufficient ExitFilled event is emitted on host within the same block. - // @param tokenIn_RU - The address of the token the user supplies as the input on the rollup for the trade. - // @param tokenOut_H - The address of the token the user expects to receive on host. - // @param recipient_H - The address of the recipient of tokenOut_H on host. - // @param deadline - The deadline by which the exit order must be fulfilled. - // @param amountIn_RU - The amount of tokenIn_RU the user supplies as the input on the rollup for the trade. - // @param amountOutMinimum_H - The minimum amount of tokenOut_H the user expects to receive on host. - // @custom:reverts Expired if the deadline has passed. - // @custom:emits Exit if the exit transaction succeeds. + /// @notice Expresses an intent to exit the rollup with ERC20s. + /// @dev Exits are modeled as a swap between two tokens. + /// tokenIn_RU is provided on the rollup; in exchange, + /// tokenOut_H is expected to be received on host. + /// Exits may "swap" native rollup Ether for host WETH - + /// two assets that represent the same underlying token and should have roughly the same value - + /// or they may be a more "true" swap of rollup USDC for host WETH. + /// Fees paid to the Builders for fulfilling the exit orders + /// can be included within the "exchange rate" between tokenIn and tokenOut. + /// @dev The Builder claims the tokenIn_RU from the contract by submitting a transaction to `sweep` the tokens within the same block. + /// @dev The Rollup STF MUST NOT apply `submitExit` transactions to the rollup state + /// UNLESS a sufficient ExitFilled event is emitted on host within the same block. + /// @param tokenIn_RU - The address of the token the user supplies as the input on the rollup for the trade. + /// @param tokenOut_H - The address of the token the user expects to receive on host. + /// @param recipient_H - The address of the recipient of tokenOut_H on host. + /// @param deadline - The deadline by which the exit order must be fulfilled. + /// @param amountIn_RU - The amount of tokenIn_RU the user supplies as the input on the rollup for the trade. + /// @param amountOutMinimum_H - The minimum amount of tokenOut_H the user expects to receive on host. + /// @custom:reverts Expired if the deadline has passed. + /// @custom:emits Exit if the exit transaction succeeds. function submitExit( address tokenIn_RU, address tokenOut_H, @@ -135,7 +135,7 @@ contract RollupPassage { uint256 amountOutMinimum_H ) external { // check that the deadline hasn't passed - if (block.timestamp >= deadline) revert Expired(); + if (block.timestamp >= deadline) revert OrderExpired(); // transfer the tokens from the user to the contract IERC20(tokenIn_RU).transferFrom(msg.sender, address(this), amountIn_RU); @@ -144,33 +144,33 @@ contract RollupPassage { emit Exit(tokenIn_RU, tokenOut_H, recipient_H, deadline, amountIn_RU, amountOutMinimum_H); } - // @notice Expresses an intent to exit the rollup with native Ether. - // @dev See `submitExit` above for dev details on how exits work. - // @dev tokenIn_RU is set to address(0), native rollup Ether. - // amountIn_RU is set to msg.value. - // @param tokenOut_H - The address of the token the user expects to receive on host. - // @param recipient_H - The address of the recipient of tokenOut_H on host. - // @param deadline - The deadline by which the exit order must be fulfilled. - // @param amountOutMinimum_H - The minimum amount of tokenOut_H the user expects to receive on host. - // @custom:reverts Expired if the deadline has passed. - // @custom:emits Exit if the exit transaction succeeds. + /// @notice Expresses an intent to exit the rollup with native Ether. + /// @dev See `submitExit` above for dev details on how exits work. + /// @dev tokenIn_RU is set to address(0), native rollup Ether. + /// amountIn_RU is set to msg.value. + /// @param tokenOut_H - The address of the token the user expects to receive on host. + /// @param recipient_H - The address of the recipient of tokenOut_H on host. + /// @param deadline - The deadline by which the exit order must be fulfilled. + /// @param amountOutMinimum_H - The minimum amount of tokenOut_H the user expects to receive on host. + /// @custom:reverts Expired if the deadline has passed. + /// @custom:emits Exit if the exit transaction succeeds. function submitExit(address tokenOut_H, address recipient_H, uint256 deadline, uint256 amountOutMinimum_H) external payable { // check that the deadline hasn't passed - if (block.timestamp >= deadline) revert Expired(); + if (block.timestamp >= deadline) revert OrderExpired(); // emit the exit event emit Exit(address(0), tokenOut_H, recipient_H, deadline, msg.value, amountOutMinimum_H); } - // @notice Transfer the entire balance of tokens to the recipient. - // @dev Called by the Builder within the same block as `submitExit` transactions to claim the amounts of `tokenIn`. - // @dev Builder MUST ensure that no other account calls `sweep` before them. - // @param recipient - The address to receive the tokens. - // @param tokens - The addresses of the tokens to transfer. - // TODO: should there be more granular control for the builder to specify a different recipient for each token? + /// @notice Transfer the entire balance of tokens to the recipient. + /// @dev Called by the Builder within the same block as `submitExit` transactions to claim the amounts of `tokenIn`. + /// @dev Builder MUST ensure that no other account calls `sweep` before them. + /// @param recipient - The address to receive the tokens. + /// @param tokens - The addresses of the tokens to transfer. + /// TODO: should there be more granular control for the builder to specify a different recipient for each token? function sweep(address recipient, address[] calldata tokens) public { for (uint256 i = 0; i < tokens.length; i++) { IERC20 token = IERC20(tokens[i]); @@ -179,10 +179,10 @@ contract RollupPassage { emit Sweep(recipient); } - // @notice Transfer the entire balance of native Ether to the recipient. - // @dev Called by the Builder within the same block as `submitExit` transactions to claim the amounts of native Ether. - // @dev Builder MUST ensure that no other account calls `sweepETH` before them. - // @param recipient - The address to receive the native Ether. + /// @notice Transfer the entire balance of native Ether to the recipient. + /// @dev Called by the Builder within the same block as `submitExit` transactions to claim the amounts of native Ether. + /// @dev Builder MUST ensure that no other account calls `sweepETH` before them. + /// @param recipient - The address to receive the native Ether. function sweepETH(address payable recipient) public { recipient.transfer(address(this).balance); emit Sweep(recipient); From 27c8d78b43e6ae06cc7883fc68764887bcf6dec8 Mon Sep 17 00:00:00 2001 From: Anna Carroll Date: Tue, 16 Apr 2024 16:00:58 -0500 Subject: [PATCH 2/5] update: redo block hash construction --- src/Zenith.sol | 205 ++++++++++++++++++++++++++++--------------------- 1 file changed, 119 insertions(+), 86 deletions(-) diff --git a/src/Zenith.sol b/src/Zenith.sol index 38cc7cb..d6dfddc 100644 --- a/src/Zenith.sol +++ b/src/Zenith.sol @@ -7,125 +7,158 @@ import {AccessControlDefaultAdminRules} from "openzeppelin-contracts/contracts/access/extensions/AccessControlDefaultAdminRules.sol"; contract Zenith is HostPassage, AccessControlDefaultAdminRules { - // @notice Role that allows an address to sign commitments to rollup blocks. + struct BlockHeader { + uint256 rollupChainId; + uint256 sequence; + uint256 gasLimit; + uint256 confirmBy; + address rewardAddress; + } + + /// @notice Role that allows an address to sign commitments to rollup blocks. bytes32 public constant SEQUENCER_ROLE = bytes32("SEQUENCER_ROLE"); - // @notice The sequence number of the next block that can be submitted. - uint256 public nextSequence; - - // @notice Thrown when a block submission is attempted with a sequence number that is not the next block. - // @dev Blocks must be submitted in strict increasing order. - // @param expected - the sequence number of the next block that can be submitted. - // @param actual - the sequence number of the block that was submitted. - error BadSequence(uint256 expected, uint256 actual); - - // @notice Thrown when a block submission is attempted with a bad signature. - // @dev Can indicate that the commit signer is not a permissioned sequencer, OR - // that the signed data was malformed or invalid. - // @param signer - the signer derived from the commit hash & signature. - // If this is an unexpected value, the commit data might be malformed or invalid. - // @param commit - the commit hash derived from the block data. - // If this value is expected, the signer was not a sequencer permissioned with the SEQUENCER_ROLE. - error BadSignature(address signer, bytes32 commit); - - // @notice Emitted when a new rollup block is successfully submitted. - // @param sequence - the sequence number of the block. - // @param sequencer - the address of the sequencer that signed the block. - // @param blobIndices - the indices of the 4844 blob hashes for the block data. - // TODO: can an off-chain observer easily get the blob data from the transaction using the blob indices? - event BlockSubmitted(uint256 indexed sequence, address indexed sequencer, uint32[] blobIndices); - - // @notice Initializes the Admin role. - // @dev See `AccessControlDefaultAdminRules` for information on contract administration. - // - Admin role can grant and revoke Sequencer roles. - // - Admin role can be transferred via two-step process with a 1 day timelock. - // @param admin - the address that will be the initial admin. + /// @notice The sequence number of the next block that can be submitted. + /// rollupChainId => nextSequence number + mapping(uint256 => uint256) public nextSequence; + + /// @notice Thrown when a block submission is attempted with a sequence number that is not the next block. + /// @dev Blocks must be submitted in strict increasing order. + /// @param expected - the correct next sequence number for the given rollup chainId. + error BadSequence(uint256 expected); + + /// @notice Thrown when a block submission is attempted where confirmBy time has passed. + error BlockExpired(); + + /// @notice Thrown when a block submission is attempted with a signature over malformed data. + /// @param hashes - the encoded blob hashes attached to the transaction. + /// this is the most useful data to debug a bad signature off-chain. + /// @param v - the v component of the Sequencer's ECSDA signature over the block commitment. + /// @param r - the r component of the Sequencer's ECSDA signature over the block commitment. + /// @param s - the s component of the Sequencer's ECSDA signature over the block commitment. + error BadSignature(bytes hashes, uint8 v, bytes32 r, bytes32 s); + + /// @notice Thrown when a block submission is attempted signed by a non-permissioned sequencer. + /// @param sequencer - the signer of the block data that is not a permissioned sequencer. + error NotSequencer(address sequencer); + + /// @notice Emitted when a new rollup block is successfully submitted. + /// @param sequencer - the address of the sequencer that signed the block. + /// @param header - the block header information for the block. + /// @param blobIndices - the indices of the 4844 blob hashes for the block data. + event BlockSubmitted(address indexed sequencer, BlockHeader indexed header, uint32[] blobIndices); + + /// @notice Initializes the Admin role. + /// @dev See `AccessControlDefaultAdminRules` for information on contract administration. + /// - Admin role can grant and revoke Sequencer roles. + /// - Admin role can be transferred via two-step process with a 1 day timelock. + /// @param admin - the address that will be the initial admin. constructor(address admin) AccessControlDefaultAdminRules(1 days, admin) {} - // @notice Submit a rollup block with block data stored in 4844 blobs. - // @dev Blocks are submitted by Builders, with an attestation to the block data signed by a Sequencer. - // @param blockSequence - the sequence number of the block. - // @param blobIndices - the indices of the 4844 blob hashes for the block data. - // @param v - the v component of the Sequencer's ECSDA signature over the block commitment. - // @param r - the r component of the Sequencer's ECSDA signature over the block commitment. - // @param s - the s component of the Sequencer's ECSDA signature over the block commitment. - // @custom:reverts BadSequence if the sequence number is not the next block. - // @custom:reverts BadSignature if the signature provided commits to different block data, - // OR if the signer is not a permissioned sequencer. - // @custom:emits BlockSubmitted if the block is successfully submitted. - function submitBlock(uint256 blockSequence, uint32[] memory blobIndices, uint8 v, bytes32 r, bytes32 s) external { + /// @notice Submit a rollup block with block data stored in 4844 blobs. + /// @dev Blocks are submitted by Builders, with an attestation to the block data signed by a Sequencer. + /// @param header - the header information for the rollup block. + /// @param blobIndices - the indices of the 4844 blob hashes for the block data. + /// @param v - the v component of the Sequencer's ECSDA signature over the block commitment. + /// @param r - the r component of the Sequencer's ECSDA signature over the block commitment. + /// @param s - the s component of the Sequencer's ECSDA signature over the block commitment. + /// @custom:reverts BadSequence if the sequence number is not the next block for the given rollup chainId. + /// @custom:reverts BlockExpired if the confirmBy time has passed. + /// @custom:reverts BadSignature if the signature provided commits to different block data. + /// @custom:reverts NotSequencer if the signer is not a permissioned sequencer. + /// @custom:emits BlockSubmitted if the block is successfully submitted. + function submitBlock(BlockHeader memory header, uint32[] memory blobIndices, uint8 v, bytes32 r, bytes32 s) + external + { // assert that the sequence number is valid and increment it - uint256 _nextSequence = nextSequence++; - if (_nextSequence != blockSequence) revert BadSequence(_nextSequence, blockSequence); + uint256 _nextSequence = nextSequence[header.rollupChainId]++; + if (_nextSequence != header.sequence) revert BadSequence(_nextSequence); + + // assert that confirmBy time has not passed + if (block.timestamp > header.confirmBy) revert BlockExpired(); // derive block commitment from sequence number and blobhashes - bytes32 blockCommit = blockCommitment(blockSequence, blobIndices); + (bytes32 blockCommit, bytes memory hashes) = blockCommitment(header, blobIndices); // derive sequencer from signature address sequencer = ecrecover(blockCommit, v, r, s); + // if the derived signer is address(0), the signature is invalid over the derived blockCommit + // emit the data required to inspect the signature off-chain + if (sequencer == address(0)) revert BadSignature(hashes, v, r, s); + // assert that sequencer is permissioned - if (!hasRole(SEQUENCER_ROLE, sequencer)) revert BadSignature(sequencer, blockCommit); + if (!hasRole(SEQUENCER_ROLE, sequencer)) revert NotSequencer(sequencer); // emit event - emit BlockSubmitted(blockSequence, sequencer, blobIndices); + emit BlockSubmitted(sequencer, header, blobIndices); } - // @notice Construct hash of block details that the sequencer signs. - // @dev See `getCommit` for hash data encoding. - // @dev Used to easily generate a correct commit hash off-chain for the sequencer to sign. - // @param blockSequence - the sequence number of the block. - // @param blobHashes - the 4844 blob hashes for the block data. - // @param commit - the hash of the encoded block details. - function blockCommitment(uint256 blockSequence, bytes32[] memory blobHashes) + /// @notice Construct hash of block details that the sequencer signs. + /// @dev See `getCommit` for hash data encoding. + /// @dev Used to easily generate a correct commit hash off-chain for the sequencer to sign. + /// @param header - the header information for the rollup block. + /// @param blobHashes - the 4844 blob hashes for the block data. + /// @param commit - the hash of the encoded block details. + function blockCommitment(BlockHeader memory header, bytes32[] memory blobHashes) external view returns (bytes32 commit) { - commit = getCommit(blockSequence, packHashes(blobHashes)); + commit = getCommit(header, packHashes(blobHashes)); } - // @notice Construct hash of block details that the sequencer signs. - // @dev See `getCommit` for hash data encoding. - // @dev Used within the transaction in which the block data is submitted as a 4844 blob. - // Relies on blob indices, which are used to read blob hashes from the transaction. - // @param blockSequence - the sequence number of the block. - // @param blobIndices - the indices of the 4844 blob hashes for the block data. - // @param commit - the hash of the encoded block details. - function blockCommitment(uint256 blockSequence, uint32[] memory blobIndices) - internal - view - returns (bytes32 commit) - { - commit = getCommit(blockSequence, getHashes(blobIndices)); - } - - // @notice Encode the array of blob hashes into a bytes string. - // @param blobHashes - the 4844 blob hashes for the block data. - // @return encodedHashes - the encoded blob hashes. - function packHashes(bytes32[] memory blobHashes) internal pure returns (bytes memory encodedHashes) { + /// @notice Encode the array of blob hashes into a bytes string. + /// @param blobHashes - the 4844 blob hashes for the block data. + /// @return encodedHashes - the encoded blob hashes. + function packHashes(bytes32[] memory blobHashes) public pure returns (bytes memory encodedHashes) { for (uint32 i = 0; i < blobHashes.length; i++) { encodedHashes = abi.encodePacked(encodedHashes, blobHashes[i]); } } - // @notice Encode an array of blob hashes, given their indices in the transaction. - // @param blobIndices - the indices of the 4844 blob hashes for the block data. - // @return encodedHashes - the encoded blob hashes. + /// @notice Construct hash of block details that the sequencer signs. + /// @dev See `getCommit` for hash data encoding. + /// @dev Used within the transaction in which the block data is submitted as a 4844 blob. + /// Relies on blob indices, which are used to read blob hashes from the transaction. + /// @param header - the header information for the rollup block. + /// @param blobIndices - the indices of the 4844 blob hashes for the block data. + /// @param commit - the hash of the encoded block details. + function blockCommitment(BlockHeader memory header, uint32[] memory blobIndices) + internal + view + returns (bytes32 commit, bytes memory hashes) + { + hashes = getHashes(blobIndices); + commit = getCommit(header, hashes); + } + + /// @notice Encode an array of blob hashes, given their indices in the transaction. + /// @param blobIndices - the indices of the 4844 blob hashes for the block data. + /// @return encodedHashes - the encoded blob hashes. function getHashes(uint32[] memory blobIndices) internal view returns (bytes memory encodedHashes) { for (uint32 i = 0; i < blobIndices.length; i++) { encodedHashes = abi.encodePacked(encodedHashes, blobhash(blobIndices[i])); } } - // @notice Construct hash of block details that the sequencer signs. - // @dev Hash is keccak256(abi.encodePacked("zenith", hostChainId, blockSequence, encodedBlobHashesLength, encodedBlobHashes)) - // @param blockSequence - the sequence number of the block. - // @param encodedHashes - the encoded blob hashes. - // @return commit - the hash of the encoded block details. - function getCommit(uint256 blockSequence, bytes memory encodedHashes) internal view returns (bytes32 commit) { - bytes memory encoded = - abi.encodePacked("zenith", block.chainid, blockSequence, encodedHashes.length, encodedHashes); + /// @notice Construct hash of block details that the sequencer signs. + /// @dev Hash is keccak256(abi.encodePacked("init4.sequencer.v0", hostChainId, rollupChainId, blockSequence, rollupGasLimit, confirmBy, rewardAddress, numBlobs, encodedBlobHashes)) + /// @param header - the header information for the rollup block. + /// @param encodedHashes - the encoded blob hashes. + /// @return commit - the hash of the encoded block details. + function getCommit(BlockHeader memory header, bytes memory encodedHashes) internal view returns (bytes32 commit) { + bytes memory encoded = abi.encodePacked( + "init4.sequencer.v0", + block.chainid, + header.rollupChainId, + header.sequence, + header.gasLimit, + header.confirmBy, + header.rewardAddress, + encodedHashes.length / 32, + encodedHashes + ); commit = keccak256(encoded); } } From 0d13b8ea5d7c1d6c7e3b1bba1fe429b6895b43a5 Mon Sep 17 00:00:00 2001 From: Anna Carroll Date: Tue, 16 Apr 2024 16:03:13 -0500 Subject: [PATCH 3/5] add tests for Zenith errors, events, and state changes --- test/Zenith.t.sol | 98 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 79 insertions(+), 19 deletions(-) diff --git a/test/Zenith.t.sol b/test/Zenith.t.sol index f07406c..00e36c2 100644 --- a/test/Zenith.t.sol +++ b/test/Zenith.t.sol @@ -7,45 +7,105 @@ import {Zenith} from "../src/Zenith.sol"; contract ZenithTest is Test { Zenith public target; + Zenith.BlockHeader header; + bytes32[] blobHashes; + uint32[] blobIndices; + bytes32 commit; + uint256 sequencerKey = 123; + uint256 notSequencerKey = 300; - event BlockSubmitted(uint256 indexed sequence, address indexed sequencer, uint32[] blobIndices); + event BlockSubmitted(address indexed sequencer, Zenith.BlockHeader indexed header, uint32[] blobIndices); function setUp() public { target = new Zenith(address(this)); target.grantRole(target.SEQUENCER_ROLE(), vm.addr(sequencerKey)); + + // set default block values + header.confirmBy = block.timestamp + 10 minutes; + header.gasLimit = 30_000_000; + header.rewardAddress = address(this); + header.rollupChainId = block.chainid + 1; + header.sequence = 0; // first block has index + + // set default blob info + blobIndices.push(0); + blobHashes.push(bytes32("JUNK BLOBHASH")); + // TODO: vm.blobhashes(blobHashes); + + // derive block commitment from sequence number and blobhashes + commit = target.blockCommitment(header, blobHashes); } - // BLOCKED by PR supporting vm.blobhashes: https://github.com/foundry-rs/foundry/pull/7001 - function BLOCKED_test_submitBlock() public { - // first block has index 0 - uint256 blockSequence = 0; + // cannot submit block with incorrect sequence number + function test_badSequence() public { + // change to incorrect sequence number + header.sequence = 1; - // construct array with fake blobhash - bytes32[] memory blobHashes = new bytes32[](1); - blobHashes[0] = bytes32("JUNK BLOBHASH"); + // sign block commitmenet with sequencer key + (uint8 v, bytes32 r, bytes32 s) = vm.sign(sequencerKey, commit); - uint32[] memory blobIndices = new uint32[](1); - blobIndices[0] = 0; + vm.expectRevert(abi.encodeWithSelector(Zenith.BadSequence.selector, 0)); + target.submitBlock(header, blobIndices, v, r, s); + } - // TODO: vm.blobhashes(blobHashes); + // cannot submit block with expired confirmBy time + function test_blockExpired() public { + // change to incorrect sequence number + header.confirmBy = block.timestamp - 1; - // derive block commitment from sequence number and blobhashes - bytes32 commit = target.blockCommitment(blockSequence, blobHashes); + // sign block commitmenet with sequencer key + (uint8 v, bytes32 r, bytes32 s) = vm.sign(sequencerKey, commit); + + vm.expectRevert(abi.encodeWithSelector(Zenith.BlockExpired.selector)); + target.submitBlock(header, blobIndices, v, r, s); + } + // BLOCKED by PR supporting vm.blobhashes: https://github.com/foundry-rs/foundry/pull/7001 + // can submit block successfully with acceptable data & correct signature provided + function BLOCKED_test_submitBlock() public { // sign block commitmenet with sequencer key (uint8 v, bytes32 r, bytes32 s) = vm.sign(sequencerKey, commit); // should emit BlockSubmitted event vm.expectEmit(); - emit BlockSubmitted(0, vm.addr(sequencerKey), blobIndices); - target.submitBlock(blockSequence, blobIndices, v, r, s); + emit BlockSubmitted(vm.addr(sequencerKey), header, blobIndices); + target.submitBlock(header, blobIndices, v, r, s); // should increment sequence number - assertEq(target.nextSequence(), blockSequence + 1); + assertEq(target.nextSequence(header.rollupChainId), header.sequence + 1); + } + + // cannot submit block with invalid sequencer signer from non-permissioned key + function BLOCKED_test_notSequencer() public { + // sign block commitmenet with NOT sequencer key + (uint8 v, bytes32 r, bytes32 s) = vm.sign(notSequencerKey, commit); + + vm.expectRevert(abi.encodeWithSelector(Zenith.NotSequencer.selector, vm.addr(notSequencerKey))); + target.submitBlock(header, blobIndices, v, r, s); + } + + // cannot submit block with sequencer signature over different block header data + function BLOCKED_test_badSignature_header() public { + // sign original block commitmenet with sequencer key + (uint8 v, bytes32 r, bytes32 s) = vm.sign(sequencerKey, commit); + + // change header data from what was signed by sequencer + header.confirmBy = block.timestamp + 15 minutes; + + vm.expectRevert(abi.encodeWithSelector(Zenith.BadSignature.selector, target.packHashes(blobHashes), v, r, s)); + target.submitBlock(header, blobIndices, v, r, s); } - // TODO: invalid sequencer - // TODO: invalid signature - // TODO: incorrect sequence number + // cannot submit block with sequencer signature over different blob hashes + function BLOCKED_test_badSignature_blobs() public { + // sign original block commitmenet with sequencer key + (uint8 v, bytes32 r, bytes32 s) = vm.sign(sequencerKey, commit); + + blobHashes[0] = bytes32("DIFFERENT BLOBHASH"); + // TODO: vm.blobhashes(blobHashes); + + vm.expectRevert(abi.encodeWithSelector(Zenith.BadSignature.selector, target.packHashes(blobHashes), v, r, s)); + target.submitBlock(header, blobIndices, v, r, s); + } } From d851236a5fe68f822e447844bc511afd58ab90e2 Mon Sep 17 00:00:00 2001 From: Anna Carroll Date: Tue, 16 Apr 2024 16:12:06 -0500 Subject: [PATCH 4/5] touchup natspec --- src/Zenith.sol | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Zenith.sol b/src/Zenith.sol index d6dfddc..5268ea4 100644 --- a/src/Zenith.sol +++ b/src/Zenith.sol @@ -7,38 +7,43 @@ import {AccessControlDefaultAdminRules} from "openzeppelin-contracts/contracts/access/extensions/AccessControlDefaultAdminRules.sol"; contract Zenith is HostPassage, AccessControlDefaultAdminRules { + /// @notice Block header information for the rollup block, signed by the sequencer. + /// @param rollupChainId - the chainId of the rollup chain. Any chainId is accepted by the contract. + /// @param sequence - the sequence number of the rollup block. Must be monotonically increasing. Enforced by the contract. + /// @param confirmBy - the timestamp by which the block must be submitted. Enforced by the contract. + /// @param gasLimit - the gas limit for the rollup block. Ignored by the contract; enforced by the Node. + /// @param rewardAddress - the address to receive the rollup block reward. Ignored by the contract; enforced by the Node. struct BlockHeader { uint256 rollupChainId; uint256 sequence; - uint256 gasLimit; uint256 confirmBy; + uint256 gasLimit; address rewardAddress; } - /// @notice Role that allows an address to sign commitments to rollup blocks. + /// @notice Role that allows a key to sign commitments to rollup blocks. bytes32 public constant SEQUENCER_ROLE = bytes32("SEQUENCER_ROLE"); - /// @notice The sequence number of the next block that can be submitted. + /// @notice The sequence number of the next block that can be submitted for a given rollup chainId. /// rollupChainId => nextSequence number mapping(uint256 => uint256) public nextSequence; - /// @notice Thrown when a block submission is attempted with a sequence number that is not the next block. - /// @dev Blocks must be submitted in strict increasing order. + /// @notice Thrown when a block submission is attempted with a sequence number that is not the next block for the rollup chainId. + /// @dev Blocks must be submitted in strict monotonic increasing order. /// @param expected - the correct next sequence number for the given rollup chainId. error BadSequence(uint256 expected); - /// @notice Thrown when a block submission is attempted where confirmBy time has passed. + /// @notice Thrown when a block submission is attempted when the confirmBy time has passed. error BlockExpired(); - /// @notice Thrown when a block submission is attempted with a signature over malformed data. + /// @notice Thrown when a block submission is attempted with a signature over different data. /// @param hashes - the encoded blob hashes attached to the transaction. - /// this is the most useful data to debug a bad signature off-chain. /// @param v - the v component of the Sequencer's ECSDA signature over the block commitment. /// @param r - the r component of the Sequencer's ECSDA signature over the block commitment. /// @param s - the s component of the Sequencer's ECSDA signature over the block commitment. error BadSignature(bytes hashes, uint8 v, bytes32 r, bytes32 s); - /// @notice Thrown when a block submission is attempted signed by a non-permissioned sequencer. + /// @notice Thrown when a block submission is attempted with a signature by a non-permissioned sequencer. /// @param sequencer - the signer of the block data that is not a permissioned sequencer. error NotSequencer(address sequencer); @@ -94,8 +99,8 @@ contract Zenith is HostPassage, AccessControlDefaultAdminRules { emit BlockSubmitted(sequencer, header, blobIndices); } - /// @notice Construct hash of block details that the sequencer signs. - /// @dev See `getCommit` for hash data encoding. + /// @notice Construct hash of the block data that the sequencer signs. + /// @dev See `getCommit` for hashed data encoding. /// @dev Used to easily generate a correct commit hash off-chain for the sequencer to sign. /// @param header - the header information for the rollup block. /// @param blobHashes - the 4844 blob hashes for the block data. From aec39db43dda262cba00df217403d957cab7438b Mon Sep 17 00:00:00 2001 From: Anna Carroll Date: Tue, 16 Apr 2024 16:14:01 -0500 Subject: [PATCH 5/5] touchup test comments --- test/Zenith.t.sol | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/Zenith.t.sol b/test/Zenith.t.sol index 00e36c2..7bfb39a 100644 --- a/test/Zenith.t.sol +++ b/test/Zenith.t.sol @@ -22,11 +22,11 @@ contract ZenithTest is Test { target.grantRole(target.SEQUENCER_ROLE(), vm.addr(sequencerKey)); // set default block values + header.rollupChainId = block.chainid + 1; + header.sequence = 0; // first block has index header.confirmBy = block.timestamp + 10 minutes; header.gasLimit = 30_000_000; header.rewardAddress = address(this); - header.rollupChainId = block.chainid + 1; - header.sequence = 0; // first block has index // set default blob info blobIndices.push(0); @@ -51,7 +51,7 @@ contract ZenithTest is Test { // cannot submit block with expired confirmBy time function test_blockExpired() public { - // change to incorrect sequence number + // change to expired confirmBy time header.confirmBy = block.timestamp - 1; // sign block commitmenet with sequencer key @@ -64,7 +64,7 @@ contract ZenithTest is Test { // BLOCKED by PR supporting vm.blobhashes: https://github.com/foundry-rs/foundry/pull/7001 // can submit block successfully with acceptable data & correct signature provided function BLOCKED_test_submitBlock() public { - // sign block commitmenet with sequencer key + // sign block commitmenet with correct sequencer key (uint8 v, bytes32 r, bytes32 s) = vm.sign(sequencerKey, commit); // should emit BlockSubmitted event