Draft: Initial version for precommits & role-based validator timelock #1461
Conversation
l1-contracts/contracts/state-transition/chain-deps/ZKChainStorage.sol
Outdated
Show resolved
Hide resolved
|
|
||
| let currentCommitment := mload(TXS_STATUS_ROLLING_HASH_BEGIN_BYTE()) | ||
|
|
||
| mstore(0, txHash) |
There was a problem hiding this comment.
is it safe to write to this memory address?
There was a problem hiding this comment.
yes, by this point in code, everything up to SCRATCH_SPACE_SLOTS + SCRATCH_SPACE_BEGIN_SLOT is safe to write to
| error InvalidBatchNumber(uint256 provided, uint256 expected); | ||
|
|
||
| error EmptyPrecommitData(uint256 batchNumber); | ||
|
|
||
| error PrecommitmentMismatch(uint256 batchNumber, bytes32 expected, bytes32 found); | ||
|
|
||
| error RoleAccessDenied(uint256 chainId, bytes32 role, address account); | ||
|
|
||
| error DefaultAdminTransferNotAllowed(); | ||
|
|
There was a problem hiding this comment.
Error selectors missing here.
l1-contracts/contracts/state-transition/AccessControlEnumerablePerChainUpgradeable.sol
Outdated
Show resolved
Hide resolved
l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol
Outdated
Show resolved
Hide resolved
l1-contracts/contracts/state-transition/chain-interfaces/IExecutor.sol
Outdated
Show resolved
Hide resolved
l1-contracts/contracts/state-transition/AccessControlEnumerablePerChainUpgradeable.sol
Outdated
Show resolved
Hide resolved
| @@ -174,4 +174,6 @@ struct ZKChainStorage { | |||
| /// @notice Bytecode hash of evm emulator. | |||
| /// @dev Used as an input to zkp-circuit. | |||
| bytes32 l2EvmEmulatorBytecodeHash; | |||
| /// todo | |||
| mapping (uint256 batchNumber => bytes32 precommitemnt) batchPrecommitments; | |||
There was a problem hiding this comment.
What do you think about other names? How about checkpoints?
I see that precommitments make sense because they go before the commit thus pre commitments. But the word commit might be misleading. A cryptographic commitment (commit-reveal) implies that it's not possible to change the value once committed. In our case that's not completely true as batches can be reverted.
This is more of an open discussion. I don't have a strong opinion in this particular case.
There was a problem hiding this comment.
IMO precommitments is a bit better as a universal long term name
Even "commitment" operation can be reverted, so the issue with naming is true even today
Signed-off-by: Danil <[email protected]>
| } | ||
|
|
||
| s.precommitmentForTheLatestBatch = currentPrecommitment; |
There was a problem hiding this comment.
could we name it rollingTxsHash in this way we keep the meaning, but detach it from precommit operation
| @@ -76,6 +76,8 @@ contract DiamondInit is ZKChainBase, IDiamondInit { | |||
| s.priorityTxMaxGasLimit = _initializeData.priorityTxMaxGasLimit; | |||
| s.feeParams = _initializeData.feeParams; | |||
| s.priorityTree.setup(s.priorityQueue.getTotalPriorityTxs()); | |||
| // FIXME: include it in the upgrade. | |||
There was a problem hiding this comment.
will be fixed after merging back draft-v29
What ❔
Why ❔
Checklist