Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(contract): applied new solhint rules across l1, l2, and system contracts #473

Merged
merged 15 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 27 additions & 18 deletions .solhint.json
Original file line number Diff line number Diff line change
@@ -1,31 +1,40 @@
{
"extends": "solhint:recommended",
"rules": {
"state-visibility": "off",
"func-visibility": ["warn", { "ignoreConstructors": true }],
"var-name-mixedcase": "off",
"avoid-call-value": "off",
"no-empty-blocks": "off",
"not-rely-on-time": "off",
"avoid-call-value": "error",
"avoid-low-level-calls": "off",
"no-inline-assembly": "off",
"avoid-sha3": "error",
"check-send-result": "error",
"compiler-version": ["error", "^0.8.0"],
"const-name-snakecase": "off",
"no-complex-fallback": "off",
"reason-string": "off",
"contract-name-camelcase": "off",
"gas-calldata-parameters": "error",
"gas-custom-errors": "error",
"gas-increment-by-one": "error",
"gas-length-in-loops": "error",
"gas-struct-packing": "error",
"explicit-types": "error",
"func-name-mixedcase": "off",
"custom-errors": "off",
"no-unused-vars": "error",
"func-named-parameters": ["error", 4],
"func-visibility": ["error", { "ignoreConstructors": true }],
"imports-on-top": "error",
"max-states-count": "off",
"modifier-name-mixedcase": "error",
"named-parameters-mapping": "off",
"no-complex-fallback": "off",
"no-console": "error",
"no-empty-blocks": "off",
"no-global-import": "error",
"no-inline-assembly": "off",
"no-unused-import": "error",
"explicit-types": "error",
"modifier-name-mixedcase": "error",
"imports-on-top": "error",
"no-unused-vars": "error",
"not-rely-on-time": "off",
"quotes": "error",
"use-forbidden-name": "error",
"visibility-modifier-order": "error",
"reason-string": "error",
"reentrancy": "error",
"func-named-parameters": ["error", 4],
"compiler-version": ["error", "^0.8.0"]
"state-visibility": "error",
"use-forbidden-name": "error",
"var-name-mixedcase": "off",
"visibility-modifier-order": "error"
}
}
4 changes: 4 additions & 0 deletions .solhintignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ l1-contracts/cache
l1-contracts/cache-forge
l1-contracts/lib
l1-contracts/node_modules
l1-contracts/contracts/dev-contracts
l1-contracts/test

# l1-contracts-foundry
l1-contracts-foundry/cache
Expand All @@ -18,3 +20,5 @@ l2-contracts/node_modules
# system-contracts
system-contracts/contracts/openzeppelin
system-contracts/contracts/Constants.sol
system-contracts/contracts/test-contracts
system-contracts/contracts-preprocessed
16 changes: 11 additions & 5 deletions l1-contracts-foundry/script/DeployErc20.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {Script, console2 as console} from "forge-std/Script.sol";
import {stdToml} from "forge-std/StdToml.sol";

import {Utils} from "./Utils.sol";
import {MintFailed} from "./ZkSyncScriptErrors.sol";

