Skip to content

Commit

Permalink
fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
Raid Ateir committed Dec 18, 2024
1 parent 0810ca1 commit a70eae5
Show file tree
Hide file tree
Showing 27 changed files with 113 additions and 846 deletions.
5 changes: 1 addition & 4 deletions l1-contracts/contracts/bridge/L1ERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,7 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard {
_l2TxGasPerPubdataByte: _l2TxGasPerPubdataByte,
_refundRecipient: _refundRecipient
});
// Ensuring that all the funds that were locked into this bridge were spent by the asset router / native token vault.
if (IERC20(_l1Token).allowance(address(this), address(L1_ASSET_ROUTER)) != 0) {
revert AssetRouterAllowanceNotZero();
}

depositAmount[msg.sender][_l1Token][l2TxHash] = _amount;
emit DepositInitiated({
l2DepositTxHash: l2TxHash,
Expand Down
9 changes: 4 additions & 5 deletions l1-contracts/contracts/bridge/L1Nullifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {IL1NativeTokenVault} from "./ntv/IL1NativeTokenVault.sol";
import {IL1ERC20Bridge} from "./interfaces/IL1ERC20Bridge.sol";
import {IL1AssetRouter} from "./asset-router/IL1AssetRouter.sol";
import {IAssetRouterBase} from "./asset-router/IAssetRouterBase.sol";
import {INativeTokenVault} from "./ntv/INativeTokenVault.sol";

import {IL1Nullifier, FinalizeL1DepositParams} from "./interfaces/IL1Nullifier.sol";

Expand Down Expand Up @@ -403,7 +402,7 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable,
/// @notice Internal function that handles the logic for finalizing withdrawals, supporting both the current bridge system and the legacy ERC20 bridge.
/// @param _finalizeWithdrawalParams The structure that holds all necessary data to finalize withdrawal
function _finalizeDeposit(
FinalizeL1DepositParams memory _finalizeWithdrawalParams // TODO check if works with
FinalizeL1DepositParams memory _finalizeWithdrawalParams
) internal nonReentrant whenNotPaused {
uint256 chainId = _finalizeWithdrawalParams.chainId;
uint256 l2BatchNumber = _finalizeWithdrawalParams.l2BatchNumber;
Expand Down Expand Up @@ -503,8 +502,8 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable,
/// @return assetId The ID of the bridged asset.
/// @return transferData The transfer data used to finalize withdawal.
function _verifyWithdrawal(
FinalizeL1DepositParams calldata _finalizeWithdrawalParams
) internal view returns (bytes32 assetId, bytes memory transferData) {
FinalizeL1DepositParams memory _finalizeWithdrawalParams
) internal returns (bytes32 assetId, bytes memory transferData) {
(assetId, transferData) = _parseL2WithdrawalMessage(
_finalizeWithdrawalParams.chainId,
_finalizeWithdrawalParams.message
Expand Down Expand Up @@ -551,7 +550,7 @@ contract L1Nullifier is IL1Nullifier, ReentrancyGuard, Ownable2StepUpgradeable,
function _parseL2WithdrawalMessage(
uint256 _chainId,
bytes memory _l2ToL1message
) internal view returns (bytes32 assetId, bytes memory transferData) {
) internal returns (bytes32 assetId, bytes memory transferData) {
// Please note that there are three versions of the message:
// 1. The message that is sent from `L2BaseToken` to withdraw base token.
// 2. The message that is sent from L2 Legacy Shared Bridge to withdraw ERC20 tokens or base token.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ abstract contract AssetRouterBase is IAssetRouterBase, Ownable2StepUpgradeable,
Internal Functions
//////////////////////////////////////////////////////////////*/

function _setAssetHandler(bytes32 _assetId, address _assetHandlerAddress) internal {
assetHandlerAddress[_assetId] = _assetHandlerAddress;
emit AssetHandlerRegistered(_assetId, _assetHandlerAddress);
}

/// @dev send the burn message to the asset
/// @notice Forwards the burn request for specific asset to respective asset handler.
/// @param _chainId The chain ID of the ZK chain to which to deposit.
Expand Down
9 changes: 0 additions & 9 deletions l1-contracts/contracts/bridge/asset-router/IL1AssetRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,6 @@ interface IL1AssetRouter is IAssetRouterBase, IL1SharedBridgeLegacy {
bytes32 indexed additionalData
);

event LegacyDepositInitiated(
uint256 indexed chainId,
bytes32 indexed l2DepositTxHash,
address indexed from,
address to,
address l1Asset,
uint256 amount
);

/// @notice Initiates a deposit by locking funds on the contract and sending the request
/// 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
Expand Down
5 changes: 5 additions & 0 deletions l1-contracts/contracts/bridge/asset-router/IL2AssetRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,9 @@ interface IL2AssetRouter is IAssetRouterBase {
/// @dev Used to set the assetHandlerAddress for a given assetId.
/// @dev Will be used by ZK Gateway
function setAssetHandlerAddress(uint256 _originChainId, bytes32 _assetId, address _assetAddress) external;

/// @notice Function that allows native token vault to register itself as the asset handler for
/// a legacy asset.
/// @param _assetId The assetId of the legacy token.
function setLegacyTokenAssetHandler(bytes32 _assetId) external;
}
31 changes: 1 addition & 30 deletions l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {AssetRouterBase} from "./AssetRouterBase.sol";
import {IL1AssetHandler} from "../interfaces/IL1AssetHandler.sol";
import {IL1ERC20Bridge} from "../interfaces/IL1ERC20Bridge.sol";
import {IAssetHandler} from "../interfaces/IAssetHandler.sol";
import {IL1Nullifier, FinalizeL1DepositParams} from "../interfaces/IL1Nullifier.sol";
import {IL1Nullifier} from "../interfaces/IL1Nullifier.sol";
import {INativeTokenVault} from "../ntv/INativeTokenVault.sol";
import {IL2SharedBridgeLegacyFunctions} from "../interfaces/IL2SharedBridgeLegacyFunctions.sol";

Expand Down Expand Up @@ -381,15 +381,6 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {

/// @notice Ensures that token is registered with native token vault.
/// @dev Only used when deposit is made with legacy data encoding format.
<<<<<<< HEAD
/// @param _token The L1 token address which should be registered with native token vault.
/// @return assetId The asset ID of the token provided.
function _ensureTokenRegisteredWithNTV(address _token) internal returns (bytes32 assetId) {
assetId = nativeTokenVault.getAssetId(block.chainid, _token);
if (nativeTokenVault.tokenAddress(assetId) == address(0)) {
nativeTokenVault.registerToken(_token);
}
=======
/// @param _token The native token address which should be registered with native token vault.
/// @return assetId The asset ID of the token provided.
function _ensureTokenRegisteredWithNTV(address _token) internal override returns (bytes32 assetId) {
Expand All @@ -399,7 +390,6 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
}
nativeTokenVault.ensureTokenIsRegistered(_token);
assetId = nativeTokenVault.assetId(_token);
>>>>>>> origin/oz-audit-sep-base
}

/// @inheritdoc IL1AssetRouter
Expand Down Expand Up @@ -584,21 +574,6 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
bytes calldata _message,
bytes32[] calldata _merkleProof
) external override {
<<<<<<< HEAD
/// @dev We use a deprecated field to support L2->L1 legacy withdrawals, which were started
/// by the legacy bridge.
address legacyL2Bridge = L1_NULLIFIER.__DEPRECATED_l2BridgeAddress(_chainId);
FinalizeL1DepositParams memory finalizeWithdrawalParams = FinalizeL1DepositParams({
chainId: _chainId,
l2BatchNumber: _l2BatchNumber,
l2MessageIndex: _l2MessageIndex,
l2Sender: legacyL2Bridge == address(0) ? L2_ASSET_ROUTER_ADDR : legacyL2Bridge,
l2TxNumberInBatch: _l2TxNumberInBatch,
message: _message,
merkleProof: _merkleProof
});
L1_NULLIFIER.finalizeDeposit(finalizeWithdrawalParams);
=======
L1_NULLIFIER.finalizeWithdrawal({
_chainId: _chainId,
_l2BatchNumber: _l2BatchNumber,
Expand All @@ -607,7 +582,6 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
_message: _message,
_merkleProof: _merkleProof
});
>>>>>>> origin/oz-audit-sep-base
}

/// @dev Withdraw funds from the initiated deposit, that failed when finalizing on L2.
Expand Down Expand Up @@ -642,8 +616,6 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
_merkleProof: _merkleProof
});
}
<<<<<<< HEAD
=======

/// @notice Legacy read method, which forwards the call to L1Nullifier to check if withdrawal was finalized
function isWithdrawalFinalized(
Expand All @@ -660,5 +632,4 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard {
function l2BridgeAddress(uint256 _chainId) external view override returns (address) {
return L1_NULLIFIER.l2BridgeAddress(_chainId);
}
>>>>>>> origin/oz-audit-sep-base
}
25 changes: 16 additions & 9 deletions l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {AddressAliasHelper} from "../../vendor/AddressAliasHelper.sol";
import {L2_NATIVE_TOKEN_VAULT_ADDR, L2_BRIDGEHUB_ADDR} from "../../common/L2ContractAddresses.sol";
import {L2ContractHelper} from "../../common/libraries/L2ContractHelper.sol";
import {DataEncoding} from "../../common/libraries/DataEncoding.sol";
import {EmptyAddress, InvalidCaller, AmountMustBeGreaterThanZero, AssetIdNotSupported} from "../../common/L1ContractErrors.sol";
import {TokenNotLegacy, EmptyAddress, InvalidCaller, AmountMustBeGreaterThanZero, AssetIdNotSupported} from "../../common/L1ContractErrors.sol";

/// @author Matter Labs
/// @custom:security-contact [email protected]
Expand Down Expand Up @@ -68,6 +68,13 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter {
_;
}

modifier onlyNTV() {
if (msg.sender != L2_NATIVE_TOKEN_VAULT_ADDR) {
revert InvalidCaller(msg.sender);
}
_;
}

/// @dev Disable the initialization to prevent Parity hack.
/// @dev this contract is deployed in the L2GenesisUpgrade, and is meant as direct deployment without a proxy.
/// @param _l1AssetRouter The address of the L1 Bridge contract.
Expand Down Expand Up @@ -108,6 +115,12 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter {
_setAssetHandlerAddressThisChain(L2_NATIVE_TOKEN_VAULT_ADDR, _assetRegistrationData, _assetHandlerAddress);
}

function setLegacyTokenAssetHandler(bytes32 _assetId) external override onlyNTV {
// Note, that it is an asset handler, but not asset deployment tracker,
// which is located on L1.
_setAssetHandler(_assetId, L2_NATIVE_TOKEN_VAULT_ADDR);
}

/*//////////////////////////////////////////////////////////////
Receive transaction Functions
//////////////////////////////////////////////////////////////*/
Expand All @@ -120,13 +133,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter {
uint256,
bytes32 _assetId,
bytes calldata _transferData
)
public
payable
override(AssetRouterBase, IAssetRouterBase)
onlyAssetRouterCounterpartOrSelf(L1_CHAIN_ID)
nonReentrant
{
) public override(AssetRouterBase, IAssetRouterBase) onlyAssetRouterCounterpartOrSelf(L1_CHAIN_ID) {
if (_assetId == BASE_TOKEN_ASSET_ID) {
revert AssetIdNotSupported(BASE_TOKEN_ASSET_ID);
}
Expand All @@ -144,7 +151,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter {
/// @dev do not rely on this function, it will be deprecated in the future
/// @param _assetId The asset id of the withdrawn asset
/// @param _assetData The data that is passed to the asset handler contract
function withdraw(bytes32 _assetId, bytes memory _assetData) public override {
function withdraw(bytes32 _assetId, bytes calldata _assetData) external returns (bytes32) {
_withdrawSender(_assetId, _assetData, msg.sender, true);
}

Expand Down
24 changes: 22 additions & 2 deletions l1-contracts/contracts/bridge/interfaces/IL1Nullifier.sol
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ interface IL1Nullifier {

function setL1AssetRouter(address _l1AssetRouter) external;

function __DEPRECATED_chainBalance(uint256 _chainId, address _token) external view returns (uint256);
function chainBalance(uint256 _chainId, address _token) external view returns (uint256);

function __DEPRECATED_l2BridgeAddress(uint256 _chainId) external view returns (address);
function l2BridgeAddress(uint256 _chainId) external view returns (address);

function transferTokenToNTV(address _token) external;

Expand Down Expand Up @@ -108,4 +108,24 @@ interface IL1Nullifier {
uint16 _l2TxNumberInBatch,
bytes32[] calldata _merkleProof
) external;

/// @notice Legacy function to finalize withdrawal via the same
/// interface as the old L1SharedBridge.
/// @dev Note, that we need to keep this interface, since the `L2AssetRouter`
/// will continue returning the previous address as the `l1SharedBridge`. The value
/// returned by it is used in the SDK for finalizing withdrawals.
/// @param _chainId The chain ID of the transaction to check
/// @param _l2BatchNumber The L2 batch number where the withdrawal 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 _message The L2 withdraw data, stored in an L2 -> L1 message
/// @param _merkleProof The Merkle proof of the inclusion L2 -> L1 message about withdrawal initialization
function finalizeWithdrawal(
uint256 _chainId,
uint256 _l2BatchNumber,
uint256 _l2MessageIndex,
uint16 _l2TxNumberInBatch,
bytes calldata _message,
bytes32[] calldata _merkleProof
) external;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ pragma solidity ^0.8.20;
/// @author Matter Labs
/// @custom:security-contact [email protected]
interface IL2SharedBridgeLegacy {
event FinalizeDeposit(
address indexed l1Sender,
address indexed l2Receiver,
address indexed l2Token,
uint256 amount
);

function withdraw(address _l1Receiver, address _l2Token, uint256 _amount) external;

function l1TokenAddress(address _l2Token) external view returns (address);
Expand All @@ -17,5 +24,5 @@ interface IL2SharedBridgeLegacy {

function deployBeaconProxy(bytes32 _salt) external returns (address);

function sendMessageToL1(bytes calldata _message) external;
function sendMessageToL1(bytes calldata _message) external returns (bytes32);
}
12 changes: 10 additions & 2 deletions l1-contracts/contracts/bridge/ntv/INativeTokenVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ interface INativeTokenVault {

/// @notice The AssetRouter contract
function ASSET_ROUTER() external view returns (IAssetRouterBase);

/// @notice The chain ID of the L1 chain
function L1_CHAIN_ID() external view returns (uint256);

/// @notice Returns the chain ID of the origin chain for a given asset ID
function originChainId(bytes32 assetId) external view returns (uint256);

Expand All @@ -25,15 +29,19 @@ interface INativeTokenVault {
/// @notice No access control is ok, since the bridging of tokens should be permissionless. This requires permissionless registration.
function registerToken(address _l1Token) external;

/// @notice Used to get the assetId of a token
function getAssetId(uint256 _chainId, address _tokenAddress) external view returns (bytes32);
/// @notice Ensures that the native token is registered with the NTV.
/// @dev This function is used to ensure that the token is registered with the NTV.
function ensureTokenIsRegistered(address _nativeToken) external;

/// @notice Used to get the the ERC20 data for a token
function getERC20Getters(address _token, uint256 _originChainId) external view returns (bytes memory);

/// @notice Used to get the token address of an assetId
function tokenAddress(bytes32 assetId) external view returns (address);

/// @notice Used to get the assetId of a token
function assetId(address token) external view returns (bytes32);

/// @notice Used to get the expected bridged token address corresponding to its native counterpart
function calculateCreate2TokenAddress(uint256 _originChainId, address _originToken) external view returns (address);
}
2 changes: 1 addition & 1 deletion l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken
bytes32,
address,
address _assetHandlerAddressOnCounterpart
) external view override onlyAssetRouter {
) external view onlyAssetRouter {
if (_assetHandlerAddressOnCounterpart != L2_NATIVE_TOKEN_VAULT_ADDR) {
revert WrongCounterpart();
}
Expand Down
3 changes: 2 additions & 1 deletion l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@ import {NativeTokenVault} from "./NativeTokenVault.sol";

import {IL2SharedBridgeLegacy} from "../interfaces/IL2SharedBridgeLegacy.sol";
import {BridgedStandardERC20} from "../BridgedStandardERC20.sol";
import {IL2AssetRouter} from "../asset-router/IL2AssetRouter.sol";

import {L2_DEPLOYER_SYSTEM_CONTRACT_ADDR, L2_ASSET_ROUTER_ADDR} from "../../common/L2ContractAddresses.sol";
import {L2ContractHelper, IContractDeployer} from "../../common/libraries/L2ContractHelper.sol";

import {SystemContractsCaller} from "../../common/libraries/SystemContractsCaller.sol";
import {DataEncoding} from "../../common/libraries/DataEncoding.sol";

import {EmptyAddress, EmptyBytes32, AddressMismatch, DeployFailed, AssetIdNotSupported, ZeroAddress} from "../../common/L1ContractErrors.sol";
import {AssetIdAlreadyRegistered, NoLegacySharedBridge, TokenIsNotLegacy, EmptyAddress, EmptyBytes32, AddressMismatch, DeployFailed, AssetIdNotSupported} from "../../common/L1ContractErrors.sol";

/// @author Matter Labs
/// @custom:security-contact [email protected]
Expand Down
7 changes: 0 additions & 7 deletions l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -334,13 +334,6 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2
return BridgeHelper.getERC20Getters(_token, _originChainId);
}

/// @notice Returns the parsed assetId.
/// @param _nativeToken The address of the token to be parsed.
/// @dev Shows the assetId for a given chain and token address
function getAssetId(uint256 _chainId, address _nativeToken) external pure override returns (bytes32) {
return DataEncoding.encodeNTVAssetId(_chainId, _nativeToken);
}

/// @notice Registers a native token address for the vault.
/// @dev It does not perform any checks for the correctnesss of the token contract.
/// @param _nativeToken The address of the token to be registered.
Expand Down
Loading

0 comments on commit a70eae5

Please sign in to comment.