From 9c5145a5db76d1806eb5d9e1363eb5cc4d3167d0 Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Tue, 28 May 2024 16:05:21 +0200 Subject: [PATCH 01/12] better checking log senders --- .../unit/concrete/Executor/Committing.t.sol | 95 ++++++++++++++----- 1 file changed, 73 insertions(+), 22 deletions(-) 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..699bb74c1 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol @@ -5,12 +5,39 @@ import {Vm} from "forge-std/Test.sol"; import {Utils, L2_BOOTLOADER_ADDRESS, L2_SYSTEM_CONTEXT_ADDRESS} from "../Utils/Utils.sol"; import {ExecutorTest} from "./_Executor_Shared.t.sol"; -import {IExecutor, MAX_NUMBER_OF_BLOBS} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; +import {IExecutor, MAX_NUMBER_OF_BLOBS, PubdataSource} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; 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 {IStateTransitionManager} from "contracts/state-transition/IStateTransitionManager.sol"; contract CommittingTest is ExecutorTest { + function test_revertWhen_wrongCaller(address randomCaller) public { + if (randomCaller != validator) { + IExecutor.CommitBatchInfo[] memory newCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + newCommitBatchInfoArray[0] = newCommitBatchInfo; + + vm.prank(randomCaller); + vm.expectRevert("Hyperchain: not validator"); + executor.commitBatches(genesisStoredBatchInfo, newCommitBatchInfoArray); + } + } + + function test_RevertWhen_wrongProtocolVersion() public { + IExecutor.CommitBatchInfo[] memory newCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + newCommitBatchInfoArray[0] = newCommitBatchInfo; + + vm.mockCall( + stmAddress, + abi.encodeWithSelector(IStateTransitionManager.protocolVersionIsActive.selector), + abi.encode(bool(false)) + ); + + vm.prank(validator); + vm.expectRevert("Executor facet: wrong protocol version"); + executor.commitBatches(genesisStoredBatchInfo, newCommitBatchInfoArray); + } + function test_RevertWhen_CommittingWithWrongLastCommittedBatchData() public { IExecutor.CommitBatchInfo[] memory newCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); newCommitBatchInfoArray[0] = newCommitBatchInfo; @@ -24,6 +51,23 @@ contract CommittingTest is ExecutorTest { executor.commitBatches(wrongGenesisStoredBatchInfo, newCommitBatchInfoArray); } + function test_RevertWhen_wrongPubdataSource(uint8 pubdataSource) public { + if (pubdataSource == uint8(PubdataSource.Blob) || pubdataSource == uint8(PubdataSource.Calldata)) { + return; + } + + IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; + wrongNewCommitBatchInfo.batchNumber = 1; + wrongNewCommitBatchInfo.pubdataCommitments[0] = bytes1(pubdataSource); + + IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + + vm.prank(validator); + vm.expectRevert(bytes.concat("us")); + executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); + } + function test_RevertWhen_CommittingWithWrongOrderOfBatches() public { IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; wrongNewCommitBatchInfo.batchNumber = 2; // wrong batch number @@ -257,28 +301,41 @@ contract CommittingTest is ExecutorTest { } function test_RevertWhen_SystemLogIsFromIncorrectAddress() public { - bytes32[7] memory values = [ + bytes32[13] memory values = [ bytes32(""), bytes32(0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563), bytes32(""), bytes32(""), bytes32(""), keccak256(""), + bytes32(""), + bytes32(""), + bytes32(""), + bytes32(""), + bytes32(""), + bytes32(""), bytes32("") ]; - bytes[7] memory errors = [ + bytes[13] memory errors = [ bytes.concat("lm"), bytes.concat("ln"), bytes.concat("lb"), bytes.concat("sc"), bytes.concat("sv"), bytes.concat("bl"), - bytes.concat("bk") + bytes.concat("bk"), + bytes.concat("pc"), + bytes.concat("pc"), + bytes.concat("pc"), + bytes.concat("pc"), + bytes.concat("pc"), + bytes.concat("pc") ]; for (uint256 i = 0; i < values.length; i++) { bytes[] memory wrongL2Logs = Utils.createSystemLogs(); + address wrongAddress = makeAddr("randomAddress"); wrongL2Logs[i] = Utils.constructL2Log(true, wrongAddress, i, values[i]); @@ -293,6 +350,14 @@ contract CommittingTest is ExecutorTest { vm.expectRevert(errors[i]); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } + + // logs[13] = constructL2Log( + // true, + // L2_BOOTLOADER_ADDRESS, + // uint256(SystemLogKey.EXPECTED_SYSTEM_CONTRACT_UPGRADE_TX_HASH_KEY), + // bytes32(0) + // ); + } function test_RevertWhen_SystemLogIsMissing() public { @@ -407,17 +472,10 @@ contract CommittingTest is ExecutorTest { correctCommitBatchInfoArray[0].pubdataCommitments = pubdataCommitment; vm.prank(validator); - - vm.recordLogs(); - + vm.expectEmit(true, false, false, false, address(executor)); + emit BlockCommit(1, bytes32(0), bytes32(0)); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); - Vm.Log[] memory entries = vm.getRecordedLogs(); - - assertEq(entries.length, 1); - assertEq(entries[0].topics[0], keccak256("BlockCommit(uint256,bytes32,bytes32)")); - assertEq(entries[0].topics[1], bytes32(uint256(1))); // batchNumber - uint256 totalBatchesCommitted = getters.getTotalBatchesCommitted(); assertEq(totalBatchesCommitted, 1); @@ -480,17 +538,10 @@ contract CommittingTest is ExecutorTest { correctCommitBatchInfoArray[0].pubdataCommitments = pubdataCommitment; vm.prank(validator); - - vm.recordLogs(); - + vm.expectEmit(true, false, false, false, address(executor)); + emit BlockCommit(1, bytes32(0), bytes32(0)); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); - Vm.Log[] memory entries = vm.getRecordedLogs(); - - assertEq(entries.length, 1); - assertEq(entries[0].topics[0], keccak256("BlockCommit(uint256,bytes32,bytes32)")); - assertEq(entries[0].topics[1], bytes32(uint256(1))); // batchNumber - uint256 totalBatchesCommitted = getters.getTotalBatchesCommitted(); assertEq(totalBatchesCommitted, 1); From da8c2987d88e2eb7f5e9b55633c6b3cb503fa9bf Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Wed, 29 May 2024 12:47:41 +0200 Subject: [PATCH 02/12] executor tests: add utils facet --- .../concrete/Executor/_Executor_Shared.t.sol | 17 ++++++++++++++++- .../test/foundry/unit/concrete/Utils/Utils.sol | 3 ++- .../foundry/unit/concrete/Utils/UtilsFacet.sol | 4 ++++ 3 files changed, 22 insertions(+), 2 deletions(-) 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..799d14a56 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 @@ -20,8 +20,11 @@ import {IExecutor} from "contracts/state-transition/chain-interfaces/IExecutor.s import {IVerifier} from "contracts/state-transition/chain-interfaces/IVerifier.sol"; import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; import {TestnetVerifier} from "contracts/state-transition/TestnetVerifier.sol"; +import {Utils} from "foundry-test/unit/concrete/Utils/Utils.sol"; +import {UtilsFacet} from "foundry-test/unit/concrete/Utils/UtilsFacet.sol"; contract ExecutorTest is Test { + address internal stmAddress; address internal owner; address internal validator; address internal randomSigner; @@ -30,6 +33,7 @@ contract ExecutorTest is Test { TestExecutor internal executor; GettersFacet internal getters; MailboxFacet internal mailbox; + UtilsFacet internal utils; bytes32 internal newCommittedBlockBatchHash; bytes32 internal newCommittedBlockCommitment; uint256 internal currentTimestamp; @@ -41,6 +45,8 @@ contract ExecutorTest is Test { IExecutor.StoredBatchInfo internal genesisStoredBatchInfo; IExecutor.ProofInput internal proofInput; + event BlockCommit(uint256 indexed batchNumber, bytes32 indexed batchHash, bytes32 indexed commitment); + function getAdminSelectors() private view returns (bytes4[] memory) { bytes4[] memory selectors = new bytes4[](11); selectors[0] = admin.setPendingAdmin.selector; @@ -133,6 +139,7 @@ contract ExecutorTest is Test { admin = new AdminFacet(); getters = new GettersFacet(); mailbox = new MailboxFacet(eraChainId); + utils = new UtilsFacet(); DummyStateTransitionManager stateTransitionManager = new DummyStateTransitionManager(); vm.mockCall( @@ -140,6 +147,7 @@ contract ExecutorTest is Test { abi.encodeWithSelector(IStateTransitionManager.protocolVersionIsActive.selector), abi.encode(bool(true)) ); + stmAddress = address(stateTransitionManager); DiamondInit diamondInit = new DiamondInit(); bytes8 dummyHash = 0x1234567890123456; @@ -183,7 +191,7 @@ contract ExecutorTest is Test { bytes memory diamondInitData = abi.encodeWithSelector(diamondInit.initialize.selector, params); - Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](4); + Diamond.FacetCut[] memory facetCuts = new Diamond.FacetCut[](5); facetCuts[0] = Diamond.FacetCut({ facet: address(admin), action: Diamond.Action.Add, @@ -208,6 +216,12 @@ contract ExecutorTest is Test { isFreezable: true, selectors: getMailboxSelectors() }); + facetCuts[4] = Diamond.FacetCut({ + facet: address(utils), + action: Diamond.Action.Add, + isFreezable: true, + selectors: Utils.getUtilsFacetSelectors() + }); Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ facetCuts: facetCuts, @@ -222,6 +236,7 @@ contract ExecutorTest is Test { getters = GettersFacet(address(diamondProxy)); mailbox = MailboxFacet(address(diamondProxy)); admin = AdminFacet(address(diamondProxy)); + utils = UtilsFacet(address(diamondProxy)); // Initiate the token multiplier to enable L1 -> L2 transactions. vm.prank(address(stateTransitionManager)); diff --git a/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol b/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol index b52d1e122..fc772b59f 100644 --- a/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol +++ b/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol @@ -256,7 +256,7 @@ library Utils { } function getUtilsFacetSelectors() public pure returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](38); + bytes4[] memory selectors = new bytes4[](39); selectors[0] = UtilsFacet.util_setChainId.selector; selectors[1] = UtilsFacet.util_getChainId.selector; selectors[2] = UtilsFacet.util_setBridgehub.selector; @@ -295,6 +295,7 @@ library Utils { selectors[35] = UtilsFacet.util_getIsFrozen.selector; selectors[36] = UtilsFacet.util_setTransactionFilterer.selector; selectors[37] = UtilsFacet.util_setBaseTokenGasPriceMultiplierDenominator.selector; + selectors[38] = UtilsFacet.util_setl2SystemContractsUpgradeTxHash.selector; return selectors; } diff --git a/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol b/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol index 01864697d..26a23c0fd 100644 --- a/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol +++ b/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol @@ -162,6 +162,10 @@ contract UtilsFacet is ZkSyncHyperchainBase { return s.isFrozen; } + function util_setl2SystemContractsUpgradeTxHash(bytes32 _l2SystemContractsUpgradeTxHash) external { + s.l2SystemContractsUpgradeTxHash = _l2SystemContractsUpgradeTxHash; + } + // add this to be excluded from coverage report function test() internal virtual {} } From 47ea92423446b647afce12f17383c05fdf2ed51f Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Wed, 29 May 2024 12:48:14 +0200 Subject: [PATCH 03/12] executor test: reverts on decoding l2 logs --- .../unit/concrete/Executor/Committing.t.sol | 71 ++++++++++++++----- 1 file changed, 54 insertions(+), 17 deletions(-) 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 699bb74c1..eb563ac69 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol @@ -300,8 +300,17 @@ contract CommittingTest is ExecutorTest { executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } + function _getCommitInfoWithLogs(bytes[] memory logs) internal returns (IExecutor.CommitBatchInfo[] memory) { + IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(logs); + IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + + return wrongNewCommitBatchInfoArray; + } + function test_RevertWhen_SystemLogIsFromIncorrectAddress() public { - bytes32[13] memory values = [ + bytes32[15] memory values = [ bytes32(""), bytes32(0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563), bytes32(""), @@ -314,10 +323,12 @@ contract CommittingTest is ExecutorTest { bytes32(""), bytes32(""), bytes32(""), + bytes32(""), + bytes32(""), bytes32("") ]; - bytes[13] memory errors = [ + bytes[15] memory errors = [ bytes.concat("lm"), bytes.concat("ln"), bytes.concat("lb"), @@ -330,34 +341,62 @@ contract CommittingTest is ExecutorTest { bytes.concat("pc"), bytes.concat("pc"), bytes.concat("pc"), - bytes.concat("pc") + bytes.concat("pc"), + bytes.concat("bu"), + bytes.concat("ul") ]; + address wrongAddress = makeAddr("randomAddress"); + for (uint256 i = 0; i < values.length; i++) { - bytes[] memory wrongL2Logs = Utils.createSystemLogs(); + bytes[] memory wrongL2Logs = Utils.createSystemLogsWithUpgradeTransaction(bytes32("")); + bytes[] memory tooManyLogs = new bytes[](15); + for (uint256 i = 0; i < wrongL2Logs.length; i++) { + tooManyLogs[i] = wrongL2Logs[i]; + } - address wrongAddress = makeAddr("randomAddress"); - wrongL2Logs[i] = Utils.constructL2Log(true, wrongAddress, i, values[i]); + tooManyLogs[i] = Utils.constructL2Log(true, wrongAddress, i, values[i]); IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; - wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(wrongL2Logs); - + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(tooManyLogs); IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; vm.prank(validator); - vm.expectRevert(errors[i]); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } - // logs[13] = constructL2Log( - // true, - // L2_BOOTLOADER_ADDRESS, - // uint256(SystemLogKey.EXPECTED_SYSTEM_CONTRACT_UPGRADE_TX_HASH_KEY), - // bytes32(0) - // ); + bytes[] memory logsWithWrongHash = Utils.createSystemLogsWithUpgradeTransaction(bytes32("")); + logsWithWrongHash[logsWithWrongHash.length - 1] = Utils.constructL2Log( + true, + L2_BOOTLOADER_ADDRESS, + logsWithWrongHash.length - 1, + bytes32("1") + ); + IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(logsWithWrongHash); + IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + vm.prank(validator); + vm.expectRevert(bytes.concat("ut")); + executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); + } + + function test_RevertWhen_SystemLogsBadLength() public { + utils.util_setl2SystemContractsUpgradeTxHash(bytes32("not default")); + bytes[] memory logs = Utils.createSystemLogsWithUpgradeTransaction(bytes32("not default")); + delete logs[0]; + + IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(logs); + IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + + vm.prank(validator); + vm.expectRevert(bytes.concat("b8")); + executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } function test_RevertWhen_SystemLogIsMissing() public { @@ -367,12 +406,10 @@ contract CommittingTest is ExecutorTest { IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(l2Logs); - IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; vm.prank(validator); - vm.expectRevert(bytes.concat("b7")); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } From 160600fd66cd6817bfd01761523b54ed5e3a4dde Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Mon, 3 Jun 2024 12:51:52 +0200 Subject: [PATCH 04/12] reverts on committing with validium mode --- .../unit/concrete/Executor/Committing.t.sol | 60 +++++++++++++++++-- .../concrete/Executor/_Executor_Shared.t.sol | 2 +- 2 files changed, 56 insertions(+), 6 deletions(-) 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 eb563ac69..0b0df5536 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol @@ -4,11 +4,11 @@ pragma solidity 0.8.24; import {Vm} from "forge-std/Test.sol"; import {Utils, L2_BOOTLOADER_ADDRESS, L2_SYSTEM_CONTEXT_ADDRESS} from "../Utils/Utils.sol"; import {ExecutorTest} from "./_Executor_Shared.t.sol"; - +import {VerifierParams, FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZkSyncHyperchainStorage.sol"; import {IExecutor, MAX_NUMBER_OF_BLOBS, PubdataSource} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; 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 {L2_PUBDATA_CHUNK_PUBLISHER_ADDR, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR} from "contracts/common/L2ContractAddresses.sol"; import {IStateTransitionManager} from "contracts/state-transition/IStateTransitionManager.sol"; contract CommittingTest is ExecutorTest { @@ -52,9 +52,7 @@ contract CommittingTest is ExecutorTest { } function test_RevertWhen_wrongPubdataSource(uint8 pubdataSource) public { - if (pubdataSource == uint8(PubdataSource.Blob) || pubdataSource == uint8(PubdataSource.Calldata)) { - return; - } + vm.assume(pubdataSource != uint8(PubdataSource.Blob) && pubdataSource != uint8(PubdataSource.Calldata)); IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; wrongNewCommitBatchInfo.batchNumber = 1; @@ -150,6 +148,58 @@ contract CommittingTest is ExecutorTest { executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } + function test_RevertWhen_validiumDataNotEmpty() public { + FeeParams memory feeParams = defaultFeeParams(); + feeParams.pubdataPricingMode = PubdataPricingMode.Validium; + utils.util_setFeeParams(feeParams); + + bytes[] memory wrongL2Logs = Utils.createSystemLogs(); + bytes memory pubdataCommitment = "\x01"; + + wrongL2Logs[uint256(uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY))] = Utils.constructL2Log( + true, + L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, + uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY), + bytes32("") + ); + + IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(wrongL2Logs); + IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + + vm.prank(validator); + vm.expectRevert(bytes.concat("EF: v0l")); + executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); + + wrongL2Logs[uint256(uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY))] = Utils.constructL2Log( + true, + L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, + uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY), + bytes32("f") + ); + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(wrongL2Logs); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + vm.prank(validator); + vm.expectRevert(bytes.concat("v0h")); + executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); + + wrongL2Logs = Utils.createSystemLogs(); + wrongL2Logs[uint256(uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY))] = Utils.constructL2Log( + true, + L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, + uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY), + bytes32("") + ); + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(wrongL2Logs); + wrongNewCommitBatchInfo.pubdataCommitments = pubdataCommitment; + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + + vm.prank(validator); + vm.expectRevert(bytes.concat("tb")); + executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); + } + function test_RevertWhen_CommittingWithWrongPreviousBatchHash() public { bytes32 wrongPreviousBatchHash = Utils.randomBytes32("wrongPreviousBatchHash"); bytes[] memory wrongL2Logs = Utils.createSystemLogs(); 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 799d14a56..b8a837d6a 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 @@ -116,7 +116,7 @@ contract ExecutorTest is Test { return selectors; } - function defaultFeeParams() private pure returns (FeeParams memory feeParams) { + function defaultFeeParams() internal pure returns (FeeParams memory feeParams) { feeParams = FeeParams({ pubdataPricingMode: PubdataPricingMode.Rollup, batchOverheadL1Gas: 1_000_000, From 5523b079be66e6e4c70b967a41ab61314f9f59c7 Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Mon, 3 Jun 2024 13:03:39 +0200 Subject: [PATCH 05/12] test too long commitment pubdata --- .../unit/concrete/Executor/Committing.t.sol | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) 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 0b0df5536..30e8905ca 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol @@ -5,7 +5,7 @@ import {Vm} from "forge-std/Test.sol"; import {Utils, L2_BOOTLOADER_ADDRESS, L2_SYSTEM_CONTEXT_ADDRESS} from "../Utils/Utils.sol"; import {ExecutorTest} from "./_Executor_Shared.t.sol"; import {VerifierParams, FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZkSyncHyperchainStorage.sol"; -import {IExecutor, MAX_NUMBER_OF_BLOBS, PubdataSource} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; +import {IExecutor, MAX_NUMBER_OF_BLOBS, PubdataSource, BLOB_SIZE_BYTES} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; 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, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR} from "contracts/common/L2ContractAddresses.sol"; @@ -200,6 +200,21 @@ contract CommittingTest is ExecutorTest { executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } + function test_RevertWhen_tooLongPubdataCommitment() public { + IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; + wrongNewCommitBatchInfo.batchNumber = 1; + bytes memory pubdataCommitment = new bytes(BLOB_SIZE_BYTES + 1); + wrongNewCommitBatchInfo.pubdataCommitments = pubdataCommitment; + wrongNewCommitBatchInfo.pubdataCommitments[0] = bytes1(uint8(PubdataSource.Calldata)); + + IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + + vm.prank(validator); + vm.expectRevert(bytes.concat("cz")); + executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); + } + function test_RevertWhen_CommittingWithWrongPreviousBatchHash() public { bytes32 wrongPreviousBatchHash = Utils.randomBytes32("wrongPreviousBatchHash"); bytes[] memory wrongL2Logs = Utils.createSystemLogs(); From 7d6470800360070d6f7c5f34381647d80034abce Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Mon, 3 Jun 2024 17:32:44 +0200 Subject: [PATCH 06/12] test commit with wrong timestamp and pubdata hash --- .../unit/concrete/Executor/Committing.t.sol | 71 ++++++++++++++++++- .../foundry/unit/concrete/Utils/Utils.sol | 3 +- .../unit/concrete/Utils/UtilsFacet.sol | 4 ++ 3 files changed, 76 insertions(+), 2 deletions(-) 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 30e8905ca..6b1119f5c 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.24; import {Vm} from "forge-std/Test.sol"; -import {Utils, L2_BOOTLOADER_ADDRESS, L2_SYSTEM_CONTEXT_ADDRESS} from "../Utils/Utils.sol"; +import {Utils, L2_BOOTLOADER_ADDRESS, L2_SYSTEM_CONTEXT_ADDRESS, DEFAULT_L2_LOGS_TREE_ROOT_HASH} from "../Utils/Utils.sol"; import {ExecutorTest} from "./_Executor_Shared.t.sol"; import {VerifierParams, FeeParams, PubdataPricingMode} from "contracts/state-transition/chain-deps/ZkSyncHyperchainStorage.sol"; import {IExecutor, MAX_NUMBER_OF_BLOBS, PubdataSource, BLOB_SIZE_BYTES} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; @@ -125,6 +125,43 @@ contract CommittingTest is ExecutorTest { executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } + function test_RevertWhen_previousTimestampWrong() public { + IExecutor.StoredBatchInfo memory someStoredBatchInfo = IExecutor.StoredBatchInfo({ + batchNumber: 1, + batchHash: bytes32(""), + indexRepeatedStorageChanges: 0, + numberOfLayer1Txs: 0, + priorityOperationsHash: keccak256(""), + l2LogsTreeRoot: DEFAULT_L2_LOGS_TREE_ROOT_HASH, + timestamp: 101, + commitment: bytes32("") + }); + + bytes32 _storedBatchHash = keccak256(abi.encode(someStoredBatchInfo)); + utils.util_setStoredBatchHashes(1, _storedBatchHash); + utils.util_setTotalBatchesCommitted(1); + + bytes[] memory wrongL2Logs = Utils.createSystemLogs(); + wrongL2Logs[uint256(uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY))] = Utils.constructL2Log( + true, + L2_SYSTEM_CONTEXT_ADDRESS, + uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY), + Utils.packBatchTimestampAndBlockTimestamp(100, 100) + ); + + IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(wrongL2Logs); + wrongNewCommitBatchInfo.timestamp = uint64(100); + wrongNewCommitBatchInfo.batchNumber = 2; + + IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + + vm.prank(validator); + vm.expectRevert(bytes.concat("h3")); + executor.commitBatches(someStoredBatchInfo, wrongNewCommitBatchInfoArray); + } + function test_RevertWhen_CommittingTooBigLastL2BatchTimestamp() public { uint64 wrongNewBatchTimestamp = 0xffffffff; bytes[] memory wrongL2Logs = Utils.createSystemLogs(); @@ -215,6 +252,38 @@ contract CommittingTest is ExecutorTest { executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } + function hashSlice(bytes memory data, uint256 start, uint256 end) internal pure returns (bytes32) { + bytes memory slice = new bytes(end - start); + for (uint256 i = start; i < end; i++) { + slice[i - start] = data[i]; + } + return keccak256(slice); + } + + function test_RevertWhen_pubdataHashAndCommitmentMissmatch() public { + bytes[] memory wrongL2Logs = Utils.createSystemLogs(); + bytes memory pubdataCommitment = new bytes(64); + bytes32 pubdataCommitmentHash = hashSlice(pubdataCommitment, 1, pubdataCommitment.length - 32); + wrongL2Logs[uint256(uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY))] = Utils.constructL2Log( + true, + L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, + uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY), + bytes32("") + ); + IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; + wrongNewCommitBatchInfo.batchNumber = 1; + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(wrongL2Logs); + wrongNewCommitBatchInfo.pubdataCommitments = pubdataCommitment; + wrongNewCommitBatchInfo.pubdataCommitments[0] = bytes1(uint8(PubdataSource.Calldata)); + + IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + + vm.prank(validator); + vm.expectRevert(bytes.concat("wp")); + executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); + } + function test_RevertWhen_CommittingWithWrongPreviousBatchHash() public { bytes32 wrongPreviousBatchHash = Utils.randomBytes32("wrongPreviousBatchHash"); bytes[] memory wrongL2Logs = Utils.createSystemLogs(); diff --git a/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol b/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol index fc772b59f..d37a9889e 100644 --- a/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol +++ b/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol @@ -256,7 +256,7 @@ library Utils { } function getUtilsFacetSelectors() public pure returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](39); + bytes4[] memory selectors = new bytes4[](40); selectors[0] = UtilsFacet.util_setChainId.selector; selectors[1] = UtilsFacet.util_getChainId.selector; selectors[2] = UtilsFacet.util_setBridgehub.selector; @@ -296,6 +296,7 @@ library Utils { selectors[36] = UtilsFacet.util_setTransactionFilterer.selector; selectors[37] = UtilsFacet.util_setBaseTokenGasPriceMultiplierDenominator.selector; selectors[38] = UtilsFacet.util_setl2SystemContractsUpgradeTxHash.selector; + selectors[39] = UtilsFacet.util_setTotalBatchesCommitted.selector; return selectors; } diff --git a/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol b/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol index 26a23c0fd..5df50d4b0 100644 --- a/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol +++ b/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol @@ -166,6 +166,10 @@ contract UtilsFacet is ZkSyncHyperchainBase { s.l2SystemContractsUpgradeTxHash = _l2SystemContractsUpgradeTxHash; } + function util_setTotalBatchesCommitted(uint256 _totalBatchesCommitted) external { + s.totalBatchesCommitted = _totalBatchesCommitted; + } + // add this to be excluded from coverage report function test() internal virtual {} } From bce8056a936f622821ebe8b660f6e576cf27f2f1 Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Wed, 12 Jun 2024 13:38:13 +0200 Subject: [PATCH 07/12] commit with system upgrade --- .../unit/concrete/Executor/Committing.t.sol | 129 ++++++++++++++---- .../concrete/Executor/_Executor_Shared.t.sol | 34 +---- .../foundry/unit/concrete/Utils/Utils.sol | 8 +- .../unit/concrete/Utils/UtilsFacet.sol | 6 +- 4 files changed, 111 insertions(+), 66 deletions(-) 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 6b1119f5c..857176d2f 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol @@ -233,6 +233,7 @@ contract CommittingTest is ExecutorTest { wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; vm.prank(validator); + // it goes through to the next revert vm.expectRevert(bytes.concat("tb")); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } @@ -485,6 +486,7 @@ contract CommittingTest is ExecutorTest { for (uint256 i = 0; i < values.length; i++) { bytes[] memory wrongL2Logs = Utils.createSystemLogsWithUpgradeTransaction(bytes32("")); bytes[] memory tooManyLogs = new bytes[](15); + for (uint256 i = 0; i < wrongL2Logs.length; i++) { tooManyLogs[i] = wrongL2Logs[i]; } @@ -513,13 +515,14 @@ contract CommittingTest is ExecutorTest { wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(logsWithWrongHash); IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + vm.prank(validator); vm.expectRevert(bytes.concat("ut")); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } - function test_RevertWhen_SystemLogsBadLength() public { - utils.util_setl2SystemContractsUpgradeTxHash(bytes32("not default")); + function test_RevertWhen_systemLogsBadLength() public { + utils.util_setL2SystemContractsUpgradeTxHash(bytes32("not default")); bytes[] memory logs = Utils.createSystemLogsWithUpgradeTransaction(bytes32("not default")); delete logs[0]; @@ -534,19 +537,97 @@ contract CommittingTest is ExecutorTest { } function test_RevertWhen_SystemLogIsMissing() public { - for (uint256 i = 0; i < 7; i++) { - bytes[] memory l2Logs = Utils.createSystemLogs(); - delete l2Logs[i]; + bytes[] memory logs = Utils.createSystemLogs(); + delete logs[0]; - IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; - wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(l2Logs); - IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); - wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; + IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; + wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(logs); + IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; - vm.prank(validator); - vm.expectRevert(bytes.concat("b7")); - executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); - } + vm.prank(validator); + vm.expectRevert(bytes.concat("b7")); + executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); + } + + function test_successfullyCommitBatchWithSystemUpgrade() public { + // upgrade batch number must be zero to proceed + utils.util_setL2SystemContractsUpgradeBatchNumber(0); + // upgrade tx hash must be non zero to proceed + utils.util_setL2SystemContractsUpgradeTxHash(bytes32("not default")); + + // first new batch logs must contain upgrade transaction + // ugrade batch number will be batch number of this batch + bytes[] memory upgradeLogs = Utils.createSystemLogsWithUpgradeTransaction(bytes32("not default")); + uint64 upgradeBatchNumber = 1; + + upgradeLogs[uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY)] = Utils.constructL2Log( + true, + L2_SYSTEM_CONTEXT_ADDRESS, + uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY), + Utils.packBatchTimestampAndBlockTimestamp(currentTimestamp, currentTimestamp) + ); + upgradeLogs[uint256(SystemLogKey.BLOB_ONE_HASH_KEY)] = Utils.constructL2Log( + true, + L2_PUBDATA_CHUNK_PUBLISHER_ADDR, + uint256(SystemLogKey.BLOB_ONE_HASH_KEY), + 0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563 + ); + + IExecutor.CommitBatchInfo memory upgradeBatch = newCommitBatchInfo; + upgradeBatch.batchNumber = upgradeBatchNumber; + upgradeBatch.systemLogs = Utils.encodePacked(upgradeLogs); + upgradeBatch.pubdataCommitments = abi.encodePacked( + "\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00", + bytes32(uint256(0xbeef)) + ); + + bytes32[] memory blobHashes = new bytes32[](MAX_NUMBER_OF_BLOBS); + blobHashes[0] = 0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563; + + bytes32[] memory blobCommitments = new bytes32[](MAX_NUMBER_OF_BLOBS); + blobCommitments[0] = bytes32(uint256(0xbeef)); + + bytes32 expectedBatchCommitment = Utils.createBatchCommitment( + upgradeBatch, + bytes32(""), + blobCommitments, + blobHashes + ); + + IExecutor.CommitBatchInfo[] memory batchInfoArray = new IExecutor.CommitBatchInfo[](1); + batchInfoArray[0] = upgradeBatch; + + vm.prank(validator); + // solhint-disable-next-line func-named-parameters + vm.expectEmit(true, true, true, true, address(executor)); + emit BlockCommit(1, upgradeBatch.newStateRoot, expectedBatchCommitment); + executor.commitBatches(genesisStoredBatchInfo, batchInfoArray); + + assertEq(getters.getTotalBatchesCommitted(), 1); + assertEq(getters.getL2SystemContractsUpgradeBatchNumber(), upgradeBatchNumber); + assertEq(getters.getL2SystemContractsUpgradeTxHash(), bytes32("not default")); + + IExecutor.StoredBatchInfo memory lastBatch = IExecutor.StoredBatchInfo({ + batchNumber: 1, + batchHash: Utils.randomBytes32("newStateRoot"), + indexRepeatedStorageChanges: 0, + numberOfLayer1Txs: 0, + priorityOperationsHash: keccak256(""), + l2LogsTreeRoot: DEFAULT_L2_LOGS_TREE_ROOT_HASH, + timestamp: currentTimestamp, + commitment: expectedBatchCommitment + }); + + assertEq(utils.util_getStoredBatchHashes(1), keccak256(abi.encode(lastBatch))); + + batchInfoArray[0].batchNumber += 1; + + // this proves that upgrade batch number is not zero in second call + // and it doesn't go through branch with upgrade + vm.expectRevert(bytes.concat("ut")); + vm.prank(validator); + executor.commitBatches(lastBatch, batchInfoArray); } function test_SuccessfullyCommitBatch() public { @@ -588,21 +669,12 @@ contract CommittingTest is ExecutorTest { correctCommitBatchInfoArray[0] = correctNewCommitBatchInfo; vm.prank(validator); - - vm.recordLogs(); + // solhint-disable-next-line func-named-parameters + vm.expectEmit(true, true, true, true, address(executor)); + emit BlockCommit(1, correctNewCommitBatchInfo.newStateRoot, expectedBatchCommitment); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); - - Vm.Log[] memory entries = vm.getRecordedLogs(); - - assertEq(entries.length, 1); - assertEq(entries[0].topics[0], keccak256("BlockCommit(uint256,bytes32,bytes32)")); - assertEq(entries[0].topics[1], bytes32(uint256(1))); // batchNumber - assertEq(entries[0].topics[2], correctNewCommitBatchInfo.newStateRoot); // batchHash - assertEq(entries[0].topics[3], expectedBatchCommitment); // commitment - - uint256 totalBatchesCommitted = getters.getTotalBatchesCommitted(); - assertEq(totalBatchesCommitted, 1); + assertEq(getters.getTotalBatchesCommitted(), 1); } function test_SuccessfullyCommitBatchWithOneBlob() public { @@ -709,13 +781,12 @@ contract CommittingTest is ExecutorTest { correctCommitBatchInfoArray[0].pubdataCommitments = pubdataCommitment; vm.prank(validator); + // solhint-disable-next-line func-named-parameters vm.expectEmit(true, false, false, false, address(executor)); emit BlockCommit(1, bytes32(0), bytes32(0)); executor.commitBatches(genesisStoredBatchInfo, correctCommitBatchInfoArray); - uint256 totalBatchesCommitted = getters.getTotalBatchesCommitted(); - assertEq(totalBatchesCommitted, 1); - + assertEq(getters.getTotalBatchesCommitted(), 1); vm.clearMockedCalls(); } 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 b8a837d6a..46e97362e 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 @@ -72,38 +72,6 @@ contract ExecutorTest is Test { return selectors; } - function getGettersSelectors() public view returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](28); - selectors[0] = getters.getVerifier.selector; - selectors[1] = getters.getAdmin.selector; - selectors[2] = getters.getPendingAdmin.selector; - selectors[3] = getters.getTotalBlocksCommitted.selector; - selectors[4] = getters.getTotalBlocksVerified.selector; - selectors[5] = getters.getTotalBlocksExecuted.selector; - selectors[6] = getters.getTotalPriorityTxs.selector; - selectors[7] = getters.getFirstUnprocessedPriorityTx.selector; - selectors[8] = getters.getPriorityQueueSize.selector; - selectors[9] = getters.priorityQueueFrontOperation.selector; - selectors[10] = getters.isValidator.selector; - selectors[11] = getters.l2LogsRootHash.selector; - selectors[12] = getters.storedBatchHash.selector; - selectors[13] = getters.getL2BootloaderBytecodeHash.selector; - selectors[14] = getters.getL2DefaultAccountBytecodeHash.selector; - selectors[15] = getters.getVerifierParams.selector; - selectors[16] = getters.isDiamondStorageFrozen.selector; - selectors[17] = getters.getPriorityTxMaxGasLimit.selector; - selectors[18] = getters.isEthWithdrawalFinalized.selector; - selectors[19] = getters.facets.selector; - selectors[20] = getters.facetFunctionSelectors.selector; - selectors[21] = getters.facetAddresses.selector; - selectors[22] = getters.facetAddress.selector; - selectors[23] = getters.isFunctionFreezable.selector; - selectors[24] = getters.isFacetFreezable.selector; - selectors[25] = getters.getTotalBatchesCommitted.selector; - selectors[26] = getters.getTotalBatchesVerified.selector; - selectors[27] = getters.getTotalBatchesExecuted.selector; - return selectors; - } function getMailboxSelectors() private view returns (bytes4[] memory) { bytes4[] memory selectors = new bytes4[](6); @@ -208,7 +176,7 @@ contract ExecutorTest is Test { facet: address(getters), action: Diamond.Action.Add, isFreezable: false, - selectors: getGettersSelectors() + selectors: Utils.getGettersSelectors() }); facetCuts[3] = Diamond.FacetCut({ facet: address(mailbox), diff --git a/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol b/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol index d37a9889e..7efb79073 100644 --- a/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol +++ b/l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol @@ -210,7 +210,7 @@ library Utils { } function getGettersSelectors() public pure returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](29); + bytes4[] memory selectors = new bytes4[](30); selectors[0] = GettersFacet.getVerifier.selector; selectors[1] = GettersFacet.getAdmin.selector; selectors[2] = GettersFacet.getPendingAdmin.selector; @@ -240,6 +240,7 @@ library Utils { selectors[26] = GettersFacet.getTotalBatchesVerified.selector; selectors[27] = GettersFacet.getTotalBatchesExecuted.selector; selectors[28] = GettersFacet.getL2SystemContractsUpgradeTxHash.selector; + selectors[29] = GettersFacet.getL2SystemContractsUpgradeBatchNumber.selector; return selectors; } @@ -256,7 +257,7 @@ library Utils { } function getUtilsFacetSelectors() public pure returns (bytes4[] memory) { - bytes4[] memory selectors = new bytes4[](40); + bytes4[] memory selectors = new bytes4[](41); selectors[0] = UtilsFacet.util_setChainId.selector; selectors[1] = UtilsFacet.util_getChainId.selector; selectors[2] = UtilsFacet.util_setBridgehub.selector; @@ -295,8 +296,9 @@ library Utils { selectors[35] = UtilsFacet.util_getIsFrozen.selector; selectors[36] = UtilsFacet.util_setTransactionFilterer.selector; selectors[37] = UtilsFacet.util_setBaseTokenGasPriceMultiplierDenominator.selector; - selectors[38] = UtilsFacet.util_setl2SystemContractsUpgradeTxHash.selector; + selectors[38] = UtilsFacet.util_setL2SystemContractsUpgradeTxHash.selector; selectors[39] = UtilsFacet.util_setTotalBatchesCommitted.selector; + selectors[40] = UtilsFacet.util_setL2SystemContractsUpgradeBatchNumber.selector; return selectors; } diff --git a/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol b/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol index 5df50d4b0..10a54ad48 100644 --- a/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol +++ b/l1-contracts/test/foundry/unit/concrete/Utils/UtilsFacet.sol @@ -162,7 +162,7 @@ contract UtilsFacet is ZkSyncHyperchainBase { return s.isFrozen; } - function util_setl2SystemContractsUpgradeTxHash(bytes32 _l2SystemContractsUpgradeTxHash) external { + function util_setL2SystemContractsUpgradeTxHash(bytes32 _l2SystemContractsUpgradeTxHash) external { s.l2SystemContractsUpgradeTxHash = _l2SystemContractsUpgradeTxHash; } @@ -170,6 +170,10 @@ contract UtilsFacet is ZkSyncHyperchainBase { s.totalBatchesCommitted = _totalBatchesCommitted; } + function util_setL2SystemContractsUpgradeBatchNumber(uint256 _l2SystemContractsUpgradeBatchNumber) external { + s.l2SystemContractsUpgradeBatchNumber = _l2SystemContractsUpgradeBatchNumber; + } + // add this to be excluded from coverage report function test() internal virtual {} } From 232e0d23c87e3e0955b39fe59f594c43460b2b70 Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Thu, 13 Jun 2024 13:57:29 +0200 Subject: [PATCH 08/12] test executing multiple batches consider upgrade --- .../unit/concrete/Executor/Executing.t.sol | 133 +++++++++++++++++- .../concrete/Executor/_Executor_Shared.t.sol | 1 + 2 files changed, 132 insertions(+), 2 deletions(-) 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..08f18792e 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Executing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Executing.t.sol @@ -54,6 +54,74 @@ contract ExecutingTest is ExecutorTest { executor.proveBatches(genesisStoredBatchInfo, storedBatchInfoArray, proofInput); } + function _commitAndProveMultipleBatches( + uint256 _count, + uint256 _startBatchNumber + ) internal returns (IExecutor.StoredBatchInfo[] memory storedBatchInfos) { + uint256 timestamp = currentTimestamp + 1; + + IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](_count); + IExecutor.StoredBatchInfo memory lastCommited = newStoredBatchInfo; + IExecutor.StoredBatchInfo memory lastProved = newStoredBatchInfo; + + for (uint256 i = 0; i < _count; i++) { + uint256 batchTimestamp = timestamp + i; + bytes[] memory correctL2Logs = Utils.createSystemLogs(); + + correctL2Logs[uint256(uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY))] = Utils + .constructL2Log( + true, + L2_SYSTEM_CONTEXT_ADDRESS, + uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY), + Utils.packBatchTimestampAndBlockTimestamp(batchTimestamp, batchTimestamp) + ); + + correctL2Logs[uint256(uint256(SystemLogKey.PREV_BATCH_HASH_KEY))] = Utils.constructL2Log( + true, + L2_SYSTEM_CONTEXT_ADDRESS, + uint256(SystemLogKey.PREV_BATCH_HASH_KEY), + lastCommited.batchHash + ); + + bytes memory l2Logs = Utils.encodePacked(correctL2Logs); + + IExecutor.CommitBatchInfo memory commitBatch = newCommitBatchInfo; + commitBatch.systemLogs = l2Logs; + commitBatch.timestamp = uint64(batchTimestamp); + commitBatch.batchNumber = uint64(_startBatchNumber + i); + + IExecutor.CommitBatchInfo[] memory commitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + commitBatchInfoArray[0] = commitBatch; + + vm.recordLogs(); + vm.prank(validator); + executor.commitBatches(lastCommited, commitBatchInfoArray); + + Vm.Log[] memory entries = vm.getRecordedLogs(); + lastCommited = IExecutor.StoredBatchInfo({ + batchNumber: uint64(_startBatchNumber + i), + batchHash: entries[0].topics[2], + indexRepeatedStorageChanges: 0, + numberOfLayer1Txs: 0, + priorityOperationsHash: keccak256(""), + l2LogsTreeRoot: 0, + timestamp: batchTimestamp, + commitment: entries[0].topics[3] + }); + + IExecutor.StoredBatchInfo[] memory toProve = new IExecutor.StoredBatchInfo[](1); + toProve[0] = lastCommited; + + vm.prank(validator); + executor.proveBatches(lastProved, toProve, proofInput); + + storedBatchInfoArray[i] = lastCommited; + lastProved = lastCommited; + } + + return storedBatchInfoArray; + } + function test_RevertWhen_ExecutingBlockWithWrongBatchNumber() public { IExecutor.StoredBatchInfo memory wrongNewStoredBatchInfo = newStoredBatchInfo; wrongNewStoredBatchInfo.batchNumber = 10; // Correct is 1 @@ -254,10 +322,71 @@ contract ExecutingTest is ExecutorTest { IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](1); storedBatchInfoArray[0] = newStoredBatchInfo; + // solhint-disable-next-line func-named-parameters + vm.expectEmit(true, true, true, true, address(executor)); + emit BlockExecution( + newStoredBatchInfo.batchNumber, + newStoredBatchInfo.batchHash, + newStoredBatchInfo.commitment + ); + + vm.prank(validator); + executor.executeBatches(storedBatchInfoArray); + + assertEq(getters.getTotalBlocksExecuted(), 1); + assertEq(getters.l2LogsRootHash(newStoredBatchInfo.batchNumber), newStoredBatchInfo.l2LogsTreeRoot); + } + + function test_shouldDeleteSystemUpgradeData(uint256 batchNumberBefore) public { + IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](4); + IExecutor.StoredBatchInfo[] memory newbatches = _commitAndProveMultipleBatches(3, 2); + storedBatchInfoArray[0] = newStoredBatchInfo; + storedBatchInfoArray[1] = newbatches[0]; + storedBatchInfoArray[2] = newbatches[1]; + storedBatchInfoArray[3] = newbatches[2]; + + batchNumberBefore = bound(batchNumberBefore, 1, newbatches[2].batchNumber); + utils.util_setL2SystemContractsUpgradeBatchNumber(1); + utils.util_setL2SystemContractsUpgradeTxHash(bytes32("upgrade hash")); + + vm.prank(validator); + executor.executeBatches(storedBatchInfoArray); + + assertEq(getters.getTotalBlocksExecuted(), 4); + assertEq(getters.getL2SystemContractsUpgradeBatchNumber(), 0); + assertEq(getters.getL2SystemContractsUpgradeTxHash(), bytes32(0)); + assertEq(getters.l2LogsRootHash(newbatches[2].batchNumber), newbatches[2].l2LogsTreeRoot); + } + + function test_shouldNotDeleteSystemUpgradeData(uint256 batchNumberAfter) public { + IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](4); + IExecutor.StoredBatchInfo[] memory newbatches = _commitAndProveMultipleBatches(3, 2); + storedBatchInfoArray[0] = newStoredBatchInfo; + storedBatchInfoArray[1] = newbatches[0]; + storedBatchInfoArray[2] = newbatches[1]; + storedBatchInfoArray[3] = newbatches[2]; + + batchNumberAfter = bound(batchNumberAfter, newbatches[2].batchNumber + 1, type(uint256).max); + utils.util_setL2SystemContractsUpgradeBatchNumber(batchNumberAfter); + utils.util_setL2SystemContractsUpgradeTxHash(bytes32("upgrade hash")); + vm.prank(validator); executor.executeBatches(storedBatchInfoArray); - uint256 totalBlocksExecuted = getters.getTotalBlocksExecuted(); - assertEq(totalBlocksExecuted, 1); + assertEq(getters.getTotalBlocksExecuted(), 4); + assertEq(getters.getL2SystemContractsUpgradeBatchNumber(), batchNumberAfter); + assertEq(getters.getL2SystemContractsUpgradeTxHash(), bytes32("upgrade hash")); + assertEq(getters.l2LogsRootHash(newbatches[2].batchNumber), newbatches[2].l2LogsTreeRoot); + } + + function test_shouldBeCalledOnlyByValidator(address caller) public { + vm.assume(caller != validator); + + IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](1); + storedBatchInfoArray[0] = newStoredBatchInfo; + + vm.expectRevert("Hyperchain: not validator"); + vm.prank(caller); + executor.executeBatches(storedBatchInfoArray); } } 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 46e97362e..a8c050d4a 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 @@ -46,6 +46,7 @@ contract ExecutorTest is Test { IExecutor.ProofInput internal proofInput; event BlockCommit(uint256 indexed batchNumber, bytes32 indexed batchHash, bytes32 indexed commitment); + event BlockExecution(uint256 indexed batchNumber, bytes32 indexed batchHash, bytes32 indexed commitment); function getAdminSelectors() private view returns (bytes4[] memory) { bytes4[] memory selectors = new bytes4[](11); From baeb80ebecb2c3c101df5b714e9071c000569b23 Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Fri, 14 Jun 2024 12:59:31 +0200 Subject: [PATCH 09/12] executor tests reverts in proving --- .../unit/concrete/Executor/Proving.t.sol | 112 +++++++++++++++++- 1 file changed, 108 insertions(+), 4 deletions(-) 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..12d0c4384 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Proving.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Proving.t.sol @@ -6,8 +6,9 @@ import {Utils, L2_SYSTEM_CONTEXT_ADDRESS} from "../Utils/Utils.sol"; import {ExecutorTest} from "./_Executor_Shared.t.sol"; -import {COMMIT_TIMESTAMP_NOT_OLDER} from "contracts/common/Config.sol"; +import {COMMIT_TIMESTAMP_NOT_OLDER, PUBLIC_INPUT_SHIFT} from "contracts/common/Config.sol"; import {IExecutor, SystemLogKey} from "contracts/state-transition/chain-interfaces/IExecutor.sol"; +import {IVerifier} from "contracts/state-transition/chain-interfaces/IVerifier.sol"; contract ProvingTest is ExecutorTest { function setUp() public { @@ -47,6 +48,73 @@ contract ProvingTest is ExecutorTest { }); } + function _commitMultipleBatches( + uint256 _count, + uint256 _startBatchNumber + ) internal returns (IExecutor.StoredBatchInfo[] memory storedBatchInfos) { + uint256 timestamp = currentTimestamp + 1; + + IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](_count); + IExecutor.StoredBatchInfo memory lastCommited = newStoredBatchInfo; + + for (uint256 i = 0; i < _count; i++) { + uint256 batchTimestamp = timestamp + i; + bytes[] memory correctL2Logs = Utils.createSystemLogs(); + + correctL2Logs[uint256(uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY))] = Utils + .constructL2Log( + true, + L2_SYSTEM_CONTEXT_ADDRESS, + uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY), + Utils.packBatchTimestampAndBlockTimestamp(batchTimestamp, batchTimestamp) + ); + + correctL2Logs[uint256(uint256(SystemLogKey.PREV_BATCH_HASH_KEY))] = Utils.constructL2Log( + true, + L2_SYSTEM_CONTEXT_ADDRESS, + uint256(SystemLogKey.PREV_BATCH_HASH_KEY), + lastCommited.batchHash + ); + + bytes memory l2Logs = Utils.encodePacked(correctL2Logs); + + IExecutor.CommitBatchInfo memory commitBatch = newCommitBatchInfo; + commitBatch.systemLogs = l2Logs; + commitBatch.timestamp = uint64(batchTimestamp); + commitBatch.batchNumber = uint64(_startBatchNumber + i); + + IExecutor.CommitBatchInfo[] memory commitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); + commitBatchInfoArray[0] = commitBatch; + + vm.recordLogs(); + vm.prank(validator); + executor.commitBatches(lastCommited, commitBatchInfoArray); + + Vm.Log[] memory entries = vm.getRecordedLogs(); + lastCommited = IExecutor.StoredBatchInfo({ + batchNumber: uint64(_startBatchNumber + i), + batchHash: entries[0].topics[2], + indexRepeatedStorageChanges: 0, + numberOfLayer1Txs: 0, + priorityOperationsHash: keccak256(""), + l2LogsTreeRoot: 0, + timestamp: batchTimestamp, + commitment: entries[0].topics[3] + }); + + storedBatchInfoArray[i] = lastCommited; + } + + return storedBatchInfoArray; + } + + function test_RevertWhen_wrongCaller(address _caller) public { + vm.assume(_caller != validator); + + vm.expectRevert("Hyperchain: not validator"); + executor.proveBatches(genesisStoredBatchInfo, new IExecutor.StoredBatchInfo[](0), proofInput); + } + function test_RevertWhen_ProvingWithWrongPreviousBlockData() public { IExecutor.StoredBatchInfo memory wrongPreviousStoredBatchInfo = genesisStoredBatchInfo; wrongPreviousStoredBatchInfo.batchNumber = 10; // Correct is 0 @@ -86,15 +154,51 @@ contract ProvingTest is ExecutorTest { executor.proveBatches(genesisStoredBatchInfo, storedBatchInfoArray, proofInput); } + function test_RevertWhen_proveMultipleBatches() public { + IExecutor.StoredBatchInfo[] memory storedBatchInfos = _commitMultipleBatches(2, 2); + IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](3); + storedBatchInfoArray[0] = newStoredBatchInfo; + storedBatchInfoArray[1] = storedBatchInfos[0]; + storedBatchInfoArray[2] = storedBatchInfos[1]; + + vm.prank(validator); + vm.expectRevert(bytes.concat("t4")); + executor.proveBatches(genesisStoredBatchInfo, storedBatchInfoArray, proofInput); + } + + function test_RevertWhen_verifyFailed() public { + IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](1); + storedBatchInfoArray[0] = newStoredBatchInfo; + + vm.prank(validator); + uint256[] memory proofPublicInput = new uint256[](1); + proofPublicInput[0] = + uint256(keccak256(abi.encodePacked(genesisStoredBatchInfo.commitment, newStoredBatchInfo.commitment))) >> + PUBLIC_INPUT_SHIFT; + + vm.mockCall( + testnetVerifierAddress, + abi.encodeWithSelector( + IVerifier.verify.selector, + proofPublicInput, + proofInput.serializedProof, + proofInput.recursiveAggregationInput + ), + abi.encode(false) + ); + vm.expectRevert(bytes.concat("p")); + executor.proveBatches(genesisStoredBatchInfo, storedBatchInfoArray, proofInput); + } + function test_SuccessfulProve() public { IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](1); storedBatchInfoArray[0] = newStoredBatchInfo; vm.prank(validator); + vm.expectEmit(true, false, false, false, address(executor)); + emit BlocksVerification({previousLastVerifiedBatch: 0, currentLastVerifiedBatch: 1}); executor.proveBatches(genesisStoredBatchInfo, storedBatchInfoArray, proofInput); - - uint256 totalBlocksVerified = getters.getTotalBlocksVerified(); - assertEq(totalBlocksVerified, 1); + assertEq(getters.getTotalBlocksVerified(), 1); } } From 0f54bb712f98fc0795cd53a6a4f2f481b6a0becb Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Fri, 14 Jun 2024 13:00:05 +0200 Subject: [PATCH 10/12] executor tests: remove revert after changes in contract --- .../unit/concrete/Executor/Committing.t.sol | 39 +------------------ .../concrete/Executor/_Executor_Shared.t.sol | 3 ++ 2 files changed, 4 insertions(+), 38 deletions(-) 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 857176d2f..f63bc3e67 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol @@ -185,21 +185,12 @@ contract CommittingTest is ExecutorTest { executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } - function test_RevertWhen_validiumDataNotEmpty() public { + function test_RevertWhen_validiumDataWrongCommitmentsLength() public { FeeParams memory feeParams = defaultFeeParams(); feeParams.pubdataPricingMode = PubdataPricingMode.Validium; utils.util_setFeeParams(feeParams); bytes[] memory wrongL2Logs = Utils.createSystemLogs(); - bytes memory pubdataCommitment = "\x01"; - - wrongL2Logs[uint256(uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY))] = Utils.constructL2Log( - true, - L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, - uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY), - bytes32("") - ); - IExecutor.CommitBatchInfo memory wrongNewCommitBatchInfo = newCommitBatchInfo; wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(wrongL2Logs); IExecutor.CommitBatchInfo[] memory wrongNewCommitBatchInfoArray = new IExecutor.CommitBatchInfo[](1); @@ -208,34 +199,6 @@ contract CommittingTest is ExecutorTest { vm.prank(validator); vm.expectRevert(bytes.concat("EF: v0l")); executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); - - wrongL2Logs[uint256(uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY))] = Utils.constructL2Log( - true, - L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, - uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY), - bytes32("f") - ); - wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(wrongL2Logs); - wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; - vm.prank(validator); - vm.expectRevert(bytes.concat("v0h")); - executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); - - wrongL2Logs = Utils.createSystemLogs(); - wrongL2Logs[uint256(uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY))] = Utils.constructL2Log( - true, - L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, - uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY), - bytes32("") - ); - wrongNewCommitBatchInfo.systemLogs = Utils.encodePacked(wrongL2Logs); - wrongNewCommitBatchInfo.pubdataCommitments = pubdataCommitment; - wrongNewCommitBatchInfoArray[0] = wrongNewCommitBatchInfo; - - vm.prank(validator); - // it goes through to the next revert - vm.expectRevert(bytes.concat("tb")); - executor.commitBatches(genesisStoredBatchInfo, wrongNewCommitBatchInfoArray); } function test_RevertWhen_tooLongPubdataCommitment() public { 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 a8c050d4a..b88fda83b 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 @@ -29,6 +29,7 @@ contract ExecutorTest is Test { address internal validator; address internal randomSigner; address internal blobVersionedHashRetriever; + address internal testnetVerifierAddress; AdminFacet internal admin; TestExecutor internal executor; GettersFacet internal getters; @@ -47,6 +48,7 @@ contract ExecutorTest is Test { event BlockCommit(uint256 indexed batchNumber, bytes32 indexed batchHash, bytes32 indexed commitment); event BlockExecution(uint256 indexed batchNumber, bytes32 indexed batchHash, bytes32 indexed commitment); + event BlocksVerification(uint256 indexed previousLastVerifiedBatch, uint256 indexed currentLastVerifiedBatch); function getAdminSelectors() private view returns (bytes4[] memory) { bytes4[] memory selectors = new bytes4[](11); @@ -133,6 +135,7 @@ contract ExecutorTest is Test { }); TestnetVerifier testnetVerifier = new TestnetVerifier(); + testnetVerifierAddress = address(testnetVerifier); InitializeData memory params = InitializeData({ // TODO REVIEW From 21c46f6af988046f503a48790b01a476604b0727 Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Fri, 14 Jun 2024 13:04:52 +0200 Subject: [PATCH 11/12] lint fixes --- .../foundry/unit/concrete/Executor/Executing.t.sol | 14 +++++++------- .../foundry/unit/concrete/Executor/Proving.t.sol | 10 +++++----- 2 files changed, 12 insertions(+), 12 deletions(-) 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 08f18792e..3889acbab 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Executing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Executing.t.sol @@ -61,7 +61,7 @@ contract ExecutingTest is ExecutorTest { uint256 timestamp = currentTimestamp + 1; IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](_count); - IExecutor.StoredBatchInfo memory lastCommited = newStoredBatchInfo; + IExecutor.StoredBatchInfo memory lastCommitted = newStoredBatchInfo; IExecutor.StoredBatchInfo memory lastProved = newStoredBatchInfo; for (uint256 i = 0; i < _count; i++) { @@ -80,7 +80,7 @@ contract ExecutingTest is ExecutorTest { true, L2_SYSTEM_CONTEXT_ADDRESS, uint256(SystemLogKey.PREV_BATCH_HASH_KEY), - lastCommited.batchHash + lastCommitted.batchHash ); bytes memory l2Logs = Utils.encodePacked(correctL2Logs); @@ -95,10 +95,10 @@ contract ExecutingTest is ExecutorTest { vm.recordLogs(); vm.prank(validator); - executor.commitBatches(lastCommited, commitBatchInfoArray); + executor.commitBatches(lastCommitted, commitBatchInfoArray); Vm.Log[] memory entries = vm.getRecordedLogs(); - lastCommited = IExecutor.StoredBatchInfo({ + lastCommitted = IExecutor.StoredBatchInfo({ batchNumber: uint64(_startBatchNumber + i), batchHash: entries[0].topics[2], indexRepeatedStorageChanges: 0, @@ -110,13 +110,13 @@ contract ExecutingTest is ExecutorTest { }); IExecutor.StoredBatchInfo[] memory toProve = new IExecutor.StoredBatchInfo[](1); - toProve[0] = lastCommited; + toProve[0] = lastCommitted; vm.prank(validator); executor.proveBatches(lastProved, toProve, proofInput); - storedBatchInfoArray[i] = lastCommited; - lastProved = lastCommited; + storedBatchInfoArray[i] = lastCommitted; + lastProved = lastCommitted; } return storedBatchInfoArray; 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 12d0c4384..1477a21fa 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Proving.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Proving.t.sol @@ -55,7 +55,7 @@ contract ProvingTest is ExecutorTest { uint256 timestamp = currentTimestamp + 1; IExecutor.StoredBatchInfo[] memory storedBatchInfoArray = new IExecutor.StoredBatchInfo[](_count); - IExecutor.StoredBatchInfo memory lastCommited = newStoredBatchInfo; + IExecutor.StoredBatchInfo memory lastCommitted = newStoredBatchInfo; for (uint256 i = 0; i < _count; i++) { uint256 batchTimestamp = timestamp + i; @@ -73,7 +73,7 @@ contract ProvingTest is ExecutorTest { true, L2_SYSTEM_CONTEXT_ADDRESS, uint256(SystemLogKey.PREV_BATCH_HASH_KEY), - lastCommited.batchHash + lastCommitted.batchHash ); bytes memory l2Logs = Utils.encodePacked(correctL2Logs); @@ -88,10 +88,10 @@ contract ProvingTest is ExecutorTest { vm.recordLogs(); vm.prank(validator); - executor.commitBatches(lastCommited, commitBatchInfoArray); + executor.commitBatches(lastCommitted, commitBatchInfoArray); Vm.Log[] memory entries = vm.getRecordedLogs(); - lastCommited = IExecutor.StoredBatchInfo({ + lastCommitted = IExecutor.StoredBatchInfo({ batchNumber: uint64(_startBatchNumber + i), batchHash: entries[0].topics[2], indexRepeatedStorageChanges: 0, @@ -102,7 +102,7 @@ contract ProvingTest is ExecutorTest { commitment: entries[0].topics[3] }); - storedBatchInfoArray[i] = lastCommited; + storedBatchInfoArray[i] = lastCommitted; } return storedBatchInfoArray; From 695613d3ab67af4c68c07161be5bc0db66f0383e Mon Sep 17 00:00:00 2001 From: tommysr <47206288+tommysr@users.noreply.github.com> Date: Fri, 14 Jun 2024 13:09:01 +0200 Subject: [PATCH 12/12] prettier fix some lints --- .../test/foundry/unit/concrete/Executor/Committing.t.sol | 2 +- .../test/foundry/unit/concrete/Executor/_Executor_Shared.t.sol | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) 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 f63bc3e67..b1b51d7d8 100644 --- a/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol @@ -224,7 +224,7 @@ contract CommittingTest is ExecutorTest { return keccak256(slice); } - function test_RevertWhen_pubdataHashAndCommitmentMissmatch() public { + function test_RevertWhen_pubdataHashAndCommitmentMismatch() public { bytes[] memory wrongL2Logs = Utils.createSystemLogs(); bytes memory pubdataCommitment = new bytes(64); bytes32 pubdataCommitmentHash = hashSlice(pubdataCommitment, 1, pubdataCommitment.length - 32); 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 b88fda83b..10fa5ddc3 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 @@ -75,7 +75,6 @@ contract ExecutorTest is Test { return selectors; } - function getMailboxSelectors() private view returns (bytes4[] memory) { bytes4[] memory selectors = new bytes4[](6); selectors[0] = mailbox.proveL2MessageInclusion.selector;