Skip to content

Commit

Permalink
Merge pull request #1425 from matter-labs/vd-zks-329-make-contracts-p…
Browse files Browse the repository at this point in the history
…ass-the-lint

Style changes to contracts, fix solhint lints
  • Loading branch information
dvush authored Feb 26, 2021
2 parents 0f73cc0 + 4d43e59 commit 28fd5d7
Show file tree
Hide file tree
Showing 25 changed files with 175 additions and 195 deletions.
5 changes: 1 addition & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,7 @@ jobs:
run: |
ci_run zk
ci_run zk fmt --check
ci_run zk lint rust --check
ci_run zk lint js --check
ci_run zk lint ts --check
ci_run zk lint md --check
ci_run zk lint --check
unit-tests:
runs-on: [self-hosted, CI-worker]
Expand Down
54 changes: 27 additions & 27 deletions contracts/contracts/Bytes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -159,89 +159,89 @@ library Bytes {
}

/// Reads byte stream
/// @return new_offset - offset + amount of bytes read
/// @return newOffset - offset + amount of bytes read
/// @return data - actually read data
// NOTE: theoretically possible overflow of (_offset + _length)
function read(
bytes memory _data,
uint256 _offset,
uint256 _length
) internal pure returns (uint256 new_offset, bytes memory data) {
) internal pure returns (uint256 newOffset, bytes memory data) {
data = slice(_data, _offset, _length);
new_offset = _offset + _length;
newOffset = _offset + _length;
}

