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

Resolve some of the leftover todos #765

Merged
merged 18 commits into from
Sep 5, 2024
Merged
Show file tree
Hide file tree
Changes from 17 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
4 changes: 2 additions & 2 deletions docs/Overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,12 @@ Each L2 -> L1 system log will have a key that is part of the following:
```solidity
enum SystemLogKey {
L2_TO_L1_LOGS_TREE_ROOT_KEY,
TOTAL_L2_TO_L1_PUBDATA_KEY,
STATE_DIFF_HASH_KEY,
PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY,
PREV_BATCH_HASH_KEY,
CHAINED_PRIORITY_TXN_HASH_KEY,
NUMBER_OF_LAYER_1_TXS_KEY,
L2_DA_VALIDATOR_OUTPUT_HASH_KEY,
USED_L2_DA_VALIDATOR_ADDRESS_KEY,
EXPECTED_SYSTEM_CONTRACT_UPGRADE_TX_HASH_KEY
}
```
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,12 @@ contract AdminFacet is ZkSyncHyperchainBase, IAdmin {

require(currentProtocolVersion == protocolVersion, "STM: protocolVersion not up to date");

if (block.chainid != L1_CHAIN_ID) {
// We assume that GW -> L1 transactions can never fail and provide no recovery mechanism from it.
// That's why we need to bound the gas that can be consumed during such a migration.
require(s.totalBatchesCommitted == s.totalBatchesExecuted, "Af: not all batches executed");
}

s.settlementLayer = _settlementLayer;
chainBridgeMintData = abi.encode(prepareChainCommitment());
}
Expand Down Expand Up @@ -391,37 +397,4 @@ contract AdminFacet is ZkSyncHyperchainBase, IAdmin {

commitment.batchHashes = batchHashes;
}

// function recoverFromFailedMigrationToGateway(
// uint256 _settlementLayerChainId,
// uint256 _l2BatchNumber,
// uint256 _l2MessageIndex,
// uint16 _l2TxNumberInBatch,
// bytes32[] calldata _merkleProof
// ) external onlyAdmin {
// require(s.settlementLayerState == SettlementLayerState.MigratedFromL1, "not migrated L1");

// bytes32 migrationHash = s.settlementLayerMigrationHash;
// require(migrationHash != bytes32(0), "can not recover when there is no migration");

// require(
// IBridgehub(s.bridgehub).proveL1ToL2TransactionStatus(
// _settlementLayerChainId,
// migrationHash,
// _l2BatchNumber,
// _l2MessageIndex,
// _l2TxNumberInBatch,
// _merkleProof,
// TxStatus.Failure
// ),
// "Migration not failed"
// );

// s.settlementLayerState = SettlementLayerState.ActiveOnL1;
// s.settlementLayerChainId = 0;
// s.settlementLayerMigrationHash = bytes32(0);

// // We do not need to perform any additional actions, since no changes related to the chain commitment can be performed
// // while the chain is in the "migrated" state.
// }
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {L2_BOOTLOADER_ADDRESS, L2_TO_L1_MESSENGER_SYSTEM_CONTRACT_ADDR, L2_SYSTE
import {IStateTransitionManager} from "../../IStateTransitionManager.sol";
import {PriorityTree, PriorityOpsBatchInfo} from "../../libraries/PriorityTree.sol";
import {IL1DAValidator, L1DAValidatorOutput} from "../../chain-interfaces/IL1DAValidator.sol";
import {BatchNumberMismatch, TimeNotReached, ValueMismatch, HashMismatch, NonIncreasingTimestamp, TimestampError, InvalidLogSender, TxHashMismatch, UnexpectedSystemLog, LogAlreadyProcessed, InvalidProtocolVersion, CanOnlyProcessOneBatch, BatchHashMismatch, UpgradeBatchNumberIsNotZero, NonSequentialBatch, CantExecuteUnprovenBatches, SystemLogsSizeTooBig, InvalidNumberOfBlobs, VerifiedBatchesExceedsCommittedBatches, InvalidProof, RevertedBatchNotAfterNewLastBatch, CantRevertExecutedBatch, L2TimestampTooBig, PriorityOperationsRollingHashMismatch} from "../../../common/L1ContractErrors.sol";
import {MissingSystemLogs, BatchNumberMismatch, TimeNotReached, ValueMismatch, HashMismatch, NonIncreasingTimestamp, TimestampError, InvalidLogSender, TxHashMismatch, UnexpectedSystemLog, LogAlreadyProcessed, InvalidProtocolVersion, CanOnlyProcessOneBatch, BatchHashMismatch, UpgradeBatchNumberIsNotZero, NonSequentialBatch, CantExecuteUnprovenBatches, SystemLogsSizeTooBig, InvalidNumberOfBlobs, VerifiedBatchesExceedsCommittedBatches, InvalidProof, RevertedBatchNotAfterNewLastBatch, CantRevertExecutedBatch, L2TimestampTooBig, PriorityOperationsRollingHashMismatch} 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 @@ -216,16 +216,15 @@ contract ExecutorFacet is ZkSyncHyperchainBase, IExecutor {
}
}

