Merge branch 'dev' into rescue_playground

.drone.yml

@@ -33,6 +33,7 @@ steps:
   - export PATH=$ZKSYNC_HOME/bin:$PATH
   - export CARGO_HOME=$ZKSYNC_HOME/target/cargo
   - ci-prepare-env.sh
+  - zksync dummy-prover ensure-disabled
   - zksync env ci
   - zksync yarn
   - zksync db-wait

.githooks/pre-commit

@@ -0,0 +1,25 @@
+#!/bin/sh
+#
+# Pre-commit hook verifying that inappropriate code will not be committed.
+
+# Colors for the terminal output
+RED='\033[0;31m'
+NC='\033[0m' # No Color
+
+# Check that `rustfmt` rules are not violated.
+if ! cargo fmt -- --check; then
+    echo "${RED}Commit error!${NC}"
+    echo "Please format the code via 'cargo fmt', cannot commit unformatted code"
+    exit 1
+fi
+
+VERFIER_CONTRACT_FILE="contracts/contracts/Verifier.sol"
+
+# Check if diff for contract contains setting the `DUMMY_VERIFIER` to the true.
+if git diff --cached $VERFIER_CONTRACT_FILE | grep -lq 'constant DUMMY_VERIFIER = true'; then
+    echo "${RED}Commit error!${NC}"
+    echo "It seems that line 'constant DUMMY_VERIFIER = true' in 'Verifier.sol' is staged to be committed"
+    echo "Cannot commit the code with enabled DUMMY_VERIFIER"
+    echo "Please disable the DUMMY_VERIFIER and try to commit changes again"
+    exit 1
+fi

README.md

@@ -112,6 +112,17 @@ zksync dockerhub-push
 
 # Development
 
+## Committing changes
+
+`zksync` uses pre-commit git hooks for basic code integrity checks. Hooks are set up automatically
+within the workspace initialization process. These hooks will not allow to commit the code which does
+not pass several checks.
+
+Currently the following criteria are checked:
+
+- Code should always be formatted via `cargo fmt`.
+- Dummy Prover should not be staged for commit (see below for the explanation).
+
 ## Database migrations
 
 - 

bin/.setup_env

@@ -2,6 +2,7 @@
 
 pushd `dirname $0`/.. > /dev/null
 
+# Setup env itself
 if [ -z "$ZKSYNC_ENV" ]
 then
 
@@ -36,3 +37,8 @@ else
 fi
 
 popd > /dev/null
+
+# Setup the git hooks folder.
+if ! git config --local core.hooksPath; then
+    git config --local core.hooksPath $ZKSYNC_HOME/.githooks/
+fi

bin/dummy-prover

@@ -7,11 +7,12 @@ NC='\033[0m' # No Color
 
 USAGE='Usage: zksync dummy-prover [-h|--help|run|status|enable|disable]
 where:
-    -h | --help     show this message
-    run           run the Dummy Prover (default)
-    status        get the status of the Dummy Prover (enabled/disabled)
-    enable        enables the Dummy Prover support
-    disable       disables the Dummy Prover support'
+    -h | --help       show this message
+    run               run the Dummy Prover (default)
+    status            get the status of the Dummy Prover (enabled/disabled)
+    enable            enables the Dummy Prover support
+    disable           disables the Dummy Prover support
+    ensure-disabled   checks that Dummy Prover is disabled and exits with code 1 otherwise'
 
 if [ -z $ZKSYNC_ENV ];
 then 
@@ -72,6 +73,22 @@ function disable_dummy_prover {
     exit 0
 }
 
+function ensure_dummy_prover_disabled {
+    # Checks for the `DUMMY_VERIFIER` constant to be `false`.
+    # This is mandatory e.g. for CI and deploy since we don't want to accidentally obtain
+    # a dummy verifier deployed.
+
+    if f grep -lq 'constant DUMMY_VERIFIER = true' $ZKSYNC_HOME/contracts/contracts/Verifier.sol; then
+      # Dummy verifier enabled, the restriction is violated.
+      echo "DUMMY_VERIFIER constant in Verifier.sol is set to 'true', which is not allowed."
+      echo "Change the DUMMY_VERIFIER constant value to 'false'"
+      exit 1
+    else
+      # Dummy verifier disabled, it's OK.
+      exit 0
+    fi
+}
+
 case $COMMAND in
   run)
       run_dummy_prover
@@ -85,6 +102,9 @@ case $COMMAND in
   disable)
       disable_dummy_prover
     ;;
+  ensure-disabled)
+      ensure_dummy_prover_disabled
+    ;;
   -h | --help)
       echo "$USAGE"
       exit 0

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