From 8532ada784cc7e2454a4de83723ad01205a89310 Mon Sep 17 00:00:00 2001 From: Vlad Bochok <41153528+vladbochok@users.noreply.github.com> Date: Wed, 12 Jun 2024 13:53:29 +0200 Subject: [PATCH] Fix audit issue L-02 (#433) --- .../contracts/bridge/L1SharedBridge.sol | 19 +++++++++++++++-- .../contracts/bridgehub/Bridgehub.sol | 21 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/bridge/L1SharedBridge.sol b/l1-contracts/contracts/bridge/L1SharedBridge.sol index 6d89fb5cb..0651380f8 100644 --- a/l1-contracts/contracts/bridge/L1SharedBridge.sol +++ b/l1-contracts/contracts/bridge/L1SharedBridge.sol @@ -148,7 +148,7 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade } /// @dev This sets the first post upgrade batch for era, used to check old withdrawals - /// @param _eraLegacyBridgeLastDepositBatch The the zkSync Era batch number that processes the last deposit tx initiated by the legacy bridge + /// @param _eraLegacyBridgeLastDepositBatch The the zkSync Era batch number that processes the last deposit tx initiated by the legacy bridge /// @param _eraLegacyBridgeLastDepositTxNumber The tx number in the _eraLegacyBridgeLastDepositBatch of the last deposit tx initiated by the legacy bridge function setEraLegacyBridgeLastDepositTime( uint256 _eraLegacyBridgeLastDepositBatch, @@ -160,7 +160,10 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade eraLegacyBridgeLastDepositTxNumber = _eraLegacyBridgeLastDepositTxNumber; } - /// @dev transfer tokens from legacy erc20 bridge or mailbox and set chainBalance as part of migration process + /// @dev Transfer tokens from legacy erc20 bridge or mailbox and set chainBalance as part of migration process. + /// @param _token The address of token to be transferred (address(1) for ether and contract address for ERC20). + /// @param _target The hyperchain or bridge contract address from where to transfer funds. + /// @param _targetChainId The chain ID of the corresponding hyperchain. function transferFundsFromLegacy(address _token, address _target, uint256 _targetChainId) external onlyOwner { if (_token == ETH_TOKEN_ADDRESS) { uint256 balanceBefore = address(this).balance; @@ -195,6 +198,10 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade /// @notice Allows bridgehub to acquire mintValue for L1->L2 transactions. /// @dev If the corresponding L2 transaction fails, refunds are issued to a refund recipient on L2. + /// @param _chainId The chain ID of the hyperchain to which deposit. + /// @param _prevMsgSender The `msg.sender` address from the external call that initiated current one. + /// @param _l1Token The L1 token address which is deposited. + /// @param _amount The total amount of tokens to be bridged. function bridgehubDepositBaseToken( uint256 _chainId, address _prevMsgSender, @@ -230,6 +237,10 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade } /// @notice Initiates a deposit transaction within Bridgehub, used by `requestL2TransactionTwoBridges`. + /// @param _chainId The chain ID of the hyperchain to which deposit. + /// @param _prevMsgSender The `msg.sender` address from the external call that initiated current one. + /// @param _l2Value The L2 `msg.value` from the L1 -> L2 deposit transaction. + /// @param _data The calldata for the second bridge deposit. function bridgehubDeposit( uint256 _chainId, address _prevMsgSender, @@ -295,6 +306,9 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade /// @notice Confirms the acceptance of a transaction by the Mailbox, as part of the L2 transaction process within Bridgehub. /// This function is utilized by `requestL2TransactionTwoBridges` to validate the execution of a transaction. + /// @param _chainId The chain ID of the hyperchain to which confirm the deposit. + /// @param _txDataHash The keccak256 hash of abi.encode(msgSender, l1Token, amount) + /// @param _txHash The hash of the L1->L2 transaction to confirm the deposit. function bridgehubConfirmL2Transaction( uint256 _chainId, bytes32 _txDataHash, @@ -645,6 +659,7 @@ contract L1SharedBridge is IL1SharedBridge, ReentrancyGuard, Ownable2StepUpgrade /// of processing an L2 transaction where tokens would be minted. /// @dev If the token is bridged for the first time, the L2 token contract will be deployed. Note however, that the /// newly-deployed token does not support any custom logic, i.e. rebase tokens' functionality is not supported. + /// @param _prevMsgSender The `msg.sender` address from the external call that initiated current one. /// @param _l2Receiver The account address that should receive funds on L2 /// @param _l1Token The L1 token address which is deposited /// @param _amount The total amount of tokens to be bridged diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index 58dd91304..c88d85622 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -158,6 +158,12 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus //// Mailbox forwarder /// @notice forwards function call to Mailbox based on ChainId + /// @param _chainId The chain ID of the hyperchain where to prove L2 message inclusion. + /// @param _batchNumber The executed L2 batch number in which the message appeared + /// @param _index The position in the L2 logs Merkle tree of the l2Log that was sent with the message + /// @param _message Information about the sent message: sender address, the message itself, tx index in the L2 batch where the message was sent + /// @param _proof Merkle proof for inclusion of L2 log that was sent with the message + /// @return Whether the proof is valid function proveL2MessageInclusion( uint256 _chainId, uint256 _batchNumber, @@ -170,6 +176,12 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus } /// @notice forwards function call to Mailbox based on ChainId + /// @param _chainId The chain ID of the hyperchain where to prove L2 log inclusion. + /// @param _batchNumber The executed L2 batch number in which the log appeared + /// @param _index The position of the l2log in the L2 logs Merkle tree + /// @param _log Information about the sent log + /// @param _proof Merkle proof for inclusion of the L2 log + /// @return Whether the proof is correct and L2 log is included in batch function proveL2LogInclusion( uint256 _chainId, uint256 _batchNumber, @@ -182,6 +194,15 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus } /// @notice forwards function call to Mailbox based on ChainId + /// @param _chainId The chain ID of the hyperchain where to prove L1->L2 tx status. + /// @param _l2TxHash The L2 canonical transaction hash + /// @param _l2BatchNumber The L2 batch number where the transaction was processed + /// @param _l2MessageIndex The position in the L2 logs Merkle tree of the l2Log that was sent with the message + /// @param _l2TxNumberInBatch The L2 transaction number in the batch, in which the log was sent + /// @param _merkleProof The Merkle proof of the processing L1 -> L2 transaction + /// @param _status The execution status of the L1 -> L2 transaction (true - success & 0 - fail) + /// @return Whether the proof is correct and the transaction was actually executed with provided status + /// NOTE: It may return `false` for incorrect proof, but it doesn't mean that the L1 -> L2 transaction has an opposite status! function proveL1ToL2TransactionStatus( uint256 _chainId, bytes32 _l2TxHash,