Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

N-07 Naming Issues #577

Merged
merged 2 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 {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";

Check failure on line 11 in l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol

View workflow job for this annotation

GitHub Actions / lint

imported name PubdataPerBatchIsLessThanTxn is not used

Check failure on line 11 in l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol

View workflow job for this annotation

GitHub Actions / lint

imported name PubdataPerBatchIsLessThanTxn is not used

Check failure on line 11 in l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol

View workflow job for this annotation

GitHub Actions / lint

imported name PubdataPerBatchIsLessThanTxn is not used

// 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 @@
// 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
Loading