From d05ff616585f6771787ae79eec47dbeef2088883 Mon Sep 17 00:00:00 2001 From: Stanislav Bezkorovainyi Date: Mon, 24 Jun 2024 10:29:58 +0200 Subject: [PATCH] Some clarification (#541) --- .../contracts/bridgehub/Bridgehub.sol | 8 +-- .../contracts/bridgehub/IMessageRoot.sol | 2 +- .../contracts/bridgehub/MessageRoot.sol | 68 +++++++++++++++---- .../bridgehub/STMDeploymentTracker.sol | 24 ++++--- l1-contracts/contracts/common/Config.sol | 2 + .../IStateTransitionManager.sol | 11 ++- .../StateTransitionManager.sol | 13 ++-- .../chain-deps/facets/Admin.sol | 6 +- .../chain-interfaces/IAdmin.sol | 8 +-- l1-contracts/scripts/sync-layer.ts | 4 +- .../test/unit_tests/synclayer.spec.ts | 5 +- 11 files changed, 98 insertions(+), 53 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index aa7675bc3..3ce6c847d 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -457,11 +457,11 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus require(settlementLayer[_chainId] == block.chainid, "BH: not current SL"); settlementLayer[_chainId] = _settlementChainId; - bytes memory stmMintData = IStateTransitionManager(stateTransitionManager[_chainId]).bridgeBurn( + bytes memory stmMintData = IStateTransitionManager(stateTransitionManager[_chainId]).forwardedBridgeBurn( _chainId, _stmData ); - bytes memory chainMintData = IZkSyncHyperchain(getHyperchain(_chainId)).bridgeBurn( + bytes memory chainMintData = IZkSyncHyperchain(getHyperchain(_chainId)).forwardedBridgeBurn( getHyperchain(_settlementChainId), _prevMsgSender, _chainData @@ -487,10 +487,10 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus stateTransitionManager[_chainId] = stm; address hyperchain = getHyperchain(_chainId); if (hyperchain == address(0)) { - hyperchain = IStateTransitionManager(stm).bridgeMint(_chainId, _stmData); + hyperchain = IStateTransitionManager(stm).forwardedBridgeMint(_chainId, _stmData); } - IZkSyncHyperchain(hyperchain).bridgeMint(_chainMintData); + IZkSyncHyperchain(hyperchain).forwardedBridgeMint(_chainMintData); return address(0); } diff --git a/l1-contracts/contracts/bridgehub/IMessageRoot.sol b/l1-contracts/contracts/bridgehub/IMessageRoot.sol index 63ac8a601..4cf57dd21 100644 --- a/l1-contracts/contracts/bridgehub/IMessageRoot.sol +++ b/l1-contracts/contracts/bridgehub/IMessageRoot.sol @@ -11,5 +11,5 @@ interface IMessageRoot { function chainMessageRoot(uint256 _chainId) external view returns (bytes32); - function addChainBatchRoot(uint256 _chainId, bytes32 _chainBatchRoot, bool _updateFullTree) external; + function addChainBatchRoot(uint256 _chainId, bytes32 _chainBatchRoot) external; } diff --git a/l1-contracts/contracts/bridgehub/MessageRoot.sol b/l1-contracts/contracts/bridgehub/MessageRoot.sol index 544e1c35f..f8d1e93f7 100644 --- a/l1-contracts/contracts/bridgehub/MessageRoot.sol +++ b/l1-contracts/contracts/bridgehub/MessageRoot.sol @@ -21,6 +21,11 @@ import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {FullMerkle} from "../common/libraries/FullMerkle.sol"; +import {MAX_NUMBER_OF_HYPERCHAINS} from "../common/Config.sol"; + +// FIXME: explain why it can not collide with real root +bytes32 constant EMPTY_LOG_ROOT = keccak256("New Tree zero hash"); + contract MessageRoot is IMessageRoot, ReentrancyGuard, Ownable2StepUpgradeable, PausableUpgradeable { using FullMerkle for FullMerkle.FullTree; using DynamicIncrementalMerkle for DynamicIncrementalMerkle.Bytes32PushTree; @@ -67,11 +72,14 @@ contract MessageRoot is IMessageRoot, ReentrancyGuard, Ownable2StepUpgradeable, } function addNewChain(uint256 _chainId) external onlyBridgehub { - chainIndex[_chainId] = chainCount; - chainIndexToId[chainCount] = _chainId; + uint256 cachedChainCount = chainCount; + require(cachedChainCount < MAX_NUMBER_OF_HYPERCHAINS, "MR: too many chains"); + + chainIndex[_chainId] = cachedChainCount; + chainIndexToId[cachedChainCount] = _chainId; ++chainCount; // slither-disable-next-line unused-return - bytes32 initialHash = chainTree[_chainId].setup(keccak256("New Tree zero hash")); + bytes32 initialHash = chainTree[_chainId].setup(EMPTY_LOG_ROOT); // slither-disable-next-line unused-return sharedTree.pushNewLeaf(initialHash); } @@ -81,27 +89,57 @@ contract MessageRoot is IMessageRoot, ReentrancyGuard, Ownable2StepUpgradeable, } /// @dev add a new chainBatchRoot to the chainTree - function addChainBatchRoot( - uint256 _chainId, - bytes32 _chainBatchRoot, - bool _updateFullTree - ) external onlyChain(_chainId) { + function addChainBatchRoot(uint256 _chainId, bytes32 _chainBatchRoot) external onlyChain(_chainId) { bytes32 chainRoot; // slither-disable-next-line unused-return (, chainRoot) = chainTree[_chainId].push(_chainBatchRoot); - if (_updateFullTree) { - // slither-disable-next-line unused-return - sharedTree.updateLeaf(chainIndex[_chainId], chainRoot); - } + // slither-disable-next-line unused-return + sharedTree.updateLeaf(chainIndex[_chainId], chainRoot); } - function updateFullTree() external { - bytes32[] memory newLeaves; - for (uint256 i = 0; i < chainCount; ++i) { + function updateFullTree() public { + uint256 cachedChainCount = chainCount; + bytes32[] memory newLeaves = new bytes32[](cachedChainCount); + for (uint256 i = 0; i < cachedChainCount; ++i) { newLeaves[i] = chainTree[chainIndexToId[i]].root(); } // slither-disable-next-line unused-return sharedTree.updateAllLeaves(newLeaves); } + + /// @notice To be called by the bootloader by the L1Messenger at the end of the batch to produce the final root and send it to the underlying layer. + /// @return pubdata The pubdata to be relayed to the DA layer. + function clearTreeAndProvidePubdata() external returns (bytes memory pubdata) { + // FIXME: access control: only to be called by the l1 messenger. + + uint256 cachedChainCount = chainCount; + + // We will send the updated roots for all chains. + // While it will mean that we'll pay even for unchanged roots: + // - It is the simplest approach + // - The alternative is to send pairs of (chainId, root), which is less efficient if at least half of the chains are active. + // + // There are of course ways to optimize it further, but it will be done in the future. + bytes memory pubdata = new bytes(cachedChainCount * 32); + + for (uint256 i = 0; i < cachedChainCount; i++) { + // It is the responsibility of each chain to provide the roots of its L2->L1 messages if it wants to see those. + // However, for the security of the system as a whole, the chain roots need to be provided for all chains. + + bytes32 chainRoot = chainTree[chainIndexToId[i]].root(); + + assembly { + mstore(add(pubdata, add(32, mul(i, 32))), chainRoot) + } + + // Clearing up the state. + // Note that it *does not* delete any storage slots, so in terms of pubdata savings, it is useless. + // However, the chains paid for these changes anyway, so it is considered acceptable. + // In the future, further optimizations will be available. + chainTree[chainIndexToId[i]].setup(EMPTY_LOG_ROOT); + } + + updateFullTree(); + } } diff --git a/l1-contracts/contracts/bridgehub/STMDeploymentTracker.sol b/l1-contracts/contracts/bridgehub/STMDeploymentTracker.sol index a55bb03bf..b4392d92e 100644 --- a/l1-contracts/contracts/bridgehub/STMDeploymentTracker.sol +++ b/l1-contracts/contracts/bridgehub/STMDeploymentTracker.sol @@ -52,7 +52,16 @@ contract STMDeploymentTracker is ISTMDeploymentTracker, ReentrancyGuard, Ownable BRIDGE_HUB.setAssetHandlerAddressInitial(bytes32(uint256(uint160(_stmAddress))), _stmAddress); } - /// @dev registerSTMAssetOnL2SharedBridge, use via requestL2TransactionTwoBridges + /// @notice The function responsible for registering the L2 counterpart of an STM asset on the L2 Bridgehub. + /// @dev The function is called by the Bridgehub contract during the `Bridgehub.requestL2TransactionTwoBridges`. + /// @dev Since the L2 settlment layers `_chainId` might potentially have ERC20 tokens as native assets, + /// there are two ways to perform the L1->L2 transaction: + /// - via the `Bridgehub.requestL2TransactionDirect`. However, this would require the STMDeploymentTracker to + /// hahndle the ERC20 balances to be used in the transaction. + /// - via the `Bridgehub.requestL2TransactionTwoBridges`. This way it will be the sender that provides the funds + /// for the L2 transaction. + /// The second approach is used due to its simplicity even though it gives the sender slightly more control over the call: + /// `gasLimit`, etc. // slither-disable-next-line locked-ether function bridgehubDeposit( uint256 _chainId, @@ -66,14 +75,9 @@ contract STMDeploymentTracker is ISTMDeploymentTracker, ReentrancyGuard, Ownable // solhint-disable-next-line gas-custom-errors require(_prevMsgSender == owner(), "STMDT: not owner"); - (bool _registerOnL2Bridgehub, address _stmL1Address, address _stmL2Address) = abi.decode( - _data, - (bool, address, address) - ); + (address _stmL1Address, address _stmL2Address) = abi.decode(_data, (address, address)); - if (_registerOnL2Bridgehub) { - request = _registerSTMAssetOnL2Bridgehub(_chainId, _stmL1Address, _stmL2Address); - } + request = _registerSTMAssetOnL2Bridgehub(_chainId, _stmL1Address, _stmL2Address); } // todo this has to be put in L1SharedBridge via TwoBridges. Hard, because we have to have multiple msg types @@ -125,7 +129,9 @@ contract STMDeploymentTracker is ISTMDeploymentTracker, ReentrancyGuard, Ownable } /// @dev we need to implement this for the bridgehub - function bridgehubConfirmL2Transaction(uint256 _chainId, bytes32 _txDataHash, bytes32 _txHash) external {} + function bridgehubConfirmL2Transaction(uint256 _chainId, bytes32 _txDataHash, bytes32 _txHash) external { + // This function is typically used on bridges for e.g. + } function getAssetId(address _l1STM) public view override returns (bytes32) { return keccak256(abi.encode(block.chainid, address(this), bytes32(uint256(uint160(_l1STM))))); diff --git a/l1-contracts/contracts/common/Config.sol b/l1-contracts/contracts/common/Config.sol index 5b7c56be4..9d46af1a8 100644 --- a/l1-contracts/contracts/common/Config.sol +++ b/l1-contracts/contracts/common/Config.sol @@ -116,6 +116,8 @@ address constant NATIVE_TOKEN_VAULT_VIRTUAL_ADDRESS = address(2); /// @dev https://eips.ethereum.org/EIPS/eip-1352 address constant BRIDGEHUB_MIN_SECOND_BRIDGE_ADDRESS = address(uint160(type(uint16).max)); +uint256 constant MAX_NUMBER_OF_HYPERCHAINS = 100; + /// FIXME: move to a different file struct StoredBatchHashInfo { diff --git a/l1-contracts/contracts/state-transition/IStateTransitionManager.sol b/l1-contracts/contracts/state-transition/IStateTransitionManager.sol index 5039b1b55..aa9c7b01d 100644 --- a/l1-contracts/contracts/state-transition/IStateTransitionManager.sol +++ b/l1-contracts/contracts/state-transition/IStateTransitionManager.sol @@ -154,13 +154,12 @@ interface IStateTransitionManager { event BridgeInitialize(address indexed l1Token, string name, string symbol, uint8 decimals); - event BridgeMint(address indexed _account, uint256 _amount); - - event BridgeBurn(address indexed _account, uint256 _amount); - - function bridgeBurn(uint256 _chainId, bytes calldata _data) external returns (bytes memory _bridgeMintData); + function forwardedBridgeBurn( + uint256 _chainId, + bytes calldata _data + ) external returns (bytes memory _bridgeMintData); - function bridgeMint(uint256 _chainId, bytes calldata _data) external returns (address); + function forwardedBridgeMint(uint256 _chainId, bytes calldata _data) external returns (address); function bridgeClaimFailedBurn( uint256 _chainId, diff --git a/l1-contracts/contracts/state-transition/StateTransitionManager.sol b/l1-contracts/contracts/state-transition/StateTransitionManager.sol index a842d9e48..2aabb97f7 100644 --- a/l1-contracts/contracts/state-transition/StateTransitionManager.sol +++ b/l1-contracts/contracts/state-transition/StateTransitionManager.sol @@ -414,18 +414,23 @@ contract StateTransitionManager is IStateTransitionManager, ReentrancyGuard, Own // TODO: emit event } - /// @dev we can move assets using these - function bridgeBurn( + /// @notice Called by the bridgehub during the migration of a chain to another settlement layer. + /// @param _chainId The chain id of the chain to be migrated. + /// @param _data The data needed to perform the migration. + function forwardedBridgeBurn( uint256 _chainId, bytes calldata _data - ) external view override onlyBridgehub returns (bytes memory stmBridgeMintData) { + ) external view override onlyBridgehub returns (bytes memory stmForwardedBridgeMintData) { (address _newSyncLayerAdmin, bytes memory _diamondCut) = abi.decode(_data, (address, bytes)); require(_newSyncLayerAdmin != address(0), "STM: admin zero"); // todo check protocol version return abi.encode(IBridgehub(BRIDGE_HUB).baseToken(_chainId), _newSyncLayerAdmin, protocolVersion, _diamondCut); } - function bridgeMint( + /// @notice Called by the bridgehub during the migration of a chain to the current settlement layer. + /// @param _chainId The chain id of the chain to be migrated. + /// @param _stmData The data returned from `forwardedBridgeBurn` for the chain. + function forwardedBridgeMint( uint256 _chainId, bytes calldata _stmData ) external override onlyBridgehub returns (address chainAddress) { diff --git a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol index 0d16bffa8..34cfbac84 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol @@ -242,7 +242,7 @@ contract AdminFacet is ZkSyncHyperchainBase, IAdmin { //////////////////////////////////////////////////////////////*/ /// @dev we can move assets using these - function bridgeBurn( + function forwardedBridgeBurn( address _syncLayer, address _prevMsgSender, bytes calldata @@ -264,7 +264,7 @@ contract AdminFacet is ZkSyncHyperchainBase, IAdmin { chainBridgeMintData = abi.encode(_prepareChainCommitment()); } - function bridgeMint(bytes calldata _data) external payable override { + function forwardedBridgeMint(bytes calldata _data) external payable override { HyperchainCommitment memory _commitment = abi.decode(_data, (HyperchainCommitment)); uint256 batchesExecuted = _commitment.totalBatchesExecuted; @@ -310,7 +310,7 @@ contract AdminFacet is ZkSyncHyperchainBase, IAdmin { emit MigrationComplete(); } - function bridgeClaimFailedBurn( + function forwardedBridgeClaimFailedBurn( uint256 _chainId, bytes32 _assetInfo, address _prevMsgSender, diff --git a/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol b/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol index d74951d05..3f75f40f8 100644 --- a/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol +++ b/l1-contracts/contracts/state-transition/chain-interfaces/IAdmin.sol @@ -123,20 +123,18 @@ interface IAdmin is IZkSyncHyperchainBase { event BridgeMint(address indexed _account, uint256 _amount); - event BridgeBurn(address indexed _account, uint256 _amount); - - function bridgeBurn( + function forwardedBridgeBurn( address _syncLayer, address _prevMsgSender, bytes calldata _data ) external payable returns (bytes memory _bridgeMintData); - function bridgeClaimFailedBurn( + function forwardedBridgeClaimFailedBurn( uint256 _chainId, bytes32 _assetInfo, address _prevMsgSender, bytes calldata _data ) external payable; - function bridgeMint(bytes calldata _data) external payable; + function forwardedBridgeMint(bytes calldata _data) external payable; } diff --git a/l1-contracts/scripts/sync-layer.ts b/l1-contracts/scripts/sync-layer.ts index 59c761918..6179d83e8 100644 --- a/l1-contracts/scripts/sync-layer.ts +++ b/l1-contracts/scripts/sync-layer.ts @@ -384,8 +384,8 @@ async function registerSLContractsOnL1(deployer: Deployer) { secondBridgeAddress: stmDeploymentTracker.address, secondBridgeValue: 0, secondBridgeCalldata: ethers.utils.defaultAbiCoder.encode( - ["bool", "address", "address"], - [true, l1STM.address, getAddressFromEnv("SYNC_LAYER_STATE_TRANSITION_PROXY_ADDR")] + ["address", "address"], + [l1STM.address, getAddressFromEnv("SYNC_LAYER_STATE_TRANSITION_PROXY_ADDR")] ), }, ]) diff --git a/l1-contracts/test/unit_tests/synclayer.spec.ts b/l1-contracts/test/unit_tests/synclayer.spec.ts index d3d0bdc49..013f72639 100644 --- a/l1-contracts/test/unit_tests/synclayer.spec.ts +++ b/l1-contracts/test/unit_tests/synclayer.spec.ts @@ -124,10 +124,7 @@ describe("Synclayer", function () { refundRecipient: migratingDeployer.deployWallet.address, secondBridgeAddress: stmDeploymentTracker.address, secondBridgeValue: 0, - secondBridgeCalldata: ethers.utils.defaultAbiCoder.encode( - ["bool", "address", "address"], - [true, stm.address, stm.address] - ), + secondBridgeCalldata: ethers.utils.defaultAbiCoder.encode(["address", "address"], [stm.address, stm.address]), }, ]) );