diff --git a/l1-contracts/contracts/bridge/L1SharedBridge.sol b/l1-contracts/contracts/bridge/L1SharedBridge.sol index 0823fbd7e..532f03e6b 100644 --- a/l1-contracts/contracts/bridge/L1SharedBridge.sol +++ b/l1-contracts/contracts/bridge/L1SharedBridge.sol @@ -22,31 +22,7 @@ import {ETH_TOKEN_ADDRESS, TWO_BRIDGES_MAGIC_VALUE} from "../common/Config.sol"; import {IBridgehub, L2TransactionRequestTwoBridgesInner, L2TransactionRequestDirect} from "../bridgehub/IBridgehub.sol"; import {IGetters} from "../state-transition/chain-interfaces/IGetters.sol"; import {L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR} from "../common/L2ContractAddresses.sol"; -import { - Unauthorized, - ZeroAddress, - SharedBridgeValueAlreadySet, - SharedBridgeKey, - NoFundsTransferred, - ZeroBalance, - ValueMismatch, - NonEmptyMsgValue, - L2BridgeNotDeployed, - TokenNotSupported, - WithdrawIncorrectAmount, - EmptyDeposit, - DepositExists, - AddressAlreadyUsed, - InvalidProof, - DepositDNE, - InsufficientFunds, - DepositFailed, - ShareadBridgeValueNotSet, - WithdrawalAlreadyFinalized, - WithdrawFailed, - MalformedMessage, - InvalidSelector -} from "../common/L1ContractErrors.sol"; +import {Unauthorized, ZeroAddress, SharedBridgeValueAlreadySet, SharedBridgeKey, NoFundsTransferred, ZeroBalance, ValueMismatch, NonEmptyMsgValue, L2BridgeNotDeployed, TokenNotSupported, WithdrawIncorrectAmount, EmptyDeposit, DepositExists, AddressAlreadyUsed, InvalidProof, DepositDNE, InsufficientFunds, DepositFailed, ShareadBridgeValueNotSet, WithdrawalAlreadyFinalized, WithdrawFailed, MalformedMessage, InvalidSelector} from "../common/L1ContractErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -326,12 +302,12 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade // The token has non-standard transfer logic if (withdrawAmount != _depositAmount) { revert WithdrawIncorrectAmount(); - } + } } // empty deposit amount if (amount == 0) { revert EmptyDeposit(); - } + } bytes32 txDataHash = keccak256(abi.encode(_prevMsgSender, _l1Token, amount)); if (!hyperbridgingEnabled[_chainId]) { @@ -539,7 +515,7 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade /// @param _l2BatchNumber The L2 batch number for the withdrawal. /// @return Whether withdrawal was initiated on zkSync Era before Legacy Bridge upgrade. function _isEraLegacyTokenWithdrawal(uint256 _chainId, uint256 _l2BatchNumber) internal view returns (bool) { - if((_chainId == ERA_CHAIN_ID) && eraPostLegacyBridgeUpgradeFirstBatch == 0) { + if ((_chainId == ERA_CHAIN_ID) && eraPostLegacyBridgeUpgradeFirstBatch == 0) { revert ShareadBridgeValueNotSet(SharedBridgeKey.LegacyBridgeFirstBatch); } return (_chainId == ERA_CHAIN_ID) && (_l2BatchNumber < eraPostLegacyBridgeUpgradeFirstBatch); diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index 7260ef4cf..9eb5cd04a 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -13,20 +13,7 @@ import {IZkSyncHyperchain} from "../state-transition/chain-interfaces/IZkSyncHyp import {ETH_TOKEN_ADDRESS, TWO_BRIDGES_MAGIC_VALUE, BRIDGEHUB_MIN_SECOND_BRIDGE_ADDRESS} from "../common/Config.sol"; import {BridgehubL2TransactionRequest, L2Message, L2Log, TxStatus} from "../common/Messaging.sol"; import {AddressAliasHelper} from "../vendor/AddressAliasHelper.sol"; -import { - Unauthorized, - STMAlreadyRegistered, - STMNotRegistered, - TokenAlreadyRegistered, - TokenNotRegistered, - InvalidChainId, - WethBridgeNotSet, - BridgeHubAlreadyRegistered, - ValueMismatch, - InsufficientFunds, - NonEmptyMsgValue, - AddressTooLow -} from "../common/L1ContractErrors.sol"; +import {Unauthorized, STMAlreadyRegistered, STMNotRegistered, TokenAlreadyRegistered, TokenNotRegistered, InvalidChainId, WethBridgeNotSet, BridgeHubAlreadyRegistered, ValueMismatch, InsufficientFunds, NonEmptyMsgValue, AddressTooLow} from "../common/L1ContractErrors.sol"; contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, PausableUpgradeable { /// @notice all the ether is held by the weth bridge @@ -145,7 +132,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus if (_chainId == 0) { revert InvalidChainId(); } - if (_chainId <= type(uint48).max) { + if (_chainId > type(uint48).max) { revert InvalidChainId(); } diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 7caf0abc4..f4a4baad9 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -1,55 +1,177 @@ // SPDX-License-Identifier: MIT pragma solidity 0.8.24; +// 0x8e4a23d6 error Unauthorized(address caller); +// 0x95b66fe9 error EmptyDeposit(); +// 0x626ade30 error ValueMismatch(uint256 expected, uint256 actual); +// 0xae899454 error WithdrawalAlreadyFinalized(); +// 0xd92e233d error ZeroAddress(); +// 0xcac5fc40 error SharedBridgeValueAlreadySet(SharedBridgeKey); +// 0xcab098d8 error NoFundsTransferred(); +// 0x669567ea error ZeroBalance(); +// 0x536ec84b error NonEmptyMsgValue(); +// 0xa4f62e33 error L2BridgeNotDeployed(uint256 chainId); +// 0x06439c6b error TokenNotSupported(address token); +// 0xc92b7c8f error WithdrawIncorrectAmount(); +// 0xad2fa98e error DepositExists(); +// 0x1ff9d522 error AddressAlreadyUsed(address addr); +// 0x09bde339 error InvalidProof(); +// 0x8c04ba2d error DepositDNE(); +// 0x356680b7 error InsufficientFunds(); +// 0x79cacff1 error DepositFailed(); +// 0xfc1a3c3a error ShareadBridgeValueNotSet(SharedBridgeKey); +// 0x750b219c error WithdrawFailed(); +// 0x16509b9a error MalformedMessage(); +// 0x12ba286f error InvalidSelector(bytes4 func); +// 0xd0bc70cf error STMAlreadyRegistered(); +// 0x09865e10 error STMNotRegistered(); +// 0x4f4b634e error TokenAlreadyRegistered(address token); +// 0xddef98d7 error TokenNotRegistered(address token); +// 0x7a47c9a2 error InvalidChainId(); +// 0xe4ed5fcc error WethBridgeNotSet(); +// 0x6cf12312 error BridgeHubAlreadyRegistered(); +// 0x1eee5481 error AddressTooLow(address); +// 0xdf3a8fdd error SlotOccupied(); +// 0x43e266b0 error MalformedBytecode(BytecodeError); +// 0x72afcbf4 error OperationShouldBeReady(); +// 0xee454a75 error OperationShouldBePending(); +// 0x1a21feed error OperationExists(); +// 0x4fbe5dba error InvalidDelay(); +// 0x9b48e060 error PreviousOperationNotExecuted(); +// 0x0b08d5be error HashMismatch(bytes32 expected, bytes32 actual); +// 0xb615c2b1 error HyperchainLimitReached(); +// 0x71dcf049 error TimeNotReached(); +// 0xf0b4e88f error TooMuchGas(); +// 0x59170bf0 error MalformedCalldata(); +// 0x79e12cc3 error FacetIsFrozen(bytes4 func); -error PubdataBatchIsLessThanTxn(); +// 0x2a4a14df +error PubdataPerBatchIsLessThanTxn(); +// 0x6f1cf752 error InvalidPubdataPricingMode(); +// 0xaa7feadc error InvalidValue(); +// 0x78d2ed02 error ChainAlreadyLive(); +// 0x5428eae7 error InvalidProtocolVersion(); +// 0x682dabb4 error DiamondFreezeIncorrectState(); +// 0xc5d09071 +error InvalidPubdataMode(); +// 0x0af806e0 +error InvalidHash(); +// 0x9094af7e +error InvalidPubdataLength(); +// 0xd018e08e +error NonIncreasingTimestamp(); +// 0x2d50c33b +error TimestampError(); +// 0x1b6825bb +error LogAlreadyProcessed(uint8); +// 0xc1780bd6 +error InvalidLogSender(address sender, uint256 logKey); +// 0x6aa39880 +error UnexpectedSystemLog(uint256 logKey); +// 0xfa44b527 +error MissingSystemLogs(uint256 expected, uint256 actual); +// 0xe85392f9 +error CanOnlyProcessOneBatch(); +// 0x55ad3fd3 +error BatchHashMismatch(bytes32 exected, bytes32 actual); +// 0xf093c2e5 +error UpgradeBatchNumberIsNotZero(); +// 0x0105f9c0 +error NonSequentialBatch(); +// 0x00c6ead2 +error CantExecuteUnprovenBatches(); +// 0x213eb372 +error SystemLogsSizeOverflow(); +// 0xd8e9405c +error InvalidNumberOfBlobs(uint256 expected, uint256 numCommitments, uint256 numHashes); +// 0x2dbdba00 +error VerifyProofCommittedVerifiedMismatch(); +// 0xdab52f4b +error RevertedBatchBeforeNewBatch(); +// 0xe18cb383 +error CantRevertExecutedBatch(); +// 0x4daa985d +error PointEvalFailed(bytes); +// 0xfc7ab1d3 +error EmptyBlobVersionHash(uint256 index); +// 0x92290acc +error TxHashMismatch(); +error NonEmptyBlobVersionHash(uint256 index); +error BlobHashCommitmentError(uint256 index, bool blobHashEmpty, bool blobCommitmentEmpty); +error OnlyEraSupported(); +error BatchNotExecuted(uint256 batchNumber); +error HashedLogIsDefault(); +error BaseTokenGasPriceDenominatorNotSet(); +error TransactionNotAllowed(); +error GasPerPubdataMismatch(); +error TooManyFactoryDeps(); +error MsgValueTooLow(uint256 required, uint256 provided); +error NoFunctionsForDiamondCut(); +error UndefinedDiamondCutAction(); +error AddressHasNoCode(address); +error FacetExists(bytes4 selector, address); +error NonZeroAddress(address); +error SelectorsMustAllHaveSameFreezability(); +error NonEmptyCalldata(); +error BadReturnData(); +error MerklePathEmpty(); +error MerklePathOutOfBounds(); +error MerkleIndexOutOfBounds(); +error QueueIsEmpty(); +error InvalidUpgradeTxn(UpgradeTxVerifyParam); +error NotEnoughGas(); +error InvalidTxType(uint256 txType); +error NewProtocolVersionNotInUpgradeTxn(); +error UnexpectedNumberOfFactoryDeps(); +error PreviousUpgradeNotFinalized(bytes32 txHash); +error PreviousUpgradeNotCleaned(); enum SharedBridgeKey { PostUpgradeFirstBatch, @@ -64,3 +186,19 @@ enum BytecodeError { Length, WordsMustBeOdd } + +enum UpgradeTxVerifyParam { + From, + To, + Paymaster, + Value, + MaxFeePerGas, + MaxPriorityFeePerGas, + Reserved0, + Reserved1, + Reserved2, + Reserved3, + Signature, + PaymasterInput, + ReservedDynamic +} diff --git a/l1-contracts/contracts/common/libraries/L2ContractHelper.sol b/l1-contracts/contracts/common/libraries/L2ContractHelper.sol index cb563b546..4422c4e84 100644 --- a/l1-contracts/contracts/common/libraries/L2ContractHelper.sol +++ b/l1-contracts/contracts/common/libraries/L2ContractHelper.sol @@ -34,7 +34,7 @@ library L2ContractHelper { // bytecode length in words must be odd if (bytecodeLenInWords % 2 == 0) { revert MalformedBytecode(BytecodeError.WordsMustBeOdd); - } + } hashedBytecode = sha256(_bytecode) & 0x00000000FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF; // Setting the version of the hash hashedBytecode = (hashedBytecode | bytes32(uint256(1 << 248))); diff --git a/l1-contracts/contracts/governance/Governance.sol b/l1-contracts/contracts/governance/Governance.sol index 4107965db..92ada8a6a 100644 --- a/l1-contracts/contracts/governance/Governance.sol +++ b/l1-contracts/contracts/governance/Governance.sol @@ -4,15 +4,7 @@ pragma solidity 0.8.24; import {Ownable2Step} from "@openzeppelin/contracts/access/Ownable2Step.sol"; import {IGovernance} from "./IGovernance.sol"; -import { - ZeroAddress, - Unauthorized, - OperationShouldBeReady, - OperationShouldBePending, - OperationExists, - InvalidDelay, - PreviousOperationNotExecuted -} from "../common/L1ContractErrors.sol"; +import {ZeroAddress, Unauthorized, OperationShouldBeReady, OperationShouldBePending, OperationExists, InvalidDelay, PreviousOperationNotExecuted} from "../common/L1ContractErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev diff --git a/l1-contracts/contracts/state-transition/StateTransitionManager.sol b/l1-contracts/contracts/state-transition/StateTransitionManager.sol index 0caaccf2f..f2f9e6d02 100644 --- a/l1-contracts/contracts/state-transition/StateTransitionManager.sol +++ b/l1-contracts/contracts/state-transition/StateTransitionManager.sol @@ -21,12 +21,7 @@ import {ProposedUpgrade} from "../upgrades/BaseZkSyncUpgrade.sol"; import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {REQUIRED_L2_GAS_PRICE_PER_PUBDATA, L2_TO_L1_LOG_SERIALIZE_SIZE, DEFAULT_L2_LOGS_TREE_ROOT_HASH, EMPTY_STRING_KECCAK, SYSTEM_UPGRADE_L2_TX_TYPE, PRIORITY_TX_MAX_GAS_LIMIT} from "../common/Config.sol"; import {VerifierParams} from "./chain-interfaces/IVerifier.sol"; -import { - Unauthorized, - ZeroAddress, - HashMismatch, - HyperchainLimitReached -} from "../common/L1ContractErrors.sol"; +import {Unauthorized, ZeroAddress, HashMismatch, HyperchainLimitReached} from "../common/L1ContractErrors.sol"; /// @title State Transition Manager contract /// @author Matter Labs diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol index bda5b7b13..b75f19070 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol @@ -8,18 +8,7 @@ import {MAX_GAS_PER_TRANSACTION} from "../../../common/Config.sol"; import {FeeParams, PubdataPricingMode} from "../ZkSyncHyperchainStorage.sol"; import {ZkSyncHyperchainBase} from "./ZkSyncHyperchainBase.sol"; import {IStateTransitionManager} from "../../IStateTransitionManager.sol"; -import { - Unauthorized, - TooMuchGas, - PubdataBatchIsLessThanTxn, - InvalidPubdataPricingMode, - InvalidValue, - ChainAlreadyLive, - HashMismatch, - ValueMismatch, - InvalidProtocolVersion, - DiamondFreezeIncorrectState -} from "../../../common/L1ContractErrors.sol"; +import {Unauthorized, TooMuchGas, PubdataPerBatchIsLessThanTxn, InvalidPubdataPricingMode, InvalidValue, ChainAlreadyLive, HashMismatch, ValueMismatch, InvalidProtocolVersion, DiamondFreezeIncorrectState} from "../../../common/L1ContractErrors.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IZkSyncHyperchainBase} from "../../chain-interfaces/IZkSyncHyperchainBase.sol"; @@ -85,7 +74,7 @@ contract AdminFacet is ZkSyncHyperchainBase, IAdmin { // Double checking that the new fee params are valid, i.e. // the maximal pubdata per batch is not less than the maximal pubdata per priority transaction. if (_newFeeParams.maxPubdataPerBatch < _newFeeParams.priorityTxMaxPubdata) { - revert PubdataBatchIsLessThanTxn(); + revert PubdataPerBatchIsLessThanTxn(); } FeeParams memory oldFeeParams = s.feeParams; @@ -93,7 +82,7 @@ contract AdminFacet is ZkSyncHyperchainBase, IAdmin { // we cannot change pubdata pricing mode if (_newFeeParams.pubdataPricingMode != oldFeeParams.pubdataPricingMode) { revert InvalidPubdataPricingMode(); - } + } s.feeParams = _newFeeParams; @@ -170,9 +159,9 @@ contract AdminFacet is ZkSyncHyperchainBase, IAdmin { Diamond.DiamondStorage storage diamondStorage = Diamond.getDiamondStorage(); // diamond proxy is frozen already - if (diamondStorage.isFrozen) { - revert DiamondFreezeIncorrectState(); - } + if (diamondStorage.isFrozen) { + revert DiamondFreezeIncorrectState(); + } diamondStorage.isFrozen = true; emit Freeze(); @@ -183,9 +172,9 @@ contract AdminFacet is ZkSyncHyperchainBase, IAdmin { Diamond.DiamondStorage storage diamondStorage = Diamond.getDiamondStorage(); // diamond proxy is not frozen - if (!diamondStorage.isFrozen) { - revert DiamondFreezeIncorrectState(); - } + if (!diamondStorage.isFrozen) { + revert DiamondFreezeIncorrectState(); + } diamondStorage.isFrozen = false; emit Unfreeze(); diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol index 6439d0e35..a6a652a66 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Executor.sol @@ -11,6 +11,7 @@ import {UnsafeBytes} from "../../../common/libraries/UnsafeBytes.sol"; import {L2_BOOTLOADER_ADDRESS, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR, L2_PUBDATA_CHUNK_PUBLISHER_ADDR} from "../../../common/L2ContractAddresses.sol"; import {PubdataPricingMode} from "../ZkSyncHyperchainStorage.sol"; import {IStateTransitionManager} from "../../IStateTransitionManager.sol"; +import {ValueMismatch, InvalidPubdataMode, InvalidHash, InvalidPubdataLength, HashMismatch, NonIncreasingTimestamp, TimestampError, InvalidLogSender, TxHashMismatch, UnexpectedSystemLog, MissingSystemLogs, LogAlreadyProcessed, InvalidProtocolVersion, CanOnlyProcessOneBatch, BatchHashMismatch, UpgradeBatchNumberIsNotZero, NonSequentialBatch, CantExecuteUnprovenBatches, SystemLogsSizeOverflow, InvalidNumberOfBlobs, VerifyProofCommittedVerifiedMismatch, InvalidProof, RevertedBatchBeforeNewBatch, CantRevertExecutedBatch, PointEvalFailed, EmptyBlobVersionHash, NonEmptyBlobVersionHash, BlobHashCommitmentError} from "../../../common/L1ContractErrors.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IZkSyncHyperchainBase} from "../../chain-interfaces/IZkSyncHyperchainBase.sol"; @@ -33,16 +34,20 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { CommitBatchInfo calldata _newBatch, bytes32 _expectedSystemContractUpgradeTxHash ) internal view returns (StoredBatchInfo memory) { - require(_newBatch.batchNumber == _previousBatch.batchNumber + 1, "f"); // only commit next batch + // only commit next batch + if (_newBatch.batchNumber != _previousBatch.batchNumber + 1) { + revert ValueMismatch(_previousBatch.batchNumber + 1, _newBatch.batchNumber); + } uint8 pubdataSource = uint8(bytes1(_newBatch.pubdataCommitments[0])); PubdataPricingMode pricingMode = s.feeParams.pubdataPricingMode; - require( - pricingMode == PubdataPricingMode.Validium || - pubdataSource == uint8(PubdataSource.Calldata) || - pubdataSource == uint8(PubdataSource.Blob), - "us" - ); + if ( + pricingMode != PubdataPricingMode.Validium && + pubdataSource != uint8(PubdataSource.Calldata) && + pubdataSource != uint8(PubdataSource.Blob) + ) { + revert InvalidPubdataMode(); + } // Check that batch contain all meta information for L2 logs. // Get the chained hash of priority transaction hashes. @@ -51,19 +56,26 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { bytes32[] memory blobCommitments = new bytes32[](MAX_NUMBER_OF_BLOBS); if (pricingMode == PubdataPricingMode.Validium) { // skipping data validation for validium, we just check that the data is empty - require(logOutput.pubdataHash == 0x00, "v0h"); - require(_newBatch.pubdataCommitments.length == 1, "EF: v0l"); + if (logOutput.pubdataHash != 0x00) { + revert InvalidHash(); + } + if (_newBatch.pubdataCommitments.length != 1) { + revert InvalidPubdataLength(); + } } else if (pubdataSource == uint8(PubdataSource.Blob)) { // In this scenario, pubdataCommitments is a list of: opening point (16 bytes) || claimed value (32 bytes) || commitment (48 bytes) || proof (48 bytes)) = 144 bytes blobCommitments = _verifyBlobInformation(_newBatch.pubdataCommitments[1:], logOutput.blobHashes); } else if (pubdataSource == uint8(PubdataSource.Calldata)) { // In this scenario pubdataCommitments is actual pubdata consisting of l2 to l1 logs, l2 to l1 message, compressed smart contract bytecode, and compressed state diffs - require(_newBatch.pubdataCommitments.length <= BLOB_SIZE_BYTES, "cz"); - require( - logOutput.pubdataHash == - keccak256(_newBatch.pubdataCommitments[1:_newBatch.pubdataCommitments.length - 32]), - "wp" - ); + if (_newBatch.pubdataCommitments.length > BLOB_SIZE_BYTES) { + revert InvalidPubdataLength(); + } + if ( + logOutput.pubdataHash != + keccak256(_newBatch.pubdataCommitments[1:_newBatch.pubdataCommitments.length - 32]) + ) { + revert InvalidHash(); + } blobCommitments[0] = bytes32( _newBatch.pubdataCommitments[_newBatch.pubdataCommitments.length - 32:_newBatch .pubdataCommitments @@ -71,11 +83,17 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { ); } - require(_previousBatch.batchHash == logOutput.previousBatchHash, "l"); + if (_previousBatch.batchHash != logOutput.previousBatchHash) { + revert HashMismatch(logOutput.previousBatchHash, _previousBatch.batchHash); + } // Check that the priority operation hash in the L2 logs is as expected - require(logOutput.chainedPriorityTxsHash == _newBatch.priorityOperationsHash, "t"); + if (logOutput.chainedPriorityTxsHash != _newBatch.priorityOperationsHash) { + revert HashMismatch(logOutput.chainedPriorityTxsHash, _newBatch.priorityOperationsHash); + } // Check that the number of processed priority operations is as expected - require(logOutput.numberOfLayer1Txs == _newBatch.numberOfLayer1Txs, "ta"); + if (logOutput.numberOfLayer1Txs != _newBatch.numberOfLayer1Txs) { + revert ValueMismatch(logOutput.numberOfLayer1Txs, _newBatch.numberOfLayer1Txs); + } // Check the timestamp of the new batch _verifyBatchTimestamp(logOutput.packedBatchAndL2BlockTimestamp, _newBatch.timestamp, _previousBatch.timestamp); @@ -112,11 +130,15 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { ) internal view { // Check that the timestamp that came from the system context is expected uint256 batchTimestamp = _packedBatchAndL2BlockTimestamp >> 128; - require(batchTimestamp == _expectedBatchTimestamp, "tb"); + if (batchTimestamp != _expectedBatchTimestamp) { + revert TimestampError(); + } // While the fact that _previousBatchTimestamp < batchTimestamp is already checked on L2, // we double check it here for clarity - require(_previousBatchTimestamp < batchTimestamp, "h3"); + if (_previousBatchTimestamp >= batchTimestamp) { + revert NonIncreasingTimestamp(); + } uint256 lastL2BlockTimestamp = _packedBatchAndL2BlockTimestamp & PACKED_L2_BLOCK_TIMESTAMP_MASK; @@ -124,8 +146,14 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { // So here we need to only double check that: // - The timestamp of the batch is not too small. // - The timestamp of the last L2 block is not too big. - require(block.timestamp - COMMIT_TIMESTAMP_NOT_OLDER <= batchTimestamp, "h1"); // New batch timestamp is too small - require(lastL2BlockTimestamp <= block.timestamp + COMMIT_TIMESTAMP_APPROXIMATION_DELTA, "h2"); // The last L2 block timestamp is too big + // New batch timestamp is too small + if (block.timestamp - COMMIT_TIMESTAMP_NOT_OLDER > batchTimestamp) { + revert TimestampError(); + } + // The last L2 block timestamp is too big + if (lastL2BlockTimestamp > block.timestamp + COMMIT_TIMESTAMP_APPROXIMATION_DELTA) { + revert TimestampError(); + } } /// @dev Check that L2 logs are proper and batch contain all meta information for them @@ -157,47 +185,65 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { (bytes32 logValue, ) = UnsafeBytes.readBytes32(emittedL2Logs, i + L2_LOG_VALUE_OFFSET); // Ensure that the log hasn't been processed already - require(!_checkBit(processedLogs, uint8(logKey)), "kp"); + if (_checkBit(processedLogs, uint8(logKey))) { + revert LogAlreadyProcessed(uint8(logKey)); + } processedLogs = _setBit(processedLogs, uint8(logKey)); // Need to check that each log was sent by the correct address. if (logKey == uint256(SystemLogKey.L2_TO_L1_LOGS_TREE_ROOT_KEY)) { - require(logSender == L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, "lm"); + if (logSender != L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR) { + revert InvalidLogSender(logSender, logKey); + } logOutput.l2LogsTreeRoot = logValue; } else if (logKey == uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY)) { - require(logSender == L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, "ln"); + if (logSender != L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR) { + revert InvalidLogSender(logSender, logKey); + } logOutput.pubdataHash = logValue; } else if (logKey == uint256(SystemLogKey.STATE_DIFF_HASH_KEY)) { - require(logSender == L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, "lb"); + if (logSender != L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR) { + revert InvalidLogSender(logSender, logKey); + } logOutput.stateDiffHash = logValue; } else if (logKey == uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY)) { - require(logSender == L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR, "sc"); + if (logSender != L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR) { + revert InvalidLogSender(logSender, logKey); + } logOutput.packedBatchAndL2BlockTimestamp = uint256(logValue); } else if (logKey == uint256(SystemLogKey.PREV_BATCH_HASH_KEY)) { - require(logSender == L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR, "sv"); + if (logSender != L2_SYSTEM_CONTEXT_SYSTEM_CONTRACT_ADDR) { + revert InvalidLogSender(logSender, logKey); + } logOutput.previousBatchHash = logValue; } else if (logKey == uint256(SystemLogKey.CHAINED_PRIORITY_TXN_HASH_KEY)) { - require(logSender == L2_BOOTLOADER_ADDRESS, "bl"); + if (logSender != L2_BOOTLOADER_ADDRESS) { + revert InvalidLogSender(logSender, logKey); + } logOutput.chainedPriorityTxsHash = logValue; } else if (logKey == uint256(SystemLogKey.NUMBER_OF_LAYER_1_TXS_KEY)) { - require(logSender == L2_BOOTLOADER_ADDRESS, "bk"); + if (logSender != L2_BOOTLOADER_ADDRESS) { + revert InvalidLogSender(logSender, logKey); + } logOutput.numberOfLayer1Txs = uint256(logValue); } else if ( logKey >= uint256(SystemLogKey.BLOB_ONE_HASH_KEY) && logKey <= uint256(SystemLogKey.BLOB_SIX_HASH_KEY) ) { - require(logSender == L2_PUBDATA_CHUNK_PUBLISHER_ADDR, "pc"); + if (logSender != L2_PUBDATA_CHUNK_PUBLISHER_ADDR) { + revert InvalidLogSender(logSender, logKey); + } uint8 blobNumber = uint8(logKey) - uint8(SystemLogKey.BLOB_ONE_HASH_KEY); - // While the fact that `blobNumber` is a valid blob number is implicitly checked by the fact - // that Solidity provides array overflow protection, we still double check it manually in case - // we accidentally put `unchecked` at the top of the loop and generally for better error messages. - require(blobNumber < MAX_NUMBER_OF_BLOBS, "b6"); logOutput.blobHashes[blobNumber] = logValue; } else if (logKey == uint256(SystemLogKey.EXPECTED_SYSTEM_CONTRACT_UPGRADE_TX_HASH_KEY)) { - require(logSender == L2_BOOTLOADER_ADDRESS, "bu"); - require(_expectedSystemContractUpgradeTxHash == logValue, "ut"); + if (logSender != L2_BOOTLOADER_ADDRESS) { + revert InvalidLogSender(logSender, logKey); + } + if (_expectedSystemContractUpgradeTxHash != logValue) { + revert TxHashMismatch(); + } } else if (logKey > uint256(SystemLogKey.EXPECTED_SYSTEM_CONTRACT_UPGRADE_TX_HASH_KEY)) { - revert("ul"); + revert UnexpectedSystemLog(logKey); } } @@ -205,9 +251,13 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { // Without the protocol upgrade we expect 13 logs: 2^13 - 1 = 8191 // With the protocol upgrade we expect 14 logs: 2^14 - 1 = 16383 if (_expectedSystemContractUpgradeTxHash == bytes32(0)) { - require(processedLogs == 8191, "b7"); + if (processedLogs != 8191) { + revert MissingSystemLogs(8191, processedLogs); + } } else { - require(processedLogs == 16383, "b8"); + if (processedLogs != 16383) { + revert MissingSystemLogs(16383, processedLogs); + } } } @@ -239,14 +289,21 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { // 2. A chain might become out of sync if it launches while we are in the middle of a protocol upgrade. This would mean they cannot process their genesis upgrade // as their protocolversion would be outdated, and they also cannot process the protocol upgrade tx as they have a pending upgrade. // 3. The protocol upgrade is increased in the BaseZkSyncUpgrade, in the executor only the systemContractsUpgradeTxHash is checked - require( - IStateTransitionManager(s.stateTransitionManager).protocolVersionIsActive(s.protocolVersion), - "Executor facet: wrong protocol version" - ); + if (!IStateTransitionManager(s.stateTransitionManager).protocolVersionIsActive(s.protocolVersion)) { + revert InvalidProtocolVersion(); + } // With the new changes for EIP-4844, namely the restriction on number of blobs per block, we only allow for a single batch to be committed at a time. - require(_newBatchesData.length == 1, "e4"); + if (_newBatchesData.length != 1) { + revert CanOnlyProcessOneBatch(); + } // Check that we commit batches after last committed batch - require(s.storedBatchHashes[s.totalBatchesCommitted] == _hashStoredBatchInfo(_lastCommittedBatchData), "i"); // incorrect previous batch data + if (s.storedBatchHashes[s.totalBatchesCommitted] != _hashStoredBatchInfo(_lastCommittedBatchData)) { + // incorrect previous batch data + revert BatchHashMismatch( + s.storedBatchHashes[s.totalBatchesCommitted], + _hashStoredBatchInfo(_lastCommittedBatchData) + ); + } bytes32 systemContractsUpgradeTxHash = s.l2SystemContractsUpgradeTxHash; // Upgrades are rarely done so we optimize a case with no active system contracts upgrade. @@ -298,7 +355,9 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { // While the logic of the contract ensures that the s.l2SystemContractsUpgradeBatchNumber is 0 when this function is called, // this check is added just in case. Since it is a hot read, it does not encure noticeable gas cost. - require(s.l2SystemContractsUpgradeBatchNumber == 0, "ik"); + if (s.l2SystemContractsUpgradeBatchNumber != 0) { + revert UpgradeBatchNumberIsNotZero(); + } // Save the batch number where the upgrade transaction was executed. s.l2SystemContractsUpgradeBatchNumber = _newBatchesData[0].batchNumber; @@ -338,14 +397,17 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { /// @dev _executedBatchIdx is an index in the array of the batches that we want to execute together function _executeOneBatch(StoredBatchInfo memory _storedBatch, uint256 _executedBatchIdx) internal { uint256 currentBatchNumber = _storedBatch.batchNumber; - require(currentBatchNumber == s.totalBatchesExecuted + _executedBatchIdx + 1, "k"); // Execute batches in order - require( - _hashStoredBatchInfo(_storedBatch) == s.storedBatchHashes[currentBatchNumber], - "exe10" // executing batch should be committed - ); + if (currentBatchNumber != s.totalBatchesExecuted + _executedBatchIdx + 1) { + revert NonSequentialBatch(); + } + if (_hashStoredBatchInfo(_storedBatch) != s.storedBatchHashes[currentBatchNumber]) { + revert BatchHashMismatch(s.storedBatchHashes[currentBatchNumber], _hashStoredBatchInfo(_storedBatch)); + } bytes32 priorityOperationsHash = _collectOperationsFromPriorityQueue(_storedBatch.numberOfLayer1Txs); - require(priorityOperationsHash == _storedBatch.priorityOperationsHash, "x"); // priority operations hash does not match to expected + if (priorityOperationsHash != _storedBatch.priorityOperationsHash) { + revert TxHashMismatch(); + } // Save root hash of L2 -> L1 logs tree s.l2LogsRootHashes[currentBatchNumber] = _storedBatch.l2LogsTreeRoot; @@ -373,7 +435,9 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { uint256 newTotalBatchesExecuted = s.totalBatchesExecuted + nBatches; s.totalBatchesExecuted = newTotalBatchesExecuted; - require(newTotalBatchesExecuted <= s.totalBatchesVerified, "n"); // Can't execute batches more than committed and proven currently. + if (newTotalBatchesExecuted > s.totalBatchesVerified) { + revert CantExecuteUnprovenBatches(); + } uint256 batchWhenUpgradeHappened = s.l2SystemContractsUpgradeBatchNumber; if (batchWhenUpgradeHappened != 0 && batchWhenUpgradeHappened <= newTotalBatchesExecuted) { @@ -414,22 +478,31 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { uint256[] memory proofPublicInput = new uint256[](committedBatchesLength); // Check that the batch passed by the validator is indeed the first unverified batch - require(_hashStoredBatchInfo(_prevBatch) == s.storedBatchHashes[currentTotalBatchesVerified], "t1"); + if (_hashStoredBatchInfo(_prevBatch) != s.storedBatchHashes[currentTotalBatchesVerified]) { + revert BatchHashMismatch( + s.storedBatchHashes[currentTotalBatchesVerified], + _hashStoredBatchInfo(_prevBatch) + ); + } bytes32 prevBatchCommitment = _prevBatch.commitment; for (uint256 i = 0; i < committedBatchesLength; i = i.uncheckedInc()) { currentTotalBatchesVerified = currentTotalBatchesVerified.uncheckedInc(); - require( - _hashStoredBatchInfo(_committedBatches[i]) == s.storedBatchHashes[currentTotalBatchesVerified], - "o1" - ); + if (_hashStoredBatchInfo(_committedBatches[i]) != s.storedBatchHashes[currentTotalBatchesVerified]) { + revert BatchHashMismatch( + s.storedBatchHashes[currentTotalBatchesVerified], + _hashStoredBatchInfo(_committedBatches[i]) + ); + } bytes32 currentBatchCommitment = _committedBatches[i].commitment; proofPublicInput[i] = _getBatchProofPublicInput(prevBatchCommitment, currentBatchCommitment); prevBatchCommitment = currentBatchCommitment; } - require(currentTotalBatchesVerified <= s.totalBatchesCommitted, "q"); + if (currentTotalBatchesVerified > s.totalBatchesCommitted) { + revert VerifyProofCommittedVerifiedMismatch(); + } _verifyProof(proofPublicInput, _proof); @@ -439,14 +512,18 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { function _verifyProof(uint256[] memory proofPublicInput, ProofInput calldata _proof) internal view { // We can only process 1 batch proof at a time. - require(proofPublicInput.length == 1, "t4"); + if (proofPublicInput.length != 1) { + revert CanOnlyProcessOneBatch(); + } bool successVerifyProof = s.verifier.verify( proofPublicInput, _proof.serializedProof, _proof.recursiveAggregationInput ); - require(successVerifyProof, "p"); // Proof verification fail + if (!successVerifyProof) { + revert InvalidProof(); + } } /// @dev Gets zk proof public input @@ -469,8 +546,12 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { } function _revertBatches(uint256 _newLastBatch) internal { - require(s.totalBatchesCommitted > _newLastBatch, "v1"); // The last committed batch is less than new last batch - require(_newLastBatch >= s.totalBatchesExecuted, "v2"); // Already executed batches cannot be reverted + if (s.totalBatchesCommitted <= _newLastBatch) { + revert RevertedBatchBeforeNewBatch(); + } + if (_newLastBatch < s.totalBatchesExecuted) { + revert CantRevertExecutedBatch(); + } if (_newLastBatch < s.totalBatchesVerified) { s.totalBatchesVerified = _newLastBatch; @@ -531,7 +612,9 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { bytes32[] memory _blobCommitments, bytes32[] memory _blobHashes ) internal pure returns (bytes memory) { - require(_batch.systemLogs.length <= MAX_L2_TO_L1_LOGS_COMMITMENT_BYTES, "pu"); + if (_batch.systemLogs.length > MAX_L2_TO_L1_LOGS_COMMITMENT_BYTES) { + revert SystemLogsSizeOverflow(); + } bytes32 l2ToL1LogsHash = keccak256(_batch.systemLogs); @@ -556,8 +639,9 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { ) internal pure returns (bytes32[] memory blobAuxOutputWords) { // These invariants should be checked by the caller of this function, but we double check // just in case. - require(_blobCommitments.length == MAX_NUMBER_OF_BLOBS, "b10"); - require(_blobHashes.length == MAX_NUMBER_OF_BLOBS, "b11"); + if (_blobCommitments.length != MAX_NUMBER_OF_BLOBS || _blobHashes.length != MAX_NUMBER_OF_BLOBS) { + revert InvalidNumberOfBlobs(MAX_NUMBER_OF_BLOBS, _blobCommitments.length, _blobHashes.length); + } // for each blob we have: // linear hash (hash of preimage from system logs) and @@ -605,9 +689,13 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { // We verify that the point evaluation precompile call was successful by testing the latter 32 bytes of the // response is equal to BLS_MODULUS as defined in https://eips.ethereum.org/EIPS/eip-4844#point-evaluation-precompile - require(success, "failed to call point evaluation precompile"); + if (!success) { + revert PointEvalFailed(precompileInput); + } (, uint256 result) = abi.decode(data, (uint256, uint256)); - require(result == BLS_MODULUS, "precompile unexpected output"); + if (result != BLS_MODULUS) { + revert PointEvalFailed(abi.encode(result)); + } } /// @dev Verifies that the blobs contain the correct data by calling the point evaluation precompile. For the precompile we need: @@ -620,16 +708,24 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { ) internal view returns (bytes32[] memory blobCommitments) { uint256 versionedHashIndex = 0; - require(_pubdataCommitments.length > 0, "pl"); - require(_pubdataCommitments.length <= PUBDATA_COMMITMENT_SIZE * MAX_NUMBER_OF_BLOBS, "bd"); - require(_pubdataCommitments.length % PUBDATA_COMMITMENT_SIZE == 0, "bs"); + if (_pubdataCommitments.length == 0) { + revert InvalidPubdataLength(); + } + if (_pubdataCommitments.length > PUBDATA_COMMITMENT_SIZE * MAX_NUMBER_OF_BLOBS) { + revert InvalidPubdataLength(); + } + if (_pubdataCommitments.length % PUBDATA_COMMITMENT_SIZE != 0) { + revert InvalidPubdataLength(); + } blobCommitments = new bytes32[](MAX_NUMBER_OF_BLOBS); // solhint-disable-next-line gas-length-in-loops for (uint256 i = 0; i < _pubdataCommitments.length; i += PUBDATA_COMMITMENT_SIZE) { bytes32 blobVersionedHash = _getBlobVersionedHash(versionedHashIndex); - require(blobVersionedHash != bytes32(0), "vh"); + if (blobVersionedHash == bytes32(0)) { + revert EmptyBlobVersionHash(versionedHashIndex); + } // First 16 bytes is the opening point. While we get the point as 16 bytes, the point evaluation precompile // requires it to be 32 bytes. The blob commitment must use the opening point as 16 bytes though. @@ -653,16 +749,19 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor { // This check is required because we want to ensure that there aren't any extra blobs trying to be published. // Calling the BLOBHASH opcode with an index > # blobs - 1 yields bytes32(0) bytes32 versionedHash = _getBlobVersionedHash(versionedHashIndex); - require(versionedHash == bytes32(0), "lh"); + if (versionedHash != bytes32(0)) { + revert NonEmptyBlobVersionHash(versionedHashIndex); + } // We verify that for each set of blobHash/blobCommitment are either both empty // or there are values for both. for (uint256 i = 0; i < MAX_NUMBER_OF_BLOBS; ++i) { - require( - (_blobHashes[i] == bytes32(0) && blobCommitments[i] == bytes32(0)) || - (_blobHashes[i] != bytes32(0) && blobCommitments[i] != bytes32(0)), - "bh" - ); + if ( + (_blobHashes[i] == bytes32(0) && blobCommitments[i] != bytes32(0)) || + (_blobHashes[i] != bytes32(0) && blobCommitments[i] == bytes32(0)) + ) { + revert BlobHashCommitmentError(i, _blobHashes[i] == bytes32(0), blobCommitments[i] == bytes32(0)); + } } } diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol index 1769dceca..d45f38663 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol @@ -10,6 +10,7 @@ import {PriorityQueue, PriorityOperation} from "../../../state-transition/librar import {UncheckedMath} from "../../../common/libraries/UncheckedMath.sol"; import {IGetters} from "../../chain-interfaces/IGetters.sol"; import {ILegacyGetters} from "../../chain-interfaces/ILegacyGetters.sol"; +import {InvalidSelector} from "../../../common/L1ContractErrors.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IZkSyncHyperchainBase} from "../../chain-interfaces/IZkSyncHyperchainBase.sol"; @@ -180,7 +181,9 @@ contract GettersFacet is ZkSyncHyperchainBase, IGetters, ILegacyGetters { /// @inheritdoc IGetters function isFunctionFreezable(bytes4 _selector) external view returns (bool) { Diamond.DiamondStorage storage ds = Diamond.getDiamondStorage(); - require(ds.selectorToFacet[_selector].facetAddress != address(0), "g2"); + if (ds.selectorToFacet[_selector].facetAddress == address(0)) { + revert InvalidSelector(_selector); + } return ds.selectorToFacet[_selector].isFreezable; } diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol index f5e5f0590..e9f96c685 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Mailbox.sol @@ -20,6 +20,8 @@ import {L2_BOOTLOADER_ADDRESS, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR} from ".. import {IL1SharedBridge} from "../../../bridge/interfaces/IL1SharedBridge.sol"; +import {OnlyEraSupported, BatchNotExecuted, HashedLogIsDefault, BaseTokenGasPriceDenominatorNotSet, TransactionNotAllowed, GasPerPubdataMismatch, TooManyFactoryDeps, MsgValueTooLow} from "../../../common/L1ContractErrors.sol"; + // While formally the following import is not used, it is needed to inherit documentation from it import {IZkSyncHyperchainBase} from "../../chain-interfaces/IZkSyncHyperchainBase.sol"; @@ -42,7 +44,9 @@ contract MailboxFacet is ZkSyncHyperchainBase, IMailbox { /// @inheritdoc IMailbox function transferEthToSharedBridge() external onlyBaseTokenBridge { - require(s.chainId == ERA_CHAIN_ID, "Mailbox: transferEthToSharedBridge only available for Era on mailbox"); + if (s.chainId != ERA_CHAIN_ID) { + revert OnlyEraSupported(); + } uint256 amount = address(this).balance; address baseTokenBridgeAddress = s.baseTokenBridge; @@ -113,7 +117,9 @@ contract MailboxFacet is ZkSyncHyperchainBase, IMailbox { L2Log memory _log, bytes32[] calldata _proof ) internal view returns (bool) { - require(_batchNumber <= s.totalBatchesExecuted, "xx"); + if (_batchNumber > s.totalBatchesExecuted) { + revert BatchNotExecuted(_batchNumber); + } bytes32 hashedLog = keccak256( // solhint-disable-next-line func-named-parameters @@ -121,7 +127,9 @@ contract MailboxFacet is ZkSyncHyperchainBase, IMailbox { ); // Check that hashed log is not the default one, // otherwise it means that the value is out of range of sent L2 -> L1 logs - require(hashedLog != L2_L1_LOGS_TREE_DEFAULT_LEAF_HASH, "tw"); + if (hashedLog == L2_L1_LOGS_TREE_DEFAULT_LEAF_HASH) { + revert HashedLogIsDefault(); + } // It is ok to not check length of `_proof` array, as length // of leaf preimage (which is `L2_TO_L1_LOG_SERIALIZE_SIZE`) is not @@ -162,7 +170,9 @@ contract MailboxFacet is ZkSyncHyperchainBase, IMailbox { /// @return The price of L2 gas in the base token function _deriveL2GasPrice(uint256 _l1GasPrice, uint256 _gasPerPubdata) internal view returns (uint256) { FeeParams memory feeParams = s.feeParams; - require(s.baseTokenGasPriceMultiplierDenominator > 0, "Mailbox: baseTokenGasPriceDenominator not set"); + if (s.baseTokenGasPriceMultiplierDenominator == 0) { + revert BaseTokenGasPriceDenominatorNotSet(); + } uint256 l1GasPriceConverted = (_l1GasPrice * s.baseTokenGasPriceMultiplierNominator) / s.baseTokenGasPriceMultiplierDenominator; uint256 pubdataPriceBaseToken; @@ -191,7 +201,9 @@ contract MailboxFacet is ZkSyncHyperchainBase, IMailbox { bytes calldata _message, bytes32[] calldata _merkleProof ) external nonReentrant { - require(s.chainId == ERA_CHAIN_ID, "Mailbox: finalizeEthWithdrawal only available for Era on mailbox"); + if (s.chainId != ERA_CHAIN_ID) { + revert OnlyEraSupported(); + } IL1SharedBridge(s.baseTokenBridge).finalizeWithdrawal({ _chainId: ERA_CHAIN_ID, _l2BatchNumber: _l2BatchNumber, @@ -212,7 +224,9 @@ contract MailboxFacet is ZkSyncHyperchainBase, IMailbox { bytes[] calldata _factoryDeps, address _refundRecipient ) external payable returns (bytes32 canonicalTxHash) { - require(s.chainId == ERA_CHAIN_ID, "Mailbox: legacy interface only available for Era"); + if (s.chainId != ERA_CHAIN_ID) { + revert OnlyEraSupported(); + } canonicalTxHash = _requestL2TransactionSender( BridgehubL2TransactionRequest({ sender: msg.sender, @@ -239,17 +253,18 @@ contract MailboxFacet is ZkSyncHyperchainBase, IMailbox { ) internal nonReentrant returns (bytes32 canonicalTxHash) { // Check that the transaction is allowed by the filterer (if the filterer is set). if (s.transactionFilterer != address(0)) { - require( - ITransactionFilterer(s.transactionFilterer).isTransactionAllowed({ + if ( + !ITransactionFilterer(s.transactionFilterer).isTransactionAllowed({ sender: _request.sender, contractL2: _request.contractL2, mintValue: _request.mintValue, l2Value: _request.l2Value, l2Calldata: _request.l2Calldata, refundRecipient: _request.refundRecipient - }), - "tf" - ); + }) + ) { + revert TransactionNotAllowed(); + } } // Enforcing that `_request.l2GasPerPubdataByteLimit` equals to a certain constant number. This is needed @@ -257,7 +272,9 @@ contract MailboxFacet is ZkSyncHyperchainBase, IMailbox { // VERY IMPORTANT: nobody should rely on this constant to be fixed and every contract should give their users the ability to provide the // ability to provide `_request.l2GasPerPubdataByteLimit` for each independent transaction. // CHANGING THIS CONSTANT SHOULD BE A CLIENT-SIDE CHANGE. - require(_request.l2GasPerPubdataByteLimit == REQUIRED_L2_GAS_PRICE_PER_PUBDATA, "qp"); + if (_request.l2GasPerPubdataByteLimit != REQUIRED_L2_GAS_PRICE_PER_PUBDATA) { + revert GasPerPubdataMismatch(); + } WritePriorityOpParams memory params; params.request = _request; @@ -268,13 +285,17 @@ contract MailboxFacet is ZkSyncHyperchainBase, IMailbox { function _requestL2Transaction(WritePriorityOpParams memory _params) internal returns (bytes32 canonicalTxHash) { BridgehubL2TransactionRequest memory request = _params.request; - require(request.factoryDeps.length <= MAX_NEW_FACTORY_DEPS, "uj"); + if (request.factoryDeps.length > MAX_NEW_FACTORY_DEPS) { + revert TooManyFactoryDeps(); + } _params.txId = s.priorityQueue.getTotalPriorityTxs(); // Checking that the user provided enough ether to pay for the transaction. _params.l2GasPrice = _deriveL2GasPrice(tx.gasprice, request.l2GasPerPubdataByteLimit); uint256 baseCost = _params.l2GasPrice * request.l2GasLimit; - require(request.mintValue >= baseCost + request.l2Value, "mv"); // The `msg.value` doesn't cover the transaction cost + if (request.mintValue < baseCost + request.l2Value) { + revert MsgValueTooLow(baseCost + request.l2Value, request.mintValue); + } request.refundRecipient = AddressAliasHelper.actualRefundRecipient(request.refundRecipient, request.sender); // Change the sender address if it is a smart contract to prevent address collision between L1 and L2. diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/ZkSyncHyperchainBase.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/ZkSyncHyperchainBase.sol index 59662d409..0910fcab3 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/ZkSyncHyperchainBase.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/ZkSyncHyperchainBase.sol @@ -5,6 +5,8 @@ pragma solidity 0.8.24; import {ZkSyncHyperchainStorage} from "../ZkSyncHyperchainStorage.sol"; import {ReentrancyGuard} from "../../../common/ReentrancyGuard.sol"; +import {Unauthorized} from "../../../common/L1ContractErrors.sol"; + /// @title Base contract containing functions accessible to the other facets. /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -14,44 +16,52 @@ contract ZkSyncHyperchainBase is ReentrancyGuard { /// @notice Checks that the message sender is an active admin modifier onlyAdmin() { - require(msg.sender == s.admin, "Hyperchain: not admin"); + if (msg.sender != s.admin) { + revert Unauthorized(msg.sender); + } _; } /// @notice Checks if validator is active modifier onlyValidator() { - require(s.validators[msg.sender], "Hyperchain: not validator"); + if (!s.validators[msg.sender]) { + revert Unauthorized(msg.sender); + } _; } modifier onlyStateTransitionManager() { - require(msg.sender == s.stateTransitionManager, "Hyperchain: not state transition manager"); + if (msg.sender != s.stateTransitionManager) { + revert Unauthorized(msg.sender); + } _; } modifier onlyBridgehub() { - require(msg.sender == s.bridgehub, "Hyperchain: not bridgehub"); + if (msg.sender != s.bridgehub) { + revert Unauthorized(msg.sender); + } _; } modifier onlyAdminOrStateTransitionManager() { - require( - msg.sender == s.admin || msg.sender == s.stateTransitionManager, - "Hyperchain: Only by admin or state transition manager" - ); + if (msg.sender != s.admin && msg.sender != s.stateTransitionManager) { + revert Unauthorized(msg.sender); + } _; } modifier onlyValidatorOrStateTransitionManager() { - require( - s.validators[msg.sender] || msg.sender == s.stateTransitionManager, - "Hyperchain: Only by validator or state transition manager" - ); + if (!s.validators[msg.sender] && msg.sender != s.stateTransitionManager) { + revert Unauthorized(msg.sender); + } _; } modifier onlyBaseTokenBridge() { - require(msg.sender == s.baseTokenBridge, "Hyperchain: Only base token bridge can call this function"); + if (msg.sender != s.baseTokenBridge) { + revert Unauthorized(msg.sender); + } _; } } diff --git a/l1-contracts/contracts/state-transition/libraries/Diamond.sol b/l1-contracts/contracts/state-transition/libraries/Diamond.sol index 8b3ecd873..98cde91f3 100644 --- a/l1-contracts/contracts/state-transition/libraries/Diamond.sol +++ b/l1-contracts/contracts/state-transition/libraries/Diamond.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.24; import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import {UncheckedMath} from "../../common/libraries/UncheckedMath.sol"; +import {NoFunctionsForDiamondCut, UndefinedDiamondCutAction, AddressHasNoCode, FacetExists, ZeroAddress, NonZeroAddress, SelectorsMustAllHaveSameFreezability, NonEmptyCalldata, MalformedCalldata, BadReturnData} from "../../common/L1ContractErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -105,7 +106,9 @@ library Diamond { bool isFacetFreezable = facetCuts[i].isFreezable; bytes4[] memory selectors = facetCuts[i].selectors; - require(selectors.length > 0, "B"); // no functions for diamond cut + if (selectors.length == 0) { + revert NoFunctionsForDiamondCut(); + } if (action == Action.Add) { _addFunctions(facet, selectors, isFacetFreezable); @@ -114,7 +117,7 @@ library Diamond { } else if (action == Action.Remove) { _removeFunctions(facet, selectors); } else { - revert("C"); // undefined diamond cut action + revert UndefinedDiamondCutAction(); } } @@ -130,7 +133,9 @@ library Diamond { // Facet with no code cannot be added. // This check also verifies that the facet does not have zero address, since it is the // address with which 0x00000000 selector is associated. - require(_facet.code.length > 0, "G"); + if (_facet.code.length == 0) { + revert AddressHasNoCode(_facet); + } // Add facet to the list of facets if the facet address is new one _saveFacetIfNew(_facet); @@ -139,7 +144,9 @@ library Diamond { for (uint256 i = 0; i < selectorsLength; i = i.uncheckedInc()) { bytes4 selector = _selectors[i]; SelectorToFacet memory oldFacet = ds.selectorToFacet[selector]; - require(oldFacet.facetAddress == address(0), "J"); // facet for this selector already exists + if (oldFacet.facetAddress != address(0)) { + revert FacetExists(selector, oldFacet.facetAddress); + } _addOneFunction(_facet, selector, _isFacetFreezable); } @@ -153,13 +160,18 @@ library Diamond { // Facet with no code cannot be added. // This check also verifies that the facet does not have zero address, since it is the // address with which 0x00000000 selector is associated. - require(_facet.code.length > 0, "K"); + if (_facet.code.length == 0) { + revert AddressHasNoCode(_facet); + } uint256 selectorsLength = _selectors.length; for (uint256 i = 0; i < selectorsLength; i = i.uncheckedInc()) { bytes4 selector = _selectors[i]; SelectorToFacet memory oldFacet = ds.selectorToFacet[selector]; - require(oldFacet.facetAddress != address(0), "L"); // it is impossible to replace the facet with zero address + // it is impossible to replace the facet with zero address + if (oldFacet.facetAddress == address(0)) { + revert ZeroAddress(); + } _removeOneFunction(oldFacet.facetAddress, selector); // Add facet to the list of facets if the facet address is a new one @@ -173,13 +185,19 @@ library Diamond { function _removeFunctions(address _facet, bytes4[] memory _selectors) private { DiamondStorage storage ds = getDiamondStorage(); - require(_facet == address(0), "a1"); // facet address must be zero + // facet address must be zero + if (_facet != address(0)) { + revert NonZeroAddress(_facet); + } uint256 selectorsLength = _selectors.length; for (uint256 i = 0; i < selectorsLength; i = i.uncheckedInc()) { bytes4 selector = _selectors[i]; SelectorToFacet memory oldFacet = ds.selectorToFacet[selector]; - require(oldFacet.facetAddress != address(0), "a2"); // Can't delete a non-existent facet + // Can't delete a non-existent facet + if (oldFacet.facetAddress == address(0)) { + revert ZeroAddress(); + } _removeOneFunction(oldFacet.facetAddress, selector); } @@ -213,7 +231,9 @@ library Diamond { // so all the selectors in a facet will have the same freezability if (selectorPosition != 0) { bytes4 selector0 = ds.facetToSelectors[_facet].selectors[0]; - require(_isSelectorFreezable == ds.selectorToFacet[selector0].isFreezable, "J1"); + if (_isSelectorFreezable != ds.selectorToFacet[selector0].isFreezable) { + revert SelectorsMustAllHaveSameFreezability(); + } } ds.selectorToFacet[_selector] = SelectorToFacet({ @@ -278,14 +298,17 @@ library Diamond { /// @dev Used as a final step of diamond cut to execute the logic of the initialization for changed facets function _initializeDiamondCut(address _init, bytes memory _calldata) private { if (_init == address(0)) { - require(_calldata.length == 0, "H"); // Non-empty calldata for zero address + // Non-empty calldata for zero address + if (_calldata.length != 0) { + revert NonEmptyCalldata(); + } } else { // Do not check whether `_init` is a contract since later we check that it returns data. (bool success, bytes memory data) = _init.delegatecall(_calldata); if (!success) { // If the returndata is too small, we still want to produce some meaningful error if (data.length <= 4) { - revert("I"); // delegatecall failed + revert MalformedCalldata(); } assembly { @@ -295,8 +318,12 @@ library Diamond { // Check that called contract returns magic value to make sure that contract logic // supposed to be used as diamond cut initializer. - require(data.length == 32, "lp"); - require(abi.decode(data, (bytes32)) == DIAMOND_INIT_SUCCESS_RETURN_VALUE, "lp1"); + if (data.length != 32) { + revert BadReturnData(); + } + if (abi.decode(data, (bytes32)) != DIAMOND_INIT_SUCCESS_RETURN_VALUE) { + revert BadReturnData(); + } } } } diff --git a/l1-contracts/contracts/state-transition/libraries/Merkle.sol b/l1-contracts/contracts/state-transition/libraries/Merkle.sol index ec31073aa..04437704d 100644 --- a/l1-contracts/contracts/state-transition/libraries/Merkle.sol +++ b/l1-contracts/contracts/state-transition/libraries/Merkle.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {UncheckedMath} from "../../common/libraries/UncheckedMath.sol"; +import {MerklePathEmpty, MerklePathOutOfBounds, MerkleIndexOutOfBounds} from "../../common/L1ContractErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -21,9 +22,15 @@ library Merkle { bytes32 _itemHash ) internal pure returns (bytes32) { uint256 pathLength = _path.length; - require(pathLength > 0, "xc"); - require(pathLength < 256, "bt"); - require(_index < (1 << pathLength), "px"); + if (pathLength == 0) { + revert MerklePathEmpty(); + } + if (pathLength >= 256) { + revert MerklePathOutOfBounds(); + } + if (_index >= (1 << pathLength)) { + revert MerkleIndexOutOfBounds(); + } bytes32 currentHash = _itemHash; for (uint256 i; i < pathLength; i = i.uncheckedInc()) { diff --git a/l1-contracts/contracts/state-transition/libraries/PriorityQueue.sol b/l1-contracts/contracts/state-transition/libraries/PriorityQueue.sol index cb43068df..635270c7f 100644 --- a/l1-contracts/contracts/state-transition/libraries/PriorityQueue.sol +++ b/l1-contracts/contracts/state-transition/libraries/PriorityQueue.sol @@ -2,6 +2,8 @@ pragma solidity 0.8.24; +import {QueueIsEmpty} from "../../common/L1ContractErrors.sol"; + /// @notice The structure that contains meta information of the L2 transaction that was requested from L1 /// @dev The weird size of fields was selected specifically to minimize the structure storage size /// @param canonicalTxHash Hashed L2 transaction data that is needed to process it @@ -62,7 +64,10 @@ library PriorityQueue { /// @return The first unprocessed priority operation from the queue function front(Queue storage _queue) internal view returns (PriorityOperation memory) { - require(!_queue.isEmpty(), "D"); // priority queue is empty + // priority queue is empty + if (_queue.isEmpty()) { + revert QueueIsEmpty(); + } return _queue.data[_queue.head]; } @@ -70,7 +75,10 @@ library PriorityQueue { /// @notice Remove the first unprocessed priority operation from the queue /// @return priorityOperation that was popped from the priority queue function popFront(Queue storage _queue) internal returns (PriorityOperation memory priorityOperation) { - require(!_queue.isEmpty(), "s"); // priority queue is empty + // priority queue is empty + if (_queue.isEmpty()) { + revert QueueIsEmpty(); + } // Save value into the stack to avoid double reading from the storage uint256 head = _queue.head; diff --git a/l1-contracts/contracts/state-transition/libraries/TransactionValidator.sol b/l1-contracts/contracts/state-transition/libraries/TransactionValidator.sol index 71ef18a86..a83edd5fa 100644 --- a/l1-contracts/contracts/state-transition/libraries/TransactionValidator.sol +++ b/l1-contracts/contracts/state-transition/libraries/TransactionValidator.sol @@ -6,6 +6,7 @@ import {Math} from "@openzeppelin/contracts/utils/math/Math.sol"; import {L2CanonicalTransaction} from "../../common/Messaging.sol"; import {TX_SLOT_OVERHEAD_L2_GAS, MEMORY_OVERHEAD_GAS, L1_TX_INTRINSIC_L2_GAS, L1_TX_DELTA_544_ENCODING_BYTES, L1_TX_DELTA_FACTORY_DEPS_L2_GAS, L1_TX_MIN_L2_GAS_BASE, L1_TX_INTRINSIC_PUBDATA, L1_TX_DELTA_FACTORY_DEPS_PUBDATA} from "../../common/Config.sol"; +import {TooMuchGas, InvalidPubdataLength, InvalidUpgradeTxn, UpgradeTxVerifyParam, NotEnoughGas} from "../../common/L1ContractErrors.sol"; /// @title zkSync Library for validating L1 -> L2 transactions /// @author Matter Labs @@ -25,39 +26,70 @@ library TransactionValidator { uint256 l2GasForTxBody = getTransactionBodyGasLimit(_transaction.gasLimit, _encoded.length); // Ensuring that the transaction is provable - require(l2GasForTxBody <= _priorityTxMaxGasLimit, "ui"); + if (l2GasForTxBody > _priorityTxMaxGasLimit) { + revert TooMuchGas(); + } // Ensuring that the transaction cannot output more pubdata than is processable - require(l2GasForTxBody / _transaction.gasPerPubdataByteLimit <= _priorityTxMaxPubdata, "uk"); + if (l2GasForTxBody / _transaction.gasPerPubdataByteLimit > _priorityTxMaxPubdata) { + revert InvalidPubdataLength(); + } // Ensuring that the transaction covers the minimal costs for its processing: // hashing its content, publishing the factory dependencies, etc. - require( + if ( getMinimalPriorityTransactionGasLimit( _encoded.length, _transaction.factoryDeps.length, _transaction.gasPerPubdataByteLimit - ) <= l2GasForTxBody, - "up" - ); + ) > l2GasForTxBody + ) { + revert NotEnoughGas(); + } } /// @dev Used to validate upgrade transactions /// @param _transaction The transaction to validate function validateUpgradeTransaction(L2CanonicalTransaction memory _transaction) internal pure { // Restrict from to be within system contract range (0...2^16 - 1) - require(_transaction.from <= type(uint16).max, "ua"); - require(_transaction.to <= type(uint160).max, "ub"); - require(_transaction.paymaster == 0, "uc"); - require(_transaction.value == 0, "ud"); - require(_transaction.maxFeePerGas == 0, "uq"); - require(_transaction.maxPriorityFeePerGas == 0, "ux"); - require(_transaction.reserved[0] == 0, "ue"); - require(_transaction.reserved[1] <= type(uint160).max, "uf"); - require(_transaction.reserved[2] == 0, "ug"); - require(_transaction.reserved[3] == 0, "uo"); - require(_transaction.signature.length == 0, "uh"); - require(_transaction.paymasterInput.length == 0, "ul1"); - require(_transaction.reservedDynamic.length == 0, "um"); + if (_transaction.from > type(uint16).max) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.From); + } + if (_transaction.to > type(uint160).max) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.To); + } + if (_transaction.paymaster != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.Paymaster); + } + if (_transaction.value != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.Value); + } + if (_transaction.maxFeePerGas != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.MaxFeePerGas); + } + if (_transaction.maxPriorityFeePerGas != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.MaxPriorityFeePerGas); + } + if (_transaction.reserved[0] != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.Reserved0); + } + if (_transaction.reserved[1] > type(uint160).max) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.Reserved1); + } + if (_transaction.reserved[2] != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.Reserved2); + } + if (_transaction.reserved[3] != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.Reserved3); + } + if (_transaction.signature.length != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.Signature); + } + if (_transaction.paymasterInput.length != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.PaymasterInput); + } + if (_transaction.reservedDynamic.length != 0) { + revert InvalidUpgradeTxn(UpgradeTxVerifyParam.ReservedDynamic); + } } /// @dev Calculates the approximate minimum gas limit required for executing a priority transaction. @@ -112,7 +144,10 @@ library TransactionValidator { ) internal pure returns (uint256 txBodyGasLimit) { uint256 overhead = getOverheadForTransaction(_encodingLength); - require(_totalGasLimit >= overhead, "my"); // provided gas limit doesn't cover transaction overhead + // provided gas limit doesn't cover transaction overhead + if (_totalGasLimit < overhead) { + revert NotEnoughGas(); + } unchecked { // We enforce the fact that `_totalGasLimit >= overhead` explicitly above. txBodyGasLimit = _totalGasLimit - overhead; diff --git a/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol b/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol index 60b43648a..adb39d825 100644 --- a/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol +++ b/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol @@ -9,6 +9,7 @@ import {L2ContractHelper} from "../common/libraries/L2ContractHelper.sol"; import {TransactionValidator} from "../state-transition/libraries/TransactionValidator.sol"; import {MAX_NEW_FACTORY_DEPS, SYSTEM_UPGRADE_L2_TX_TYPE, MAX_ALLOWED_PROTOCOL_VERSION_DELTA} from "../common/Config.sol"; import {L2CanonicalTransaction} from "../common/Messaging.sol"; +import {TimeNotReached, InvalidTxType, NewProtocolVersionNotInUpgradeTxn, TooManyFactoryDeps, UnexpectedNumberOfFactoryDeps, InvalidProtocolVersion, PreviousUpgradeNotFinalized, PreviousUpgradeNotCleaned, InvalidHash} from "../common/L1ContractErrors.sol"; /// @notice The struct that represents the upgrade proposal. /// @param l2ProtocolUpgradeTx The system upgrade transaction. @@ -68,7 +69,9 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { // of the L1 block at which the upgrade occurred. This means that using timestamp as a signifier of "upgraded" // on the L2 side would be inaccurate. The effects of this "back-dating" of L2 upgrade batches will be reduced // as the permitted delay window is reduced in the future. - require(block.timestamp >= _proposedUpgrade.upgradeTimestamp, "Upgrade is not ready yet"); + if (block.timestamp < _proposedUpgrade.upgradeTimestamp) { + revert TimeNotReached(); + } _setNewProtocolVersion(_proposedUpgrade.newProtocolVersion); _upgradeL1Contract(_proposedUpgrade.l1ContractsUpgradeCalldata); @@ -186,7 +189,9 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { return bytes32(0); } - require(_l2ProtocolUpgradeTx.txType == SYSTEM_UPGRADE_L2_TX_TYPE, "L2 system upgrade tx type is wrong"); + if (_l2ProtocolUpgradeTx.txType != SYSTEM_UPGRADE_L2_TX_TYPE) { + revert InvalidTxType(_l2ProtocolUpgradeTx.txType); + } bytes memory encodedTransaction = abi.encode(_l2ProtocolUpgradeTx); @@ -201,10 +206,9 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { // We want the hashes of l2 system upgrade transactions to be unique. // This is why we require that the `nonce` field is unique to each upgrade. - require( - _l2ProtocolUpgradeTx.nonce == _newProtocolVersion, - "The new protocol version should be included in the L2 system upgrade tx" - ); + if (_l2ProtocolUpgradeTx.nonce != _newProtocolVersion) { + revert NewProtocolVersionNotInUpgradeTxn(); + } _verifyFactoryDeps(_factoryDeps, _l2ProtocolUpgradeTx.factoryDeps); @@ -219,15 +223,18 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { /// @param _factoryDeps The list of factory deps /// @param _expectedHashes The list of expected bytecode hashes function _verifyFactoryDeps(bytes[] calldata _factoryDeps, uint256[] calldata _expectedHashes) private pure { - require(_factoryDeps.length == _expectedHashes.length, "Wrong number of factory deps"); - require(_factoryDeps.length <= MAX_NEW_FACTORY_DEPS, "Factory deps can be at most 32"); + if (_factoryDeps.length != _expectedHashes.length) { + revert UnexpectedNumberOfFactoryDeps(); + } + if (_factoryDeps.length > MAX_NEW_FACTORY_DEPS) { + revert TooManyFactoryDeps(); + } uint256 length = _factoryDeps.length; for (uint256 i = 0; i < length; ++i) { - require( - L2ContractHelper.hashL2Bytecode(_factoryDeps[i]) == bytes32(_expectedHashes[i]), - "Wrong factory dep hash" - ); + if (L2ContractHelper.hashL2Bytecode(_factoryDeps[i]) != bytes32(_expectedHashes[i])) { + revert InvalidHash(); + } } } @@ -235,22 +242,21 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { /// @param _newProtocolVersion The new protocol version function _setNewProtocolVersion(uint256 _newProtocolVersion) internal virtual { uint256 previousProtocolVersion = s.protocolVersion; - require( - _newProtocolVersion > previousProtocolVersion, - "New protocol version is not greater than the current one" - ); - require( - _newProtocolVersion - previousProtocolVersion <= MAX_ALLOWED_PROTOCOL_VERSION_DELTA, - "Too big protocol version difference" - ); + if (_newProtocolVersion <= previousProtocolVersion) { + revert InvalidProtocolVersion(); + } + if (_newProtocolVersion - previousProtocolVersion > MAX_ALLOWED_PROTOCOL_VERSION_DELTA) { + revert InvalidProtocolVersion(); + } // If the previous upgrade had an L2 system upgrade transaction, we require that it is finalized. // Note it is important to keep this check, as otherwise hyperchains might skip upgrades by overwriting - require(s.l2SystemContractsUpgradeTxHash == bytes32(0), "Previous upgrade has not been finalized"); - require( - s.l2SystemContractsUpgradeBatchNumber == 0, - "The batch number of the previous upgrade has not been cleaned" - ); + if (s.l2SystemContractsUpgradeTxHash != bytes32(0)) { + revert PreviousUpgradeNotFinalized(s.l2SystemContractsUpgradeTxHash); + } + if (s.l2SystemContractsUpgradeBatchNumber != 0) { + revert PreviousUpgradeNotCleaned(); + } s.protocolVersion = _newProtocolVersion; emit NewProtocolVersion(previousProtocolVersion, _newProtocolVersion); diff --git a/l1-contracts/contracts/upgrades/UpgradeHyperchains.sol b/l1-contracts/contracts/upgrades/UpgradeHyperchains.sol index ded2df649..f7d74fefd 100644 --- a/l1-contracts/contracts/upgrades/UpgradeHyperchains.sol +++ b/l1-contracts/contracts/upgrades/UpgradeHyperchains.sol @@ -5,6 +5,7 @@ pragma solidity 0.8.24; import {Diamond} from "../state-transition/libraries/Diamond.sol"; import {BaseZkSyncUpgrade, ProposedUpgrade} from "./BaseZkSyncUpgrade.sol"; import {ETH_TOKEN_ADDRESS} from "../common/Config.sol"; +import {InvalidChainId, ZeroAddress} from "./ZkSyncUpgradeErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -15,10 +16,18 @@ contract UpgradeHyperchains is BaseZkSyncUpgrade { function upgrade(ProposedUpgrade calldata _proposedUpgrade) public override returns (bytes32) { (uint256 chainId, address bridgehubAddress, address stateTransitionManager, address sharedBridgeAddress) = abi .decode(_proposedUpgrade.postUpgradeCalldata, (uint256, address, address, address)); - require(chainId != 0, "UpgradeHyperchain: 1"); - require(bridgehubAddress != address(0), "UpgradeHyperchain: 2"); - require(stateTransitionManager != address(0), "UpgradeHyperchain: 3"); - require(sharedBridgeAddress != address(0), "UpgradeHyperchain: 4"); + if (chainId == 0) { + revert InvalidChainId(); + } + if (bridgehubAddress == address(0)) { + revert ZeroAddress(); + } + if (stateTransitionManager == address(0)) { + revert ZeroAddress(); + } + if (sharedBridgeAddress == address(0)) { + revert ZeroAddress(); + } s.chainId = chainId; s.bridgehub = bridgehubAddress; diff --git a/l1-contracts/contracts/upgrades/Upgrade_v1_4_1.sol b/l1-contracts/contracts/upgrades/Upgrade_v1_4_1.sol index a25c79775..62a3469cc 100644 --- a/l1-contracts/contracts/upgrades/Upgrade_v1_4_1.sol +++ b/l1-contracts/contracts/upgrades/Upgrade_v1_4_1.sol @@ -5,6 +5,7 @@ pragma solidity 0.8.24; import {Diamond} from "../state-transition/libraries/Diamond.sol"; import {BaseZkSyncUpgrade, ProposedUpgrade} from "./BaseZkSyncUpgrade.sol"; import {PubdataPricingMode, FeeParams} from "../state-transition/chain-deps/ZkSyncHyperchainStorage.sol"; +import {PubdataPerBatchIsLessThanTxn} from "../common/L1ContractErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -24,7 +25,9 @@ contract Upgrade_v1_4_1 is BaseZkSyncUpgrade { function changeFeeParams(FeeParams memory _newFeeParams) private { // Double checking that the new fee params are valid, i.e. // the maximal pubdata per batch is not less than the maximal pubdata per priority transaction. - require(_newFeeParams.maxPubdataPerBatch >= _newFeeParams.priorityTxMaxPubdata, "n6"); + if (_newFeeParams.maxPubdataPerBatch < _newFeeParams.priorityTxMaxPubdata) { + revert PubdataPerBatchIsLessThanTxn(); + } FeeParams memory oldFeeParams = s.feeParams; s.feeParams = _newFeeParams; diff --git a/l1-contracts/contracts/upgrades/ZkSyncUpgradeErrors.sol b/l1-contracts/contracts/upgrades/ZkSyncUpgradeErrors.sol index d2e78011b..ab81fddf2 100644 --- a/l1-contracts/contracts/upgrades/ZkSyncUpgradeErrors.sol +++ b/l1-contracts/contracts/upgrades/ZkSyncUpgradeErrors.sol @@ -6,3 +6,5 @@ error ProtocolVersionShouldBeGreater(uint256 _oldProtocolVersion, uint256 _newPr error ProtocolVersionDeltaTooLarge(uint256 _proposedDelta, uint256 _maxDelta); error PreviousUpgradeNotFinalized(); error PreviousUpgradeBatchNotCleared(); +error InvalidChainId(); +error ZeroAddress(); diff --git a/l1-contracts/test/foundry/unit/concrete/Bridgehub/experimental_bridge.t.sol b/l1-contracts/test/foundry/unit/concrete/Bridgehub/experimental_bridge.t.sol index 3dcc863fc..78207525d 100644 --- a/l1-contracts/test/foundry/unit/concrete/Bridgehub/experimental_bridge.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Bridgehub/experimental_bridge.t.sol @@ -15,6 +15,7 @@ import {IL1SharedBridge} from "contracts/bridge/interfaces/IL1SharedBridge.sol"; import {L2Message, L2Log, TxStatus, BridgehubL2TransactionRequest} from "contracts/common/Messaging.sol"; import {ETH_TOKEN_ADDRESS, REQUIRED_L2_GAS_PRICE_PER_PUBDATA, MAX_NEW_FACTORY_DEPS} from "contracts/common/Config.sol"; +import {SlotOccupied, STMAlreadyRegistered, TokenAlreadyRegistered, Unauthorized, NonEmptyMsgValue, STMNotRegistered, InvalidChainId} from "contracts/common/L1ContractErrors.sol"; contract ExperimentalBridgeTest is Test { using stdStorage for StdStorage; @@ -43,7 +44,7 @@ contract ExperimentalBridgeTest is Test { address defaultOwner = bridgeHub.owner(); // Now, the `reentrancyGuardInitializer` should prevent anyone from calling `initialize` since we have called the constructor of the contract - vm.expectRevert(bytes("1B")); + vm.expectRevert(SlotOccupied.selector); bridgeHub.initialize(bridgeOwner); vm.store( @@ -84,7 +85,7 @@ contract ExperimentalBridgeTest is Test { function test_randomCallerCannotSetDeployer(address randomCaller, address randomDeployer) public { if (randomCaller != bridgeHub.owner() && randomCaller != bridgeHub.admin()) { vm.prank(randomCaller); - vm.expectRevert(bytes("Bridgehub: not owner or admin")); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, randomCaller)); bridgeHub.setPendingAdmin(randomDeployer); // The deployer shouldn't have changed. @@ -104,7 +105,7 @@ contract ExperimentalBridgeTest is Test { // An address that has already been registered, cannot be registered again (at least not before calling `removeStateTransitionManager`). vm.prank(bridgeOwner); - vm.expectRevert(bytes("Bridgehub: state transition already registered")); + vm.expectRevert(STMAlreadyRegistered.selector); bridgeHub.addStateTransitionManager(randomAddressWithoutTheCorrectInterface); isSTMRegistered = bridgeHub.stateTransitionManagerIsRegistered(randomAddressWithoutTheCorrectInterface); @@ -133,7 +134,7 @@ contract ExperimentalBridgeTest is Test { // An address that has already been registered, cannot be registered again (at least not before calling `removeStateTransitionManager`). vm.prank(bridgeOwner); - vm.expectRevert(bytes("Bridgehub: state transition already registered")); + vm.expectRevert(STMAlreadyRegistered.selector); bridgeHub.addStateTransitionManager(randomAddressWithoutTheCorrectInterface); // Definitely not by a random caller @@ -153,7 +154,7 @@ contract ExperimentalBridgeTest is Test { // A non-existent STM cannot be removed vm.prank(bridgeOwner); - vm.expectRevert(bytes("Bridgehub: state transition not registered yet")); + vm.expectRevert(STMNotRegistered.selector); bridgeHub.removeStateTransitionManager(randomAddressWithoutTheCorrectInterface); // Let's first register our particular stateTransitionManager @@ -172,7 +173,7 @@ contract ExperimentalBridgeTest is Test { // An already removed STM cannot be removed again vm.prank(bridgeOwner); - vm.expectRevert(bytes("Bridgehub: state transition not registered yet")); + vm.expectRevert(STMNotRegistered.selector); bridgeHub.removeStateTransitionManager(randomAddressWithoutTheCorrectInterface); } @@ -192,7 +193,7 @@ contract ExperimentalBridgeTest is Test { // A non-existent STM cannot be removed vm.prank(bridgeOwner); - vm.expectRevert(bytes("Bridgehub: state transition not registered yet")); + vm.expectRevert(STMNotRegistered.selector); bridgeHub.removeStateTransitionManager(randomAddressWithoutTheCorrectInterface); // Let's first register our particular stateTransitionManager @@ -211,7 +212,7 @@ contract ExperimentalBridgeTest is Test { // An already removed STM cannot be removed again vm.prank(bridgeOwner); - vm.expectRevert(bytes("Bridgehub: state transition not registered yet")); + vm.expectRevert(STMNotRegistered.selector); bridgeHub.removeStateTransitionManager(randomAddressWithoutTheCorrectInterface); // Not possible by a randomcaller as well @@ -243,7 +244,7 @@ contract ExperimentalBridgeTest is Test { // An already registered token cannot be registered again vm.prank(bridgeOwner); - vm.expectRevert("Bridgehub: token already registered"); + vm.expectRevert(abi.encodeWithSelector(TokenAlreadyRegistered.selector, address(randomAddress))); bridgeHub.addToken(randomAddress); } @@ -275,7 +276,7 @@ contract ExperimentalBridgeTest is Test { // An already registered token cannot be registered again by randomCaller if (randomCaller != bridgeOwner) { vm.prank(bridgeOwner); - vm.expectRevert("Bridgehub: token already registered"); + vm.expectRevert(abi.encodeWithSelector(TokenAlreadyRegistered.selector, address(randomAddress))); bridgeHub.addToken(randomAddress); } } @@ -329,7 +330,6 @@ contract ExperimentalBridgeTest is Test { ) public { address deployerAddress = makeAddr("DEPLOYER_ADDRESS"); admin = makeAddr("NEW_CHAIN_ADMIN"); - // Diamond.DiamondCutData memory dcData; vm.prank(bridgeOwner); bridgeHub.setPendingAdmin(deployerAddress); @@ -343,7 +343,7 @@ contract ExperimentalBridgeTest is Test { if (randomCaller != deployerAddress && randomCaller != bridgeOwner) { vm.prank(randomCaller); - vm.expectRevert(bytes("Bridgehub: not owner or admin")); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, randomCaller)); bridgeHub.createNewChain({ _chainId: chainId, _stateTransitionManager: address(mockSTM), @@ -700,7 +700,7 @@ contract ExperimentalBridgeTest is Test { vm.deal(randomCaller, 1 ether); vm.prank(randomCaller); - vm.expectRevert("Bridgehub: non-eth bridge with msg.value"); + vm.expectRevert(NonEmptyMsgValue.selector); bytes32 resultantHash = bridgeHub.requestL2TransactionDirect{value: randomCaller.balance}(l2TxnReqDirect); // Now, let's call the same function with zero msg.value diff --git a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/ClaimFailedDeposit.t.sol b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/ClaimFailedDeposit.t.sol index 89a20d90d..944202b63 100644 --- a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/ClaimFailedDeposit.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/ClaimFailedDeposit.t.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.24; import {L1Erc20BridgeTest} from "./_L1Erc20Bridge_Shared.t.sol"; import {StdStorage, stdStorage} from "forge-std/Test.sol"; +import {EmptyDeposit} from "contracts/common/L1ContractErrors.sol"; contract ClaimFailedDepositTest is L1Erc20BridgeTest { using stdStorage for StdStorage; @@ -11,7 +12,7 @@ contract ClaimFailedDepositTest is L1Erc20BridgeTest { event ClaimedFailedDeposit(address indexed to, address indexed l1Token, uint256 amount); function test_RevertWhen_ClaimAmountIsZero() public { - vm.expectRevert(bytes("2T")); + vm.expectRevert(EmptyDeposit.selector); bytes32[] memory merkleProof; bridge.claimFailedDeposit({ diff --git a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Deposit.t.sol b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Deposit.t.sol index 67f16aab8..20085bcb4 100644 --- a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Deposit.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Deposit.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {L1Erc20BridgeTest} from "./_L1Erc20Bridge_Shared.t.sol"; +import {EmptyDeposit, ValueMismatch} from "contracts/common/L1ContractErrors.sol"; contract DepositTest is L1Erc20BridgeTest { event DepositInitiated( @@ -14,7 +15,7 @@ contract DepositTest is L1Erc20BridgeTest { ); function test_RevertWhen_depositAmountIsZero() public { - vm.expectRevert(bytes("0T")); + vm.expectRevert(EmptyDeposit.selector); bridge.deposit({ _l2Receiver: randomSigner, _l1Token: address(token), @@ -26,7 +27,7 @@ contract DepositTest is L1Erc20BridgeTest { } function test_RevertWhen_legacyDepositAmountIsZero() public { - vm.expectRevert(bytes("0T")); + vm.expectRevert(EmptyDeposit.selector); bridge.deposit({ _l2Receiver: randomSigner, _l1Token: address(token), @@ -84,7 +85,7 @@ contract DepositTest is L1Erc20BridgeTest { uint256 amount = 2; vm.prank(alice); feeOnTransferToken.approve(address(bridge), amount); - vm.expectRevert(bytes("3T")); + vm.expectRevert(abi.encodeWithSelector(ValueMismatch.selector, amount, amount - 1)); vm.prank(alice); bridge.deposit({ _l2Receiver: randomSigner, @@ -99,7 +100,7 @@ contract DepositTest is L1Erc20BridgeTest { uint256 amount = 4; vm.prank(alice); feeOnTransferToken.approve(address(bridge), amount); - vm.expectRevert(bytes("3T")); + vm.expectRevert(abi.encodeWithSelector(ValueMismatch.selector, amount, amount - 1)); vm.prank(alice); bridge.deposit({ _l2Receiver: randomSigner, diff --git a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/FinalizeWithdrawal.sol b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/FinalizeWithdrawal.sol index 830c77953..2e8165301 100644 --- a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/FinalizeWithdrawal.sol +++ b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/FinalizeWithdrawal.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.24; import {L1Erc20BridgeTest} from "./_L1Erc20Bridge_Shared.t.sol"; import {StdStorage, stdStorage} from "forge-std/Test.sol"; +import {WithdrawalAlreadyFinalized} from "contracts/common/L1ContractErrors.sol"; contract FinalizeWithdrawalTest is L1Erc20BridgeTest { using stdStorage for StdStorage; @@ -22,7 +23,7 @@ contract FinalizeWithdrawalTest is L1Erc20BridgeTest { assertTrue(bridge.isWithdrawalFinalized(l2BatchNumber, l2MessageIndex)); - vm.expectRevert(bytes("pw")); + vm.expectRevert(WithdrawalAlreadyFinalized.selector); bytes32[] memory merkleProof; bridge.finalizeWithdrawal({ _l2BatchNumber: l2BatchNumber, diff --git a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Initialization.t.sol b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Initialization.t.sol index 281ec169a..d3e5c9357 100644 --- a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Initialization.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Initialization.t.sol @@ -3,10 +3,11 @@ pragma solidity 0.8.24; import {L1Erc20BridgeTest} from "./_L1Erc20Bridge_Shared.t.sol"; +import {SlotOccupied} from "contracts/common/L1ContractErrors.sol"; contract InitializationTest is L1Erc20BridgeTest { function test_RevertWhen_DoubleInitialization() public { - vm.expectRevert(bytes("1B")); + vm.expectRevert(SlotOccupied.selector); bridge.initialize(); } } diff --git a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Reentrancy.t.sol b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Reentrancy.t.sol index 7a6183f93..5ef56f41f 100644 --- a/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Reentrancy.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Bridges/L1Erc20Bridge/Reentrancy.t.sol @@ -5,6 +5,7 @@ pragma solidity 0.8.24; import {StdStorage, stdStorage} from "forge-std/Test.sol"; import {L1Erc20BridgeTest} from "./_L1Erc20Bridge_Shared.t.sol"; import {ReenterL1ERC20Bridge} from "contracts/dev-contracts/test/ReenterL1ERC20Bridge.sol"; +import {SlotOccupied} from "contracts/common/L1ContractErrors.sol"; contract ReentrancyTest is L1Erc20BridgeTest { using stdStorage for StdStorage; @@ -15,7 +16,7 @@ contract ReentrancyTest is L1Erc20BridgeTest { token.approve(address(bridgeReenterItself), amount); vm.prank(alice); - vm.expectRevert(bytes("r1")); + vm.expectRevert(SlotOccupied.selector); bridgeReenterItself.deposit({ _l2Receiver: randomSigner, _l1Token: address(token), @@ -32,7 +33,7 @@ contract ReentrancyTest is L1Erc20BridgeTest { token.approve(address(bridgeReenterItself), amount); vm.prank(alice); - vm.expectRevert(bytes("r1")); + vm.expectRevert(SlotOccupied.selector); bridgeReenterItself.deposit({ _l2Receiver: randomSigner, _l1Token: address(token), @@ -54,7 +55,7 @@ contract ReentrancyTest is L1Erc20BridgeTest { vm.prank(alice); bytes32[] memory merkleProof; - vm.expectRevert(bytes("r1")); + vm.expectRevert(SlotOccupied.selector); bridgeReenterItself.claimFailedDeposit({ _depositSender: alice, _l1Token: address(token), @@ -71,7 +72,7 @@ contract ReentrancyTest is L1Erc20BridgeTest { uint256 l2MessageIndex = 4; vm.prank(alice); - vm.expectRevert(bytes("r1")); + vm.expectRevert(SlotOccupied.selector); bytes32[] memory merkleProof; bridgeReenterItself.finalizeWithdrawal({ _l2BatchNumber: l2BatchNumber, diff --git a/l1-contracts/test/foundry/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeFails.t.sol b/l1-contracts/test/foundry/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeFails.t.sol index 15ef94080..05d62363a 100644 --- a/l1-contracts/test/foundry/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeFails.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeFails.t.sol @@ -14,11 +14,12 @@ import {IMailbox} from "contracts/state-transition/chain-interfaces/IMailbox.sol import {IL1ERC20Bridge} from "contracts/bridge/interfaces/IL1ERC20Bridge.sol"; import {L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR} from "contracts/common/L2ContractAddresses.sol"; import {IGetters} from "contracts/state-transition/chain-interfaces/IGetters.sol"; +import {ZeroAddress, ValueMismatch, NonEmptyMsgValue, DepositExists, ValueMismatch, NonEmptyMsgValue, TokenNotSupported, WithdrawIncorrectAmount, EmptyDeposit, L2BridgeNotDeployed, WithdrawIncorrectAmount, InvalidProof, NoFundsTransferred, InsufficientFunds, DepositDNE, WithdrawalAlreadyFinalized, InsufficientFunds, MalformedMessage, InvalidSelector} from "contracts/common/L1ContractErrors.sol"; /// We are testing all the specified revert and require cases. contract L1SharedBridgeFailTest is L1SharedBridgeTest { function test_initialize_wrongOwner() public { - vm.expectRevert("ShB owner 0"); + vm.expectRevert(ZeroAddress.selector); new TransparentUpgradeableProxy( address(sharedBridgeImpl), admin, @@ -30,7 +31,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { function test_bridgehubDepositBaseToken_EthwrongMsgValue() public { vm.deal(bridgehubAddress, amount); vm.prank(bridgehubAddress); - vm.expectRevert("L1SharedBridge: msg.value not equal to amount"); + vm.expectRevert(abi.encodeWithSelector(ValueMismatch.selector, amount, uint256(0))); sharedBridge.bridgehubDepositBaseToken(chainId, alice, ETH_TOKEN_ADDRESS, amount); } @@ -40,7 +41,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { vm.prank(alice); token.approve(address(sharedBridge), amount); vm.prank(bridgehubAddress); - vm.expectRevert("ShB m.v > 0 b d.it"); + vm.expectRevert(NonEmptyMsgValue.selector); sharedBridge.bridgehubDepositBaseToken{value: amount}(chainId, alice, address(token), amount); } @@ -51,8 +52,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { vm.mockCall(address(token), abi.encodeWithSelector(IERC20.balanceOf.selector), abi.encode(10)); - bytes memory message = bytes("3T"); - vm.expectRevert(message); + vm.expectRevert(abi.encodeWithSelector(ValueMismatch.selector, uint256(100), uint256(0))); vm.prank(bridgehubAddress); sharedBridge.bridgehubDepositBaseToken(chainId, alice, address(token), amount); } @@ -67,14 +67,14 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encodeWithSelector(IBridgehub.baseToken.selector), abi.encode(address(token)) ); - vm.expectRevert("ShB l2 bridge not deployed"); + vm.expectRevert(abi.encodeWithSelector(L2BridgeNotDeployed.selector, chainId)); // solhint-disable-next-line func-named-parameters sharedBridge.bridgehubDeposit{value: amount}(chainId, alice, 0, abi.encode(ETH_TOKEN_ADDRESS, 0, bob)); } function test_bridgehubDeposit_Erc_weth() public { vm.prank(bridgehubAddress); - vm.expectRevert("ShB: WETH deposit not supported"); + vm.expectRevert(abi.encodeWithSelector(TokenNotSupported.selector, l1WethAddress)); // solhint-disable-next-line func-named-parameters sharedBridge.bridgehubDeposit(chainId, alice, 0, abi.encode(l1WethAddress, amount, bob)); } @@ -86,7 +86,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encodeWithSelector(IBridgehub.baseToken.selector), abi.encode(ETH_TOKEN_ADDRESS) ); - vm.expectRevert("ShB: baseToken deposit not supported"); + vm.expectRevert(abi.encodeWithSelector(TokenNotSupported.selector, ETH_TOKEN_ADDRESS)); // solhint-disable-next-line func-named-parameters sharedBridge.bridgehubDeposit(chainId, alice, 0, abi.encode(ETH_TOKEN_ADDRESS, 0, bob)); } @@ -101,7 +101,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encodeWithSelector(IBridgehub.baseToken.selector), abi.encode(address(token)) ); - vm.expectRevert("ShB wrong withdraw amount"); + vm.expectRevert(WithdrawIncorrectAmount.selector); // solhint-disable-next-line func-named-parameters sharedBridge.bridgehubDeposit(chainId, alice, 0, abi.encode(ETH_TOKEN_ADDRESS, amount, bob)); } @@ -117,7 +117,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encodeWithSelector(IBridgehub.baseToken.selector), abi.encode(ETH_TOKEN_ADDRESS) ); - vm.expectRevert("ShB m.v > 0 for BH d.it 2"); + vm.expectRevert(NonEmptyMsgValue.selector); // solhint-disable-next-line func-named-parameters sharedBridge.bridgehubDeposit{value: amount}(chainId, alice, 0, abi.encode(address(token), amount, bob)); } @@ -133,8 +133,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encode(ETH_TOKEN_ADDRESS) ); vm.mockCall(address(token), abi.encodeWithSelector(IERC20.balanceOf.selector), abi.encode(10)); - bytes memory message = bytes("5T"); - vm.expectRevert(message); + vm.expectRevert(WithdrawIncorrectAmount.selector); // solhint-disable-next-line func-named-parameters sharedBridge.bridgehubDeposit(chainId, alice, 0, abi.encode(address(token), amount, bob)); } @@ -146,8 +145,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encodeWithSelector(IBridgehub.baseToken.selector), abi.encode(address(token)) ); - bytes memory message = bytes("6T"); - vm.expectRevert(message); + vm.expectRevert(EmptyDeposit.selector); // solhint-disable-next-line func-named-parameters sharedBridge.bridgehubDeposit(chainId, alice, 0, abi.encode(ETH_TOKEN_ADDRESS, 0, bob)); } @@ -156,7 +154,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { bytes32 txDataHash = keccak256(abi.encode(alice, address(token), amount)); _setSharedBridgeDepositHappened(chainId, txHash, txDataHash); vm.prank(bridgehubAddress); - vm.expectRevert("ShB tx hap"); + vm.expectRevert(DepositExists.selector); sharedBridge.bridgehubConfirmL2Transaction(chainId, txDataHash, txHash); } @@ -167,8 +165,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encode(address(0)) ); vm.prank(bridgehubAddress); - bytes memory message = bytes("yn"); - vm.expectRevert(message); + vm.expectRevert(InvalidProof.selector); sharedBridge.claimFailedDeposit({ _chainId: chainId, _depositSender: alice, @@ -201,8 +198,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encode(true) ); - bytes memory message = bytes("y1"); - vm.expectRevert(message); + vm.expectRevert(NoFundsTransferred.selector); sharedBridge.claimFailedDeposit({ _chainId: chainId, _depositSender: alice, @@ -235,7 +231,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encode(true) ); - vm.expectRevert("ShB: d.it not hap"); + vm.expectRevert(DepositDNE.selector); sharedBridge.claimFailedDeposit({ _chainId: chainId, _depositSender: alice, @@ -272,7 +268,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encode(true) ); - vm.expectRevert("ShB n funds"); + vm.expectRevert(InsufficientFunds.selector); sharedBridge.claimFailedDeposit({ _chainId: chainId, _depositSender: alice, @@ -303,7 +299,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { amount ); - vm.expectRevert("ShB: legacy withdrawal"); + vm.expectRevert(WithdrawalAlreadyFinalized.selector); sharedBridge.finalizeWithdrawal({ _chainId: eraChainId, _l2BatchNumber: legacyBatchNumber, @@ -347,7 +343,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { amount ); - vm.expectRevert("Withdrawal is already finalized"); + vm.expectRevert(WithdrawalAlreadyFinalized.selector); sharedBridge.finalizeWithdrawal({ _chainId: eraChainId, _l2BatchNumber: legacyBatchNumber, @@ -380,8 +376,8 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { address(token), amount ); - vm.expectRevert("Withdrawal is already finalized 2"); + vm.expectRevert(WithdrawalAlreadyFinalized.selector); sharedBridge.finalizeWithdrawal({ _chainId: eraChainId, _l2BatchNumber: legacyBatchNumber, @@ -422,8 +418,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encode(true) ); - vm.expectRevert("ShB not enough funds 2"); - + vm.expectRevert(InsufficientFunds.selector); sharedBridge.finalizeWithdrawal({ _chainId: chainId, _l2BatchNumber: l2BatchNumber, @@ -464,8 +459,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { abi.encode(false) ); - vm.expectRevert("ShB withd w proof"); - + vm.expectRevert(InvalidProof.selector); sharedBridge.finalizeWithdrawal({ _chainId: chainId, _l2BatchNumber: l2BatchNumber, @@ -487,7 +481,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { bytes memory message = abi.encodePacked(IMailbox.finalizeEthWithdrawal.selector); - vm.expectRevert("ShB wrong msg len"); + vm.expectRevert(MalformedMessage.selector); sharedBridge.finalizeWithdrawal({ _chainId: chainId, _l2BatchNumber: l2BatchNumber, @@ -510,8 +504,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { bytes memory message = abi.encodePacked(IL1ERC20Bridge.finalizeWithdrawal.selector, alice, amount); // should have more data here - vm.expectRevert("ShB wrong msg len 2"); - + vm.expectRevert(MalformedMessage.selector); sharedBridge.finalizeWithdrawal({ _chainId: eraChainId, _l2BatchNumber: l2BatchNumber, @@ -534,7 +527,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { // notice that the selector is wrong bytes memory message = abi.encodePacked(IMailbox.proveL2LogInclusion.selector, alice, amount); - vm.expectRevert("ShB Incorrect message function selector"); + vm.expectRevert(abi.encodeWithSelector(InvalidSelector.selector, IMailbox.proveL2LogInclusion.selector)); sharedBridge.finalizeWithdrawal({ _chainId: eraChainId, _l2BatchNumber: l2BatchNumber, @@ -553,7 +546,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { vm.prank(owner); sharedBridge.initializeChainGovernance(eraChainId, address(0)); - vm.expectRevert("ShB b. n dep"); + vm.expectRevert(abi.encodeWithSelector(L2BridgeNotDeployed.selector, eraChainId)); vm.prank(l1ERC20BridgeAddress); sharedBridge.depositLegacyErc20Bridge({ _prevMsgSender: alice, @@ -571,7 +564,7 @@ contract L1SharedBridgeFailTest is L1SharedBridgeTest { uint256 l2TxGasPerPubdataByte = 100; address refundRecipient = address(0); - vm.expectRevert("ShB: WETH deposit not supported 2"); + vm.expectRevert(abi.encodeWithSelector(TokenNotSupported.selector, l1WethAddress)); vm.prank(l1ERC20BridgeAddress); sharedBridge.depositLegacyErc20Bridge({ _prevMsgSender: alice, diff --git a/l1-contracts/test/foundry/unit/concrete/DiamondCut/FacetCut.t.sol b/l1-contracts/test/foundry/unit/concrete/DiamondCut/FacetCut.t.sol index adceecddb..30cd35f71 100644 --- a/l1-contracts/test/foundry/unit/concrete/DiamondCut/FacetCut.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/DiamondCut/FacetCut.t.sol @@ -9,6 +9,7 @@ import {ExecutorFacet} from "contracts/state-transition/chain-deps/facets/Execut import {GettersFacet} from "contracts/state-transition/chain-deps/facets/Getters.sol"; import {MailboxFacet} from "contracts/state-transition/chain-deps/facets/Mailbox.sol"; import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; +import {FacetExists, SelectorsMustAllHaveSameFreezability, AddressHasNoCode, NonZeroAddress, ZeroAddress} from "contracts/common/L1ContractErrors.sol"; contract FacetCutTest is DiamondCutTest { MailboxFacet private mailboxFacet; @@ -88,7 +89,9 @@ contract FacetCutTest is DiamondCutTest { diamondCutTestContract.diamondCut(diamondCutData); - vm.expectRevert(abi.encodePacked("J")); + vm.expectRevert( + abi.encodeWithSelector(FacetExists.selector, Utils.getMailboxSelectors()[0], address(mailboxFacet)) + ); diamondCutTestContract.diamondCut(diamondCutData); } @@ -107,7 +110,7 @@ contract FacetCutTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("G")); + vm.expectRevert(abi.encodeWithSelector(AddressHasNoCode.selector, address(0))); diamondCutTestContract.diamondCut(diamondCutData); } @@ -126,7 +129,7 @@ contract FacetCutTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("L")); + vm.expectRevert(ZeroAddress.selector); diamondCutTestContract.diamondCut(diamondCutData); } @@ -145,7 +148,7 @@ contract FacetCutTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("a1")); + vm.expectRevert(abi.encodeWithSelector(NonZeroAddress.selector, address(mailboxFacet))); diamondCutTestContract.diamondCut(diamondCutData); } @@ -288,7 +291,7 @@ contract FacetCutTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("G")); + vm.expectRevert(abi.encodeWithSelector(AddressHasNoCode.selector, address(1))); diamondCutTestContract.diamondCut(diamondCutData1); } @@ -310,7 +313,7 @@ contract FacetCutTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("K")); + vm.expectRevert(abi.encodeWithSelector(AddressHasNoCode.selector, address(1))); diamondCutTestContract.diamondCut(diamondCutData1); } @@ -341,7 +344,7 @@ contract FacetCutTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("J1")); + vm.expectRevert(SelectorsMustAllHaveSameFreezability.selector); diamondCutTestContract.diamondCut(diamondCutData); } @@ -377,7 +380,7 @@ contract FacetCutTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("J1")); + vm.expectRevert(SelectorsMustAllHaveSameFreezability.selector); diamondCutTestContract.diamondCut(diamondCutData); } diff --git a/l1-contracts/test/foundry/unit/concrete/DiamondCut/Initialization.t.sol b/l1-contracts/test/foundry/unit/concrete/DiamondCut/Initialization.t.sol index cbcb012a5..ae05a2c5f 100644 --- a/l1-contracts/test/foundry/unit/concrete/DiamondCut/Initialization.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/DiamondCut/Initialization.t.sol @@ -6,6 +6,7 @@ import {RevertFallback} from "contracts/dev-contracts/RevertFallback.sol"; import {ReturnSomething} from "contracts/dev-contracts/ReturnSomething.sol"; import {DiamondCutTestContract} from "contracts/dev-contracts/test/DiamondCutTestContract.sol"; import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; +import {BadReturnData, MalformedCalldata, NonEmptyCalldata} from "contracts/common/L1ContractErrors.sol"; contract InitializationTest is DiamondCutTest { address private revertFallbackAddress; @@ -28,7 +29,7 @@ contract InitializationTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("I")); + vm.expectRevert(MalformedCalldata.selector); diamondCutTestContract.diamondCut(diamondCutData); } @@ -41,7 +42,7 @@ contract InitializationTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("lp")); + vm.expectRevert(BadReturnData.selector); diamondCutTestContract.diamondCut(diamondCutData); } @@ -54,7 +55,7 @@ contract InitializationTest is DiamondCutTest { initCalldata: bytes("0x11") }); - vm.expectRevert(abi.encodePacked("H")); + vm.expectRevert(NonEmptyCalldata.selector); diamondCutTestContract.diamondCut(diamondCutData); } @@ -67,7 +68,7 @@ contract InitializationTest is DiamondCutTest { initCalldata: bytes("") }); - vm.expectRevert(abi.encodePacked("lp1")); + vm.expectRevert(BadReturnData.selector); diamondCutTestContract.diamondCut(diamondCutData); } } diff --git a/l1-contracts/test/foundry/unit/concrete/DiamondCut/UpgradeLogic.t.sol b/l1-contracts/test/foundry/unit/concrete/DiamondCut/UpgradeLogic.t.sol index 41c07d9a2..c279f8cd6 100644 --- a/l1-contracts/test/foundry/unit/concrete/DiamondCut/UpgradeLogic.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/DiamondCut/UpgradeLogic.t.sol @@ -14,6 +14,7 @@ import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; import {Utils} from "../Utils/Utils.sol"; import {InitializeData} from "contracts/state-transition/chain-deps/DiamondInit.sol"; import {DummyStateTransitionManager} from "contracts/dev-contracts/test/DummyStateTransitionManager.sol"; +import {Unauthorized, DiamondFreezeIncorrectState} from "contracts/common/L1ContractErrors.sol"; contract UpgradeLogicTest is DiamondCutTest { DiamondProxy private diamondProxy; @@ -118,8 +119,7 @@ contract UpgradeLogicTest is DiamondCutTest { function test_RevertWhen_EmergencyFreezeWhenUnauthorizedGovernor() public { vm.startPrank(randomSigner); - - vm.expectRevert(abi.encodePacked("Hyperchain: not state transition manager")); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, randomSigner)); proxyAsAdmin.freezeDiamond(); } @@ -128,14 +128,14 @@ contract UpgradeLogicTest is DiamondCutTest { proxyAsAdmin.freezeDiamond(); - vm.expectRevert(abi.encodePacked("a9")); + vm.expectRevert(DiamondFreezeIncorrectState.selector); proxyAsAdmin.freezeDiamond(); } function test_RevertWhen_UnfreezingWhenNotFrozen() public { vm.startPrank(stateTransitionManager); - vm.expectRevert(abi.encodePacked("a7")); + vm.expectRevert(DiamondFreezeIncorrectState.selector); proxyAsAdmin.unfreezeDiamond(); } diff --git a/l1-contracts/test/foundry/unit/concrete/Executor/Authorization.t.sol b/l1-contracts/test/foundry/unit/concrete/Executor/Authorization.t.sol index 498fb21a2..8cc19a11d 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Authorization.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Authorization.t.sol @@ -6,6 +6,7 @@ import {Utils} from "../Utils/Utils.sol"; import {ExecutorTest} from "./_Executor_Shared.t.sol"; import {IExecutor} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract AuthorizationTest is ExecutorTest { IExecutor.StoredBatchInfo private storedBatchInfo; @@ -43,7 +44,7 @@ contract AuthorizationTest is ExecutorTest { vm.prank(randomSigner); - vm.expectRevert(bytes.concat("Hyperchain: not validator")); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, randomSigner)); executor.commitBatches(storedBatchInfo, commitBatchInfoArray); } @@ -53,7 +54,7 @@ contract AuthorizationTest is ExecutorTest { vm.prank(owner); - vm.expectRevert(bytes.concat("Hyperchain: not validator")); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, owner)); executor.proveBatches(storedBatchInfo, storedBatchInfoArray, proofInput); } @@ -63,7 +64,7 @@ contract AuthorizationTest is ExecutorTest { vm.prank(randomSigner); - vm.expectRevert(bytes.concat("Hyperchain: not validator")); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, randomSigner)); executor.executeBatches(storedBatchInfoArray); } } diff --git a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol index ce3e5947c..2b37d7dac 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol @@ -9,6 +9,7 @@ import {IExecutor, MAX_NUMBER_OF_BLOBS} from "contracts/state-transition/chain-i import {SystemLogKey} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; import {POINT_EVALUATION_PRECOMPILE_ADDR} from "contracts/common/Config.sol"; import {L2_PUBDATA_CHUNK_PUBLISHER_ADDR} from "contracts/common/L2ContractAddresses.sol"; +import {EmptyBlobVersionHash, CanOnlyProcessOneBatch, TimestampError, LogAlreadyProcessed, InvalidLogSender, UnexpectedSystemLog, HashMismatch, BatchHashMismatch, ValueMismatch, MissingSystemLogs, InvalidPubdataLength, NonEmptyBlobVersionHash, BlobHashCommitmentError} from "contracts/common/L1ContractErrors.sol"; contract CommittingTest is ExecutorTest { function test_RevertWhen_CommittingWithWrongLastCommittedBatchData() public { @@ -20,7 +21,13 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("i")); + vm.expectRevert( + abi.encodeWithSelector( + BatchHashMismatch.selector, + keccak256(abi.encode(genesisStoredBatchInfo)), + keccak256(abi.encode(wrongGenesisStoredBatchInfo)) + ) + ); executor.commitBatches(wrongGenesisStoredBatchInfo, newCommitBatchInfoArray); } @@ -33,7 +40,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("f")); + vm.expectRevert(abi.encodeWithSelector(ValueMismatch.selector, uint256(1), uint256(2))); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -56,7 +63,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("tb")); + vm.expectRevert(TimestampError.selector); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -79,7 +86,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("h1")); + vm.expectRevert(TimestampError.selector); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -102,7 +109,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("h2")); + vm.expectRevert(abi.encodeWithSelector(TimestampError.selector)); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -124,7 +131,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("l")); + vm.expectRevert(abi.encodeWithSelector(HashMismatch.selector, wrongPreviousBatchHash, bytes32(0))); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -140,7 +147,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("b7")); + vm.expectRevert(abi.encodeWithSelector(MissingSystemLogs.selector, 8191, 8183)); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -166,7 +173,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("kp")); + vm.expectRevert(abi.encodeWithSelector(LogAlreadyProcessed.selector, 3)); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -188,7 +195,13 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("sc")); + vm.expectRevert( + abi.encodeWithSelector( + InvalidLogSender.selector, + address(0), + uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY) + ) + ); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -210,7 +223,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("t")); + vm.expectRevert(abi.encodeWithSelector(HashMismatch.selector, wrongChainedPriorityHash, keccak256(""))); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -232,7 +245,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("ta")); + vm.expectRevert(abi.encodeWithSelector(ValueMismatch.selector, uint256(bytes32(bytes1(0x01))), uint256(2))); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -245,14 +258,14 @@ contract CommittingTest is ExecutorTest { ); IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; - wrongNewCommitBatchInfo.systemLogs = abi.encodePacked(bytes4(0x00000008), wrongL2Logs); + wrongNewCommitBatchInfo.systemLogs = abi.encodePacked(wrongL2Logs); IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; vm.prank(validator); - vm.expectRevert(bytes.concat("ul")); + vm.expectRevert(abi.encodeWithSelector(UnexpectedSystemLog.selector, uint256(119))); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -290,7 +303,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(errors[i]); + vm.expectRevert(abi.encodeWithSelector(InvalidLogSender.selector, wrongAddress, i)); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } } @@ -308,7 +321,8 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("b7")); + uint256 allLogsProcessed = uint256(8191); + vm.expectRevert(abi.encodeWithSelector(MissingSystemLogs.selector, 8191, allLogsProcessed ^ (1 << i))); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } } @@ -506,7 +520,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes("e4")); + vm.expectRevert(abi.encodeWithSelector(CanOnlyProcessOneBatch.selector)); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); } @@ -530,7 +544,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes("pl")); + vm.expectRevert(InvalidPubdataLength.selector); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); } @@ -555,7 +569,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes("bs")); + vm.expectRevert(InvalidPubdataLength.selector); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); } @@ -580,7 +594,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes("bd")); + vm.expectRevert(InvalidPubdataLength.selector); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); } @@ -624,7 +638,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes("lh")); + vm.expectRevert(abi.encodeWithSelector(NonEmptyBlobVersionHash.selector, uint256(1))); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); vm.clearMockedCalls(); @@ -661,7 +675,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes("vh")); + vm.expectRevert(abi.encodeWithSelector(EmptyBlobVersionHash.selector, 0)); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); vm.clearMockedCalls(); @@ -706,7 +720,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes("lh")); + vm.expectRevert(abi.encodeWithSelector(NonEmptyBlobVersionHash.selector, uint256(1))); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); vm.clearMockedCalls(); @@ -769,7 +783,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes("bh")); + vm.expectRevert(abi.encodeWithSelector(BlobHashCommitmentError.selector, uint256(1), true, false)); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); } @@ -819,7 +833,7 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes("bh")); + vm.expectRevert(abi.encodeWithSelector(BlobHashCommitmentError.selector, uint256(1), false, true)); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); vm.clearMockedCalls(); diff --git a/l1-contracts/test/foundry/unit/concrete/Executor/Executing.t.sol b/l1-contracts/test/foundry/unit/concrete/Executor/Executing.t.sol index 3360150d5..279c97fa2 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Executing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Executing.t.sol @@ -9,6 +9,7 @@ import {ExecutorTest} from "./_Executor_Shared.t.sol"; import {L2_BOOTLOADER_ADDRESS} from "contracts/common/L2ContractAddresses.sol"; import {COMMIT_TIMESTAMP_NOT_OLDER, REQUIRED_L2_GAS_PRICE_PER_PUBDATA} from "contracts/common/Config.sol"; import {IExecutor, SystemLogKey} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; +import {BatchHashMismatch, NonSequentialBatch, CantExecuteUnprovenBatches, QueueIsEmpty, TxHashMismatch} from "contracts/common/L1ContractErrors.sol"; contract ExecutingTest is ExecutorTest { function setUp() public { @@ -62,7 +63,7 @@ contract ExecutingTest is ExecutorTest { storedBatchInfoArray[0] = wrongNewStoredBatchInfo; vm.prank(validator); - vm.expectRevert(bytes.concat("k")); + vm.expectRevert(NonSequentialBatch.selector); executor.executeBatches(storedBatchInfoArray); } @@ -74,7 +75,13 @@ contract ExecutingTest is ExecutorTest { storedBatchInfoArray[0] = wrongNewStoredBatchInfo; vm.prank(validator); - vm.expectRevert(bytes.concat("exe10")); + vm.expectRevert( + abi.encodeWithSelector( + BatchHashMismatch.selector, + keccak256(abi.encode(newStoredBatchInfo)), + keccak256(abi.encode(wrongNewStoredBatchInfo)) + ) + ); executor.executeBatches(storedBatchInfoArray); } @@ -86,7 +93,7 @@ contract ExecutingTest is ExecutorTest { storedBatchInfoArray[0] = newStoredBatchInfo; vm.prank(validator); - vm.expectRevert(bytes.concat("n")); + vm.expectRevert(CantExecuteUnprovenBatches.selector); executor.executeBatches(storedBatchInfoArray); } @@ -143,7 +150,7 @@ contract ExecutingTest is ExecutorTest { executor.proveBatches(genesisStoredBatchInfo, correctNewStoredBatchInfoArray, proofInput); vm.prank(validator); - vm.expectRevert(bytes.concat("s")); + vm.expectRevert(QueueIsEmpty.selector); executor.executeBatches(correctNewStoredBatchInfoArray); } @@ -220,7 +227,7 @@ contract ExecutingTest is ExecutorTest { }); vm.prank(validator); - vm.expectRevert(bytes.concat("x")); + vm.expectRevert(TxHashMismatch.selector); executor.executeBatches(correctNewStoredBatchInfoArray); } @@ -245,8 +252,12 @@ contract ExecutingTest is ExecutorTest { IExecutor.StoredBatchInfo memory genesisBlock = genesisStoredBatchInfo; genesisBlock.batchHash = wrongPreviousBatchHash; + bytes32 storedBatchHash = getters.storedBlockHash(1); + vm.prank(validator); - vm.expectRevert(bytes.concat("i")); + vm.expectRevert( + abi.encodeWithSelector(BatchHashMismatch.selector, storedBatchHash, keccak256(abi.encode(genesisBlock))) + ); executor.commitBatches(genesisBlock, correctNewCommitBatchInfoArray); } diff --git a/l1-contracts/test/foundry/unit/concrete/Executor/Proving.t.sol b/l1-contracts/test/foundry/unit/concrete/Executor/Proving.t.sol index 184de78e2..5d86c8bc6 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Proving.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Proving.t.sol @@ -8,6 +8,7 @@ import {ExecutorTest} from "./_Executor_Shared.t.sol"; import {COMMIT_TIMESTAMP_NOT_OLDER} from "contracts/common/Config.sol"; import {IExecutor, SystemLogKey} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; +import {VerifyProofCommittedVerifiedMismatch, BatchHashMismatch} from "contracts/common/L1ContractErrors.sol"; contract ProvingTest is ExecutorTest { function setUp() public { @@ -56,7 +57,13 @@ contract ProvingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("t1")); + vm.expectRevert( + abi.encodeWithSelector( + BatchHashMismatch.selector, + keccak256(abi.encode(genesisStoredBatchInfo)), + keccak256(abi.encode(wrongPreviousStoredBatchInfo)) + ) + ); executor.proveBatches(wrongPreviousStoredBatchInfo, storedBatchInfoArray, proofInput); } @@ -69,7 +76,13 @@ contract ProvingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("o1")); + vm.expectRevert( + abi.encodeWithSelector( + BatchHashMismatch.selector, + keccak256(abi.encode(newStoredBatchInfo)), + keccak256(abi.encode(wrongNewStoredBatchInfo)) + ) + ); executor.proveBatches(genesisStoredBatchInfo, storedBatchInfoArray, proofInput); } @@ -82,7 +95,7 @@ contract ProvingTest is ExecutorTest { vm.prank(validator); - vm.expectRevert(bytes.concat("q")); + vm.expectRevert(VerifyProofCommittedVerifiedMismatch.selector); executor.proveBatches(genesisStoredBatchInfo, storedBatchInfoArray, proofInput); } diff --git a/l1-contracts/test/foundry/unit/concrete/Executor/Reverting.t.sol b/l1-contracts/test/foundry/unit/concrete/Executor/Reverting.t.sol index 9419c9dad..ccd12c2c8 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Reverting.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Reverting.t.sol @@ -8,6 +8,7 @@ import {ExecutorTest} from "./_Executor_Shared.t.sol"; import {COMMIT_TIMESTAMP_NOT_OLDER} from "contracts/common/Config.sol"; import {IExecutor, SystemLogKey} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; +import {RevertedBatchBeforeNewBatch} from "contracts/common/L1ContractErrors.sol"; contract RevertingTest is ExecutorTest { function setUp() public { @@ -55,7 +56,7 @@ contract RevertingTest is ExecutorTest { function test_RevertWhen_RevertingMoreBatchesThanAlreadyCommitted() public { vm.prank(validator); - vm.expectRevert(bytes.concat("v1")); + vm.expectRevert(RevertedBatchBeforeNewBatch.selector); executor.revertBatches(10); } diff --git a/l1-contracts/test/foundry/unit/concrete/Executor/_Executor_Shared.t.sol b/l1-contracts/test/foundry/unit/concrete/Executor/_Executor_Shared.t.sol index b96602f63..d81e9cc30 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/_Executor_Shared.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/_Executor_Shared.t.sol @@ -67,7 +67,7 @@ contract ExecutorTest is Test { } function getGettersSelectors() public view returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](28); + bytes4[] memory selectors = new bytes4[](29); selectors[0] = getters.getVerifier.selector; selectors[1] = getters.getAdmin.selector; selectors[2] = getters.getPendingAdmin.selector; @@ -96,6 +96,7 @@ contract ExecutorTest is Test { selectors[25] = getters.getTotalBatchesCommitted.selector; selectors[26] = getters.getTotalBatchesVerified.selector; selectors[27] = getters.getTotalBatchesExecuted.selector; + selectors[28] = getters.storedBlockHash.selector; return selectors; } diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/Authorization.t.sol b/l1-contracts/test/foundry/unit/concrete/Governance/Authorization.t.sol index 540870032..5cc75bf06 100644 --- a/l1-contracts/test/foundry/unit/concrete/Governance/Authorization.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Governance/Authorization.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {GovernanceTest} from "./_Governance_Shared.t.sol"; import {IGovernance} from "contracts/governance/IGovernance.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract Authorization is GovernanceTest { function test_RevertWhen_SchedulingByUnauthorisedAddress() public { @@ -33,21 +34,21 @@ contract Authorization is GovernanceTest { function test_RevertWhen_ExecutingByUnauthorisedAddress() public { vm.prank(randomSigner); - vm.expectRevert("Only the owner and security council are allowed to call this function"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, randomSigner)); IGovernance.Operation memory op = operationWithOneCallZeroSaltAndPredecessor(address(eventOnFallback), 0, ""); governance.execute(op); } function test_RevertWhen_ExecutingInstantByUnauthorisedAddress() public { vm.prank(randomSigner); - vm.expectRevert("Only security council is allowed to call this function"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, randomSigner)); IGovernance.Operation memory op = operationWithOneCallZeroSaltAndPredecessor(address(eventOnFallback), 0, ""); governance.executeInstant(op); } function test_RevertWhen_ExecutingInstantByOwner() public { vm.prank(owner); - vm.expectRevert("Only security council is allowed to call this function"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, owner)); IGovernance.Operation memory op = operationWithOneCallZeroSaltAndPredecessor(address(eventOnFallback), 0, ""); governance.executeInstant(op); } @@ -60,37 +61,37 @@ contract Authorization is GovernanceTest { function test_RevertWhen_UpdateDelayByUnauthorisedAddress() public { vm.prank(randomSigner); - vm.expectRevert("Only governance contract itself is allowed to call this function"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, randomSigner)); governance.updateDelay(0); } function test_RevertWhen_UpdateDelayByOwner() public { vm.prank(owner); - vm.expectRevert("Only governance contract itself is allowed to call this function"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, owner)); governance.updateDelay(0); } function test_RevertWhen_UpdateDelayBySecurityCouncil() public { vm.prank(securityCouncil); - vm.expectRevert("Only governance contract itself is allowed to call this function"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, securityCouncil)); governance.updateDelay(0); } function test_RevertWhen_UpdateSecurityCouncilByUnauthorisedAddress() public { vm.prank(randomSigner); - vm.expectRevert("Only governance contract itself is allowed to call this function"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, randomSigner)); governance.updateSecurityCouncil(address(0)); } function test_RevertWhen_UpdateSecurityCouncilByOwner() public { vm.prank(owner); - vm.expectRevert("Only governance contract itself is allowed to call this function"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, owner)); governance.updateSecurityCouncil(address(0)); } function test_RevertWhen_UpdateSecurityCouncilBySecurityCouncil() public { vm.prank(securityCouncil); - vm.expectRevert("Only governance contract itself is allowed to call this function"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, securityCouncil)); governance.updateSecurityCouncil(address(0)); } } diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/Executing.t.sol b/l1-contracts/test/foundry/unit/concrete/Governance/Executing.t.sol index 160cee2f6..a00f54f5c 100644 --- a/l1-contracts/test/foundry/unit/concrete/Governance/Executing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Governance/Executing.t.sol @@ -7,6 +7,7 @@ import {Utils} from "../Utils/Utils.sol"; import {GovernanceTest} from "./_Governance_Shared.t.sol"; import {IGovernance} from "contracts/governance/IGovernance.sol"; +import {OperationShouldBeReady, OperationShouldBePending, OperationExists, PreviousOperationNotExecuted, InvalidDelay} from "contracts/common/L1ContractErrors.sol"; contract ExecutingTest is GovernanceTest { using stdStorage for StdStorage; @@ -51,7 +52,7 @@ contract ExecutingTest is GovernanceTest { vm.startPrank(owner); IGovernance.Operation memory op = operationWithOneCallZeroSaltAndPredecessor(address(eventOnFallback), 0, ""); governance.scheduleTransparent(op, 10000); - vm.expectRevert("Operation must be ready before execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(op); } @@ -65,7 +66,7 @@ contract ExecutingTest is GovernanceTest { governance.scheduleTransparent(validOp, 0); IGovernance.Operation memory invalidOp = operationWithOneCallZeroSaltAndPredecessor(address(0), 0, ""); - vm.expectRevert("Operation must be ready before execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(invalidOp); } @@ -83,7 +84,7 @@ contract ExecutingTest is GovernanceTest { 1, "" ); - vm.expectRevert("Operation must be ready before execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(invalidOp); } @@ -101,7 +102,7 @@ contract ExecutingTest is GovernanceTest { 0, "00" ); - vm.expectRevert("Operation must be ready before execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(invalidOp); } @@ -133,7 +134,7 @@ contract ExecutingTest is GovernanceTest { invalidOp.predecessor = governance.hashOperation(executedOp); // Failed to execute operation that wasn't scheduled - vm.expectRevert("Operation must be ready before execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(invalidOp); } @@ -152,7 +153,7 @@ contract ExecutingTest is GovernanceTest { "" ); invalidOp.salt = Utils.randomBytes32("wrongSalt"); - vm.expectRevert("Operation must be ready before execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(invalidOp); } @@ -166,7 +167,7 @@ contract ExecutingTest is GovernanceTest { ); invalidOp.predecessor = Utils.randomBytes32("randomPredecessor"); governance.scheduleTransparent(invalidOp, 0); - vm.expectRevert("Predecessor operation not completed"); + vm.expectRevert(PreviousOperationNotExecuted.selector); governance.execute(invalidOp); } @@ -181,7 +182,7 @@ contract ExecutingTest is GovernanceTest { governance.scheduleTransparent(op, 0); executeOpAndCheck(op); - vm.expectRevert("Operation must be ready before execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(op); } @@ -193,7 +194,7 @@ contract ExecutingTest is GovernanceTest { 0, "1122" ); - vm.expectRevert("Operation must be ready before execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(op); } @@ -205,7 +206,7 @@ contract ExecutingTest is GovernanceTest { 0, "1122" ); - vm.expectRevert("Operation must be pending before execution"); + vm.expectRevert(OperationShouldBePending.selector); governance.executeInstant(op); } @@ -219,7 +220,7 @@ contract ExecutingTest is GovernanceTest { ); governance.scheduleTransparent(op, 0); governance.cancel(governance.hashOperation(op)); - vm.expectRevert("Operation must be ready before execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(op); } @@ -247,7 +248,7 @@ contract ExecutingTest is GovernanceTest { ); governance.scheduleTransparent(op, 0); executeOpAndCheck(op); - vm.expectRevert("Operation with this proposal id already exists"); + vm.expectRevert(OperationExists.selector); governance.scheduleTransparent(op, 0); } @@ -270,7 +271,7 @@ contract ExecutingTest is GovernanceTest { function test_RevertWhen_CancelNonExistingOperation() public { vm.startPrank(owner); - vm.expectRevert("Operation must be pending"); + vm.expectRevert(OperationShouldBePending.selector); governance.cancel(bytes32(0)); } @@ -279,7 +280,7 @@ contract ExecutingTest is GovernanceTest { stdstore.target(address(governance)).sig(governance.minDelay.selector).checked_write(1000); IGovernance.Operation memory op = operationWithOneCallZeroSaltAndPredecessor(address(revertFallback), 0, ""); - vm.expectRevert("Proposed delay is less than minimum delay"); + vm.expectRevert(InvalidDelay.selector); governance.scheduleTransparent(op, 0); } } diff --git a/l1-contracts/test/foundry/unit/concrete/Governance/Reentrancy.t.sol b/l1-contracts/test/foundry/unit/concrete/Governance/Reentrancy.t.sol index 1076d1015..54a1e0a5f 100644 --- a/l1-contracts/test/foundry/unit/concrete/Governance/Reentrancy.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Governance/Reentrancy.t.sol @@ -9,6 +9,7 @@ import {GovernanceTest} from "./_Governance_Shared.t.sol"; import {IGovernance} from "contracts/governance/IGovernance.sol"; import {ReenterGovernance} from "contracts/dev-contracts/test/ReenterGovernance.sol"; +import {OperationShouldBeReady, OperationShouldBePending} from "contracts/common/L1ContractErrors.sol"; contract ReentrancyTest is GovernanceTest { using stdStorage for StdStorage; @@ -88,7 +89,7 @@ contract ReentrancyTest is GovernanceTest { vm.startPrank(address(reenterGovernance)); governance.scheduleTransparent(op, 0); - vm.expectRevert("Operation must be ready after execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(op); } @@ -108,7 +109,7 @@ contract ReentrancyTest is GovernanceTest { vm.startPrank(address(reenterGovernance)); governance.scheduleTransparent(op, 0); - vm.expectRevert("Operation must be pending after execution"); + vm.expectRevert(OperationShouldBePending.selector); governance.executeInstant(op); } @@ -125,7 +126,7 @@ contract ReentrancyTest is GovernanceTest { vm.startPrank(address(reenterGovernance)); governance.scheduleTransparent(op, 0); - vm.expectRevert("Operation must be ready after execution"); + vm.expectRevert(OperationShouldBeReady.selector); governance.execute(op); } @@ -145,7 +146,7 @@ contract ReentrancyTest is GovernanceTest { vm.startPrank(address(reenterGovernance)); governance.scheduleTransparent(op, 0); - vm.expectRevert("Operation must be pending after execution"); + vm.expectRevert(OperationShouldBePending.selector); governance.executeInstant(op); } } diff --git a/l1-contracts/test/foundry/unit/concrete/ValidatorTimelock/ValidatorTimelock.t.sol b/l1-contracts/test/foundry/unit/concrete/ValidatorTimelock/ValidatorTimelock.t.sol index dacc45160..619d58e7c 100644 --- a/l1-contracts/test/foundry/unit/concrete/ValidatorTimelock/ValidatorTimelock.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/ValidatorTimelock/ValidatorTimelock.t.sol @@ -6,6 +6,7 @@ import {Utils} from "../Utils/Utils.sol"; import {ValidatorTimelock, IExecutor} from "contracts/state-transition/ValidatorTimelock.sol"; import {DummyStateTransitionManagerForValidatorTimelock} from "contracts/dev-contracts/test/DummyStateTransitionManagerForValidatorTimelock.sol"; import {IStateTransitionManager} from "contracts/state-transition/IStateTransitionManager.sol"; +import {Unauthorized, TimeNotReached} from "contracts/common/L1ContractErrors.sol"; contract ValidatorTimelockTest is Test { /// @notice A new validator has been added. @@ -270,7 +271,7 @@ contract ValidatorTimelockTest is Test { function test_RevertWhen_addValidatorNotAdmin() public { assert(validator.validators(chainId, bob) == false); - vm.expectRevert("ValidatorTimelock: only chain admin"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, address(this))); validator.addValidator(chainId, bob); assert(validator.validators(chainId, bob) == false); @@ -279,7 +280,7 @@ contract ValidatorTimelockTest is Test { function test_RevertWhen_removeValidatorNotAdmin() public { assert(validator.validators(chainId, alice) == true); - vm.expectRevert("ValidatorTimelock: only chain admin"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, address(this))); validator.removeValidator(chainId, alice); assert(validator.validators(chainId, alice) == true); @@ -309,7 +310,7 @@ contract ValidatorTimelockTest is Test { batchesToCommit[0] = batchToCommit; vm.prank(bob); - vm.expectRevert(bytes("ValidatorTimelock: only validator")); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, bob)); validator.commitBatches(storedBatch, batchesToCommit); } @@ -319,12 +320,12 @@ contract ValidatorTimelockTest is Test { } function test_RevertWhen_revertBatchesNotValidator() public { - vm.expectRevert("ValidatorTimelock: only validator"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, address(this))); validator.revertBatches(lastBatchNumber); } function test_RevertWhen_revertBatchesSharedBridgeNotValidator() public { - vm.expectRevert("ValidatorTimelock: only validator"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, address(this))); validator.revertBatchesSharedBridge(chainId, lastBatchNumber); } @@ -336,7 +337,7 @@ contract ValidatorTimelockTest is Test { IExecutor.StoredBatchInfo[] memory batchesToProve = new IExecutor.StoredBatchInfo[](1); batchesToProve[0] = batchToProve; - vm.expectRevert("ValidatorTimelock: only validator"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, address(this))); validator.proveBatches(prevBatch, batchesToProve, proof); } @@ -349,7 +350,7 @@ contract ValidatorTimelockTest is Test { batchesToProve[0] = batchToProve; vm.prank(bob); - vm.expectRevert("ValidatorTimelock: only validator"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, bob)); validator.proveBatchesSharedBridge(chainId, prevBatch, batchesToProve, proof); } @@ -360,7 +361,7 @@ contract ValidatorTimelockTest is Test { storedBatches[0] = storedBatch; vm.prank(bob); - vm.expectRevert("ValidatorTimelock: only validator"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, bob)); validator.executeBatches(storedBatches); } @@ -371,7 +372,7 @@ contract ValidatorTimelockTest is Test { storedBatches[0] = storedBatch; vm.prank(bob); - vm.expectRevert("ValidatorTimelock: only validator"); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, bob)); validator.executeBatchesSharedBridge(chainId, storedBatches); } @@ -400,7 +401,7 @@ contract ValidatorTimelockTest is Test { vm.prank(dan); vm.warp(timestamp + executionDelay - 1); - vm.expectRevert(bytes("5c")); + vm.expectRevert(TimeNotReached.selector); validator.executeBatches(storedBatches); } @@ -429,7 +430,7 @@ contract ValidatorTimelockTest is Test { vm.prank(alice); vm.warp(timestamp + executionDelay - 1); - vm.expectRevert(bytes("5c")); + vm.expectRevert(TimeNotReached.selector); validator.executeBatchesSharedBridge(chainId, storedBatches); } } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/CreateNewChain.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/CreateNewChain.t.sol index dc66d38b9..ba69c6bd7 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/CreateNewChain.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/CreateNewChain.t.sol @@ -3,21 +3,27 @@ pragma solidity 0.8.24; import {StateTransitionManagerTest} from "./_StateTransitionManager_Shared.t.sol"; import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; +import {Unauthorized, HashMismatch} from "contracts/common/L1ContractErrors.sol"; contract createNewChainTest is StateTransitionManagerTest { function test_RevertWhen_InitialDiamondCutHashMismatch() public { Diamond.DiamondCutData memory initialDiamondCutData = getDiamondCutData(sharedBridge); - - vm.expectRevert(bytes("STM: initial cutHash mismatch")); - + Diamond.DiamondCutData memory correctDiamondCutData = getDiamondCutData(address(diamondInit)); + + vm.expectRevert( + abi.encodeWithSelector( + HashMismatch.selector, + keccak256(abi.encode(correctDiamondCutData)), + keccak256(abi.encode(initialDiamondCutData)) + ) + ); createNewChain(initialDiamondCutData); } function test_RevertWhen_CalledNotByBridgehub() public { Diamond.DiamondCutData memory initialDiamondCutData = getDiamondCutData(diamondInit); - vm.expectRevert(bytes("STM: only bridgehub")); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, governor)); chainContractAddress.createNewChain({ _chainId: chainId, _baseToken: baseToken, diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/FreezeChain.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/FreezeChain.t.sol index 590d5d4ab..02ae770d3 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/FreezeChain.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/FreezeChain.t.sol @@ -3,6 +3,8 @@ pragma solidity 0.8.24; import {StateTransitionManagerTest} from "./_StateTransitionManager_Shared.t.sol"; import {GettersFacet} from "contracts/state-transition/chain-deps/facets/Getters.sol"; +import {IAdmin} from "contracts/state-transition/chain-interfaces/IAdmin.sol"; +import {FacetIsFrozen} from "contracts/common/L1ContractErrors.sol"; contract freezeChainTest is StateTransitionManagerTest { function test_FreezingChain() public { @@ -19,11 +21,11 @@ contract freezeChainTest is StateTransitionManagerTest { chainContractAddress.freezeChain(block.chainid); // Repeated call should revert - vm.expectRevert(bytes.concat("q1")); // storage frozen + vm.expectRevert(abi.encodeWithSelector(FacetIsFrozen.selector, IAdmin.freezeDiamond.selector)); // storage frozen chainContractAddress.freezeChain(block.chainid); // Call fails as storage is frozen - vm.expectRevert(bytes.concat("q1")); + vm.expectRevert(abi.encodeWithSelector(FacetIsFrozen.selector, GettersFacet.isDiamondStorageFrozen.selector)); isChainFrozen = gettersFacet.isDiamondStorageFrozen(); } } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/StateTransitionOwnerZero.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/StateTransitionOwnerZero.t.sol index 60c606a3d..a5c0759bc 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/StateTransitionOwnerZero.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/StateTransitionOwnerZero.t.sol @@ -5,6 +5,7 @@ import {TransparentUpgradeableProxy} from "@openzeppelin/contracts/proxy/transpa import {StateTransitionManagerTest} from "./_StateTransitionManager_Shared.t.sol"; import {StateTransitionManager} from "contracts/state-transition/StateTransitionManager.sol"; import {StateTransitionManagerInitializeData} from "contracts/state-transition/IStateTransitionManager.sol"; +import {ZeroAddress} from "contracts/common/L1ContractErrors.sol"; contract initializingSTMOwnerZeroTest is StateTransitionManagerTest { function test_InitializingSTMWithGovernorZeroShouldRevert() public { @@ -19,7 +20,7 @@ contract initializingSTMOwnerZeroTest is StateTransitionManagerTest { protocolVersion: 0 }); - vm.expectRevert(bytes("STM: owner zero")); + vm.expectRevert(ZeroAddress.selector); new TransparentUpgradeableProxy( address(stateTransitionManager), admin, diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/_StateTransitionManager_Shared.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/_StateTransitionManager_Shared.t.sol index 7f7b7dfad..08c41a330 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/_StateTransitionManager_Shared.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/StateTransitionManager/_StateTransitionManager_Shared.t.sol @@ -18,6 +18,7 @@ import {InitializeDataNewChain} from "contracts/state-transition/chain-interface import {StateTransitionManager} from "contracts/state-transition/StateTransitionManager.sol"; import {StateTransitionManagerInitializeData} from "contracts/state-transition/IStateTransitionManager.sol"; import {TestnetVerifier} from "contracts/state-transition/TestnetVerifier.sol"; +import {ZeroAddress} from "contracts/common/L1ContractErrors.sol"; contract StateTransitionManagerTest is Test { StateTransitionManager internal stateTransitionManager; @@ -89,7 +90,7 @@ contract StateTransitionManagerTest is Test { protocolVersion: 0 }); - vm.expectRevert(bytes.concat("STM: owner zero")); + vm.expectRevert(ZeroAddress.selector); new TransparentUpgradeableProxy( address(stateTransitionManager), admin, diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/DiamondInit/Initialize.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/DiamondInit/Initialize.t.sol index 3c2b01dd5..aaa8412f5 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/DiamondInit/Initialize.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/DiamondInit/Initialize.t.sol @@ -11,6 +11,7 @@ import {DiamondProxy} from "contracts/state-transition/chain-deps/DiamondProxy.s import {InitializeData} from "contracts/state-transition/chain-interfaces/IDiamondInit.sol"; import {IVerifier} from "contracts/state-transition/chain-interfaces/IVerifier.sol"; import {MAX_GAS_PER_TRANSACTION} from "contracts/common/Config.sol"; +import {MalformedCalldata} from "contracts/common/L1ContractErrors.sol"; contract InitializeTest is DiamondInitTest { function test_revertWhen_verifierIsZeroAddress() public { @@ -23,7 +24,7 @@ contract InitializeTest is DiamondInitTest { initCalldata: abi.encodeWithSelector(DiamondInit.initialize.selector, initializeData) }); - vm.expectRevert(bytes.concat("vt")); + vm.expectRevert(MalformedCalldata.selector); new DiamondProxy(block.chainid, diamondCutData); } @@ -37,7 +38,7 @@ contract InitializeTest is DiamondInitTest { initCalldata: abi.encodeWithSelector(DiamondInit.initialize.selector, initializeData) }); - vm.expectRevert(bytes.concat("vy")); + vm.expectRevert(MalformedCalldata.selector); new DiamondProxy(block.chainid, diamondCutData); } @@ -51,7 +52,7 @@ contract InitializeTest is DiamondInitTest { initCalldata: abi.encodeWithSelector(DiamondInit.initialize.selector, initializeData) }); - vm.expectRevert(bytes.concat("hc")); + vm.expectRevert(MalformedCalldata.selector); new DiamondProxy(block.chainid, diamondCutData); } @@ -65,7 +66,7 @@ contract InitializeTest is DiamondInitTest { initCalldata: abi.encodeWithSelector(DiamondInit.initialize.selector, initializeData) }); - vm.expectRevert(bytes.concat("vu")); + vm.expectRevert(MalformedCalldata.selector); new DiamondProxy(block.chainid, diamondCutData); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/DiamondProxy/DiamondProxy.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/DiamondProxy/DiamondProxy.t.sol index ba1ece9db..1fbcde964 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/DiamondProxy/DiamondProxy.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/DiamondProxy/DiamondProxy.t.sol @@ -12,6 +12,7 @@ import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; import {DiamondProxy} from "contracts/state-transition/chain-deps/DiamondProxy.sol"; import {ZkSyncHyperchainBase} from "contracts/state-transition/chain-deps/facets/ZkSyncHyperchainBase.sol"; import {TestnetVerifier} from "contracts/state-transition/TestnetVerifier.sol"; +import {FacetIsFrozen, ValueMismatch, InvalidSelector} from "contracts/common/L1ContractErrors.sol"; contract TestFacet is ZkSyncHyperchainBase { function func() public pure returns (bool) { @@ -59,7 +60,7 @@ contract DiamondProxyTest is Test { initCalldata: abi.encodeWithSelector(DiamondInit.initialize.selector, initializeData) }); - vm.expectRevert(abi.encodePacked("pr")); + vm.expectRevert(abi.encodeWithSelector(ValueMismatch.selector, block.chainid, block.chainid + 1)); new DiamondProxy(block.chainid + 1, diamondCutData); } @@ -107,7 +108,7 @@ contract DiamondProxyTest is Test { DiamondProxy diamondProxy = new DiamondProxy(block.chainid, diamondCutData); TestFacet testFacet = TestFacet(address(diamondProxy)); - vm.expectRevert(abi.encodePacked("F")); + vm.expectRevert(abi.encodeWithSelector(InvalidSelector.selector, TestFacet.func.selector)); testFacet.func(); } @@ -126,7 +127,7 @@ contract DiamondProxyTest is Test { utilsFacet.util_setIsFrozen(true); - vm.expectRevert(abi.encodePacked("q1")); + vm.expectRevert(abi.encodeWithSelector(FacetIsFrozen.selector, TestFacet.func.selector)); testFacet.func(); } @@ -167,7 +168,7 @@ contract DiamondProxyTest is Test { DiamondProxy diamondProxy = new DiamondProxy(block.chainid, diamondCutData); TestFacet testFacet = TestFacet(address(diamondProxy)); - vm.expectRevert(bytes("F")); + vm.expectRevert(abi.encodeWithSelector(InvalidSelector.selector, TestFacet.func.selector)); testFacet.func(); } } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/AcceptAdmin.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/AcceptAdmin.t.sol index fe8c99db6..ab85ecdb0 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/AcceptAdmin.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/AcceptAdmin.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract AcceptAdminTest is AdminTest { event NewPendingAdmin(address indexed oldPendingAdmin, address indexed newPendingAdmin); @@ -11,9 +12,8 @@ contract AcceptAdminTest is AdminTest { function test_revertWhen_calledByNonPendingAdmin() public { address nonPendingAdmin = makeAddr("nonPendingAdmin"); - vm.expectRevert(bytes.concat("n4")); - vm.startPrank(nonPendingAdmin); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonPendingAdmin)); adminFacet.acceptAdmin(); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ChangeFeeParams.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ChangeFeeParams.t.sol index 1575af870..cf80192de 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ChangeFeeParams.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ChangeFeeParams.t.sol @@ -3,9 +3,9 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; -import {ERROR_ONLY_ADMIN_OR_STATE_TRANSITION_MANAGER} from "../Base/_Base_Shared.t.sol"; import {FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZkSyncHyperchainStorage.sol"; +import {Unauthorized, PubdataPerBatchIsLessThanTxn} from "contracts/common/L1ContractErrors.sol"; contract ChangeFeeParamsTest is AdminTest { event NewFeeParams(FeeParams oldFeeParams, FeeParams newFeeParams); @@ -37,7 +37,7 @@ contract ChangeFeeParamsTest is AdminTest { }); vm.startPrank(nonStateTransitionManager); - vm.expectRevert(ERROR_ONLY_ADMIN_OR_STATE_TRANSITION_MANAGER); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonStateTransitionManager)); adminFacet.changeFeeParams(newFeeParams); } @@ -55,7 +55,7 @@ contract ChangeFeeParamsTest is AdminTest { minimalL2GasPrice: 250_000_000 }); - vm.expectRevert(bytes.concat("n6")); + vm.expectRevert(PubdataPerBatchIsLessThanTxn.selector); vm.startPrank(stateTransitionManager); adminFacet.changeFeeParams(newFeeParams); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ExecuteUpgrade.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ExecuteUpgrade.t.sol index d90d00df5..a54d280b1 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ExecuteUpgrade.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ExecuteUpgrade.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; -import {ERROR_ONLY_STATE_TRANSITION_MANAGER} from "../Base/_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; @@ -18,7 +18,7 @@ contract ExecuteUpgradeTest is AdminTest { initCalldata: new bytes(0) }); - vm.expectRevert(ERROR_ONLY_STATE_TRANSITION_MANAGER); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonStateTransitionManager)); vm.startPrank(nonStateTransitionManager); adminFacet.executeUpgrade(diamondCutData); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/FreezeDiamond.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/FreezeDiamond.t.sol index 8e79f4fc3..77baed0ef 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/FreezeDiamond.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/FreezeDiamond.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; -import {ERROR_ONLY_STATE_TRANSITION_MANAGER} from "../Base/_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract FreezeDiamondTest is AdminTest { event Freeze(); @@ -11,7 +11,7 @@ contract FreezeDiamondTest is AdminTest { function test_revertWhen_calledByNonStateTransitionManager() public { address nonStateTransitionManager = makeAddr("nonStateTransitionManager"); - vm.expectRevert(ERROR_ONLY_STATE_TRANSITION_MANAGER); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonStateTransitionManager)); vm.startPrank(nonStateTransitionManager); adminFacet.freezeDiamond(); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPendingGovernor.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPendingGovernor.t.sol index 8dbc12bbd..359e9ce8c 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPendingGovernor.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPendingGovernor.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; -import {ERROR_ONLY_ADMIN} from "../Base/_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract SetPendingAdminTest is AdminTest { event NewPendingAdmin(address indexed oldPendingAdmin, address indexed newPendingAdmin); @@ -12,8 +12,7 @@ contract SetPendingAdminTest is AdminTest { address nonAdmin = makeAddr("nonAdmin"); address newPendingAdmin = makeAddr("newPendingAdmin"); - vm.expectRevert(ERROR_ONLY_ADMIN); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonAdmin)); vm.startPrank(nonAdmin); adminFacet.setPendingAdmin(newPendingAdmin); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPorterAvailability.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPorterAvailability.t.sol index 94e209a15..ad0708f11 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPorterAvailability.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPorterAvailability.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; -import {ERROR_ONLY_STATE_TRANSITION_MANAGER} from "../Base/_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract SetPorterAvailabilityTest is AdminTest { event IsPorterAvailableStatusUpdate(bool isPorterAvailable); @@ -12,9 +12,8 @@ contract SetPorterAvailabilityTest is AdminTest { address nonStateTransitionManager = makeAddr("nonStateTransitionManager"); bool isPorterAvailable = true; - vm.expectRevert(ERROR_ONLY_STATE_TRANSITION_MANAGER); - vm.startPrank(nonStateTransitionManager); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonStateTransitionManager)); adminFacet.setPorterAvailability(isPorterAvailable); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPriorityTxMaxGasLimit.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPriorityTxMaxGasLimit.t.sol index 8ce9e4092..5581420fe 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPriorityTxMaxGasLimit.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetPriorityTxMaxGasLimit.t.sol @@ -3,9 +3,9 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; -import {ERROR_ONLY_STATE_TRANSITION_MANAGER} from "../Base/_Base_Shared.t.sol"; import {MAX_GAS_PER_TRANSACTION} from "contracts/common/Config.sol"; +import {Unauthorized, TooMuchGas} from "contracts/common/L1ContractErrors.sol"; contract SetPriorityTxMaxGasLimitTest is AdminTest { event NewPriorityTxMaxGasLimit(uint256 oldPriorityTxMaxGasLimit, uint256 newPriorityTxMaxGasLimit); @@ -15,8 +15,7 @@ contract SetPriorityTxMaxGasLimitTest is AdminTest { uint256 newPriorityTxMaxGasLimit = 100; vm.startPrank(nonStateTransitionManager); - vm.expectRevert(ERROR_ONLY_STATE_TRANSITION_MANAGER); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonStateTransitionManager)); adminFacet.setPriorityTxMaxGasLimit(newPriorityTxMaxGasLimit); } @@ -24,9 +23,8 @@ contract SetPriorityTxMaxGasLimitTest is AdminTest { address stateTransitionManager = utilsFacet.util_getStateTransitionManager(); uint256 newPriorityTxMaxGasLimit = MAX_GAS_PER_TRANSACTION + 1; - vm.expectRevert(bytes.concat("n5")); - vm.startPrank(stateTransitionManager); + vm.expectRevert(TooMuchGas.selector); adminFacet.setPriorityTxMaxGasLimit(newPriorityTxMaxGasLimit); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetTransactionFilterer.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetTransactionFilterer.t.sol index 5a8ac9a2b..8581ec6c4 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetTransactionFilterer.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetTransactionFilterer.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract SetTransactionFiltererTest is AdminTest { event NewTransactionFilterer(address oldTransactionFilterer, address newTransactionFilterer); @@ -34,10 +35,11 @@ contract SetTransactionFiltererTest is AdminTest { } function test_revertWhen_notAdmin() public { + address nonAdmin = makeAddr("nonAdmin"); address transactionFilterer = makeAddr("transactionFilterer"); - vm.expectRevert("Hyperchain: not admin"); - vm.startPrank(makeAddr("nonAdmin")); + vm.startPrank(nonAdmin); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonAdmin)); adminFacet.setTransactionFilterer(transactionFilterer); } } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetValidator.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetValidator.t.sol index 3452ed132..77990a285 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetValidator.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/SetValidator.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; -import {ERROR_ONLY_STATE_TRANSITION_MANAGER} from "../Base/_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract SetValidatorTest is AdminTest { event ValidatorStatusUpdate(address indexed validatorAddress, bool isActive); @@ -13,9 +13,8 @@ contract SetValidatorTest is AdminTest { address validator = makeAddr("validator"); bool isActive = true; - vm.expectRevert(ERROR_ONLY_STATE_TRANSITION_MANAGER); - vm.startPrank(nonStateTransitionManager); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonStateTransitionManager)); adminFacet.setValidator(validator, isActive); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/UnfreezeDiamond.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/UnfreezeDiamond.t.sol index b7f1fa124..9667fb828 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/UnfreezeDiamond.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/UnfreezeDiamond.t.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; -import {ERROR_ONLY_STATE_TRANSITION_MANAGER} from "../Base/_Base_Shared.t.sol"; +import {Unauthorized, DiamondFreezeIncorrectState} from "contracts/common/L1ContractErrors.sol"; contract UnfreezeDiamondTest is AdminTest { event Unfreeze(); @@ -11,8 +11,7 @@ contract UnfreezeDiamondTest is AdminTest { function test_revertWhen_calledByNonStateTransitionManager() public { address nonStateTransitionManager = makeAddr("nonStateTransitionManager"); - vm.expectRevert(ERROR_ONLY_STATE_TRANSITION_MANAGER); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonStateTransitionManager)); vm.startPrank(nonStateTransitionManager); adminFacet.unfreezeDiamond(); } @@ -22,7 +21,7 @@ contract UnfreezeDiamondTest is AdminTest { utilsFacet.util_setIsFrozen(false); - vm.expectRevert(bytes.concat("a7")); + vm.expectRevert(DiamondFreezeIncorrectState.selector); vm.startPrank(admin); adminFacet.unfreezeDiamond(); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/UpgradeChainFromVersion.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/UpgradeChainFromVersion.t.sol index 3e2155995..d4274d1e2 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/UpgradeChainFromVersion.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/UpgradeChainFromVersion.t.sol @@ -3,10 +3,10 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; -import {ERROR_ONLY_ADMIN_OR_STATE_TRANSITION_MANAGER} from "../Base/_Base_Shared.t.sol"; import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; import {IStateTransitionManager} from "contracts/state-transition/IStateTransitionManager.sol"; +import {InvalidProtocolVersion, ValueMismatch, Unauthorized, HashMismatch} from "contracts/common/L1ContractErrors.sol"; contract UpgradeChainFromVersionTest is AdminTest { event ExecuteUpgrade(Diamond.DiamondCutData diamondCut); @@ -20,9 +20,8 @@ contract UpgradeChainFromVersionTest is AdminTest { initCalldata: new bytes(0) }); - vm.expectRevert(ERROR_ONLY_ADMIN_OR_STATE_TRANSITION_MANAGER); - vm.startPrank(nonAdminOrStateTransitionManager); + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonAdminOrStateTransitionManager)); adminFacet.upgradeChainFromVersion(oldProtocolVersion, diamondCutData); } @@ -46,9 +45,10 @@ contract UpgradeChainFromVersionTest is AdminTest { abi.encode(cutHashInput) ); - vm.expectRevert("AdminFacet: cutHash mismatch"); - vm.startPrank(admin); + vm.expectRevert( + abi.encodeWithSelector(HashMismatch.selector, cutHashInput, keccak256(abi.encode(diamondCutData))) + ); adminFacet.upgradeChainFromVersion(oldProtocolVersion, diamondCutData); } @@ -73,9 +73,8 @@ contract UpgradeChainFromVersionTest is AdminTest { abi.encode(cutHashInput) ); - vm.expectRevert("AdminFacet: protocolVersion mismatch in STC when upgrading"); - vm.startPrank(admin); + vm.expectRevert(abi.encodeWithSelector(ValueMismatch.selector, uint256(2), oldProtocolVersion)); adminFacet.upgradeChainFromVersion(oldProtocolVersion, diamondCutData); } @@ -100,8 +99,7 @@ contract UpgradeChainFromVersionTest is AdminTest { abi.encode(cutHashInput) ); - vm.expectRevert("AdminFacet: protocolVersion mismatch in STC after upgrading"); - + vm.expectRevert(InvalidProtocolVersion.selector); // solhint-disable-next-line func-named-parameters vm.expectEmit(true, true, true, true, address(adminFacet)); emit ExecuteUpgrade(diamondCutData); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyBridgehub.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyBridgehub.t.sol index c484a38bb..af92cde5f 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyBridgehub.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyBridgehub.t.sol @@ -2,14 +2,14 @@ pragma solidity 0.8.24; -import {ZkSyncHyperchainBaseTest, ERROR_ONLY_BRIDGEHUB} from "./_Base_Shared.t.sol"; +import {ZkSyncHyperchainBaseTest} from "./_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract OnlyBridgehubTest is ZkSyncHyperchainBaseTest { function test_revertWhen_calledByNonBridgehub() public { address nonBridgehub = makeAddr("nonBridgehub"); - vm.expectRevert(ERROR_ONLY_BRIDGEHUB); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonBridgehub)); vm.startPrank(nonBridgehub); testBaseFacet.functionWithOnlyBridgehubModifier(); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyGovernor.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyGovernor.t.sol index ba5199f92..44c397c85 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyGovernor.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyGovernor.t.sol @@ -2,14 +2,14 @@ pragma solidity 0.8.24; -import {ZkSyncHyperchainBaseTest, ERROR_ONLY_ADMIN} from "./_Base_Shared.t.sol"; +import {ZkSyncHyperchainBaseTest} from "./_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract OnlyAdminTest is ZkSyncHyperchainBaseTest { function test_revertWhen_calledByNonAdmin() public { address nonAdmin = makeAddr("nonAdmin"); - vm.expectRevert(ERROR_ONLY_ADMIN); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonAdmin)); vm.startPrank(nonAdmin); testBaseFacet.functionWithOnlyAdminModifier(); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyGovernorOrStateTransitionManager.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyGovernorOrStateTransitionManager.t.sol index da2d6fccf..2c4062d5b 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyGovernorOrStateTransitionManager.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyGovernorOrStateTransitionManager.t.sol @@ -2,14 +2,14 @@ pragma solidity 0.8.24; -import {ZkSyncHyperchainBaseTest, ERROR_ONLY_ADMIN_OR_STATE_TRANSITION_MANAGER} from "./_Base_Shared.t.sol"; +import {ZkSyncHyperchainBaseTest} from "./_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract OnlyAdminOrStateTransitionManagerTest is ZkSyncHyperchainBaseTest { function test_revertWhen_calledByNonAdmin() public { address nonAdmin = makeAddr("nonAdmin"); - vm.expectRevert(ERROR_ONLY_ADMIN_OR_STATE_TRANSITION_MANAGER); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonAdmin)); vm.startPrank(nonAdmin); testBaseFacet.functionWithOnlyAdminOrStateTransitionManagerModifier(); } @@ -17,8 +17,7 @@ contract OnlyAdminOrStateTransitionManagerTest is ZkSyncHyperchainBaseTest { function test_revertWhen_calledByNonStateTransitionManager() public { address nonStateTransitionManager = makeAddr("nonStateTransitionManager"); - vm.expectRevert(ERROR_ONLY_ADMIN_OR_STATE_TRANSITION_MANAGER); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonStateTransitionManager)); vm.startPrank(nonStateTransitionManager); testBaseFacet.functionWithOnlyAdminOrStateTransitionManagerModifier(); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyStateTransitionManager.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyStateTransitionManager.t.sol index f6aafb661..a93032c90 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyStateTransitionManager.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyStateTransitionManager.t.sol @@ -2,14 +2,14 @@ pragma solidity 0.8.24; -import {ZkSyncHyperchainBaseTest, ERROR_ONLY_STATE_TRANSITION_MANAGER} from "./_Base_Shared.t.sol"; +import {ZkSyncHyperchainBaseTest} from "./_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract OnlyStateTransitionManagerTest is ZkSyncHyperchainBaseTest { function test_revertWhen_calledByNonStateTransitionManager() public { address nonStateTransitionManager = makeAddr("nonStateTransitionManager"); - vm.expectRevert(ERROR_ONLY_STATE_TRANSITION_MANAGER); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonStateTransitionManager)); vm.startPrank(nonStateTransitionManager); testBaseFacet.functionWithOnlyStateTransitionManagerModifier(); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyValidator.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyValidator.t.sol index c834dd982..c002fec59 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyValidator.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Base/OnlyValidator.t.sol @@ -2,7 +2,8 @@ pragma solidity 0.8.24; -import {ZkSyncHyperchainBaseTest, ERROR_ONLY_VALIDATOR} from "./_Base_Shared.t.sol"; +import {ZkSyncHyperchainBaseTest} from "./_Base_Shared.t.sol"; +import {Unauthorized} from "contracts/common/L1ContractErrors.sol"; contract OnlyValidatorTest is ZkSyncHyperchainBaseTest { function test_revertWhen_calledByNonValidator() public { @@ -10,8 +11,7 @@ contract OnlyValidatorTest is ZkSyncHyperchainBaseTest { utilsFacet.util_setValidator(nonValidator, false); - vm.expectRevert(ERROR_ONLY_VALIDATOR); - + vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, nonValidator)); vm.startPrank(nonValidator); testBaseFacet.functionWithOnlyValidatorModifier(); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Getters/IsFunctionFreezable.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Getters/IsFunctionFreezable.t.sol index 0b257db68..4af9875e2 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Getters/IsFunctionFreezable.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Getters/IsFunctionFreezable.t.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {GettersFacetTest} from "./_Getters_Shared.t.sol"; +import {InvalidSelector} from "contracts/common/L1ContractErrors.sol"; contract IsFunctionFreezableTest is GettersFacetTest { function test_revertWhen_facetAddressIzZero() public { @@ -11,8 +12,7 @@ contract IsFunctionFreezableTest is GettersFacetTest { gettersFacetWrapper.util_setFacetAddress(selector, address(0)); - vm.expectRevert(bytes.concat("g2")); - + vm.expectRevert(abi.encodeWithSelector(InvalidSelector.selector, selector)); gettersFacet.isFunctionFreezable(selector); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Getters/PriorityQueueFrontOperation.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Getters/PriorityQueueFrontOperation.t.sol index d17577afc..ac8ccfeaa 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Getters/PriorityQueueFrontOperation.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Getters/PriorityQueueFrontOperation.t.sol @@ -4,10 +4,11 @@ pragma solidity 0.8.24; import {GettersFacetTest} from "./_Getters_Shared.t.sol"; import {PriorityOperation} from "contracts/state-transition/libraries/PriorityQueue.sol"; +import {QueueIsEmpty} from "contracts/common/L1ContractErrors.sol"; contract GetPriorityQueueFrontOperationTest is GettersFacetTest { function test_revertWhen_queueIsEmpty() public { - vm.expectRevert(bytes.concat("D")); + vm.expectRevert(QueueIsEmpty.selector); gettersFacet.priorityQueueFrontOperation(); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Mailbox/BridgehubRequestL2Transaction.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Mailbox/BridgehubRequestL2Transaction.t.sol index d34a6bd7d..f20aa998c 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Mailbox/BridgehubRequestL2Transaction.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Mailbox/BridgehubRequestL2Transaction.t.sol @@ -7,6 +7,7 @@ import {BridgehubL2TransactionRequest} from "contracts/common/Messaging.sol"; import {REQUIRED_L2_GAS_PRICE_PER_PUBDATA} from "contracts/common/Config.sol"; import {TransactionFiltererTrue} from "contracts/dev-contracts/test/DummyTransactionFiltererTrue.sol"; import {TransactionFiltererFalse} from "contracts/dev-contracts/test/DummyTransactionFiltererFalse.sol"; +import {TransactionNotAllowed} from "contracts/common/L1ContractErrors.sol"; contract BridgehubRequestL2TransactionTest is MailboxTest { function test_successWithoutFilterer() public { @@ -54,7 +55,7 @@ contract BridgehubRequestL2TransactionTest is MailboxTest { vm.deal(bridgehub, 100 ether); vm.prank(address(bridgehub)); - vm.expectRevert(bytes("tf")); + vm.expectRevert(TransactionNotAllowed.selector); mailboxFacet.bridgehubRequestL2Transaction(req); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/Merkle/Merkle.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/Merkle/Merkle.t.sol index 492d489c2..89514fc99 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/Merkle/Merkle.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/Merkle/Merkle.t.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.24; import {Test} from "forge-std/Test.sol"; import {MerkleTest} from "contracts/dev-contracts/test/MerkleTest.sol"; import {MerkleTreeNoSort} from "./MerkleTreeNoSort.sol"; +import {MerklePathEmpty, MerkleIndexOutOfBounds, MerklePathOutOfBounds} from "contracts/common/L1ContractErrors.sol"; contract MerkleTestTest is Test { MerkleTreeNoSort merkleTree; @@ -44,7 +45,7 @@ contract MerkleTestTest is Test { bytes32 leaf = elements[0]; bytes32[] memory proof; - vm.expectRevert(bytes("xc")); + vm.expectRevert(MerklePathEmpty.selector); merkleTest.calculateRoot(proof, 0, leaf); } @@ -52,7 +53,7 @@ contract MerkleTestTest is Test { bytes32 leaf = elements[0]; bytes32[] memory proof = merkleTree.getProof(elements, 0); - vm.expectRevert(bytes("px")); + vm.expectRevert(MerkleIndexOutOfBounds.selector); merkleTest.calculateRoot(proof, 2 ** 255, leaf); } @@ -60,7 +61,7 @@ contract MerkleTestTest is Test { bytes32 leaf = elements[0]; bytes32[] memory proof = new bytes32[](256); - vm.expectRevert(bytes("bt")); + vm.expectRevert(MerklePathOutOfBounds.selector); merkleTest.calculateRoot(proof, 0, leaf); } } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/PriorityQueue/OnEmptyQueue.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/PriorityQueue/OnEmptyQueue.sol index 7881409fc..753d5e33c 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/PriorityQueue/OnEmptyQueue.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/PriorityQueue/OnEmptyQueue.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {PriorityQueueSharedTest} from "./_PriorityQueue_Shared.t.sol"; +import {QueueIsEmpty} from "contracts/common/L1ContractErrors.sol"; contract OnEmptyQueueTest is PriorityQueueSharedTest { function test_gets() public { @@ -13,12 +14,12 @@ contract OnEmptyQueueTest is PriorityQueueSharedTest { } function test_failGetFront() public { - vm.expectRevert(bytes("D")); + vm.expectRevert(QueueIsEmpty.selector); priorityQueue.front(); } function test_failPopFront() public { - vm.expectRevert(bytes("s")); + vm.expectRevert(QueueIsEmpty.selector); priorityQueue.popFront(); } } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/PriorityQueue/PopOperations.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/PriorityQueue/PopOperations.sol index f2f7d73ba..5e43f6284 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/PriorityQueue/PopOperations.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/PriorityQueue/PopOperations.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.24; import {PriorityQueueSharedTest} from "./_PriorityQueue_Shared.t.sol"; import {PriorityOperation} from "contracts/dev-contracts/test/PriorityQueueTest.sol"; +import {QueueIsEmpty} from "contracts/common/L1ContractErrors.sol"; contract PopOperationsTest is PriorityQueueSharedTest { uint256 public constant NUMBER_OPERATIONS = 10; @@ -67,7 +68,7 @@ contract PopOperationsTest is PriorityQueueSharedTest { assertTrue(priorityQueue.isEmpty()); // And now let's go over the limit and fail. - vm.expectRevert(bytes.concat("s")); + vm.expectRevert(QueueIsEmpty.selector); priorityQueue.popFront(); } } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/TransactionValidator/ValidateL1L2Tx.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/TransactionValidator/ValidateL1L2Tx.t.sol index bb78a71b5..c2a3e4dfe 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/TransactionValidator/ValidateL1L2Tx.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/TransactionValidator/ValidateL1L2Tx.t.sol @@ -4,6 +4,7 @@ pragma solidity 0.8.24; import {TransactionValidatorSharedTest} from "./_TransactionValidator_Shared.t.sol"; import {L2CanonicalTransaction} from "contracts/common/Messaging.sol"; +import {NotEnoughGas, TooMuchGas, InvalidPubdataLength} from "contracts/common/L1ContractErrors.sol"; contract ValidateL1L2TxTest is TransactionValidatorSharedTest { function test_BasicRequestL1L2() public pure { @@ -16,7 +17,7 @@ contract ValidateL1L2TxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createTestTransaction(); // The limit is so low, that it doesn't even cover the overhead testTx.gasLimit = 0; - vm.expectRevert(bytes("my")); + vm.expectRevert(NotEnoughGas.selector); validateL1ToL2Transaction(testTx, 500000, 100000); } @@ -27,7 +28,7 @@ contract ValidateL1L2TxTest is TransactionValidatorSharedTest { // before checking that it is below the max gas limit. uint256 priorityTxMaxGasLimit = 500000; testTx.gasLimit = priorityTxMaxGasLimit + 1000000; - vm.expectRevert(bytes("ui")); + vm.expectRevert(TooMuchGas.selector); validateL1ToL2Transaction(testTx, priorityTxMaxGasLimit, 100000); } @@ -41,7 +42,7 @@ contract ValidateL1L2TxTest is TransactionValidatorSharedTest { // So if the pubdata costs per byte is 1 - then this transaction could produce 500k of pubdata. // (hypothetically, assuming all the gas was spent on writing). testTx.gasPerPubdataByteLimit = 1; - vm.expectRevert(bytes("uk")); + vm.expectRevert(InvalidPubdataLength.selector); validateL1ToL2Transaction(testTx, priorityTxMaxGasLimit, 100000); } @@ -49,7 +50,7 @@ contract ValidateL1L2TxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createTestTransaction(); uint256 priorityTxMaxGasLimit = 500000; testTx.gasLimit = 200000; - vm.expectRevert(bytes("up")); + vm.expectRevert(NotEnoughGas.selector); validateL1ToL2Transaction(testTx, priorityTxMaxGasLimit, 100000); } diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/TransactionValidator/ValidateUpgradeTransaction.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/TransactionValidator/ValidateUpgradeTransaction.t.sol index df9a8f7eb..f3ac8238c 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/TransactionValidator/ValidateUpgradeTransaction.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/libraries/TransactionValidator/ValidateUpgradeTransaction.t.sol @@ -5,6 +5,7 @@ import {TransactionValidatorSharedTest} from "./_TransactionValidator_Shared.t.s import {L2CanonicalTransaction} from "contracts/common/Messaging.sol"; import {TransactionValidator} from "contracts/state-transition/libraries/TransactionValidator.sol"; +import {InvalidUpgradeTxn, UpgradeTxVerifyParam} from "contracts/common/L1ContractErrors.sol"; contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { function test_BasicRequest() public pure { @@ -16,7 +17,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // only system contracts (address < 2^16) are allowed to send upgrade transactions. testTx.from = uint256(1000000000); - vm.expectRevert(bytes("ua")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.From)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -24,7 +25,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // Now the 'to' address it too large. testTx.to = uint256(type(uint160).max) + 100; - vm.expectRevert(bytes("ub")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.To)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -32,7 +33,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // Paymaster must be 0 - otherwise we revert. testTx.paymaster = 1; - vm.expectRevert(bytes("uc")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.Paymaster)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -40,7 +41,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // Value must be 0 - otherwise we revert. testTx.value = 1; - vm.expectRevert(bytes("ud")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.Value)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -48,7 +49,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // reserved 0 must be 0 - otherwise we revert. testTx.reserved[0] = 1; - vm.expectRevert(bytes("ue")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.Reserved0)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -56,7 +57,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // reserved 1 must be a valid address testTx.reserved[1] = uint256(type(uint160).max) + 100; - vm.expectRevert(bytes("uf")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.Reserved1)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -64,7 +65,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // reserved 2 must be 0 - otherwise we revert. testTx.reserved[2] = 1; - vm.expectRevert(bytes("ug")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.Reserved2)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -72,7 +73,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // reserved 3 be 0 - otherwise we revert. testTx.reserved[3] = 1; - vm.expectRevert(bytes("uo")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.Reserved3)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -80,7 +81,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // Signature must be 0 - otherwise we revert. testTx.signature = bytes("hello"); - vm.expectRevert(bytes("uh")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.Signature)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -88,7 +89,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // PaymasterInput must be 0 - otherwise we revert. testTx.paymasterInput = bytes("hi"); - vm.expectRevert(bytes("ul1")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.PaymasterInput)); TransactionValidator.validateUpgradeTransaction(testTx); } @@ -96,7 +97,7 @@ contract ValidateUpgradeTxTest is TransactionValidatorSharedTest { L2CanonicalTransaction memory testTx = createUpgradeTransaction(); // ReservedDynamic must be 0 - otherwise we revert. testTx.reservedDynamic = bytes("something"); - vm.expectRevert(bytes("um")); + vm.expectRevert(abi.encodeWithSelector(InvalidUpgradeTxn.selector, UpgradeTxVerifyParam.ReservedDynamic)); TransactionValidator.validateUpgradeTransaction(testTx); } } diff --git a/l1-contracts/test/test_config/constant/hardhat.json b/l1-contracts/test/test_config/constant/hardhat.json index 0b550819d..a2364bde4 100644 --- a/l1-contracts/test/test_config/constant/hardhat.json +++ b/l1-contracts/test/test_config/constant/hardhat.json @@ -3,96 +3,96 @@ "name": "DAI", "symbol": "DAI", "decimals": 18, - "address": "0x3202935CA01eADd2F0B4e7aA23EFA52315121172" + "address": "0x4fE0EE70D78132fBb5C68C1d5e0020f3c05542f2" }, { "name": "wBTC", "symbol": "wBTC", "decimals": 8, - "address": "0xe73a1c05498e492f182f9B3E3cd65b2F9f52eE94" + "address": "0x3202935CA01eADd2F0B4e7aA23EFA52315121172" }, { "name": "BAT", "symbol": "BAT", "decimals": 18, - "address": "0x7aAE2ee8317b384aDE246664933A147411b60045" + "address": "0xe73a1c05498e492f182f9B3E3cd65b2F9f52eE94" }, { "name": "GNT", "symbol": "GNT", "decimals": 18, - "address": "0x02a344d1e31e92e39c2681937645F1d668C37d4e" + "address": "0x7aAE2ee8317b384aDE246664933A147411b60045" }, { "name": "MLTT", "symbol": "MLTT", "decimals": 18, - "address": "0x28033f8EdB2F43747E55401C4a3E3b4b2cF5146C" + "address": "0x02a344d1e31e92e39c2681937645F1d668C37d4e" }, { "name": "DAIK", "symbol": "DAIK", "decimals": 18, - "address": "0x147dCc3a8E99794C2Ec79EaF1a142324D6778255" + "address": "0x28033f8EdB2F43747E55401C4a3E3b4b2cF5146C" }, { "name": "wBTCK", "symbol": "wBTCK", "decimals": 8, - "address": "0x74cA8715E29196Bfbcd7444B09203d22dDaF7d1a" + "address": "0x147dCc3a8E99794C2Ec79EaF1a142324D6778255" }, { "name": "BATK", "symbol": "BATS", "decimals": 18, - "address": "0x2eAf3eac597d23db3A8427329482aE47935141d9" + "address": "0x74cA8715E29196Bfbcd7444B09203d22dDaF7d1a" }, { "name": "GNTK", "symbol": "GNTS", "decimals": 18, - "address": "0x2B1b32d23f2be391280bFbD2B51daB8Ad2a69B9e" + "address": "0x2eAf3eac597d23db3A8427329482aE47935141d9" }, { "name": "MLTTK", "symbol": "MLTTS", "decimals": 18, - "address": "0xc2Ca10940Ad80Cd98512B767457bd44713232B5a" + "address": "0x2B1b32d23f2be391280bFbD2B51daB8Ad2a69B9e" }, { "name": "DAIL", "symbol": "DAIL", "decimals": 18, - "address": "0xd6b4CDa9F0Ef6d9F6b35Ab5424df0dD95E6a6D1b" + "address": "0xc2Ca10940Ad80Cd98512B767457bd44713232B5a" }, { "name": "wBTCL", "symbol": "wBTCP", "decimals": 8, - "address": "0x15CD4a1C10AE2D3727Dad680a1966947931588C9" + "address": "0xd6b4CDa9F0Ef6d9F6b35Ab5424df0dD95E6a6D1b" }, { "name": "BATL", "symbol": "BATW", "decimals": 18, - "address": "0x45f430CFD5Bf38eCE39f1B9A2930B3fD494619e8" + "address": "0x15CD4a1C10AE2D3727Dad680a1966947931588C9" }, { "name": "GNTL", "symbol": "GNTW", "decimals": 18, - "address": "0x801Ab08819573537C0B256d139473eF13482B3Dd" + "address": "0x45f430CFD5Bf38eCE39f1B9A2930B3fD494619e8" }, { "name": "MLTTL", "symbol": "MLTTW", "decimals": 18, - "address": "0xf55F3af54B9a507Bd0260e5844C1648921214745" + "address": "0x801Ab08819573537C0B256d139473eF13482B3Dd" }, { "name": "Wrapped Ether", "symbol": "WETH", "decimals": 18, - "address": "0xed01DF970925Cc0BB427f9E8014449C8a529D303" + "address": "0xf55F3af54B9a507Bd0260e5844C1648921214745" } -] +] \ No newline at end of file