// NOTE: theoretically possible overflow of (_offset + 1)
function readBool(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, bool r) {
new_offset = _offset + 1;
function readBool(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, bool r) {
newOffset = _offset + 1;
r = uint8(_data[_offset]) != 0;
}

// NOTE: theoretically possible overflow of (_offset + 1)
function readUint8(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, uint8 r) {
new_offset = _offset + 1;
function readUint8(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, uint8 r) {
newOffset = _offset + 1;
r = uint8(_data[_offset]);
}

// NOTE: theoretically possible overflow of (_offset + 2)
function readUInt16(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, uint16 r) {
new_offset = _offset + 2;
function readUInt16(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, uint16 r) {
newOffset = _offset + 2;
r = bytesToUInt16(_data, _offset);
}

// NOTE: theoretically possible overflow of (_offset + 3)
function readUInt24(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, uint24 r) {
new_offset = _offset + 3;
function readUInt24(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, uint24 r) {
newOffset = _offset + 3;
r = bytesToUInt24(_data, _offset);
}

// NOTE: theoretically possible overflow of (_offset + 4)
function readUInt32(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, uint32 r) {
new_offset = _offset + 4;
function readUInt32(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, uint32 r) {
newOffset = _offset + 4;
r = bytesToUInt32(_data, _offset);
}

// NOTE: theoretically possible overflow of (_offset + 16)
function readUInt128(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, uint128 r) {
new_offset = _offset + 16;
function readUInt128(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, uint128 r) {
newOffset = _offset + 16;
r = bytesToUInt128(_data, _offset);
}

// NOTE: theoretically possible overflow of (_offset + 20)
function readUInt160(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, uint160 r) {
new_offset = _offset + 20;
function readUInt160(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, uint160 r) {
newOffset = _offset + 20;
r = bytesToUInt160(_data, _offset);
}

// NOTE: theoretically possible overflow of (_offset + 20)
function readAddress(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, address r) {
new_offset = _offset + 20;
function readAddress(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, address r) {
newOffset = _offset + 20;
r = bytesToAddress(_data, _offset);
}

// NOTE: theoretically possible overflow of (_offset + 20)
function readBytes20(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, bytes20 r) {
new_offset = _offset + 20;
function readBytes20(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, bytes20 r) {
newOffset = _offset + 20;
r = bytesToBytes20(_data, _offset);
}

// NOTE: theoretically possible overflow of (_offset + 32)
function readBytes32(bytes memory _data, uint256 _offset) internal pure returns (uint256 new_offset, bytes32 r) {
new_offset = _offset + 32;
function readBytes32(bytes memory _data, uint256 _offset) internal pure returns (uint256 newOffset, bytes32 r) {
newOffset = _offset + 32;
r = bytesToBytes32(_data, _offset);
}

/// Trim bytes into single word
function trim(bytes memory _data, uint256 _new_length) internal pure returns (uint256 r) {
require(_new_length <= 0x20, "10"); // new_length is longer than word
require(_data.length >= _new_length, "11"); // data is to short
function trim(bytes memory _data, uint256 _newLength) internal pure returns (uint256 r) {
require(_newLength <= 0x20, "10"); // new_length is longer than word
require(_data.length >= _newLength, "11"); // data is to short

uint256 a;
assembly {
a := mload(add(_data, 0x20)) // load bytes into uint256
}

return a >> ((0x20 - _new_length) * 8);
return a >> ((0x20 - _newLength) * 8);
}

// Helper function for hex conversion.
Expand Down
60 changes: 30 additions & 30 deletions contracts/contracts/Config.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,94 +6,94 @@ pragma solidity ^0.7.0;
/// @author Matter Labs
contract Config {
/// @dev ERC20 tokens and ETH withdrawals gas limit, used only for complete withdrawals
uint256 constant WITHDRAWAL_GAS_LIMIT = 100000;
uint256 internal constant WITHDRAWAL_GAS_LIMIT = 100000;

/// @dev Bytes in one chunk
uint8 constant CHUNK_BYTES = 9;
uint8 internal constant CHUNK_BYTES = 9;

/// @dev zkSync address length
uint8 constant ADDRESS_BYTES = 20;
uint8 internal constant ADDRESS_BYTES = 20;

uint8 constant PUBKEY_HASH_BYTES = 20;
uint8 internal constant PUBKEY_HASH_BYTES = 20;

/// @dev Public key bytes length
uint8 constant PUBKEY_BYTES = 32;
uint8 internal constant PUBKEY_BYTES = 32;

/// @dev Ethereum signature r/s bytes length
uint8 constant ETH_SIGN_RS_BYTES = 32;
uint8 internal constant ETH_SIGN_RS_BYTES = 32;

/// @dev Success flag bytes length
uint8 constant SUCCESS_FLAG_BYTES = 1;
uint8 internal constant SUCCESS_FLAG_BYTES = 1;

/// @dev Max amount of tokens registered in the network (excluding ETH, which is hardcoded as tokenId = 0)
uint16 constant MAX_AMOUNT_OF_REGISTERED_TOKENS = $(MAX_AMOUNT_OF_REGISTERED_TOKENS);
uint16 internal constant MAX_AMOUNT_OF_REGISTERED_TOKENS = $(MAX_AMOUNT_OF_REGISTERED_TOKENS);

/// @dev Max account id that could be registered in the network
uint32 constant MAX_ACCOUNT_ID = (2**24) - 1;
uint32 internal constant MAX_ACCOUNT_ID = (2**24) - 1;

/// @dev Expected average period of block creation
uint256 constant BLOCK_PERIOD = 15 seconds;
uint256 internal constant BLOCK_PERIOD = 15 seconds;

/// @dev ETH blocks verification expectation
/// @dev Blocks can be reverted if they are not verified for at least EXPECT_VERIFICATION_IN.
/// @dev If set to 0 validator can revert blocks at any time.
uint256 constant EXPECT_VERIFICATION_IN = 0 hours / BLOCK_PERIOD;
uint256 internal constant EXPECT_VERIFICATION_IN = 0 hours / BLOCK_PERIOD;

uint256 constant NOOP_BYTES = 1 * CHUNK_BYTES;
uint256 constant DEPOSIT_BYTES = 6 * CHUNK_BYTES;
uint256 constant TRANSFER_TO_NEW_BYTES = 6 * CHUNK_BYTES;
uint256 constant PARTIAL_EXIT_BYTES = 6 * CHUNK_BYTES;
uint256 constant TRANSFER_BYTES = 2 * CHUNK_BYTES;
uint256 constant FORCED_EXIT_BYTES = 6 * CHUNK_BYTES;
uint256 internal constant NOOP_BYTES = 1 * CHUNK_BYTES;
uint256 internal constant DEPOSIT_BYTES = 6 * CHUNK_BYTES;
uint256 internal constant TRANSFER_TO_NEW_BYTES = 6 * CHUNK_BYTES;
uint256 internal constant PARTIAL_EXIT_BYTES = 6 * CHUNK_BYTES;
uint256 internal constant TRANSFER_BYTES = 2 * CHUNK_BYTES;
uint256 internal constant FORCED_EXIT_BYTES = 6 * CHUNK_BYTES;

/// @dev Full exit operation length
uint256 constant FULL_EXIT_BYTES = 6 * CHUNK_BYTES;
uint256 internal constant FULL_EXIT_BYTES = 6 * CHUNK_BYTES;

/// @dev ChangePubKey operation length
uint256 constant CHANGE_PUBKEY_BYTES = 6 * CHUNK_BYTES;
uint256 internal constant CHANGE_PUBKEY_BYTES = 6 * CHUNK_BYTES;

/// @dev Expiration delta for priority request to be satisfied (in seconds)
/// @dev NOTE: Priority expiration should be > (EXPECT_VERIFICATION_IN * BLOCK_PERIOD)
/// @dev otherwise incorrect block with priority op could not be reverted.
uint256 constant PRIORITY_EXPIRATION_PERIOD = 3 days;
uint256 internal constant PRIORITY_EXPIRATION_PERIOD = 3 days;

/// @dev Expiration delta for priority request to be satisfied (in ETH blocks)
uint256 constant PRIORITY_EXPIRATION =
uint256 internal constant PRIORITY_EXPIRATION =
$(defined(PRIORITY_EXPIRATION) ? PRIORITY_EXPIRATION : PRIORITY_EXPIRATION_PERIOD / BLOCK_PERIOD);

/// @dev Maximum number of priority request to clear during verifying the block
/// @dev Cause deleting storage slots cost 5k gas per each slot it's unprofitable to clear too many slots
/// @dev Value based on the assumption of ~750k gas cost of verifying and 5 used storage slots per PriorityOperation structure
uint64 constant MAX_PRIORITY_REQUESTS_TO_DELETE_IN_VERIFY = 6;
uint64 internal constant MAX_PRIORITY_REQUESTS_TO_DELETE_IN_VERIFY = 6;

/// @dev Reserved time for users to send full exit priority operation in case of an upgrade (in seconds)
uint256 constant MASS_FULL_EXIT_PERIOD = 9 days;
uint256 internal constant MASS_FULL_EXIT_PERIOD = 9 days;

/// @dev Reserved time for users to withdraw funds from full exit priority operation in case of an upgrade (in seconds)
uint256 constant TIME_TO_WITHDRAW_FUNDS_FROM_FULL_EXIT = 2 days;
uint256 internal constant TIME_TO_WITHDRAW_FUNDS_FROM_FULL_EXIT = 2 days;

/// @dev Notice period before activation preparation status of upgrade mode (in seconds)
/// @dev NOTE: we must reserve for users enough time to send full exit operation, wait maximum time for processing this operation and withdraw funds from it.
uint256 constant UPGRADE_NOTICE_PERIOD =
uint256 internal constant UPGRADE_NOTICE_PERIOD =
$(
defined(UPGRADE_NOTICE_PERIOD)
? UPGRADE_NOTICE_PERIOD
: MASS_FULL_EXIT_PERIOD + PRIORITY_EXPIRATION_PERIOD + TIME_TO_WITHDRAW_FUNDS_FROM_FULL_EXIT
);

/// @dev Timestamp - seconds since unix epoch
uint256 constant COMMIT_TIMESTAMP_NOT_OLDER = 24 hours;
uint256 internal constant COMMIT_TIMESTAMP_NOT_OLDER = 24 hours;

/// @dev Maximum available error between real commit block timestamp and analog used in the verifier (in seconds)
/// @dev Must be used cause miner's `block.timestamp` value can differ on some small value (as we know - 15 seconds)
uint256 constant COMMIT_TIMESTAMP_APPROXIMATION_DELTA = 15 minutes;
uint256 internal constant COMMIT_TIMESTAMP_APPROXIMATION_DELTA = 15 minutes;

/// @dev Bit mask to apply for verifier public input before verifying.
uint256 constant INPUT_MASK = $$(~uint256(0) >> 3);
uint256 internal constant INPUT_MASK = $$(~uint256(0) >> 3);

/// @dev Auth fact reset timelock
uint256 constant AUTH_FACT_RESET_TIMELOCK = 1 days;
uint256 internal constant AUTH_FACT_RESET_TIMELOCK = 1 days;

/// @dev Max deposit of ERC20 token that is possible to deposit
uint128 constant MAX_DEPOSIT_AMOUNT = $$((2**104) - 1);
uint128 internal constant MAX_DEPOSIT_AMOUNT = $$((2**104) - 1);
}
6 changes: 3 additions & 3 deletions contracts/contracts/DeployFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ contract DeployFactory is TokenDeployInit {
address _governor,
address _feeAccountAddress
) {
require(_firstValidator != address(0));
require(_governor != address(0));
require(_feeAccountAddress != address(0));
require(_firstValidator != address(0), "validator check");
require(_governor != address(0), "governor check");
require(_feeAccountAddress != address(0), "fee acc address check");

deployProxyContracts(_govTarget, _verifierTarget, _zkSyncTarget, _genesisRoot, _firstValidator, _governor);

Expand Down
1 change: 1 addition & 0 deletions contracts/contracts/Governance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ contract Governance is Config {

/// @notice Governance contract upgrade. Can be external because Proxy contract intercepts illegal calls of this function.
/// @param upgradeParameters Encoded representation of upgrade parameters
// solhint-disable-next-line no-empty-blocks
function upgrade(bytes calldata upgradeParameters) external {}

/// @notice Change current governor
Expand Down
31 changes: 11 additions & 20 deletions contracts/contracts/Operations.sol
Original file line number Diff line number Diff line change
Expand Up @@ -27,28 +27,19 @@ library Operations {

// Byte lengths

uint8 constant OP_TYPE_BYTES = 1;

uint8 constant TOKEN_BYTES = 2;

uint8 constant PUBKEY_BYTES = 32;

uint8 constant NONCE_BYTES = 4;

uint8 constant PUBKEY_HASH_BYTES = 20;

uint8 constant ADDRESS_BYTES = 20;

uint8 internal constant OP_TYPE_BYTES = 1;
uint8 internal constant TOKEN_BYTES = 2;
uint8 internal constant PUBKEY_BYTES = 32;
uint8 internal constant NONCE_BYTES = 4;
uint8 internal constant PUBKEY_HASH_BYTES = 20;
uint8 internal constant ADDRESS_BYTES = 20;
/// @dev Packed fee bytes lengths
uint8 constant FEE_BYTES = 2;

uint8 internal constant FEE_BYTES = 2;
/// @dev zkSync account id bytes lengths
uint8 constant ACCOUNT_ID_BYTES = 4;

uint8 constant AMOUNT_BYTES = 16;

uint8 internal constant ACCOUNT_ID_BYTES = 4;
uint8 internal constant AMOUNT_BYTES = 16;
/// @dev Signature (for example full exit signature) bytes length
uint8 constant SIGNATURE_BYTES = 64;
uint8 internal constant SIGNATURE_BYTES = 64;

// Deposit pubdata
struct Deposit {
Expand All @@ -59,7 +50,7 @@ library Operations {
address owner;
}

uint256 public constant PACKED_DEPOSIT_PUBDATA_BYTES =
uint256 internal constant PACKED_DEPOSIT_PUBDATA_BYTES =
OP_TYPE_BYTES + ACCOUNT_ID_BYTES + TOKEN_BYTES + AMOUNT_BYTES + ADDRESS_BYTES;

/// Deserialize deposit pubdata
Expand Down
6 changes: 3 additions & 3 deletions contracts/contracts/Ownable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pragma solidity ^0.7.0;
/// @author Matter Labs
contract Ownable {
/// @dev Storage position of the masters address (keccak256('eip1967.proxy.admin') - 1)
bytes32 private constant masterPosition = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;
bytes32 private constant MASTER_POSITION = 0xb53127684a568b3173ae13b9f8a6016e243e63b6e8ee1178d6a717850b5d6103;

/// @notice Contract constructor
/// @dev Sets msg sender address as masters address
Expand All @@ -24,7 +24,7 @@ contract Ownable {
/// @notice Returns contract masters address
/// @return master Master's address
function getMaster() public view returns (address master) {
bytes32 position = masterPosition;
bytes32 position = MASTER_POSITION;
assembly {
master := sload(position)
}
Expand All @@ -33,7 +33,7 @@ contract Ownable {
/// @dev Sets new masters address
/// @param _newMaster New master's address
function setMaster(address _newMaster) internal {
bytes32 position = masterPosition;
bytes32 position = MASTER_POSITION;
assembly {
sstore(position, _newMaster)
}
Expand Down
1 change: 1 addition & 0 deletions contracts/contracts/PlonkCore.sol
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// SPDX-License-Identifier: MIT OR Apache-2.0
// solhint-disable

pragma solidity >=0.5.0 <0.8.0;
pragma experimental ABIEncoderV2;
Expand Down
6 changes: 3 additions & 3 deletions contracts/contracts/Proxy.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import "./UpgradeableMaster.sol";
/// @author Matter Labs
contract Proxy is Upgradeable, UpgradeableMaster, Ownable {
/// @dev Storage position of "target" (actual implementation address: keccak256('eip1967.proxy.implementation') - 1)
bytes32 private constant targetPosition = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;
bytes32 private constant TARGET_POSITION = 0x360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc;

/// @notice Contract constructor
/// @dev Calls Ownable contract constructor and initialize target
Expand All @@ -37,7 +37,7 @@ contract Proxy is Upgradeable, UpgradeableMaster, Ownable {
/// @notice Returns target of contract
/// @return target Actual implementation address
function getTarget() public view returns (address target) {
bytes32 position = targetPosition;
bytes32 position = TARGET_POSITION;
assembly {
target := sload(position)
}
Expand All @@ -46,7 +46,7 @@ contract Proxy is Upgradeable, UpgradeableMaster, Ownable {
/// @notice Sets new target of contract
/// @param _newTarget New actual implementation address
function setTarget(address _newTarget) internal {
bytes32 position = targetPosition;
bytes32 position = TARGET_POSITION;
assembly {
sstore(position, _newTarget)
}
Expand Down
Loading

0 comments on commit 28fd5d7

Please sign in to comment.