diff --git a/l1-contracts/contracts/common/Config.sol b/l1-contracts/contracts/common/Config.sol index 1e23213d1..72ca11aa1 100644 --- a/l1-contracts/contracts/common/Config.sol +++ b/l1-contracts/contracts/common/Config.sol @@ -27,10 +27,11 @@ uint256 constant PRIORITY_OPERATION_L2_TX_TYPE = 255; /// @dev Denotes the type of the zkSync transaction that is used for system upgrades. uint256 constant SYSTEM_UPGRADE_L2_TX_TYPE = 254; -/// @dev The maximal allowed difference between protocol versions in an upgrade. The 100 gap is needed +/// @dev The maximal allowed difference between protocol minor versions in an upgrade. The 100 gap is needed /// in case a protocol version has been tested on testnet, but then not launched on mainnet, e.g. /// due to a bug found. -uint256 constant MAX_ALLOWED_PROTOCOL_VERSION_DELTA = 100; +/// We are allowed to jump at most 100 minor versions at a time. The major version is always expected to be 0. +uint256 constant MAX_ALLOWED_MINOR_VERSION_DELTA = 100; /// @dev The amount of time in seconds the validator has to process the priority transaction /// NOTE: The constant is set to zero for the Alpha release period diff --git a/l1-contracts/contracts/common/libraries/SemVer.sol b/l1-contracts/contracts/common/libraries/SemVer.sol new file mode 100644 index 000000000..d20f6a1d1 --- /dev/null +++ b/l1-contracts/contracts/common/libraries/SemVer.sol @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.24; + +/// @dev The number of bits dedicated to the "patch" portion of the protocol version. +/// This also defines the bit starting from which the "minor" part is located. +uint256 constant SEMVER_MINOR_OFFSET = 32; + +/// @dev The number of bits dedicated to the "patch" and "minor" portions of the protocol version. +/// This also defines the bit starting from which the "major" part is located. +/// Note, that currently, only major version of "0" is supported. +uint256 constant SEMVER_MAJOR_OFFSET = 64; + +/** + * @author Matter Labs + * @custom:security-contact security@matterlabs.dev + * @notice The library for managing SemVer for the protocol version. + */ +library SemVer { + /// @notice Unpacks the SemVer version from a single uint256 into major, minor and patch components. + /// @param _packedProtocolVersion The packed protocol version. + /// @return major The major version. + /// @return minor The minor version. + /// @return patch The patch version. + function unpackSemVer( + uint96 _packedProtocolVersion + ) internal pure returns (uint32 major, uint32 minor, uint32 patch) { + patch = uint32(_packedProtocolVersion); + minor = uint32(_packedProtocolVersion >> SEMVER_MINOR_OFFSET); + major = uint32(_packedProtocolVersion >> SEMVER_MAJOR_OFFSET); + } + + /// @notice Packs the SemVer version from the major, minor and patch components into a single uint96. + /// @param _major The major version. + /// @param _minor The minor version. + /// @param _patch The patch version. + /// @return packedProtocolVersion The packed protocol version. + function packSemVer( + uint32 _major, + uint32 _minor, + uint32 _patch + ) internal pure returns (uint96 packedProtocolVersion) { + packedProtocolVersion = + uint96(_patch) | + (uint96(_minor) << SEMVER_MINOR_OFFSET) | + (uint96(_major) << SEMVER_MAJOR_OFFSET); + } +} diff --git a/l1-contracts/contracts/dev-contracts/test/CustomUpgradeTest.sol b/l1-contracts/contracts/dev-contracts/test/CustomUpgradeTest.sol index e7c82d29c..7055ce557 100644 --- a/l1-contracts/contracts/dev-contracts/test/CustomUpgradeTest.sol +++ b/l1-contracts/contracts/dev-contracts/test/CustomUpgradeTest.sol @@ -28,16 +28,17 @@ contract CustomUpgradeTest is BaseZkSyncUpgrade { /// @notice The main function that will be called by the upgrade proxy. /// @param _proposedUpgrade The upgrade to be executed. function upgrade(ProposedUpgrade calldata _proposedUpgrade) public override returns (bytes32) { - _setNewProtocolVersion(_proposedUpgrade.newProtocolVersion); + (uint32 newMinorVersion, bool isPatchOnly) = _setNewProtocolVersion(_proposedUpgrade.newProtocolVersion); _upgradeL1Contract(_proposedUpgrade.l1ContractsUpgradeCalldata); _upgradeVerifier(_proposedUpgrade.verifier, _proposedUpgrade.verifierParams); - _setBaseSystemContracts(_proposedUpgrade.bootloaderHash, _proposedUpgrade.defaultAccountHash); + _setBaseSystemContracts(_proposedUpgrade.bootloaderHash, _proposedUpgrade.defaultAccountHash, isPatchOnly); bytes32 txHash; txHash = _setL2SystemContractUpgrade( _proposedUpgrade.l2ProtocolUpgradeTx, _proposedUpgrade.factoryDeps, - _proposedUpgrade.newProtocolVersion + newMinorVersion, + isPatchOnly ); _postUpgrade(_proposedUpgrade.postUpgradeCalldata); diff --git a/l1-contracts/contracts/state-transition/IStateTransitionManager.sol b/l1-contracts/contracts/state-transition/IStateTransitionManager.sol index d201c8333..4151a1454 100644 --- a/l1-contracts/contracts/state-transition/IStateTransitionManager.sol +++ b/l1-contracts/contracts/state-transition/IStateTransitionManager.sol @@ -131,4 +131,6 @@ interface IStateTransitionManager { uint256 _oldProtocolVersion, Diamond.DiamondCutData calldata _diamondCut ) external; + + function getSemverProtocolVersion() external view returns (uint32, uint32, uint32); } diff --git a/l1-contracts/contracts/state-transition/StateTransitionManager.sol b/l1-contracts/contracts/state-transition/StateTransitionManager.sol index c5367dd9b..812e6fa11 100644 --- a/l1-contracts/contracts/state-transition/StateTransitionManager.sol +++ b/l1-contracts/contracts/state-transition/StateTransitionManager.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.24; import {EnumerableMap} from "@openzeppelin/contracts/utils/structs/EnumerableMap.sol"; +import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; import {Diamond} from "./libraries/Diamond.sol"; import {DiamondProxy} from "./chain-deps/DiamondProxy.sol"; @@ -21,6 +22,7 @@ import {ProposedUpgrade} from "../upgrades/BaseZkSyncUpgrade.sol"; import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {REQUIRED_L2_GAS_PRICE_PER_PUBDATA, L2_TO_L1_LOG_SERIALIZE_SIZE, DEFAULT_L2_LOGS_TREE_ROOT_HASH, EMPTY_STRING_KECCAK, SYSTEM_UPGRADE_L2_TX_TYPE, PRIORITY_TX_MAX_GAS_LIMIT} from "../common/Config.sol"; import {VerifierParams} from "./chain-interfaces/IVerifier.sol"; +import {SemVer} from "../common/libraries/SemVer.sol"; /// @title State Transition Manager contract /// @author Matter Labs @@ -47,7 +49,7 @@ contract StateTransitionManager is IStateTransitionManager, ReentrancyGuard, Own /// @dev The genesisUpgrade contract address, used to setChainId address public genesisUpgrade; - /// @dev The current protocolVersion + /// @dev The current packed protocolVersion. To access human-readable version, use `getSemverProtocolVersion` function. uint256 public protocolVersion; /// @dev The timestamp when protocolVersion can be last used @@ -84,6 +86,12 @@ contract StateTransitionManager is IStateTransitionManager, ReentrancyGuard, Own _; } + /// @return The tuple of (major, minor, patch) protocol version. + function getSemverProtocolVersion() external view returns (uint32, uint32, uint32) { + // slither-disable-next-line unused-return + return SemVer.unpackSemVer(SafeCast.toUint96(protocolVersion)); + } + /// @notice Returns all the registered hyperchain addresses function getAllHyperchains() public view override returns (address[] memory chainAddresses) { uint256[] memory keys = hyperchainMap.keys(); @@ -280,6 +288,10 @@ contract StateTransitionManager is IStateTransitionManager, ReentrancyGuard, Own uint256[] memory uintEmptyArray; bytes[] memory bytesEmptyArray; + uint256 cachedProtocolVersion = protocolVersion; + // slither-disable-next-line unused-return + (, uint32 minorVersion, ) = SemVer.unpackSemVer(SafeCast.toUint96(cachedProtocolVersion)); + L2CanonicalTransaction memory l2ProtocolUpgradeTx = L2CanonicalTransaction({ txType: SYSTEM_UPGRADE_L2_TX_TYPE, from: uint256(uint160(L2_FORCE_DEPLOYER_ADDR)), @@ -289,8 +301,8 @@ contract StateTransitionManager is IStateTransitionManager, ReentrancyGuard, Own maxFeePerGas: uint256(0), maxPriorityFeePerGas: uint256(0), paymaster: uint256(0), - // Note, that the protocol version is used as "nonce" for system upgrade transactions - nonce: protocolVersion, + // Note, that the `minor` of the protocol version is used as "nonce" for system upgrade transactions + nonce: uint256(minorVersion), value: 0, reserved: [uint256(0), 0, 0, 0], data: systemContextCalldata, @@ -314,7 +326,7 @@ contract StateTransitionManager is IStateTransitionManager, ReentrancyGuard, Own l1ContractsUpgradeCalldata: new bytes(0), postUpgradeCalldata: new bytes(0), upgradeTimestamp: 0, - newProtocolVersion: protocolVersion + newProtocolVersion: cachedProtocolVersion }); Diamond.FacetCut[] memory emptyArray; @@ -325,7 +337,7 @@ contract StateTransitionManager is IStateTransitionManager, ReentrancyGuard, Own }); IAdmin(_chainContract).executeUpgrade(cutData); - emit SetChainIdUpgrade(_chainContract, l2ProtocolUpgradeTx, protocolVersion); + emit SetChainIdUpgrade(_chainContract, l2ProtocolUpgradeTx, cachedProtocolVersion); } /// @dev used to register already deployed hyperchain contracts diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol index 1769dceca..ab87d31f0 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Getters.sol @@ -2,6 +2,8 @@ pragma solidity 0.8.24; +import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; + import {ZkSyncHyperchainBase} from "./ZkSyncHyperchainBase.sol"; import {PubdataPricingMode} from "../ZkSyncHyperchainStorage.sol"; import {VerifierParams} from "../../../state-transition/chain-interfaces/IVerifier.sol"; @@ -10,6 +12,7 @@ import {PriorityQueue, PriorityOperation} from "../../../state-transition/librar import {UncheckedMath} from "../../../common/libraries/UncheckedMath.sol"; import {IGetters} from "../../chain-interfaces/IGetters.sol"; import {ILegacyGetters} from "../../chain-interfaces/ILegacyGetters.sol"; +import {SemVer} from "../../../common/libraries/SemVer.sol"; // While formally the following import is not used, it is needed to inherit documentation from it import {IZkSyncHyperchainBase} from "../../chain-interfaces/IZkSyncHyperchainBase.sol"; @@ -143,6 +146,12 @@ contract GettersFacet is ZkSyncHyperchainBase, IGetters, ILegacyGetters { return s.protocolVersion; } + /// @inheritdoc IGetters + function getSemverProtocolVersion() external view returns (uint32, uint32, uint32) { + // slither-disable-next-line unused-return + return SemVer.unpackSemVer(SafeCast.toUint96(s.protocolVersion)); + } + /// @inheritdoc IGetters function getL2SystemContractsUpgradeTxHash() external view returns (bytes32) { return s.l2SystemContractsUpgradeTxHash; diff --git a/l1-contracts/contracts/state-transition/chain-interfaces/IGetters.sol b/l1-contracts/contracts/state-transition/chain-interfaces/IGetters.sol index 4eec88868..9d77efdf3 100644 --- a/l1-contracts/contracts/state-transition/chain-interfaces/IGetters.sol +++ b/l1-contracts/contracts/state-transition/chain-interfaces/IGetters.sol @@ -85,9 +85,12 @@ interface IGetters is IZkSyncHyperchainBase { /// @return Whether the diamond is frozen or not function isDiamondStorageFrozen() external view returns (bool); - /// @return The current protocol version + /// @return The current packed protocol version. To access human-readable version, use `getSemverProtocolVersion` function. function getProtocolVersion() external view returns (uint256); + /// @return The tuple of (major, minor, patch) protocol version. + function getSemverProtocolVersion() external view returns (uint32, uint32, uint32); + /// @return The upgrade system contract transaction hash, 0 if the upgrade is not initialized function getL2SystemContractsUpgradeTxHash() external view returns (bytes32); diff --git a/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol b/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol index 07185b095..72c02f277 100644 --- a/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol +++ b/l1-contracts/contracts/upgrades/BaseZkSyncUpgrade.sol @@ -2,13 +2,16 @@ pragma solidity 0.8.24; +import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; + import {ZkSyncHyperchainBase} from "../state-transition/chain-deps/facets/ZkSyncHyperchainBase.sol"; import {VerifierParams} from "../state-transition/chain-interfaces/IVerifier.sol"; import {IVerifier} from "../state-transition/chain-interfaces/IVerifier.sol"; import {L2ContractHelper} from "../common/libraries/L2ContractHelper.sol"; import {TransactionValidator} from "../state-transition/libraries/TransactionValidator.sol"; -import {MAX_NEW_FACTORY_DEPS, SYSTEM_UPGRADE_L2_TX_TYPE, MAX_ALLOWED_PROTOCOL_VERSION_DELTA} from "../common/Config.sol"; +import {MAX_NEW_FACTORY_DEPS, SYSTEM_UPGRADE_L2_TX_TYPE, MAX_ALLOWED_MINOR_VERSION_DELTA} from "../common/Config.sol"; import {L2CanonicalTransaction} from "../common/Messaging.sol"; +import {SemVer} from "../common/libraries/SemVer.sol"; /// @notice The struct that represents the upgrade proposal. /// @param l2ProtocolUpgradeTx The system upgrade transaction. @@ -70,15 +73,16 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { // as the permitted delay window is reduced in the future. require(block.timestamp >= _proposedUpgrade.upgradeTimestamp, "Upgrade is not ready yet"); - _setNewProtocolVersion(_proposedUpgrade.newProtocolVersion); + (uint32 newMinorVersion, bool isPatchOnly) = _setNewProtocolVersion(_proposedUpgrade.newProtocolVersion); _upgradeL1Contract(_proposedUpgrade.l1ContractsUpgradeCalldata); _upgradeVerifier(_proposedUpgrade.verifier, _proposedUpgrade.verifierParams); - _setBaseSystemContracts(_proposedUpgrade.bootloaderHash, _proposedUpgrade.defaultAccountHash); + _setBaseSystemContracts(_proposedUpgrade.bootloaderHash, _proposedUpgrade.defaultAccountHash, isPatchOnly); txHash = _setL2SystemContractUpgrade( _proposedUpgrade.l2ProtocolUpgradeTx, _proposedUpgrade.factoryDeps, - _proposedUpgrade.newProtocolVersion + newMinorVersion, + isPatchOnly ); _postUpgrade(_proposedUpgrade.postUpgradeCalldata); @@ -88,11 +92,14 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { /// @notice Change default account bytecode hash, that is used on L2 /// @param _l2DefaultAccountBytecodeHash The hash of default account L2 bytecode - function _setL2DefaultAccountBytecodeHash(bytes32 _l2DefaultAccountBytecodeHash) private { + /// @param _patchOnly Whether only the patch part of the protocol version semver has changed + function _setL2DefaultAccountBytecodeHash(bytes32 _l2DefaultAccountBytecodeHash, bool _patchOnly) private { if (_l2DefaultAccountBytecodeHash == bytes32(0)) { return; } + require(!_patchOnly, "Patch only upgrade can not set new default account"); + L2ContractHelper.validateBytecodeHash(_l2DefaultAccountBytecodeHash); // Save previous value into the stack to put it into the event later @@ -105,11 +112,14 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { /// @notice Change bootloader bytecode hash, that is used on L2 /// @param _l2BootloaderBytecodeHash The hash of bootloader L2 bytecode - function _setL2BootloaderBytecodeHash(bytes32 _l2BootloaderBytecodeHash) private { + /// @param _patchOnly Whether only the patch part of the protocol version semver has changed + function _setL2BootloaderBytecodeHash(bytes32 _l2BootloaderBytecodeHash, bool _patchOnly) private { if (_l2BootloaderBytecodeHash == bytes32(0)) { return; } + require(!_patchOnly, "Patch only upgrade can not set new bootloader"); + L2ContractHelper.validateBytecodeHash(_l2BootloaderBytecodeHash); // Save previous value into the stack to put it into the event later @@ -167,25 +177,33 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { /// @notice Updates the bootloader hash and the hash of the default account /// @param _bootloaderHash The hash of the new bootloader bytecode. If zero, it will not be updated. /// @param _defaultAccountHash The hash of the new default account bytecode. If zero, it will not be updated. - function _setBaseSystemContracts(bytes32 _bootloaderHash, bytes32 _defaultAccountHash) internal { - _setL2BootloaderBytecodeHash(_bootloaderHash); - _setL2DefaultAccountBytecodeHash(_defaultAccountHash); + /// @param _patchOnly Whether only the patch part of the protocol version semver has changed. + function _setBaseSystemContracts(bytes32 _bootloaderHash, bytes32 _defaultAccountHash, bool _patchOnly) internal { + _setL2BootloaderBytecodeHash(_bootloaderHash, _patchOnly); + _setL2DefaultAccountBytecodeHash(_defaultAccountHash, _patchOnly); } /// @notice Sets the hash of the L2 system contract upgrade transaction for the next batch to be committed /// @dev If the transaction is noop (i.e. its type is 0) it does nothing and returns 0. /// @param _l2ProtocolUpgradeTx The L2 system contract upgrade transaction. + /// @param _factoryDeps The factory dependencies that are used by the transaction. + /// @param _newMinorProtocolVersion The new minor protocol version. It must be used as the `nonce` field + /// of the `_l2ProtocolUpgradeTx`. + /// @param _patchOnly Whether only the patch part of the protocol version semver has changed. /// @return System contracts upgrade transaction hash. Zero if no upgrade transaction is set. function _setL2SystemContractUpgrade( L2CanonicalTransaction calldata _l2ProtocolUpgradeTx, bytes[] calldata _factoryDeps, - uint256 _newProtocolVersion + uint32 _newMinorProtocolVersion, + bool _patchOnly ) internal returns (bytes32) { // If the type is 0, it is considered as noop and so will not be required to be executed. if (_l2ProtocolUpgradeTx.txType == 0) { return bytes32(0); } + require(!_patchOnly, "Patch only upgrade can not set upgrade transaction"); + require(_l2ProtocolUpgradeTx.txType == SYSTEM_UPGRADE_L2_TX_TYPE, "L2 system upgrade tx type is wrong"); bytes memory encodedTransaction = abi.encode(_l2ProtocolUpgradeTx); @@ -202,7 +220,7 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { // We want the hashes of l2 system upgrade transactions to be unique. // This is why we require that the `nonce` field is unique to each upgrade. require( - _l2ProtocolUpgradeTx.nonce == _newProtocolVersion, + _l2ProtocolUpgradeTx.nonce == _newMinorProtocolVersion, "The new protocol version should be included in the L2 system upgrade tx" ); @@ -232,24 +250,48 @@ abstract contract BaseZkSyncUpgrade is ZkSyncHyperchainBase { /// @notice Changes the protocol version /// @param _newProtocolVersion The new protocol version - function _setNewProtocolVersion(uint256 _newProtocolVersion) internal virtual { + function _setNewProtocolVersion( + uint256 _newProtocolVersion + ) internal virtual returns (uint32 newMinorVersion, bool patchOnly) { uint256 previousProtocolVersion = s.protocolVersion; require( _newProtocolVersion > previousProtocolVersion, "New protocol version is not greater than the current one" ); - require( - _newProtocolVersion - previousProtocolVersion <= MAX_ALLOWED_PROTOCOL_VERSION_DELTA, - "Too big protocol version difference" + // slither-disable-next-line unused-return + (uint32 previousMajorVersion, uint32 previousMinorVersion, ) = SemVer.unpackSemVer( + SafeCast.toUint96(previousProtocolVersion) ); + require(previousMajorVersion == 0, "Implementation requires that the major version is 0 at all times"); - // If the previous upgrade had an L2 system upgrade transaction, we require that it is finalized. - // Note it is important to keep this check, as otherwise hyperchains might skip upgrades by overwriting - require(s.l2SystemContractsUpgradeTxHash == bytes32(0), "Previous upgrade has not been finalized"); - require( - s.l2SystemContractsUpgradeBatchNumber == 0, - "The batch number of the previous upgrade has not been cleaned" - ); + uint32 newMajorVersion; + // slither-disable-next-line unused-return + (newMajorVersion, newMinorVersion, ) = SemVer.unpackSemVer(SafeCast.toUint96(_newProtocolVersion)); + require(newMajorVersion == 0, "Major must always be 0"); + + // Since `_newProtocolVersion > previousProtocolVersion`, and both old and new major version is 0, + // the difference between minor versions is >= 0. + uint256 minorDelta = newMinorVersion - previousMinorVersion; + + if (minorDelta == 0) { + patchOnly = true; + } + + // While this is implicitly enforced by other checks above, we still double check just in case + require(minorDelta <= MAX_ALLOWED_MINOR_VERSION_DELTA, "Too big protocol version difference"); + + // If the minor version changes also, we need to ensure that the previous upgrade has been finalized. + // In case the minor version does not change, we permit to keep the old upgrade transaction in the system, but it + // must be ensured in the other parts of the upgrade that the is not overridden. + if (!patchOnly) { + // If the previous upgrade had an L2 system upgrade transaction, we require that it is finalized. + // Note it is important to keep this check, as otherwise hyperchains might skip upgrades by overwriting + require(s.l2SystemContractsUpgradeTxHash == bytes32(0), "Previous upgrade has not been finalized"); + require( + s.l2SystemContractsUpgradeBatchNumber == 0, + "The batch number of the previous upgrade has not been cleaned" + ); + } s.protocolVersion = _newProtocolVersion; emit NewProtocolVersion(previousProtocolVersion, _newProtocolVersion); diff --git a/l1-contracts/contracts/upgrades/BaseZkSyncUpgradeGenesis.sol b/l1-contracts/contracts/upgrades/BaseZkSyncUpgradeGenesis.sol index 4dccf4565..7cf9b5ceb 100644 --- a/l1-contracts/contracts/upgrades/BaseZkSyncUpgradeGenesis.sol +++ b/l1-contracts/contracts/upgrades/BaseZkSyncUpgradeGenesis.sol @@ -2,8 +2,11 @@ pragma solidity 0.8.24; -import {MAX_ALLOWED_PROTOCOL_VERSION_DELTA} from "../common/Config.sol"; +import {SafeCast} from "@openzeppelin/contracts/utils/math/SafeCast.sol"; + import {BaseZkSyncUpgrade} from "./BaseZkSyncUpgrade.sol"; +import {MAX_ALLOWED_MINOR_VERSION_DELTA} from "../common/Config.sol"; +import {SemVer} from "../common/libraries/SemVer.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -11,24 +14,48 @@ import {BaseZkSyncUpgrade} from "./BaseZkSyncUpgrade.sol"; abstract contract BaseZkSyncUpgradeGenesis is BaseZkSyncUpgrade { /// @notice Changes the protocol version /// @param _newProtocolVersion The new protocol version - function _setNewProtocolVersion(uint256 _newProtocolVersion) internal override { + function _setNewProtocolVersion( + uint256 _newProtocolVersion + ) internal override returns (uint32 newMinorVersion, bool patchOnly) { uint256 previousProtocolVersion = s.protocolVersion; + // IMPORTANT Genesis Upgrade difference: Note this is the only thing change > to >= require( - // IMPORTANT Genesis Upgrade difference: Note this is the only thing change > to >= _newProtocolVersion >= previousProtocolVersion, "New protocol version is not greater than the current one" ); - require( - _newProtocolVersion - previousProtocolVersion <= MAX_ALLOWED_PROTOCOL_VERSION_DELTA, - "Too big protocol version difference" + // slither-disable-next-line unused-return + (uint32 previousMajorVersion, uint32 previousMinorVersion, ) = SemVer.unpackSemVer( + SafeCast.toUint96(previousProtocolVersion) ); + require(previousMajorVersion == 0, "Implementation requires that the major version is 0 at all times"); - // If the previous upgrade had an L2 system upgrade transaction, we require that it is finalized. - require(s.l2SystemContractsUpgradeTxHash == bytes32(0), "Previous upgrade has not been finalized"); - require( - s.l2SystemContractsUpgradeBatchNumber == 0, - "The batch number of the previous upgrade has not been cleaned" - ); + uint32 newMajorVersion; + // slither-disable-next-line unused-return + (newMajorVersion, newMinorVersion, ) = SemVer.unpackSemVer(SafeCast.toUint96(_newProtocolVersion)); + require(newMajorVersion == 0, "Major must always be 0"); + + // Since `_newProtocolVersion > previousProtocolVersion`, and both old and new major version is 0, + // the difference between minor versions is >= 0. + uint256 minorDelta = newMinorVersion - previousMinorVersion; + + // IMPORTANT Genesis Upgrade difference: We never set patchOnly to `true` to allow to put a system upgrade transaction there. + patchOnly = false; + + // While this is implicitly enforced by other checks above, we still double check just in case + require(minorDelta <= MAX_ALLOWED_MINOR_VERSION_DELTA, "Too big protocol version difference"); + + // If the minor version changes also, we need to ensure that the previous upgrade has been finalized. + // In case the minor version does not change, we permit to keep the old upgrade transaction in the system, but it + // must be ensured in the other parts of the upgrade that the is not overridden. + if (!patchOnly) { + // If the previous upgrade had an L2 system upgrade transaction, we require that it is finalized. + // Note it is important to keep this check, as otherwise hyperchains might skip upgrades by overwriting + require(s.l2SystemContractsUpgradeTxHash == bytes32(0), "Previous upgrade has not been finalized"); + require( + s.l2SystemContractsUpgradeBatchNumber == 0, + "The batch number of the previous upgrade has not been cleaned" + ); + } s.protocolVersion = _newProtocolVersion; emit NewProtocolVersion(previousProtocolVersion, _newProtocolVersion); diff --git a/l1-contracts/scripts/utils.ts b/l1-contracts/scripts/utils.ts index 3a972b64c..5ae1bceac 100644 --- a/l1-contracts/scripts/utils.ts +++ b/l1-contracts/scripts/utils.ts @@ -10,6 +10,9 @@ const warning = chalk.bold.yellow; export const L1_TO_L2_ALIAS_OFFSET = "0x1111000000000000000000000000000000001111"; export const GAS_MULTIPLIER = 1; +// Bit shift by 32 does not work in JS, so we have to multiply by 2^32 +export const SEMVER_MINOR_VERSION_MULTIPLIER = 4294967296; + interface SystemConfig { requiredL2GasPricePerPubdata: number; priorityTxMinimalGasPrice: number; @@ -75,3 +78,29 @@ export function print(name: string, data: any) { export function getLowerCaseAddress(address: string) { return ethers.utils.getAddress(address).toLowerCase(); } + +export function unpackStringSemVer(semver: string): [number, number, number] { + const [major, minor, patch] = semver.split("."); + return [parseInt(major), parseInt(minor), parseInt(patch)]; +} + +function unpackNumberSemVer(semver: number): [number, number, number] { + const major = 0; + const minor = Math.floor(semver / SEMVER_MINOR_VERSION_MULTIPLIER); + const patch = semver % SEMVER_MINOR_VERSION_MULTIPLIER; + return [major, minor, patch]; +} + +// The major version is always 0 for now +export function packSemver(major: number, minor: number, patch: number) { + if (major !== 0) { + throw new Error("Major version must be 0"); + } + + return minor * SEMVER_MINOR_VERSION_MULTIPLIER + patch; +} + +export function addToProtocolVersion(packedProtocolVersion: number, minor: number, patch: number) { + const [major, minorVersion, patchVersion] = unpackNumberSemVer(packedProtocolVersion); + return packSemver(major, minorVersion + minor, patchVersion + patch); +} diff --git a/l1-contracts/scripts/verify.ts b/l1-contracts/scripts/verify.ts index 36c0980b5..82e17283b 100644 --- a/l1-contracts/scripts/verify.ts +++ b/l1-contracts/scripts/verify.ts @@ -6,7 +6,7 @@ import { getNumberFromEnv, getHashFromEnv, getAddressFromEnv, ethTestConfig } fr import { Interface } from "ethers/lib/utils"; import { Deployer } from "../src.ts/deploy"; import { Wallet } from "ethers"; -import { web3Provider } from "./utils"; +import { packSemver, unpackStringSemVer, web3Provider } from "./utils"; import { getTokens } from "../src.ts/deploy-token"; const provider = web3Provider(); @@ -121,7 +121,7 @@ async function main() { const genesisRollupLeafIndex = getNumberFromEnv("CONTRACTS_GENESIS_ROLLUP_LEAF_INDEX"); const genesisBatchCommitment = getHashFromEnv("CONTRACTS_GENESIS_BATCH_COMMITMENT"); const diamondCut = await deployer.initialZkSyncHyperchainDiamondCut([]); - const protocolVersion = getNumberFromEnv("CONTRACTS_GENESIS_PROTOCOL_VERSION"); + const protocolVersion = packSemver(...unpackStringSemVer(process.env.CONTRACTS_GENESIS_PROTOCOL_SEMANTIC_VERSION)); const initCalldata2 = stateTransitionManager.encodeFunctionData("initialize", [ { diff --git a/l1-contracts/src.ts/deploy-test-process.ts b/l1-contracts/src.ts/deploy-test-process.ts index ac5665a3b..269d8a7a8 100644 --- a/l1-contracts/src.ts/deploy-test-process.ts +++ b/l1-contracts/src.ts/deploy-test-process.ts @@ -38,7 +38,7 @@ const addressConfig = JSON.parse(fs.readFileSync(`${testConfigPath}/addresses.js const testnetTokenPath = `${testConfigPath}/hardhat.json`; export async function loadDefaultEnvVarsForTests(deployWallet: Wallet) { - process.env.CONTRACTS_GENESIS_PROTOCOL_VERSION = (21).toString(); + process.env.CONTRACTS_GENESIS_PROTOCOL_SEMANTIC_VERSION = "0.21.0"; process.env.CONTRACTS_GENESIS_ROOT = ethers.constants.HashZero; process.env.CONTRACTS_GENESIS_ROLLUP_LEAF_INDEX = "0"; process.env.CONTRACTS_GENESIS_BATCH_COMMITMENT = ethers.constants.HashZero; diff --git a/l1-contracts/src.ts/deploy.ts b/l1-contracts/src.ts/deploy.ts index 5373fd352..cad6d572d 100644 --- a/l1-contracts/src.ts/deploy.ts +++ b/l1-contracts/src.ts/deploy.ts @@ -6,7 +6,13 @@ import { ethers } from "ethers"; import { hexlify, Interface } from "ethers/lib/utils"; import type { DeployedAddresses } from "./deploy-utils"; import { deployedAddressesFromEnv, deployBytecodeViaCreate2, deployViaCreate2 } from "./deploy-utils"; -import { readBatchBootloaderBytecode, readSystemContractsBytecode, SYSTEM_CONFIG } from "../scripts/utils"; +import { + packSemver, + readBatchBootloaderBytecode, + readSystemContractsBytecode, + SYSTEM_CONFIG, + unpackStringSemVer, +} from "../scripts/utils"; import { getTokens } from "./deploy-token"; import { ADDRESS_ONE, @@ -307,7 +313,7 @@ export class Deployer { const genesisRollupLeafIndex = getNumberFromEnv("CONTRACTS_GENESIS_ROLLUP_LEAF_INDEX"); const genesisBatchCommitment = getHashFromEnv("CONTRACTS_GENESIS_BATCH_COMMITMENT"); const diamondCut = await this.initialZkSyncHyperchainDiamondCut(extraFacets); - const protocolVersion = getNumberFromEnv("CONTRACTS_GENESIS_PROTOCOL_VERSION"); + const protocolVersion = packSemver(...unpackStringSemVer(process.env.CONTRACTS_GENESIS_PROTOCOL_SEMANTIC_VERSION)); const stateTransitionManager = new Interface(hardhat.artifacts.readArtifactSync("StateTransitionManager").abi); diff --git a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ExecuteUpgrade.t.sol b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ExecuteUpgrade.t.sol index d90d00df5..95c6f54af 100644 --- a/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ExecuteUpgrade.t.sol +++ b/l1-contracts/test/foundry/unit/concrete/state-transition/chain-deps/facets/Admin/ExecuteUpgrade.t.sol @@ -4,8 +4,13 @@ pragma solidity 0.8.24; import {AdminTest} from "./_Admin_Shared.t.sol"; import {ERROR_ONLY_STATE_TRANSITION_MANAGER} from "../Base/_Base_Shared.t.sol"; +import {Utils} from "../../../../Utils/Utils.sol"; +import {VerifierParams} from "contracts/state-transition/chain-interfaces/IVerifier.sol"; import {Diamond} from "contracts/state-transition/libraries/Diamond.sol"; +import {DefaultUpgrade} from "contracts/upgrades/DefaultUpgrade.sol"; +import {SemVer} from "contracts/common/libraries/SemVer.sol"; +import {ProposedUpgrade} from "contracts/upgrades/BaseZkSyncUpgrade.sol"; contract ExecuteUpgradeTest is AdminTest { event ExecuteUpgrade(Diamond.DiamondCutData diamondCut); @@ -23,6 +28,44 @@ contract ExecuteUpgradeTest is AdminTest { vm.startPrank(nonStateTransitionManager); adminFacet.executeUpgrade(diamondCutData); } + + /// TODO: This test should be removed after the migration to the semver is complete everywhere. + function test_migrateToSemVerApproach() public { + // Setting minor protocol version manually + utilsFacet.util_setProtocolVersion(22); + + VerifierParams memory verifierParams = VerifierParams({ + recursionNodeLevelVkHash: bytes32(0), + recursionLeafLevelVkHash: bytes32(0), + recursionCircuitsSetVksHash: bytes32(0) + }); + + ProposedUpgrade memory proposedUpgrade = ProposedUpgrade({ + l2ProtocolUpgradeTx: Utils.makeEmptyL2CanonicalTransaction(), + factoryDeps: new bytes[](0), + bootloaderHash: bytes32(0), + defaultAccountHash: bytes32(0), + verifier: address(0), + verifierParams: verifierParams, + l1ContractsUpgradeCalldata: hex"", + postUpgradeCalldata: hex"", + upgradeTimestamp: 0, + newProtocolVersion: SemVer.packSemVer(0, 22, 0) + }); + + DefaultUpgrade upgrade = new DefaultUpgrade(); + + Diamond.DiamondCutData memory diamondCutData = Diamond.DiamondCutData({ + facetCuts: new Diamond.FacetCut[](0), + initAddress: address(upgrade), + initCalldata: abi.encodeCall(upgrade.upgrade, (proposedUpgrade)) + }); + + address stm = utilsFacet.util_getStateTransitionManager(); + vm.startPrank(stm); + + adminFacet.executeUpgrade(diamondCutData); + } } interface IDiamondLibrary { diff --git a/l1-contracts/test/unit_tests/l2-upgrade.test.spec.ts b/l1-contracts/test/unit_tests/l2-upgrade.test.spec.ts index cda99ac34..7352ce8cc 100644 --- a/l1-contracts/test/unit_tests/l2-upgrade.test.spec.ts +++ b/l1-contracts/test/unit_tests/l2-upgrade.test.spec.ts @@ -40,8 +40,9 @@ import { makeExecutedEqualCommitted, getBatchStoredInfo, } from "./utils"; +import { packSemver, unpackStringSemVer, addToProtocolVersion } from "../../scripts/utils"; -describe("L2 upgrade test", function () { +describe.only("L2 upgrade test", function () { let proxyExecutor: ExecutorFacet; let proxyAdmin: AdminFacet; let proxyGetters: GettersFacet; @@ -59,8 +60,8 @@ describe("L2 upgrade test", function () { let verifier: string; const noopUpgradeTransaction = buildL2CanonicalTransaction({ txType: 0 }); let chainId = process.env.CHAIN_ETH_ZKSYNC_NETWORK_ID || 270; - // let priorityOperationsHash: string; let initialProtocolVersion = 0; + let initialMinorProtocolVersion = 0; before(async () => { [owner] = await hardhat.ethers.getSigners(); @@ -92,7 +93,14 @@ describe("L2 upgrade test", function () { const transferOwnershipTx = await ownable.acceptOwnership(); await transferOwnershipTx.wait(); - initialProtocolVersion = parseInt(process.env.CONTRACTS_GENESIS_PROTOCOL_VERSION); + const [initialMajor, initialMinor, initialPatch] = unpackStringSemVer( + process.env.CONTRACTS_GENESIS_PROTOCOL_SEMANTIC_VERSION + ); + if (initialMajor !== 0 || initialPatch !== 0) { + throw new Error("Initial protocol version must be 0.x.0"); + } + initialProtocolVersion = packSemver(initialMajor, initialMinor, initialPatch); + initialMinorProtocolVersion = initialMinor; chainId = deployer.chainId; verifier = deployer.addresses.StateTransition.Verifier; @@ -149,18 +157,75 @@ describe("L2 upgrade test", function () { await ( await executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { - newProtocolVersion: 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 1, 0), l2ProtocolUpgradeTx: noopUpgradeTransaction, }) ).wait(); - expect(await proxyGetters.getProtocolVersion()).to.equal(1 + initialProtocolVersion); + expect(await proxyGetters.getProtocolVersion()).to.equal(addToProtocolVersion(initialProtocolVersion, 1, 0)); storedBatch2Info = getBatchStoredInfo(batch2Info, commitment); await makeExecutedEqualCommitted(proxyExecutor, storedBatch1InfoChainIdUpgrade, [storedBatch2Info], []); }); + it("Should not allow base system contract changes during patch upgrade", async () => { + const { 0: major, 1: minor, 2: patch } = await proxyGetters.getSemverProtocolVersion(); + + const bootloaderRevertReason = await getCallRevertReason( + executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { + newProtocolVersion: packSemver(major, minor, patch + 1), + bootloaderHash: ethers.utils.hexlify(hashBytecode(ethers.utils.randomBytes(32))), + l2ProtocolUpgradeTx: noopUpgradeTransaction, + }) + ); + expect(bootloaderRevertReason).to.equal("Patch only upgrade can not set new bootloader"); + + const defaultAccountRevertReason = await getCallRevertReason( + executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { + newProtocolVersion: packSemver(major, minor, patch + 1), + defaultAccountHash: ethers.utils.hexlify(hashBytecode(ethers.utils.randomBytes(32))), + l2ProtocolUpgradeTx: noopUpgradeTransaction, + }) + ); + expect(defaultAccountRevertReason).to.equal("Patch only upgrade can not set new default account"); + }); + + it("Should not allow upgrade transaction during patch upgrade", async () => { + const { 0: major, 1: minor, 2: patch } = await proxyGetters.getSemverProtocolVersion(); + + const someTx = buildL2CanonicalTransaction({ + txType: 254, + nonce: 0, + }); + + const bootloaderRevertReason = await getCallRevertReason( + executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { + newProtocolVersion: packSemver(major, minor, patch + 1), + l2ProtocolUpgradeTx: someTx, + }) + ); + expect(bootloaderRevertReason).to.equal("Patch only upgrade can not set upgrade transaction"); + }); + + it("Should not allow major version change", async () => { + // 2**64 is the offset for a major version change + const newVersion = ethers.BigNumber.from(2).pow(64); + + const someTx = buildL2CanonicalTransaction({ + txType: 254, + nonce: 0, + }); + + const bootloaderRevertReason = await getCallRevertReason( + executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { + newProtocolVersion: newVersion, + l2ProtocolUpgradeTx: someTx, + }) + ); + expect(bootloaderRevertReason).to.equal("Major must always be 0"); + }); + it("Timestamp should behave correctly", async () => { // Upgrade was scheduled for now should work fine const timeNow = (await hardhat.ethers.provider.getBlock("latest")).timestamp; @@ -186,7 +251,7 @@ describe("L2 upgrade test", function () { const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, - newProtocolVersion: 3 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 3, 0), }) ); @@ -202,7 +267,7 @@ describe("L2 upgrade test", function () { const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, - newProtocolVersion: 3 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 4, 0), }) ); @@ -234,7 +299,7 @@ describe("L2 upgrade test", function () { const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, - newProtocolVersion: 100000, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 10000, 0), }) ); @@ -250,7 +315,7 @@ describe("L2 upgrade test", function () { const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, - newProtocolVersion: 3 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 4, 0), }) ); @@ -266,7 +331,7 @@ describe("L2 upgrade test", function () { const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, - newProtocolVersion: 3 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 4, 0), }) ); @@ -283,7 +348,7 @@ describe("L2 upgrade test", function () { const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, - newProtocolVersion: 3 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 4, 0), }) ); @@ -295,14 +360,14 @@ describe("L2 upgrade test", function () { const wrongFactoryDepHash = ethers.utils.hexlify(hashBytecode(ethers.utils.randomBytes(32))); const wrongTx = buildL2CanonicalTransaction({ factoryDeps: [wrongFactoryDepHash], - nonce: 3 + 1 + initialProtocolVersion, + nonce: 4 + initialMinorProtocolVersion, }); const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, factoryDeps: [myFactoryDep], - newProtocolVersion: 3 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 4, 0), }) ); @@ -313,14 +378,14 @@ describe("L2 upgrade test", function () { const myFactoryDep = ethers.utils.hexlify(ethers.utils.randomBytes(32)); const wrongTx = buildL2CanonicalTransaction({ factoryDeps: [], - nonce: 3 + 1 + initialProtocolVersion, + nonce: 4 + initialMinorProtocolVersion, }); const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, factoryDeps: [myFactoryDep], - newProtocolVersion: 3 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 4, 0), }) ); @@ -333,14 +398,14 @@ describe("L2 upgrade test", function () { const wrongTx = buildL2CanonicalTransaction({ factoryDeps: Array(33).fill(randomDepHash), - nonce: 3 + 1 + initialProtocolVersion, + nonce: 4 + initialMinorProtocolVersion, }); const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { l2ProtocolUpgradeTx: wrongTx, factoryDeps: Array(33).fill(myFactoryDep), - newProtocolVersion: 3 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 4, 0), }) ); @@ -364,7 +429,7 @@ describe("L2 upgrade test", function () { const myFactoryDepHash = hashBytecode(myFactoryDep); const upgradeTx = buildL2CanonicalTransaction({ factoryDeps: [myFactoryDepHash], - nonce: 4 + 1 + initialProtocolVersion, + nonce: 5 + initialMinorProtocolVersion, }); const upgrade = { @@ -375,7 +440,7 @@ describe("L2 upgrade test", function () { executeUpgradeTx: true, l2ProtocolUpgradeTx: upgradeTx, factoryDeps: [myFactoryDep], - newProtocolVersion: 4 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 5, 0), }; const upgradeReceipt = await ( @@ -402,7 +467,7 @@ describe("L2 upgrade test", function () { expect(await proxyGetters.getL2BootloaderBytecodeHash()).to.equal(bootloaderHash); expect(await proxyGetters.getL2DefaultAccountBytecodeHash()).to.equal(defaultAccountHash); expect((await proxyGetters.getVerifier()).toLowerCase()).to.equal(newVerifier.toLowerCase()); - expect(await proxyGetters.getProtocolVersion()).to.equal(4 + 1 + initialProtocolVersion); + expect(await proxyGetters.getProtocolVersion()).to.equal(addToProtocolVersion(initialProtocolVersion, 5, 0)); const newVerifierParams = await proxyGetters.getVerifierParams(); expect(newVerifierParams.recursionNodeLevelVkHash).to.equal(newerVerifierParams.recursionNodeLevelVkHash); @@ -410,8 +475,12 @@ describe("L2 upgrade test", function () { expect(newVerifierParams.recursionCircuitsSetVksHash).to.equal(newerVerifierParams.recursionCircuitsSetVksHash); expect(upgradeEvents[0].name).to.eq("NewProtocolVersion"); - expect(upgradeEvents[0].args.previousProtocolVersion.toString()).to.eq((2 + initialProtocolVersion).toString()); - expect(upgradeEvents[0].args.newProtocolVersion.toString()).to.eq((4 + 1 + initialProtocolVersion).toString()); + expect(upgradeEvents[0].args.previousProtocolVersion.toString()).to.eq( + addToProtocolVersion(initialProtocolVersion, 2, 0).toString() + ); + expect(upgradeEvents[0].args.newProtocolVersion.toString()).to.eq( + addToProtocolVersion(initialProtocolVersion, 5, 0).toString() + ); expect(upgradeEvents[1].name).to.eq("NewVerifier"); expect(upgradeEvents[1].args.oldVerifier.toLowerCase()).to.eq(verifier.toLowerCase()); @@ -434,6 +503,84 @@ describe("L2 upgrade test", function () { expect(upgradeEvents[4].args.newBytecodeHash).to.eq(defaultAccountHash); }); + it("Should successfully perform a patch upgrade even if there is a pending minor upgrade", async () => { + const currentVerifier = await proxyGetters.getVerifier(); + const currentVerifierParams = await proxyGetters.getVerifierParams(); + const currentBootloaderHash = await proxyGetters.getL2BootloaderBytecodeHash(); + const currentL2DefaultAccountBytecodeHash = await proxyGetters.getL2DefaultAccountBytecodeHash(); + + const testnetVerifierFactory = await hardhat.ethers.getContractFactory("TestnetVerifier"); + const testnetVerifierContract = await testnetVerifierFactory.deploy(); + const newVerifier = testnetVerifierContract.address; + const newerVerifierParams = buildVerifierParams({ + recursionNodeLevelVkHash: ethers.utils.hexlify(ethers.utils.randomBytes(32)), + recursionLeafLevelVkHash: ethers.utils.hexlify(ethers.utils.randomBytes(32)), + recursionCircuitsSetVksHash: ethers.utils.hexlify(ethers.utils.randomBytes(32)), + }); + + const emptyTx = buildL2CanonicalTransaction({ + txType: 0, + nonce: 0, + }); + + const upgrade = { + verifier: newVerifier, + verifierParams: newerVerifierParams, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 5, 1), + l2ProtocolUpgradeTx: emptyTx, + }; + + const upgradeReceipt = await ( + await executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, upgrade) + ).wait(); + + const defaultUpgradeFactory = await hardhat.ethers.getContractFactory("DefaultUpgrade"); + const upgradeEvents = upgradeReceipt.logs.map((log) => { + // Not all events can be parsed there, but we don't care about them + try { + const event = defaultUpgradeFactory.interface.parseLog(log); + const parsedArgs = event.args; + return { + name: event.name, + args: parsedArgs, + }; + } catch (_) { + // lint no-empty + } + }); + + // Now, we check that all the data was set as expected + expect(await proxyGetters.getL2BootloaderBytecodeHash()).to.equal(currentBootloaderHash); + expect(await proxyGetters.getL2DefaultAccountBytecodeHash()).to.equal(currentL2DefaultAccountBytecodeHash); + expect((await proxyGetters.getVerifier()).toLowerCase()).to.equal(newVerifier.toLowerCase()); + expect(await proxyGetters.getProtocolVersion()).to.equal(addToProtocolVersion(initialProtocolVersion, 5, 1)); + + const newVerifierParams = await proxyGetters.getVerifierParams(); + expect(newVerifierParams.recursionNodeLevelVkHash).to.equal(newerVerifierParams.recursionNodeLevelVkHash); + expect(newVerifierParams.recursionLeafLevelVkHash).to.equal(newerVerifierParams.recursionLeafLevelVkHash); + expect(newVerifierParams.recursionCircuitsSetVksHash).to.equal(newerVerifierParams.recursionCircuitsSetVksHash); + + expect(upgradeEvents[0].name).to.eq("NewProtocolVersion"); + expect(upgradeEvents[0].args.previousProtocolVersion.toString()).to.eq( + addToProtocolVersion(initialProtocolVersion, 5, 0).toString() + ); + expect(upgradeEvents[0].args.newProtocolVersion.toString()).to.eq( + addToProtocolVersion(initialProtocolVersion, 5, 1).toString() + ); + + expect(upgradeEvents[1].name).to.eq("NewVerifier"); + expect(upgradeEvents[1].args.oldVerifier.toLowerCase()).to.eq(currentVerifier.toLowerCase()); + expect(upgradeEvents[1].args.newVerifier.toLowerCase()).to.eq(newVerifier.toLowerCase()); + + expect(upgradeEvents[2].name).to.eq("NewVerifierParams"); + expect(upgradeEvents[2].args.oldVerifierParams[0]).to.eq(currentVerifierParams.recursionNodeLevelVkHash); + expect(upgradeEvents[2].args.oldVerifierParams[1]).to.eq(currentVerifierParams.recursionLeafLevelVkHash); + expect(upgradeEvents[2].args.oldVerifierParams[2]).to.eq(currentVerifierParams.recursionCircuitsSetVksHash); + expect(upgradeEvents[2].args.newVerifierParams[0]).to.eq(newerVerifierParams.recursionNodeLevelVkHash); + expect(upgradeEvents[2].args.newVerifierParams[1]).to.eq(newerVerifierParams.recursionLeafLevelVkHash); + expect(upgradeEvents[2].args.newVerifierParams[2]).to.eq(newerVerifierParams.recursionCircuitsSetVksHash); + }); + it("Should fail to upgrade when there is already a pending upgrade", async () => { const bootloaderHash = ethers.utils.hexlify(hashBytecode(ethers.utils.randomBytes(32))); const defaultAccountHash = ethers.utils.hexlify(hashBytecode(ethers.utils.randomBytes(32))); @@ -448,7 +595,7 @@ describe("L2 upgrade test", function () { const myFactoryDepHash = hashBytecode(myFactoryDep); const upgradeTx = buildL2CanonicalTransaction({ factoryDeps: [myFactoryDepHash], - nonce: 5 + 1 + initialProtocolVersion, + nonce: 5 + 1 + initialMinorProtocolVersion, }); const upgrade = { @@ -459,12 +606,16 @@ describe("L2 upgrade test", function () { executeUpgradeTx: true, l2ProtocolUpgradeTx: upgradeTx, factoryDeps: [myFactoryDep], - newProtocolVersion: 5 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 5 + 1, 0), }; const revertReason = await getCallRevertReason( executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, upgrade) ); - await rollBackToVersion((4 + 1 + initialProtocolVersion).toString(), stateTransitionManager, upgrade); + await rollBackToVersion( + addToProtocolVersion(initialProtocolVersion, 5, 1).toString(), + stateTransitionManager, + upgrade + ); expect(revertReason).to.equal("Previous upgrade has not been finalized"); }); @@ -631,7 +782,7 @@ describe("L2 upgrade test", function () { expect(await proxyGetters.getL2SystemContractsUpgradeBatchNumber()).to.equal(0); await ( await executeUpgrade(chainId, proxyGetters, stateTransitionManager, proxyAdmin, { - newProtocolVersion: 5 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 5 + 1, 0), l2ProtocolUpgradeTx: noopUpgradeTransaction, }) ).wait(); @@ -670,7 +821,7 @@ describe("L2 upgrade test", function () { it("Should successfully commit custom upgrade", async () => { const upgradeReceipt = await ( await executeCustomUpgrade(chainId, proxyGetters, proxyAdmin, stateTransitionManager, { - newProtocolVersion: 6 + 1 + initialProtocolVersion, + newProtocolVersion: addToProtocolVersion(initialProtocolVersion, 6 + 1, 0), l2ProtocolUpgradeTx: noopUpgradeTransaction, }) ).wait(); @@ -811,7 +962,8 @@ async function executeUpgrade( contractFactory?: ethers.ethers.ContractFactory ) { if (partialUpgrade.newProtocolVersion == null) { - const newVersion = (await proxyGetters.getProtocolVersion()).add(1); + const { 0: major, 1: minor, 2: patch } = await proxyGetters.getSemverProtocolVersion(); + const newVersion = packSemver(major, minor + 1, patch); partialUpgrade.newProtocolVersion = newVersion; } const upgrade = buildProposeUpgrade(partialUpgrade); diff --git a/l1-contracts/test/unit_tests/utils.ts b/l1-contracts/test/unit_tests/utils.ts index fd34a0e7f..f1a2b49f2 100644 --- a/l1-contracts/test/unit_tests/utils.ts +++ b/l1-contracts/test/unit_tests/utils.ts @@ -12,8 +12,9 @@ import type { ExecutorFacet } from "../../typechain"; import type { FeeParams, L2CanonicalTransaction } from "../../src.ts/utils"; import { ADDRESS_ONE, PubdataPricingMode, EMPTY_STRING_KECCAK } from "../../src.ts/utils"; +import { packSemver } from "../../scripts/utils"; -export const CONTRACTS_GENESIS_PROTOCOL_VERSION = (21).toString(); +export const CONTRACTS_GENESIS_PROTOCOL_VERSION = packSemver(0, 21, 0).toString(); // eslint-disable-next-line @typescript-eslint/no-var-requires export const IERC20_INTERFACE = require("@openzeppelin/contracts/build/contracts/IERC20"); export const DEFAULT_REVERT_REASON = "VM did not revert";