Merge pull request #418 from matter-labs/am/contracts-block-struct

Optimize `Block` structure size in contracts

contracts/contracts/Franklin.sol

@@ -362,23 +362,22 @@ contract Franklin is UpgradeableMaster, Storage, Config, Events {
 
             uint64 nPriorityRequestProcessed = totalCommittedPriorityRequests - prevTotalCommittedPriorityRequests;
 
-            createCommittedBlock(_blockNumber, _feeAccount, _newRoot, publicData, firstOnchainOpId, nOnchainOpsProcessed, nPriorityRequestProcessed);
+            createCommittedBlock(_blockNumber, _feeAccount, _newRoot, publicData, totalOnchainOps, nPriorityRequestProcessed);
             totalBlocksCommitted++;
 
             emit BlockCommitted(_blockNumber);
         }
     }
 
     /// @notice Store committed block structure to the storage.
-    /// @param _firstOnchainOpId - blocks' onchain ops start id in global operations
-    /// @param _nOnchainOpsProcessed - total number of onchain ops in block
-    /// @param _nCommittedPriorityRequests - total number of priority requests in block
+    /// @param _nCumulativeOnchainOpsProcessed - cumulative number of onchain ops
+    /// @param _nCommittedPriorityRequests - number of priority requests in block
     function createCommittedBlock(
         uint32 _blockNumber,
         uint24 _feeAccount,
         bytes32 _newRoot,
         bytes memory _publicData,
-        uint64 _firstOnchainOpId, uint64 _nOnchainOpsProcessed, uint64 _nCommittedPriorityRequests
+        uint64 _nCumulativeOnchainOpsProcessed, uint64 _nCommittedPriorityRequests
     ) internal {
         require(_publicData.length % 8 == 0, "cbb10"); // Public data size is not multiple of 8
 
@@ -394,15 +393,16 @@ contract Franklin is UpgradeableMaster, Storage, Config, Events {
             _publicData
         );
 
+        uint24 validatorId = governance.getValidatorId(msg.sender);
+
         blocks[_blockNumber] = Block(
-            msg.sender, // validator
+            validatorId, // validatorId
             uint32(block.number), // committed at
-            _firstOnchainOpId, // blocks' onchain ops start id in global operations
-            _nOnchainOpsProcessed, // total number of onchain ops in block
-            _nCommittedPriorityRequests, // total number of priority onchain ops in block
+            _nCumulativeOnchainOpsProcessed, // cumulative number of onchain ops
+            _nCommittedPriorityRequests, // number of priority onchain ops in block
+            blockChunks,
             commitment, // blocks' commitment
-            _newRoot, // new root
-            blockChunks
+            _newRoot // new root
         );
     }
 