contract DeployErc20Script is Script {
using stdToml for string;
Expand All @@ -27,7 +28,7 @@ contract DeployErc20Script is Script {
uint256 mint;
}

Config config;
Config internal config;

function run() public {
console.log("Deploying ERC20 Tokens");
Expand All @@ -52,7 +53,8 @@ contract DeployErc20Script is Script {
config.create2FactorySalt = vm.parseTomlBytes32(toml, "$.create2_factory_salt");
string[] memory tokens = vm.parseTomlKeys(toml, "$.tokens");

for (uint256 i = 0; i < tokens.length; i++) {
uint256 tokensLength = tokens.length;
for (uint256 i = 0; i < tokensLength; ++i) {
TokenDescription memory token;
string memory key = string.concat("$.tokens.", tokens[i]);
token.name = toml.readString(string.concat(key, ".name"));
Expand All @@ -66,7 +68,8 @@ contract DeployErc20Script is Script {
}

function deployTokens() internal {
for (uint256 i = 0; i < config.tokens.length; i++) {
uint256 tokensLength = config.tokens.length;
for (uint256 i = 0; i < tokensLength; ++i) {
TokenDescription memory token = config.tokens[i];
console.log("Deploying token:", token.name);
address tokenAddress = deployErc20({
Expand Down Expand Up @@ -103,7 +106,9 @@ contract DeployErc20Script is Script {
(bool success, ) = tokenAddress.call(
abi.encodeWithSignature("mint(address,uint256)", config.deployerAddress, mint)
);
require(success, "Mint failed");
if (!success) {
revert MintFailed();
}
}

return tokenAddress;
Expand All @@ -114,7 +119,8 @@ contract DeployErc20Script is Script {
vm.serializeBytes32("root", "create2_factory_salt", config.create2FactorySalt);

string memory tokens = "";
for (uint256 i = 0; i < config.tokens.length; i++) {
uint256 tokensLength = config.tokens.length;
for (uint256 i = 0; i < tokensLength; ++i) {
TokenDescription memory token = config.tokens[i];
vm.serializeString(token.symbol, "name", token.name);
vm.serializeString(token.symbol, "symbol", token.symbol);
Expand Down
17 changes: 12 additions & 5 deletions l1-contracts-foundry/script/DeployL1.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,15 @@ import {FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-de
import {L1SharedBridge} from "contracts/bridge/L1SharedBridge.sol";
import {L1ERC20Bridge} from "contracts/bridge/L1ERC20Bridge.sol";
import {DiamondProxy} from "contracts/state-transition/chain-deps/DiamondProxy.sol";
import {AddressHasNoCode} from "./ZkSyncScriptErrors.sol";

contract DeployL1Script is Script {
using stdToml for string;

address constant ADDRESS_ONE = 0x0000000000000000000000000000000000000001;
address constant DETERMINISTIC_CREATE2_ADDRESS = 0x4e59b44847b379578588920cA78FbF26c0B4956C;
address internal constant ADDRESS_ONE = 0x0000000000000000000000000000000000000001;
address internal constant DETERMINISTIC_CREATE2_ADDRESS = 0x4e59b44847b379578588920cA78FbF26c0B4956C;

// solhint-disable-next-line gas-struct-packing
struct DeployedAddresses {
BridgehubDeployedAddresses bridgehub;
StateTransitionDeployedAddresses stateTransition;
Expand All @@ -49,11 +51,13 @@ contract DeployL1Script is Script {
address create2Factory;
}

// solhint-disable-next-line gas-struct-packing
struct BridgehubDeployedAddresses {
address bridgehubImplementation;
address bridgehubProxy;
}

// solhint-disable-next-line gas-struct-packing
struct StateTransitionDeployedAddresses {
address stateTransitionProxy;
address stateTransitionImplementation;
Expand All @@ -68,13 +72,15 @@ contract DeployL1Script is Script {
address diamondProxy;
}

// solhint-disable-next-line gas-struct-packing
struct BridgesDeployedAddresses {
address erc20BridgeImplementation;
address erc20BridgeProxy;
address sharedBridgeImplementation;
address sharedBridgeProxy;
}

// solhint-disable-next-line gas-struct-packing
struct Config {
uint256 l1ChainId;
uint256 eraChainId;
Expand All @@ -84,6 +90,7 @@ contract DeployL1Script is Script {
TokensConfig tokens;
}

// solhint-disable-next-line gas-struct-packing
struct ContractsConfig {
bytes32 create2FactorySalt;
address create2FactoryAddr;
Expand Down Expand Up @@ -112,8 +119,8 @@ contract DeployL1Script is Script {
address tokenWethAddress;
}

Config config;
DeployedAddresses addresses;
Config internal config;
DeployedAddresses internal addresses;

function run() public {
console.log("Deploying L1 contracts");
Expand Down Expand Up @@ -208,7 +215,7 @@ contract DeployL1Script is Script {

if (isConfigured) {
if (config.contracts.create2FactoryAddr.code.length == 0) {
revert("Create2Factory configured address is empty");
revert AddressHasNoCode(config.contracts.create2FactoryAddr);
}
contractAddress = config.contracts.create2FactoryAddr;
console.log("Using configured Create2Factory address:", contractAddress);
Expand Down
3 changes: 2 additions & 1 deletion l1-contracts-foundry/script/InitializeL2WethToken.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {Bridgehub} from "contracts/bridgehub/Bridgehub.sol";
contract InitializeL2WethTokenScript is Script {
using stdToml for string;

// solhint-disable-next-line gas-struct-packing
struct Config {
address deployerAddress;
address create2FactoryAddr;
Expand All @@ -31,7 +32,7 @@ contract InitializeL2WethTokenScript is Script {
uint256 gasMultiplier;
}

Config config;
Config internal config;

function run() public {
initializeConfig();
Expand Down
17 changes: 10 additions & 7 deletions l1-contracts-foundry/script/RegisterHyperchain.s.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ import {VerifierParams, IVerifier} from "contracts/state-transition/chain-interf
import {FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZkSyncHyperchainStorage.sol";
import {InitializeDataNewChain as DiamondInitializeDataNewChain} from "contracts/state-transition/chain-interfaces/IDiamondInit.sol";
import {ValidatorTimelock} from "contracts/state-transition/ValidatorTimelock.sol";
import {ZksyncContract, MissingAddress, AddressHasNoCode} from "./ZkSyncScriptErrors.sol";

contract RegisterHyperchainScript is Script {
using stdToml for string;

address constant ADDRESS_ONE = 0x0000000000000000000000000000000000000001;
bytes32 constant STATE_TRANSITION_NEW_CHAIN_HASH = keccak256("NewHyperchain(uint256,address)");
address internal constant ADDRESS_ONE = 0x0000000000000000000000000000000000000001;
bytes32 internal constant STATE_TRANSITION_NEW_CHAIN_HASH = keccak256("NewHyperchain(uint256,address)");

struct Config {
ContractsConfig contracts;
Expand Down Expand Up @@ -49,6 +50,7 @@ contract RegisterHyperchainScript is Script {
uint128 baseTokenGasPriceMultiplierDenominator;
}

// solhint-disable-next-line gas-struct-packing
struct AddressesConfig {
address baseToken;
address bridgehub;
Expand All @@ -64,7 +66,7 @@ contract RegisterHyperchainScript is Script {
address newDiamondProxy;
}

Config config;
Config internal config;

function run() public {
console.log("Deploying Hyperchain");
Expand Down Expand Up @@ -146,7 +148,7 @@ contract RegisterHyperchainScript is Script {

function checkTokenAddress() internal {
if (config.addresses.baseToken == address(0)) {
revert("Token address is not set");
revert MissingAddress(ZksyncContract.BaseToken);
}

// Check if it's ethereum address
Expand All @@ -155,7 +157,7 @@ contract RegisterHyperchainScript is Script {
}

if (config.addresses.baseToken.code.length == 0) {
revert("Token address is not a contract address");
revert AddressHasNoCode(config.addresses.baseToken);
}

console.log("Using base token address:", config.addresses.baseToken);
Expand Down Expand Up @@ -248,14 +250,15 @@ contract RegisterHyperchainScript is Script {
// Get new diamond proxy address from emitted events
Vm.Log[] memory logs = vm.getRecordedLogs();
address diamondProxyAddress;
for (uint256 i = 0; i < logs.length; i++) {
uint256 logsLength = logs.length;
for (uint256 i = 0; i < logsLength; ++i) {
if (logs[i].topics[0] == STATE_TRANSITION_NEW_CHAIN_HASH) {
diamondProxyAddress = address(uint160(uint256(logs[i].topics[2])));
break;
}
}
if (diamondProxyAddress == address(0)) {
revert("Diamond proxy address not found");
revert MissingAddress(ZksyncContract.DiamondProxy);
}
config.addresses.newDiamondProxy = diamondProxyAddress;
}
Expand Down
33 changes: 19 additions & 14 deletions l1-contracts-foundry/script/Utils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity 0.8.24;

import {Vm} from "forge-std/Vm.sol";
import {ZksyncContract, FailedToDeploy, BytecodeNotSet, FailedToDeployViaCreate2} from "./ZkSyncScriptErrors.sol";

library Utils {
// Cheatcodes address, 0x7109709ECfa91a80626fF3989D68f67F5b1DD12D.
Expand Down Expand Up @@ -30,12 +31,13 @@ library Utils {

// Extract selectors from the result
string[] memory parts = vm.split(stringResult, "\n");
bytes4[] memory selectors = new bytes4[](parts.length);
for (uint256 i = 0; i < parts.length; i++) {
uint256 partsLength = parts.length;
bytes4[] memory selectors = new bytes4[](partsLength);
for (uint256 i = 0; i < partsLength; ++i) {
bytes memory part = bytes(parts[i]);
bytes memory extractedSelector = new bytes(10);
// Selector length 10 is 0x + 4 bytes
for (uint256 j = 0; j < 10; j++) {
for (uint256 j = 0; j < 10; ++j) {
extractedSelector[j] = part[j];
}
bytes4 selector = bytes4(vm.parseBytes(string(extractedSelector)));
Expand All @@ -44,16 +46,17 @@ library Utils {

// Remove `getName()` selector if existing
bool hasGetName = false;
for (uint256 i = 0; i < selectors.length; i++) {
uint256 selectorsLength = selectors.length;
for (uint256 i = 0; i < selectorsLength; ++i) {
if (selectors[i] == bytes4(keccak256("getName()"))) {
selectors[i] = selectors[selectors.length - 1];
selectors[i] = selectors[selectorsLength - 1];
hasGetName = true;
break;
}
}
if (hasGetName) {
bytes4[] memory newSelectors = new bytes4[](selectors.length - 1);
for (uint256 i = 0; i < selectors.length - 1; i++) {
bytes4[] memory newSelectors = new bytes4[](selectorsLength - 1);
for (uint256 i = 0; i < selectorsLength - 1; ++i) {
newSelectors[i] = selectors[i];
}
return newSelectors;
Expand All @@ -76,10 +79,11 @@ library Utils {
*/
function bytesToUint256(bytes memory bys) internal pure returns (uint256 value) {
// Add left padding to 32 bytes if needed
if (bys.length < 32) {
uint256 bytesLength = bys.length;
if (bytesLength < 32) {
bytes memory padded = new bytes(32);
for (uint256 i = 0; i < bys.length; i++) {
padded[i + 32 - bys.length] = bys[i];
for (uint256 i = 0; i < bytesLength; ++i) {
padded[i + 32 - bytesLength] = bys[i];
}
bys = padded;
}
Expand Down Expand Up @@ -125,8 +129,9 @@ library Utils {
child := create(0, add(bytecode, 0x20), mload(bytecode))
}
vm.stopBroadcast();
require(child != address(0), "Failed to deploy Create2Factory");
require(child.code.length > 0, "Failed to deploy Create2Factory");
if (child == address(0) || child.code.length == 0) {
revert FailedToDeploy(ZksyncContract.Create2Factory);
}
return child;
}

Expand All @@ -135,7 +140,7 @@ library Utils {
*/
function deployViaCreate2(bytes memory _bytecode, bytes32 _salt, address _factory) internal returns (address) {
if (_bytecode.length == 0) {
revert("Bytecode is not set");
revert BytecodeNotSet();
}
address contractAddress = vm.computeCreate2Address(_salt, keccak256(_bytecode), _factory);
if (contractAddress.code.length != 0) {
Expand All @@ -147,7 +152,7 @@ library Utils {
contractAddress = Utils.bytesToAddress(data);

if (!success || contractAddress == address(0) || contractAddress.code.length == 0) {
revert("Failed to deploy contract via create2");
revert FailedToDeployViaCreate2();
}

return contractAddress;
Expand Down
Loading
Loading