Skip to content

Commit cee3232

Browse files
StanislavBreadlesszksync[bot]
andauthored
Add more comments and asserts (#2058)
# What ❔ <!-- What are the changes this PR brings about? --> <!-- Example: This PR adds a PR template to the repo. --> <!-- (For bigger PRs adding more context is appreciated) --> ## Why ❔ <!-- Why are these changes done? What goal do they contribute to? What are the principles behind them? --> <!-- Example: PR templates ensure PR reviewers, observers, and future iterators are in context about the evolution of repos. --> ## Checklist <!-- Check your PR fulfills the following items. --> <!-- For draft PRs check the boxes as you complete them. --> - [ ] PR title corresponds to the body of PR (we generate changelog entries from PRs). - [ ] Tests for the changes have been added / updated. - [ ] Documentation comments have been added / updated. --------- Co-authored-by: zksync[bot] <zksync[bot]@users.noreply.github.com>
1 parent 5767133 commit cee3232

File tree

12 files changed

+505
-474
lines changed

12 files changed

+505
-474
lines changed

AllContractsHashes.json

Lines changed: 362 additions & 362 deletions
Large diffs are not rendered by default.

l1-contracts/contracts/bridge/asset-tracker/GWAssetTracker.sol

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,9 @@ contract GWAssetTracker is AssetTrackerBase, IGWAssetTracker {
352352
}
353353
}
354354
}
355+
if (msgCount != _processLogsInputs.messages.length) {
356+
revert InvalidMessage();
357+
}
355358
reconstructedLogsTree.extendUntilEnd();
356359
bytes32 localLogsRootHash = reconstructedLogsTree.root();
357360

l1-contracts/contracts/common/L1ContractErrors.sol

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,8 @@ error MockVerifierNotSupported();
250250
error MsgValueMismatch(uint256 expectedMsgValue, uint256 providedMsgValue);
251251
// 0xb385a3da
252252
error MsgValueTooLow(uint256 required, uint256 provided);
253+
// 0xedd74330
254+
error MustBeEraChain();
253255
// 0x8b7e144a
254256
error NewDeadlineExceedsMaxDeadline();
255257
// 0x6eef58d1

l1-contracts/contracts/core/bridgehub/L1Bridgehub.sol

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,11 @@ contract L1Bridgehub is BridgehubBase, IL1Bridgehub {
132132
_initData: _initData,
133133
_factoryDeps: _factoryDeps
134134
});
135+
// It is an additional protection against a malicious chain type manager
136+
if (chainAddress == address(0)) {
137+
revert ZeroAddress();
138+
}
139+
135140
_registerNewZKChain(_chainId, chainAddress, true);
136141
messageRoot.addNewChain(_chainId, 0);
137142

l1-contracts/contracts/core/chain-registration/ChainRegistrationSender.sol

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,9 @@ contract ChainRegistrationSender is
123123
/// @return the L2 transaction calldata
124124
function _getL2TxCalldata(uint256 chainToBeRegistered) internal view returns (bytes memory) {
125125
bytes32 baseTokenAssetId = BRIDGE_HUB.baseTokenAssetId(chainToBeRegistered);
126+
if (baseTokenAssetId == bytes32(0)) {
127+
revert ZKChainNotRegistered();
128+
}
126129
return abi.encodeCall(IL2Bridgehub.registerChainForInterop, (chainToBeRegistered, baseTokenAssetId));
127130
}
128131

l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,7 +435,7 @@ contract AdminFacet is ZKChainBase, IAdmin {
435435
}
436436

