Skip to content

Commit

Permalink
N-07 Naming Issues (#577)
Browse files Browse the repository at this point in the history
  • Loading branch information
koloz193 authored Jul 24, 2024
1 parent a2fb965 commit 1094312
Show file tree
Hide file tree
Showing 12 changed files with 53 additions and 47 deletions.
6 changes: 3 additions & 3 deletions gas-bound-caller/contracts/GasBoundCaller.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity 0.8.20;

import {EfficientCall} from "@matterlabs/zksync-contracts/l2/system-contracts/libraries/EfficientCall.sol";
import {ISystemContext} from "./ISystemContext.sol";
import {MaxGasLessThanGasLeft, PubdataAllowanceAndGasLeftLessThanPubdataGasAndOverhead} from "./GasBoundCallerErrors.sol";
import {InsufficientGasProvided, NotEnoughGasForPubdata} from "./GasBoundCallerErrors.sol";

ISystemContext constant SYSTEM_CONTEXT_CONTRACT = ISystemContext(address(0x800b));

Expand Down Expand Up @@ -47,7 +47,7 @@ contract GasBoundCaller {
//
// Ultimately, the entire `gas` sent to this call can be spent on compute regardless of the `_maxTotalGas` parameter.
if (_maxTotalGas < gasleft()) {
revert MaxGasLessThanGasLeft();
revert InsufficientGasProvided();
}

// This is the amount of gas that can be spent *exclusively* on pubdata in addition to the `gas` provided to this function.
Expand Down Expand Up @@ -94,7 +94,7 @@ contract GasBoundCaller {
// Here we double check that the additional cost is not higher than the maximum allowed.
// Note, that the `gasleft()` can be spent on pubdata too.
if (pubdataAllowance + gasleft() < pubdataGas + CALL_RETURN_OVERHEAD) {
revert PubdataAllowanceAndGasLeftLessThanPubdataGasAndOverhead();
revert NotEnoughGasForPubdata();
}
}

Expand Down
8 changes: 4 additions & 4 deletions gas-bound-caller/contracts/GasBoundCallerErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// We use a floating point pragma here so it can be used within other projects that interact with the zkSync ecosystem without using our exact pragma version.
pragma solidity ^0.8.21;

// 0x0921241a
error MaxGasLessThanGasLeft();
// 0xb2017838
error PubdataAllowanceAndGasLeftLessThanPubdataGasAndOverhead();
// 0x8fdb52d9
error InsufficientGasProvided();
// 0xa303e7dd
error NotEnoughGasForPubdata();
14 changes: 10 additions & 4 deletions l1-contracts/contracts/common/L1ContractErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,10 @@ error NotInitializedReentrancyGuard();
error OnlyEraSupported();
// 0x1a21feed
error OperationExists();
// 0xee454a75
error OperationShouldBePending();
// 0x72afcbf4
error OperationShouldBeReady();
// 0xeda2fbb1
error OperationMustBePending();
// 0xe1c1ff37
error OperationMustBeReady();
// 0xd7f50a9d
error PatchCantSetUpgradeTxn();
// 0x962fd7d0
Expand All @@ -195,6 +195,8 @@ error PreviousUpgradeNotCleaned();
error PreviousUpgradeNotFinalized(bytes32 txHash);
// 0xd5a99014
error PriorityOperationsRollingHashMismatch();
// 0x1a4d284a
error PriorityTxPubdataExceedsMaxPubDataPerBatch();
// 0xa461f651
error ProtocolIdMismatch(uint256 expectedProtocolVersion, uint256 providedProtocolId);
// 0x64f94ec2
Expand Down Expand Up @@ -223,6 +225,8 @@ error RemoveFunctionFacetAddressZero();
error ReplaceFunctionFacetAddressZero();
// 0xdab52f4b
error RevertedBatchBeforeNewBatch();
// 0x9a67c1cb
error RevertedBatchNotAfterNewLastBatch();
// 0xd3b6535b
error SelectorsMustAllHaveSameFreezability();
// 0x7774d2f9
Expand Down Expand Up @@ -279,6 +283,8 @@ error UpgradeBatchNumberIsNotZero();
error ValidateTxnNotEnoughGas();
// 0x626ade30
error ValueMismatch(uint256 expected, uint256 actual);
// 0xe1022469
error VerifiedBatchesExceedsCommittedBatches();
// 0x2dbdba00
error VerifyProofCommittedVerifiedMismatch();
// 0xae899454
Expand Down
12 changes: 6 additions & 6 deletions l1-contracts/contracts/governance/Governance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ pragma solidity 0.8.24;

import {Ownable2Step} from "@openzeppelin/contracts-v4/access/Ownable2Step.sol";
import {IGovernance} from "./IGovernance.sol";
import {ZeroAddress, Unauthorized, OperationShouldBeReady, OperationShouldBePending, OperationExists, InvalidDelay, PreviousOperationNotExecuted} from "../common/L1ContractErrors.sol";
import {ZeroAddress, Unauthorized, OperationMustBeReady, OperationMustBePending, OperationExists, InvalidDelay, PreviousOperationNotExecuted} from "../common/L1ContractErrors.sol";

/// @author Matter Labs
/// @custom:security-contact security@matterlabs.dev
Expand Down Expand Up @@ -160,7 +160,7 @@ contract Governance is IGovernance, Ownable2Step {
/// @param _id Proposal id value (see `hashOperation`)
function cancel(bytes32 _id) external onlyOwner {
if (!isOperationPending(_id)) {
revert OperationShouldBePending();
revert OperationMustBePending();
}
delete timestamps[_id];
emit OperationCancelled(_id);
Expand All @@ -180,15 +180,15 @@ contract Governance is IGovernance, Ownable2Step {
_checkPredecessorDone(_operation.predecessor);
// Ensure that the operation is ready to proceed.
if (!isOperationReady(id)) {
revert OperationShouldBeReady();
revert OperationMustBeReady();
}
// Execute operation.
// slither-disable-next-line reentrancy-eth
_execute(_operation.calls);
// Reconfirming that the operation is still ready after execution.
// This is needed to avoid unexpected reentrancy attacks of re-executing the same operation.
if (!isOperationReady(id)) {
revert OperationShouldBeReady();
revert OperationMustBeReady();
}
// Set operation to be done
timestamps[id] = EXECUTED_PROPOSAL_TIMESTAMP;
Expand All @@ -205,15 +205,15 @@ contract Governance is IGovernance, Ownable2Step {
_checkPredecessorDone(_operation.predecessor);
// Ensure that the operation is in a pending state before proceeding.
if (!isOperationPending(id)) {
revert OperationShouldBePending();
revert OperationMustBePending();
}
// Execute operation.
// slither-disable-next-line reentrancy-eth
_execute(_operation.calls);
// Reconfirming that the operation is still pending before execution.
// This is needed to avoid unexpected reentrancy attacks of re-executing the same operation.
if (!isOperationPending(id)) {
revert OperationShouldBePending();
revert OperationMustBePending();
}
// Set operation to be done
timestamps[id] = EXECUTED_PROPOSAL_TIMESTAMP;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +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, PubdataPerBatchIsLessThanTxn, InvalidPubdataPricingMode, ProtocolIdMismatch, ChainAlreadyLive, HashMismatch, ProtocolIdNotGreater, DenominatorIsZero, DiamondAlreadyFrozen, DiamondNotFrozen, ZeroAddress} from "../../../common/L1ContractErrors.sol";
import {Unauthorized, TooMuchGas, PriorityTxPubdataExceedsMaxPubDataPerBatch, PubdataPerBatchIsLessThanTxn, InvalidPubdataPricingMode, ProtocolIdMismatch, ChainAlreadyLive, HashMismatch, ProtocolIdNotGreater, DenominatorIsZero, DiamondAlreadyFrozen, DiamondNotFrozen, ZeroAddress} 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";
Expand Down Expand Up @@ -77,7 +77,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 PubdataPerBatchIsLessThanTxn();
revert PriorityTxPubdataExceedsMaxPubDataPerBatch();
}

FeeParams memory oldFeeParams = s.feeParams;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +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 {BatchNumberMismatch, TimeNotReached, TooManyBlobs, ValueMismatch, InvalidPubdataMode, InvalidPubdataLength, HashMismatch, NonIncreasingTimestamp, TimestampError, InvalidLogSender, TxHashMismatch, UnexpectedSystemLog, MissingSystemLogs, LogAlreadyProcessed, InvalidProtocolVersion, CanOnlyProcessOneBatch, BatchHashMismatch, UpgradeBatchNumberIsNotZero, NonSequentialBatch, CantExecuteUnprovenBatches, SystemLogsSizeTooBig, InvalidNumberOfBlobs, VerifyProofCommittedVerifiedMismatch, InvalidProof, RevertedBatchBeforeNewBatch, CantRevertExecutedBatch, PointEvalFailed, EmptyBlobVersionHash, NonEmptyBlobVersionHash, BlobHashCommitmentError, CalldataLengthTooBig, InvalidPubdataHash, L2TimestampTooBig, PriorityOperationsRollingHashMismatch, PubdataCommitmentsEmpty, PointEvalCallFailed, PubdataCommitmentsTooBig, InvalidPubdataCommitmentsSize} from "../../../common/L1ContractErrors.sol";
import {BatchNumberMismatch, TimeNotReached, TooManyBlobs, ValueMismatch, InvalidPubdataMode, InvalidPubdataLength, HashMismatch, NonIncreasingTimestamp, TimestampError, InvalidLogSender, TxHashMismatch, UnexpectedSystemLog, MissingSystemLogs, LogAlreadyProcessed, InvalidProtocolVersion, CanOnlyProcessOneBatch, BatchHashMismatch, UpgradeBatchNumberIsNotZero, NonSequentialBatch, CantExecuteUnprovenBatches, SystemLogsSizeTooBig, InvalidNumberOfBlobs, VerifiedBatchesExceedsCommittedBatches, InvalidProof, RevertedBatchNotAfterNewLastBatch, CantRevertExecutedBatch, PointEvalFailed, EmptyBlobVersionHash, NonEmptyBlobVersionHash, BlobHashCommitmentError, CalldataLengthTooBig, InvalidPubdataHash, L2TimestampTooBig, PriorityOperationsRollingHashMismatch, PubdataCommitmentsEmpty, PointEvalCallFailed, PubdataCommitmentsTooBig, InvalidPubdataCommitmentsSize} 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";
Expand Down Expand Up @@ -508,7 +508,7 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor {
prevBatchCommitment = currentBatchCommitment;
}
if (currentTotalBatchesVerified > s.totalBatchesCommitted) {
revert VerifyProofCommittedVerifiedMismatch();
revert VerifiedBatchesExceedsCommittedBatches();
}

_verifyProof(proofPublicInput, _proof);
Expand Down Expand Up @@ -554,7 +554,7 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor {

function _revertBatches(uint256 _newLastBatch) internal {
if (s.totalBatchesCommitted <= _newLastBatch) {
revert RevertedBatchBeforeNewBatch();
revert RevertedBatchNotAfterNewLastBatch();
}
if (_newLastBatch < s.totalBatchesExecuted) {
revert CantRevertExecutedBatch();
Expand Down
4 changes: 2 additions & 2 deletions l1-contracts/contracts/upgrades/Upgrade_v1_4_1.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +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";
import {PriorityTxPubdataExceedsMaxPubDataPerBatch} from "../common/L1ContractErrors.sol";

/// @author Matter Labs
/// @custom:security-contact security@matterlabs.dev
Expand All @@ -26,7 +26,7 @@ contract Upgrade_v1_4_1 is BaseZkSyncUpgrade {
// 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 PubdataPerBatchIsLessThanTxn();
revert PriorityTxPubdataExceedsMaxPubDataPerBatch();
}

FeeParams memory oldFeeParams = s.feeParams;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +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";
import {VerifiedBatchesExceedsCommittedBatches, BatchHashMismatch} from "contracts/common/L1ContractErrors.sol";

contract ProvingTest is ExecutorTest {
function setUp() public {
Expand Down Expand Up @@ -95,7 +95,7 @@ contract ProvingTest is ExecutorTest {

vm.prank(validator);

vm.expectRevert(VerifyProofCommittedVerifiedMismatch.selector);
vm.expectRevert(VerifiedBatchesExceedsCommittedBatches.selector);
executor.proveBatches(genesisStoredBatchInfo, storedBatchInfoArray, proofInput);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +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";
import {RevertedBatchNotAfterNewLastBatch} from "contracts/common/L1ContractErrors.sol";

contract RevertingTest is ExecutorTest {
function setUp() public {
Expand Down Expand Up @@ -56,7 +56,7 @@ contract RevertingTest is ExecutorTest {

function test_RevertWhen_RevertingMoreBatchesThanAlreadyCommitted() public {
vm.prank(validator);
vm.expectRevert(RevertedBatchBeforeNewBatch.selector);
vm.expectRevert(RevertedBatchNotAfterNewLastBatch.selector);
executor.revertBatches(10);
}

Expand Down
24 changes: 12 additions & 12 deletions l1-contracts/test/foundry/unit/concrete/Governance/Executing.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +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";
import {OperationMustBeReady, OperationMustBePending, OperationExists, PreviousOperationNotExecuted, InvalidDelay} from "contracts/common/L1ContractErrors.sol";

contract ExecutingTest is GovernanceTest {
using stdStorage for StdStorage;
Expand Down Expand Up @@ -52,7 +52,7 @@ contract ExecutingTest is GovernanceTest {
vm.startPrank(owner);
IGovernance.Operation memory op = operationWithOneCallZeroSaltAndPredecessor(address(eventOnFallback), 0, "");
governance.scheduleTransparent(op, 10000);
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(op);
}

Expand All @@ -66,7 +66,7 @@ contract ExecutingTest is GovernanceTest {
governance.scheduleTransparent(validOp, 0);

IGovernance.Operation memory invalidOp = operationWithOneCallZeroSaltAndPredecessor(address(0), 0, "");
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(invalidOp);
}

Expand All @@ -84,7 +84,7 @@ contract ExecutingTest is GovernanceTest {
1,
""
);
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(invalidOp);
}

Expand All @@ -102,7 +102,7 @@ contract ExecutingTest is GovernanceTest {
0,
"00"
);
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(invalidOp);
}

Expand Down Expand Up @@ -134,7 +134,7 @@ contract ExecutingTest is GovernanceTest {
invalidOp.predecessor = governance.hashOperation(executedOp);

// Failed to execute operation that wasn't scheduled
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(invalidOp);
}

Expand All @@ -153,7 +153,7 @@ contract ExecutingTest is GovernanceTest {
""
);
invalidOp.salt = Utils.randomBytes32("wrongSalt");
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(invalidOp);
}

Expand Down Expand Up @@ -182,7 +182,7 @@ contract ExecutingTest is GovernanceTest {
governance.scheduleTransparent(op, 0);
executeOpAndCheck(op);

vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(op);
}

Expand All @@ -194,7 +194,7 @@ contract ExecutingTest is GovernanceTest {
0,
"1122"
);
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(op);
}

Expand All @@ -206,7 +206,7 @@ contract ExecutingTest is GovernanceTest {
0,
"1122"
);
vm.expectRevert(OperationShouldBePending.selector);
vm.expectRevert(OperationMustBePending.selector);
governance.executeInstant(op);
}

Expand All @@ -220,7 +220,7 @@ contract ExecutingTest is GovernanceTest {
);
governance.scheduleTransparent(op, 0);
governance.cancel(governance.hashOperation(op));
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(op);
}

Expand Down Expand Up @@ -271,7 +271,7 @@ contract ExecutingTest is GovernanceTest {
function test_RevertWhen_CancelNonExistingOperation() public {
vm.startPrank(owner);

vm.expectRevert(OperationShouldBePending.selector);
vm.expectRevert(OperationMustBePending.selector);
governance.cancel(bytes32(0));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +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";
import {OperationMustBeReady, OperationMustBePending} from "contracts/common/L1ContractErrors.sol";

contract ReentrancyTest is GovernanceTest {
using stdStorage for StdStorage;
Expand Down Expand Up @@ -89,7 +89,7 @@ contract ReentrancyTest is GovernanceTest {
vm.startPrank(address(reenterGovernance));

governance.scheduleTransparent(op, 0);
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(op);
}

Expand All @@ -109,7 +109,7 @@ contract ReentrancyTest is GovernanceTest {
vm.startPrank(address(reenterGovernance));

governance.scheduleTransparent(op, 0);
vm.expectRevert(OperationShouldBePending.selector);
vm.expectRevert(OperationMustBePending.selector);
governance.executeInstant(op);
}

Expand All @@ -126,7 +126,7 @@ contract ReentrancyTest is GovernanceTest {
vm.startPrank(address(reenterGovernance));

governance.scheduleTransparent(op, 0);
vm.expectRevert(OperationShouldBeReady.selector);
vm.expectRevert(OperationMustBeReady.selector);
governance.execute(op);
}

Expand All @@ -146,7 +146,7 @@ contract ReentrancyTest is GovernanceTest {
vm.startPrank(address(reenterGovernance));

governance.scheduleTransparent(op, 0);
vm.expectRevert(OperationShouldBePending.selector);
vm.expectRevert(OperationMustBePending.selector);
governance.executeInstant(op);
}
}
Loading

0 comments on commit 1094312

Please sign in to comment.