@@ -631,9 +631,12 @@ contract Franklin is UpgradeableMaster, Storage, Config, Events {
 
         consummateOnchainOps(_blockNumber);
 
+        uint24 blockValidatorId = blocks[_blockNumber].validatorId;
+        address blockValidatorAddress = governance.getValidatorAddress(blockValidatorId);
+
         collectValidatorsFeeAndDeleteRequests(
             blocks[_blockNumber].priorityOperations,
-            blocks[_blockNumber].validator
+            blockValidatorAddress
         );
 
         totalBlocksVerified += 1;
@@ -656,8 +659,13 @@ contract Franklin is UpgradeableMaster, Storage, Config, Events {
     /// @notice (user must have possibility to withdraw funds if withdrawed)
     /// @param _blockNumber Number of block
     function consummateOnchainOps(uint32 _blockNumber) internal {
-        uint64 start = blocks[_blockNumber].operationStartId;
-        uint64 end = start + blocks[_blockNumber].onchainOperations;
+        uint64 start = 0;
+        if (_blockNumber != 0) {
+            start = blocks[_blockNumber - 1].cumulativeOnchainOperations;
+        }
+
+        uint64 end = blocks[_blockNumber].cumulativeOnchainOperations;
+
         for (uint64 current = start; current < end; ++current) {
             OnchainOperation memory op = onchainOps[current];
             if (op.opType == Operations.OpType.PartialExit) {
@@ -693,20 +701,18 @@ contract Franklin is UpgradeableMaster, Storage, Config, Events {
 
         uint32 blocksToRevert = minU32(_maxBlocksToRevert, totalBlocksCommitted - totalBlocksVerified);
         uint64 revertedPriorityRequests = 0;
-        uint64 revertedOnchainOps = 0;
 
         for (uint32 i = totalBlocksCommitted - blocksToRevert + 1; i <= totalBlocksCommitted; i++) {
             Block memory revertedBlock = blocks[i];
             require(revertedBlock.committedAtBlock > 0, "frk11"); // block not found
 
-            revertedOnchainOps += revertedBlock.onchainOperations;
             revertedPriorityRequests += revertedBlock.priorityOperations;
 
             delete blocks[i];
         }
 
         totalBlocksCommitted -= blocksToRevert;
-        totalOnchainOps -= revertedOnchainOps;
+        totalOnchainOps = blocks[totalBlocksCommitted].cumulativeOnchainOperations;
         totalCommittedPriorityRequests -= revertedPriorityRequests;
 
         emit BlocksReverted(totalBlocksVerified, totalBlocksCommitted);

contracts/contracts/Governance.sol

@@ -25,8 +25,20 @@ contract Governance is Config {
     /// @notice List of registered tokens by address
     mapping(address => uint16) public tokenIds;
 
+    /// @notice Validator information
+    struct Validator {
+        uint24 id;
+        bool isActive;
+    }
+
     /// @notice List of permitted validators
-    mapping(address => bool) public validators;
+    mapping(address => Validator) public validators;
+
+    /// @notice Mapping from validator address to id
+    mapping(uint24 => address) public validatorAddresses;
+
+    /// @notice Next validator id to insert into `validators` (0 for invalid)
+    uint24 totalValidators;
 
     constructor() public {}
 
@@ -37,7 +49,13 @@ contract Governance is Config {
         address _networkGovernor = abi.decode(initializationParameters, (address));
 
         networkGovernor = _networkGovernor;
-        validators[_networkGovernor] = true;
+
+        uint24 validatorId = totalValidators + 1;
+        Validator memory validator = Validator(validatorId, true);
+        validators[_networkGovernor] = validator;
+        validatorAddresses[validatorId] = _networkGovernor;
+
+        totalValidators += 1;
     }
 
     /// @notice Change current governor
@@ -63,11 +81,22 @@ contract Governance is Config {
     }
 
     /// @notice Change validator status (active or not active)
-    /// @param _validator Validator address
+    /// @param _validatorAddress Validator address
     /// @param _active Active flag
-    function setValidator(address _validator, bool _active) external {
+    function setValidator(address _validatorAddress, bool _active) external {
         requireGovernor(msg.sender);
-        validators[_validator] = _active;
+
+        Validator memory validator = validators[_validatorAddress];
+
+        if (validator.id == 0) {
+            validator.id = totalValidators + 1;
+            validatorAddresses[validator.id] = _validatorAddress;
+            totalValidators += 1;
+        }
+
+        validator.isActive = _active;
+
+        validators[_validatorAddress] = validator;
     }
 
     /// @notice Check if specified address is is governor
@@ -79,7 +108,25 @@ contract Governance is Config {
     /// @notice Checks if validator is active
     /// @param _address Validator address
     function requireActiveValidator(address _address) external view {
-        require(validators[_address], "grr21"); // validator is not active
+        require(validators[_address].isActive, "grr21"); // validator is not active
+    }
+
+    /// @notice Get validator's id, checking that _address is known validator's address
+    /// @param _address Validator's address
+    /// @return validator's id
+    function getValidatorId(address _address) external view returns (uint24) {
+        uint24 validatorId = validators[_address].id;
+        require(validatorId != 0, "gvi10");  // _address is not a validator's address
+        return validatorId;
+    }
+
+    /// @notice Get validator's address, checking that _validatorId is known validator's id
+    /// @param _validatorId Validator's id
+    /// @return validator's address
+    function getValidatorAddress(uint24 _validatorId) external view returns (address) {
+        address validatorAddress = validatorAddresses[_validatorId];
+        require(validatorAddress != address(0), "gva10");  // _validatorId is invalid
+        return validatorAddress;
     }
 
     /// @notice Validate token id (must be less than  or equal total tokens amount)

contracts/contracts/Storage.sol

@@ -48,20 +48,20 @@ contract Storage {
     /// @notice Rollup block data (once per block)
     /// @member validator Block producer
     /// @member committedAtBlock ETH block number at which this block was committed
-    /// @member operationStartId Index of the first operation to process for this block
-    /// @member onchainOperations Total number of operations to process for this block
+    /// @member cumulativeOnchainOperations Total number of operations in this and all previous blocks
     /// @member priorityOperations Total number of priority operations for this block
     /// @member commitment Hash of the block circuit commitment
     /// @member stateRoot New tree root hash
+    ///
+    /// Consider memory alignment when changing field order: https://solidity.readthedocs.io/en/v0.4.21/miscellaneous.html
     struct Block {
-        address validator;
+        uint24 validatorId;
         uint32 committedAtBlock;
-        uint64 operationStartId;
-        uint64 onchainOperations;
+        uint64 cumulativeOnchainOperations;
         uint64 priorityOperations;
+        uint32 chunks;
         bytes32 commitment;
         bytes32 stateRoot;
-        uint32 chunks;
     }
 
     /// @notice Blocks by Franklin block id