// FIXME: temporarily old logs were kept for backwards compatibility. This check cannot work now.
//
// We only require 8 logs to be checked, the 9th is if we are expecting a protocol upgrade
// Without the protocol upgrade we expect 8 logs: 2^8 - 1 = 255
// With the protocol upgrade we expect 9 logs: 2^9 - 1 = 511
if (_expectedSystemContractUpgradeTxHash == bytes32(0)) {
// require(processedLogs == 255, "b7");
} else {
// FIXME: do restore this code to the one that was before
require(_checkBit(processedLogs, uint8(SystemLogKey.EXPECTED_SYSTEM_CONTRACT_UPGRADE_TX_HASH_KEY)), "b8");
if (processedLogs != 127) {
revert MissingSystemLogs(127, processedLogs);
}
} else if (processedLogs != 255) {
revert MissingSystemLogs(255, processedLogs);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,6 @@ contract GettersFacet is ZkSyncHyperchainBase, IGetters, ILegacyGetters {

/// @inheritdoc IGetters
function getSettlementLayer() external view returns (address) {
// TODO: consider making private so that no one relies on it
return s.settlementLayer;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ interface IAdmin is IZkSyncHyperchainBase {
/// @notice Emitted when an upgrade is executed.
event ExecuteUpgrade(Diamond.DiamondCutData diamondCut);

/// TODO: maybe include some params
/// @notice Emitted when the migration to the new settlement layer is complete.
event MigrationComplete();

/// @notice Emitted when the contract is frozen.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@ import {PriorityOpsBatchInfo} from "../libraries/PriorityTree.sol";
/// @dev Enum used by L2 System Contracts to differentiate logs.
enum SystemLogKey {
L2_TO_L1_LOGS_TREE_ROOT_KEY,
TOTAL_L2_TO_L1_PUBDATA_KEY,
STATE_DIFF_HASH_KEY,
PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY,
PREV_BATCH_HASH_KEY,
CHAINED_PRIORITY_TXN_HASH_KEY,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,6 @@ interface IGetters is IZkSyncHyperchainBase {
/// @return isFreezable Whether the facet can be frozen by the admin or always accessible
function isFacetFreezable(address _facet) external view returns (bool isFreezable);

/// TODO
/// @return The address of the current settlement layer.
function getSettlementLayer() external view returns (address);
}

Large diffs are not rendered by default.

25 changes: 2 additions & 23 deletions l1-contracts/test/foundry/unit/concrete/Executor/Committing.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ contract CommittingTest is ExecutorTest {
vm.prank(validator);
vm.blobhashes(defaultBlobVersionedHashes);

vm.expectRevert(abi.encodeWithSelector(LogAlreadyProcessed.selector, 3));
vm.expectRevert(abi.encodeWithSelector(LogAlreadyProcessed.selector, 1));
executor.commitBatchesSharedBridge(uint256(0), genesisStoredBatchInfo, wrongNewCommitBatchInfoArray);
}

Expand Down Expand Up @@ -330,9 +330,7 @@ contract CommittingTest is ExecutorTest {
}

function test_RevertWhen_SystemLogIsFromIncorrectAddress() public {
bytes32[9] memory values = [
bytes32(""),
bytes32(""),
bytes32[7] memory values = [
bytes32(""),
bytes32(""),
bytes32(""),
Expand All @@ -342,26 +340,7 @@ contract CommittingTest is ExecutorTest {
bytes32("")
];

bytes[9] memory errors = [
bytes.concat("lm"),
bytes.concat(""),
bytes.concat(""),
bytes.concat("sc"),
bytes.concat("sv"),
bytes.concat("bl"),
bytes.concat("bk"),
bytes.concat("lp2"),
bytes.concat("vk")
];

for (uint256 i = 0; i < values.length; i++) {
// these logs are not checked by the executor, thus they can't cause a revert
if (
i == uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY) || i == uint256(SystemLogKey.STATE_DIFF_HASH_KEY)
) {
continue;
}

bytes[] memory wrongL2Logs = Utils.createSystemLogs(l2DAValidatorOutputHash);
address wrongAddress = makeAddr("randomAddress");
wrongL2Logs[i] = Utils.constructL2Log(true, wrongAddress, i, values[i]);
Expand Down
19 changes: 6 additions & 13 deletions l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -60,52 +60,45 @@ library Utils {
}

function createSystemLogs(bytes32 _outputHash) public returns (bytes[] memory) {
bytes[] memory logs = new bytes[](9);
bytes[] memory logs = new bytes[](7);
logs[0] = constructL2Log(
true,
L2_TO_L1_MESSENGER,
uint256(SystemLogKey.L2_TO_L1_LOGS_TREE_ROOT_KEY),
bytes32("")
);
logs[1] = constructL2Log(
true,
L2_TO_L1_MESSENGER,
uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY),
0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563
);
logs[2] = constructL2Log(true, L2_TO_L1_MESSENGER, uint256(SystemLogKey.STATE_DIFF_HASH_KEY), bytes32(""));
logs[3] = constructL2Log(
true,
L2_SYSTEM_CONTEXT_ADDRESS,
uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY),
bytes32("")
);
logs[4] = constructL2Log(
logs[2] = constructL2Log(
true,
L2_SYSTEM_CONTEXT_ADDRESS,
uint256(SystemLogKey.PREV_BATCH_HASH_KEY),
bytes32("")
);
logs[5] = constructL2Log(
logs[3] = constructL2Log(
true,
L2_BOOTLOADER_ADDRESS,
uint256(SystemLogKey.CHAINED_PRIORITY_TXN_HASH_KEY),
keccak256("")
);
logs[6] = constructL2Log(
logs[4] = constructL2Log(
true,
L2_BOOTLOADER_ADDRESS,
uint256(SystemLogKey.NUMBER_OF_LAYER_1_TXS_KEY),
bytes32("")
);

logs[7] = constructL2Log(
logs[5] = constructL2Log(
true,
L2_TO_L1_MESSENGER,
uint256(SystemLogKey.L2_DA_VALIDATOR_OUTPUT_HASH_KEY),
_outputHash
);
logs[8] = constructL2Log(
logs[6] = constructL2Log(
true,
L2_TO_L1_MESSENGER,
uint256(SystemLogKey.USED_L2_DA_VALIDATOR_ADDRESS_KEY),
Expand Down
41 changes: 12 additions & 29 deletions l1-contracts/test/foundry/unit/concrete/Utils/Utils.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ contract UtilsTest is Test {
function test_CreateSystemLogs() public {
bytes[] memory logs = Utils.createSystemLogs(bytes32(0));

assertEq(logs.length, 9, "logs length should be correct");
assertEq(logs.length, 7, "logs length should be correct");

assertEq(
logs[0],
Expand All @@ -60,85 +60,68 @@ contract UtilsTest is Test {

assertEq(
logs[1],
Utils.constructL2Log(
true,
L2_TO_L1_MESSENGER,
uint256(SystemLogKey.TOTAL_L2_TO_L1_PUBDATA_KEY),
0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563
),
"log[1] should be correct"
);

assertEq(
logs[2],
Utils.constructL2Log(true, L2_TO_L1_MESSENGER, uint256(SystemLogKey.STATE_DIFF_HASH_KEY), bytes32("")),
"log[2] should be correct"
);

assertEq(
logs[3],
Utils.constructL2Log(
true,
L2_SYSTEM_CONTEXT_ADDRESS,
uint256(SystemLogKey.PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY),
bytes32("")
),
"log[3] should be correct"
"log[1] should be correct"
);

assertEq(
logs[4],
logs[2],
Utils.constructL2Log(
true,
L2_SYSTEM_CONTEXT_ADDRESS,
uint256(SystemLogKey.PREV_BATCH_HASH_KEY),
bytes32("")
),
"log[4] should be correct"
"log[2] should be correct"
);

assertEq(
logs[5],
logs[3],
Utils.constructL2Log(
true,
L2_BOOTLOADER_ADDRESS,
uint256(SystemLogKey.CHAINED_PRIORITY_TXN_HASH_KEY),
keccak256("")
),
"log[5] should be correct"
"log[3] should be correct"
);

assertEq(
logs[6],
logs[4],
Utils.constructL2Log(
true,
L2_BOOTLOADER_ADDRESS,
uint256(SystemLogKey.NUMBER_OF_LAYER_1_TXS_KEY),
bytes32("")
),
"log[6] should be correct"
"log[4] should be correct"
);

assertEq(
logs[7],
logs[5],
Utils.constructL2Log(
true,
L2_TO_L1_MESSENGER,
uint256(SystemLogKey.L2_DA_VALIDATOR_OUTPUT_HASH_KEY),
bytes32(0)
),
"log[7] should be correct"
"log[5] should be correct"
);

assertEq(
logs[8],
logs[6],
Utils.constructL2Log(
true,
L2_TO_L1_MESSENGER,
uint256(SystemLogKey.USED_L2_DA_VALIDATOR_ADDRESS_KEY),
bytes32(uint256(uint160(L2_DA_VALIDATOR_ADDRESS)))
),
"log[8] should be correct"
"log[6] should be correct"
);
}

Expand Down
7 changes: 0 additions & 7 deletions l1-contracts/test/unit_tests/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export const L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR = "0x000000000000000000000000000
export const L2_BYTECODE_COMPRESSOR_ADDRESS = "0x000000000000000000000000000000000000800e";
export const DEPLOYER_SYSTEM_CONTRACT_ADDRESS = "0x0000000000000000000000000000000000008006";
export const PUBDATA_CHUNK_PUBLISHER_ADDRESS = "0x0000000000000000000000000000000000008011";
const PUBDATA_HASH = "0x290decd9548b62a8d60345a988386fc84ba6bc95484008f6362f93160ef3e563";

export const SYSTEM_UPGRADE_TX_TYPE = 254;

Expand All @@ -40,8 +39,6 @@ export function randomAddress() {

export enum SYSTEM_LOG_KEYS {
L2_TO_L1_LOGS_TREE_ROOT_KEY,
TOTAL_L2_TO_L1_PUBDATA_KEY,
STATE_DIFF_HASH_KEY,
PACKED_BATCH_AND_L2_BLOCK_TIMESTAMP_KEY,
PREV_BATCH_HASH_KEY,
CHAINED_PRIORITY_TXN_HASH_KEY,
Expand Down Expand Up @@ -213,8 +210,6 @@ export function createSystemLogs(
) {
return [
constructL2Log(true, L2_TO_L1_MESSENGER, SYSTEM_LOG_KEYS.L2_TO_L1_LOGS_TREE_ROOT_KEY, ethers.constants.HashZero),
constructL2Log(true, L2_TO_L1_MESSENGER, SYSTEM_LOG_KEYS.TOTAL_L2_TO_L1_PUBDATA_KEY, PUBDATA_HASH),
constructL2Log(true, L2_TO_L1_MESSENGER, SYSTEM_LOG_KEYS.STATE_DIFF_HASH_KEY, ethers.constants.HashZero),
constructL2Log(
true,
L2_SYSTEM_CONTEXT_ADDRESS,
Expand Down Expand Up @@ -264,8 +259,6 @@ export function createSystemLogsWithUpgrade(
) {
return [
constructL2Log(true, L2_TO_L1_MESSENGER, SYSTEM_LOG_KEYS.L2_TO_L1_LOGS_TREE_ROOT_KEY, ethers.constants.HashZero),
constructL2Log(true, L2_TO_L1_MESSENGER, SYSTEM_LOG_KEYS.TOTAL_L2_TO_L1_PUBDATA_KEY, PUBDATA_HASH),
constructL2Log(true, L2_TO_L1_MESSENGER, SYSTEM_LOG_KEYS.STATE_DIFF_HASH_KEY, ethers.constants.HashZero),
constructL2Log(
true,
L2_SYSTEM_CONTEXT_ADDRESS,
Expand Down
2 changes: 0 additions & 2 deletions l2-contracts/contracts/data-availability/DAErrors.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
pragma solidity 0.8.24;

enum PubdataField {
NumberOfLogs,
LogsHash,
MsgHash,
Bytecode,
StateDiffCompressionVersion,
Expand Down
Loading
Loading