437437
/// @inheritdoc IAdmin
438-
function allowEvmEmulation() external onlyAdmin onlyL1 returns (bytes32 canonicalTxHash) {
438+
function allowEvmEmulation() external onlyAdmin onlyL1 notPriorityMode onlyEra returns (bytes32 canonicalTxHash) {
439439
canonicalTxHash = IMailbox(address(this)).requestL2ServiceTransaction(
440440
L2_DEPLOYER_SYSTEM_CONTRACT_ADDR,
441441
abi.encodeCall(IL2ContractDeployer.setAllowedBytecodeTypesToDeploy, AllowedBytecodeTypes.EraVmAndEVM)

l1-contracts/contracts/state-transition/chain-deps/facets/ZKChainBase.sol

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ import {
1212
BaseTokenGasPriceDenominatorNotSet,
1313
Unauthorized,
1414
OnlyNormalMode,
15-
OnlyPriorityMode
15+
OnlyPriorityMode,
16+
MustBeEraChain
1617
} from "../../../common/L1ContractErrors.sol";
1718
import {GW_ASSET_TRACKER_ADDR, L2_INTEROP_CENTER_ADDR} from "../../../common/l2-helpers/L2ContractAddresses.sol";
1819
import {IL1Bridgehub} from "../../../core/bridgehub/IL1Bridgehub.sol";
@@ -70,6 +71,12 @@ contract ZKChainBase is ReentrancyGuard {
7071
_;
7172
}
7273

74+
/// @notice Ensures that the chain uses EraVM
75+
modifier onlyEra() {
76+
require(!s.zksyncOS, MustBeEraChain());
77+
_;
78+
}
79+
7380
/// @notice Allows whitelisted validators, or the `PermissionlessValidator` when Priority Mode is active.
7481
/// @dev Reverts with {Unauthorized} if `msg.sender` is not authorized for the current mode.
7582
modifier onlyValidatorOrPriorityMode() {

l1-contracts/contracts/state-transition/data-availability/RollupDAManager.sol

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,12 @@ import {L2DACommitmentScheme} from "../../common/Config.sol";
1010
/// @custom:security-contact security@matterlabs.dev
1111
/// @notice Responsible for determining which DA configurations (DAPairs) are allowed to be used
1212
/// for permanent rollups.
13+
/// @dev IMPORTANT: Rollup DA Manager must contain only compatible versions for a single protocol version.
14+
/// I.e. a separate `RollupDAManager` needs to be deployed per protocol version / chain type (ZKsync OS vs Era)
15+
/// as otherwise a malicious chain admin could switch the DA configuration to an incompatible one, causing the rollup to be unable to verify blocks and thus halt.
16+
/// This is an issue for stage1 since it may disable `PermissionlessValidator` from settling new batches.
17+
/// It is okay if two protocol versions have the same rollup DA manager if they have *exactly same* set of compatible
18+
/// DA validator pairs.
1319
contract RollupDAManager is Ownable2Step {
1420
/// @dev Mapping to track the status (enabled/disabled) of each DAPair.
1521
mapping(address l1DAValidator => mapping(L2DACommitmentScheme l2DACommitmentScheme => bool))

l1-contracts/selectors

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,8 @@ AdminFacet
257257
|----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
258258
| Error | L1DAValidatorAddressIsZero() | 0x2e89f517 |
259259
|----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
260+
| Error | MustBeEraChain() | 0xedd74330 |
261+
|----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
260262
| Error | NoFunctionsForDiamondCut() | 0xa6fef710 |
261263
|----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
262264
| Error | NonEmptyCalldata() | 0xc21b1ab7 |
@@ -445,6 +447,8 @@ AdminFacetTest
445447
|----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
446448
| Error | L1DAValidatorAddressIsZero() | 0x2e89f517 |
447449
|----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
450+
| Error | MustBeEraChain() | 0xedd74330 |
451+
|----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
448452
| Error | NoFunctionsForDiamondCut() | 0xa6fef710 |
449453
|----------+-----------------------------------------------------------------------------------------------------+--------------------------------------------------------------------|
450454
| Error | NonEmptyCalldata() | 0xc21b1ab7 |

l1-contracts/test/foundry/l1/integration/deploy-scripts/script-config/config-gateway-vote-preparation.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ force_deployments_data = "0x00"
22
gateway_chain_id = 506
33
gateway_settlement_fee = 0
44
is_zk_sync_os = true
5-
owner_address = "0xFf5024F86550361E72667aa4070FC1F0850E758e"
5+
owner_address = "0x2Ae2CC9d9344Fd83d4Ca8D0011E17407449c702a"
66
refund_recipient = "0x000000000000000000000000000000000000bEEF"
77
support_l2_legacy_shared_bridge_test = false
88
testnet_verifier = true

0 commit comments

Comments
 (0)