From 51ae0c67a9295ec01e6abf6a30c6abb09f5523ec Mon Sep 17 00:00:00 2001 From: kelemeno Date: Mon, 14 Oct 2024 12:01:19 +0100 Subject: [PATCH 01/23] kl/h01-2gw --- .../contracts/bridge/BridgedStandardERC20.sol | 3 +- .../bridge/ntv/L1NativeTokenVault.sol | 2 +- .../bridge/ntv/L2NativeTokenVault.sol | 54 ++++++++----- .../contracts/bridge/ntv/NativeTokenVault.sol | 63 +++++++++------ .../contracts/common/L1ContractErrors.sol | 2 + .../test/L2NativeTokenVaultDev.sol | 78 +++++++++++++++++++ .../l1/integration/AssetRouterTest.t.sol | 2 +- 7 files changed, 160 insertions(+), 44 deletions(-) create mode 100644 l1-contracts/contracts/dev-contracts/test/L2NativeTokenVaultDev.sol diff --git a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol index bd8d01110..c33111fa0 100644 --- a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol +++ b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol @@ -77,7 +77,7 @@ contract BridgedStandardERC20 is ERC20PermitUpgradeable, IBridgedStandardToken, /// @param _originToken Address of the origin token that can be deposited to mint this bridged token /// @param _data The additional data that the L1 bridge provide for initialization. /// In this case, it is packed `name`/`symbol`/`decimals` of the L1 token. - function bridgeInitialize(address _originToken, bytes calldata _data) external initializer returns (uint256) { + function bridgeInitialize(address _originToken, bytes calldata _data) external initializer { if (_originToken == address(0)) { revert ZeroAddress(); } @@ -129,7 +129,6 @@ contract BridgedStandardERC20 is ERC20PermitUpgradeable, IBridgedStandardToken, availableGetters = getters; emit BridgeInitialize(_originToken, decodedName, decodedSymbol, decimals_); - return chainId; } /// @notice A method to be called by the governor to update the token's metadata. diff --git a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol index be456db43..3814be790 100644 --- a/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L1NativeTokenVault.sol @@ -234,7 +234,7 @@ contract L1NativeTokenVault is IL1NativeTokenVault, IL1AssetHandler, NativeToken } } - function _deployBeaconProxy(bytes32 _salt) internal override returns (BeaconProxy proxy) { + function _deployBeaconProxy(bytes32 _salt, uint256) internal override returns (BeaconProxy proxy) { // Use CREATE2 to deploy the BeaconProxy address proxyAddress = Create2.deploy( 0, diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index e96a6d289..d93a4f957 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -91,32 +91,27 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } /// @notice Ensures that the token is deployed. - /// @param _originChainId The chain ID of the origin chain. /// @param _assetId The asset ID. /// @param _originToken The origin token address. /// @param _erc20Data The ERC20 data. /// @return expectedToken The token address. function _ensureTokenDeployed( - uint256 _originChainId, bytes32 _assetId, address _originToken, bytes memory _erc20Data ) internal override returns (address expectedToken) { - expectedToken = _assetIdCheck(_originChainId, _assetId, _originToken); + uint256 tokenOriginChainId; + (expectedToken, tokenOriginChainId) = _calculateExpectedTokenAddress(_originToken, _erc20Data); address l1LegacyToken; if (address(L2_LEGACY_SHARED_BRIDGE) != address(0)) { l1LegacyToken = L2_LEGACY_SHARED_BRIDGE.l1TokenAddress(expectedToken); } if (l1LegacyToken != address(0)) { - /// token is a legacy token, no need to deploy - if (l1LegacyToken != _originToken) { - revert AddressMismatch(_originToken, l1LegacyToken); - } - tokenAddress[_assetId] = expectedToken; + _ensureTokenDeployedInnerLegacyToken(_assetId, _originToken, _erc20Data, expectedToken, l1LegacyToken); } else { super._ensureTokenDeployedInner({ - _originChainId: _originChainId, + _tokenOriginChainId: tokenOriginChainId, _assetId: _assetId, _originToken: _originToken, _erc20Data: _erc20Data, @@ -125,13 +120,33 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } } + function _ensureTokenDeployedInnerLegacyToken( + bytes32 _assetId, + address _originToken, + bytes memory _erc20Data, + address _expectedToken, + address _l1LegacyToken + ) internal { + _assetIdCheck(L1_CHAIN_ID, _assetId, _originToken); + + /// token is a legacy token, no need to deploy + if (_l1LegacyToken != _originToken) { + revert AddressMismatch(_originToken, _l1LegacyToken); + } + + tokenAddress[_assetId] = _expectedToken; + } + /// @notice Deploys the beacon proxy for the L2 token, while using ContractDeployer system contract. /// @dev This function uses raw call to ContractDeployer to make sure that exactly `l2TokenProxyBytecodeHash` is used /// for the code of the proxy. /// @param _salt The salt used for beacon proxy deployment of L2 bridged token. /// @return proxy The beacon proxy, i.e. L2 bridged token. - function _deployBeaconProxy(bytes32 _salt) internal override returns (BeaconProxy proxy) { - if (address(L2_LEGACY_SHARED_BRIDGE) == address(0)) { + function _deployBeaconProxy( + bytes32 _salt, + uint256 _tokenOriginChainId + ) internal virtual override returns (BeaconProxy proxy) { + if (address(L2_LEGACY_SHARED_BRIDGE) == address(0) && _tokenOriginChainId != L1_CHAIN_ID) { // Deploy the beacon proxy for the L2 token (bool success, bytes memory returndata) = SystemContractsCaller.systemCallWithReturndata( @@ -173,12 +188,12 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { /// @param _l1Token The address of token on L1. /// @return Address of an L2 token counterpart. function calculateCreate2TokenAddress( - uint256 _originChainId, + uint256 _tokenOriginChainId, address _l1Token - ) public view override(INativeTokenVault, NativeTokenVault) returns (address) { + ) public view virtual override(INativeTokenVault, NativeTokenVault) returns (address) { bytes32 constructorInputHash = keccak256(abi.encode(address(bridgedTokenBeacon), "")); - bytes32 salt = _getCreate2Salt(_originChainId, _l1Token); - if (address(L2_LEGACY_SHARED_BRIDGE) != address(0)) { + bytes32 salt = _getCreate2Salt(_tokenOriginChainId, _l1Token); + if (address(L2_LEGACY_SHARED_BRIDGE) != address(0) && _tokenOriginChainId == L1_CHAIN_ID) { return L2_LEGACY_SHARED_BRIDGE.l2TokenAddress(_l1Token); } else { return @@ -192,10 +207,13 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } /// @notice Calculates the salt for the Create2 deployment of the L2 token. - function _getCreate2Salt(uint256 _originChainId, address _l1Token) internal view override returns (bytes32 salt) { - salt = _originChainId == L1_CHAIN_ID + function _getCreate2Salt( + uint256 _tokenOriginChainId, + address _l1Token + ) internal view override returns (bytes32 salt) { + salt = _tokenOriginChainId == L1_CHAIN_ID ? bytes32(uint256(uint160(_l1Token))) - : keccak256(abi.encode(_originChainId, _l1Token)); + : keccak256(abi.encode(_tokenOriginChainId, _l1Token)); } function _handleChainBalanceIncrease( diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index 5e76d7630..2280f2add 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -21,7 +21,7 @@ import {DataEncoding} from "../../common/libraries/DataEncoding.sol"; import {BridgedStandardERC20} from "../BridgedStandardERC20.sol"; import {BridgeHelper} from "../BridgeHelper.sol"; -import {EmptyDeposit, Unauthorized, TokensWithFeesNotSupported, TokenNotSupported, NonEmptyMsgValue, ValueMismatch, AddressMismatch, AssetIdMismatch, AmountMustBeGreaterThanZero, ZeroAddress} from "../../common/L1ContractErrors.sol"; +import {EmptyDeposit, Unauthorized, TokensWithFeesNotSupported, TokenNotSupported, NonEmptyMsgValue, ValueMismatch, AddressMismatch, AssetIdMismatch, AmountMustBeGreaterThanZero, ZeroAddress, DeployingBridgedTokenForNativeToken} from "../../common/L1ContractErrors.sol"; /// @author Matter Labs /// @custom:security-contact security@matterlabs.dev @@ -132,7 +132,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 (, receiver, originToken, amount, erc20Data) = DataEncoding.decodeBridgeMintData(_data); if (token == address(0)) { - token = _ensureTokenDeployed(_originChainId, _assetId, originToken, erc20Data); + token = _ensureTokenDeployed(_assetId, originToken, erc20Data); } _handleChainBalanceDecrease(_originChainId, _assetId, amount, false); IBridgedStandardToken(token).bridgeMint(receiver, amount); @@ -355,14 +355,14 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 //////////////////////////////////////////////////////////////*/ function _ensureTokenDeployed( - uint256 _originChainId, bytes32 _assetId, address _originToken, bytes memory _erc20Data ) internal virtual returns (address expectedToken) { - expectedToken = _assetIdCheck(_originChainId, _assetId, _originToken); + uint256 tokenOriginChainId; + (expectedToken, tokenOriginChainId) = _calculateExpectedTokenAddress(_originToken, _erc20Data); _ensureTokenDeployedInner({ - _originChainId: _originChainId, + _tokenOriginChainId: tokenOriginChainId, _assetId: _assetId, _originToken: _originToken, _erc20Data: _erc20Data, @@ -370,13 +370,23 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 }); } - function _assetIdCheck( - uint256 _originChainId, - bytes32 _assetId, - address _originToken - ) internal view returns (address expectedToken) { - expectedToken = calculateCreate2TokenAddress(_originChainId, _originToken); - bytes32 expectedAssetId = DataEncoding.encodeNTVAssetId(_originChainId, _originToken); + function _calculateExpectedTokenAddress( + address _originToken, + bytes memory _erc20Data + ) internal view returns (address expectedToken, uint256 tokenOriginChainId) { + tokenOriginChainId = this.tokenDataOriginChainId(_erc20Data); + expectedToken = calculateCreate2TokenAddress(tokenOriginChainId, _originToken); + } + + function tokenDataOriginChainId(bytes calldata _erc20Data) public view returns (uint256 tokenOriginChainId) { + (tokenOriginChainId, , , ) = DataEncoding.decodeTokenData(_erc20Data); + if (tokenOriginChainId == 0) { + tokenOriginChainId = L1_CHAIN_ID; + } + } + + function _assetIdCheck(uint256 _tokenOriginChainId, bytes32 _assetId, address _originToken) internal view { + bytes32 expectedAssetId = DataEncoding.encodeNTVAssetId(_tokenOriginChainId, _originToken); if (_assetId != expectedAssetId) { // Make sure that a NativeTokenVault sent the message revert AssetIdMismatch(_assetId, expectedAssetId); @@ -384,13 +394,15 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 } function _ensureTokenDeployedInner( - uint256 _originChainId, + uint256 _tokenOriginChainId, bytes32 _assetId, address _originToken, bytes memory _erc20Data, address _expectedToken ) internal { - address deployedToken = _deployBridgedToken(_originChainId, _originToken, _erc20Data); + _assetIdCheck(_tokenOriginChainId, _assetId, _originToken); + + address deployedToken = _deployBridgedToken(_tokenOriginChainId, _assetId, _originToken, _erc20Data); if (deployedToken != _expectedToken) { revert AddressMismatch(_expectedToken, deployedToken); } @@ -402,7 +414,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 /// @param _bridgeToken The address of native token. /// @return The address of bridged token. function calculateCreate2TokenAddress( - uint256 _originChainId, + uint256 _tokenOriginChainId, address _bridgeToken ) public view virtual override returns (address); @@ -411,16 +423,20 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 /// @param _erc20Data The ERC20 metadata of the token deployed. /// @return The address of the beacon proxy (bridged token). function _deployBridgedToken( - uint256 _originChainId, + uint256 _tokenOriginChainId, + bytes32 _assetId, address _originToken, bytes memory _erc20Data ) internal returns (address) { - bytes32 salt = _getCreate2Salt(_originChainId, _originToken); + bytes32 salt = _getCreate2Salt(_tokenOriginChainId, _originToken); + + BeaconProxy l2Token = _deployBeaconProxy(salt, _tokenOriginChainId); + BridgedStandardERC20(address(l2Token)).bridgeInitialize(_originToken, _erc20Data); + if (_tokenOriginChainId == block.chainid) { + revert DeployingBridgedTokenForNativeToken(); + } - BeaconProxy l2Token = _deployBeaconProxy(salt); - uint256 tokenOriginChainId = BridgedStandardERC20(address(l2Token)).bridgeInitialize(_originToken, _erc20Data); - tokenOriginChainId = tokenOriginChainId == 0 ? L1_CHAIN_ID : tokenOriginChainId; - originChainId[DataEncoding.encodeNTVAssetId(tokenOriginChainId, _originToken)] = tokenOriginChainId; + originChainId[_assetId] = _tokenOriginChainId; return address(l2Token); } @@ -436,7 +452,10 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 /// for the code of the proxy. /// @param _salt The salt used for beacon proxy deployment of the bridged token (we pass the native token address). /// @return proxy The beacon proxy, i.e. bridged token. - function _deployBeaconProxy(bytes32 _salt) internal virtual returns (BeaconProxy proxy); + function _deployBeaconProxy( + bytes32 _salt, + uint256 _tokenOriginChainId + ) internal virtual returns (BeaconProxy proxy); /*////////////////////////////////////////////////////////////// PAUSE diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 48c90d540..6db33ada2 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -87,6 +87,8 @@ error ChainIdTooBig(); error DelegateCallFailed(bytes returnData); // 0x0a8ed92c error DenominatorIsZero(); +// 0x138ee1a3 +error DeployingBridgedTokenForNativeToken(); // error DeployFailed(); // 0xc7c9660f diff --git a/l1-contracts/contracts/dev-contracts/test/L2NativeTokenVaultDev.sol b/l1-contracts/contracts/dev-contracts/test/L2NativeTokenVaultDev.sol new file mode 100644 index 000000000..46960489c --- /dev/null +++ b/l1-contracts/contracts/dev-contracts/test/L2NativeTokenVaultDev.sol @@ -0,0 +1,78 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.24; + +import {BeaconProxy} from "@openzeppelin/contracts-v4/proxy/beacon/BeaconProxy.sol"; +import {Create2} from "@openzeppelin/contracts-v4/utils/Create2.sol"; +import {IBeacon} from "@openzeppelin/contracts-v4/proxy/beacon/IBeacon.sol"; +import {UpgradeableBeacon} from "@openzeppelin/contracts-v4/proxy/beacon/UpgradeableBeacon.sol"; + +import {INativeTokenVault} from "contracts/bridge/ntv/INativeTokenVault.sol"; +import {NativeTokenVault} from "contracts/bridge/ntv/NativeTokenVault.sol"; +import {L2NativeTokenVault} from "contracts/bridge/ntv/L2NativeTokenVault.sol"; +import {BridgedStandardERC20} from "contracts/bridge/BridgedStandardERC20.sol"; + +/// @author Matter Labs +/// @notice This is used for fast debugging of the L2NTV by running it in L1 context, i.e. normal foundry instead of foundry --zksync. +contract L2NativeTokenVaultDev is L2NativeTokenVault { + constructor( + uint256 _l1ChainId, + address _aliasedOwner, + bytes32 _l2TokenProxyBytecodeHash, + address _legacySharedBridge, + address _bridgedTokenBeacon, + bool _contractsDeployedAlready, + address _wethToken, + bytes32 _baseTokenAssetId + ) + L2NativeTokenVault( + _l1ChainId, + _aliasedOwner, + _l2TokenProxyBytecodeHash, + _legacySharedBridge, + _bridgedTokenBeacon, + _contractsDeployedAlready, + _wethToken, + _baseTokenAssetId + ) + {} + + /// @notice copied from L1NTV for L1 compilation + function calculateCreate2TokenAddress( + uint256 _originChainId, + address _l1Token + ) public view override(L2NativeTokenVault) returns (address) { + bytes32 salt = _getCreate2Salt(_originChainId, _l1Token); + return + Create2.computeAddress( + salt, + keccak256(abi.encodePacked(type(BeaconProxy).creationCode, abi.encode(bridgedTokenBeacon, ""))) + ); + } + + function deployBridgedStandardERC20(address _owner) external { + _transferOwnership(_owner); + + address l2StandardToken = address(new BridgedStandardERC20{salt: bytes32(0)}()); + + UpgradeableBeacon tokenBeacon = new UpgradeableBeacon{salt: bytes32(0)}(l2StandardToken); + + tokenBeacon.transferOwnership(owner()); + bridgedTokenBeacon = IBeacon(address(tokenBeacon)); + emit L2TokenBeaconUpdated(address(bridgedTokenBeacon), l2TokenProxyBytecodeHash); + } + + function test() external pure { + // test + } + + function _deployBeaconProxy(bytes32 _salt, uint256) internal virtual override returns (BeaconProxy proxy) { + // Use CREATE2 to deploy the BeaconProxy + address proxyAddress = Create2.deploy( + 0, + _salt, + abi.encodePacked(type(BeaconProxy).creationCode, abi.encode(bridgedTokenBeacon, "")) + ); + return BeaconProxy(payable(proxyAddress)); + } +} diff --git a/l1-contracts/test/foundry/l1/integration/AssetRouterTest.t.sol b/l1-contracts/test/foundry/l1/integration/AssetRouterTest.t.sol index 83913d2a4..71e0038e3 100644 --- a/l1-contracts/test/foundry/l1/integration/AssetRouterTest.t.sol +++ b/l1-contracts/test/foundry/l1/integration/AssetRouterTest.t.sol @@ -88,7 +88,7 @@ contract AssetRouterTest is L1ContractDeployer, ZKChainDeployer, TokenDeployer, bytes memory transferData = DataEncoding.encodeBridgeMintData({ _originalCaller: ETH_TOKEN_ADDRESS, _l2Receiver: address(this), - _l1Token: ETH_TOKEN_ADDRESS, + _l1Token: _tokenAddress, _amount: 100, _erc20Metadata: BridgeHelper.getERC20Getters(_tokenAddress, chainId) }); From 18250cb7bb34f22250565eb0673fe0b6db133d99 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Mon, 14 Oct 2024 13:08:23 +0100 Subject: [PATCH 02/23] double deposit error --- l1-contracts/contracts/bridge/L1ERC20Bridge.sol | 13 ++++++++----- .../contracts/bridge/asset-router/L1AssetRouter.sol | 7 +++++++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/l1-contracts/contracts/bridge/L1ERC20Bridge.sol b/l1-contracts/contracts/bridge/L1ERC20Bridge.sol index 3eb231950..9b29b71e0 100644 --- a/l1-contracts/contracts/bridge/L1ERC20Bridge.sol +++ b/l1-contracts/contracts/bridge/L1ERC20Bridge.sol @@ -188,7 +188,7 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard { if (_l1Token == ETH_TOKEN_ADDRESS) { revert ETHDepositNotSupported(); } - uint256 amount = _depositFundsToAssetRouter(msg.sender, IERC20(_l1Token), _amount); + uint256 amount = _approveFundsToAssetRouter(msg.sender, IERC20(_l1Token), _amount); if (amount != _amount) { // The token has non-standard transfer logic revert TokensWithFeesNotSupported(); @@ -203,6 +203,8 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard { _l2TxGasPerPubdataByte: _l2TxGasPerPubdataByte, _refundRecipient: _refundRecipient }); + // clearing approval + IERC20(_l1Token).approve(address(L1_ASSET_ROUTER), 0); depositAmount[msg.sender][_l1Token][l2TxHash] = _amount; emit DepositInitiated({ l2DepositTxHash: l2TxHash, @@ -219,10 +221,11 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard { /// @dev Transfers tokens from the depositor address to the native token vault address. /// @return The difference between the contract balance before and after the transferring of funds. - function _depositFundsToAssetRouter(address _from, IERC20 _token, uint256 _amount) internal returns (uint256) { - uint256 balanceBefore = _token.balanceOf(address(L1_ASSET_ROUTER)); - _token.safeTransferFrom(_from, address(L1_ASSET_ROUTER), _amount); - uint256 balanceAfter = _token.balanceOf(address(L1_ASSET_ROUTER)); + function _approveFundsToAssetRouter(address _from, IERC20 _token, uint256 _amount) internal returns (uint256) { + uint256 balanceBefore = _token.balanceOf(address(this)); + _token.safeTransferFrom(_from, address(this), _amount); + _token.approve(address(L1_ASSET_ROUTER), _amount); + uint256 balanceAfter = _token.balanceOf(address(this)); return balanceAfter - balanceBefore; } diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index 0c3b2001f..8431b5816 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -377,10 +377,17 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { // Do the transfer if allowance to Shared bridge is bigger than amount // And if there is not enough allowance for the NTV + bool _weCanTransfer = false; if ( l1Token.allowance(_originalCaller, address(this)) >= _amount && l1Token.allowance(_originalCaller, address(nativeTokenVault)) < _amount ) { + _weCanTransfer = true; + } else if (l1Token.allowance(address(legacyBridge), address(this)) >= _amount) { + _originalCaller = address(legacyBridge); + _weCanTransfer = true; + } + if (_weCanTransfer) { // slither-disable-next-line arbitrary-send-erc20 l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount); return true; From b6b09da09899196829a5f68af10e502e34bc7892 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Mon, 14 Oct 2024 23:51:30 +0100 Subject: [PATCH 03/23] kl/h03-2gw missing chainid --- l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index 6c122ccc6..5bd99a5ab 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -180,9 +180,9 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { function _getAssetRouterWithdrawMessage( bytes32 _assetId, bytes memory _l1bridgeMintData - ) internal pure returns (bytes memory) { + ) internal view returns (bytes memory) { // solhint-disable-next-line func-named-parameters - return abi.encodePacked(IAssetRouterBase.finalizeDeposit.selector, _assetId, _l1bridgeMintData); + return abi.encodePacked(IAssetRouterBase.finalizeDeposit.selector, block.chainid, _assetId, _l1bridgeMintData); } /// @notice Encodes the message for l2ToL1log sent during withdraw initialization. From cd5fab70f85fc2d9a55cacaf71eb34342e2a66b0 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Mon, 14 Oct 2024 13:25:49 +0100 Subject: [PATCH 04/23] missing chainBalance increase --- l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index 5e76d7630..4a1ccacee 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -196,6 +196,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 address bridgedToken = tokenAddress[_assetId]; IBridgedStandardToken(bridgedToken).bridgeBurn(_originalCaller, _amount); + _handleChainBalanceIncrease(_chainId, _assetId, _amount, false); emit BridgeBurn({ chainId: _chainId, From 234fe9991af3a749cf8a27bc78268e1cf5905e78 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Mon, 14 Oct 2024 23:59:47 +0100 Subject: [PATCH 05/23] kl/h05-2gw --- l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index 5e76d7630..02e865d5d 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -259,8 +259,8 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 if (msg.value != 0) { revert NonEmptyMsgValue(); } - _handleChainBalanceIncrease(_chainId, _assetId, amount, true); amount = _depositAmount; + _handleChainBalanceIncrease(_chainId, _assetId, amount, true); if (!_depositChecked) { uint256 expectedDepositAmount = _depositFunds(_originalCaller, IERC20(nativeToken), _depositAmount); // note if _originalCaller is this contract, this will return 0. This does not happen. // The token has non-standard transfer logic From 5b6db28a3aafaebd06923b41c80dfca051e0e495 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 00:11:59 +0100 Subject: [PATCH 06/23] kl/m01-2gw --- l1-contracts/contracts/bridgehub/Bridgehub.sol | 14 ++++++++++---- l1-contracts/contracts/bridgehub/IBridgehub.sol | 2 +- l1-contracts/contracts/common/Config.sol | 5 +++++ l1-contracts/deploy-scripts/DeployL1.s.sol | 2 +- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index 8e23a9f3d..742a38472 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -18,7 +18,7 @@ import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; import {DataEncoding} from "../common/libraries/DataEncoding.sol"; import {IZKChain} from "../state-transition/chain-interfaces/IZKChain.sol"; -import {ETH_TOKEN_ADDRESS, TWO_BRIDGES_MAGIC_VALUE, BRIDGEHUB_MIN_SECOND_BRIDGE_ADDRESS, SETTLEMENT_LAYER_RELAY_SENDER} from "../common/Config.sol"; +import {ETH_TOKEN_ADDRESS, TWO_BRIDGES_MAGIC_VALUE, BRIDGEHUB_MIN_SECOND_BRIDGE_ADDRESS, SETTLEMENT_LAYER_RELAY_SENDER, L1_SETTLEMENT_LAYER_VIRTUAL_ADDRESS} from "../common/Config.sol"; import {BridgehubL2TransactionRequest, L2Message, L2Log, TxStatus} from "../common/Messaging.sol"; import {AddressAliasHelper} from "../vendor/AddressAliasHelper.sol"; import {IMessageRoot} from "./IMessageRoot.sol"; @@ -86,6 +86,9 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus /// @dev asset info used to identify chains in the Shared Bridge mapping(bytes32 ctmAssetId => address ctmAddress) public ctmAssetIdToAddress; + /// @dev ctmAddress to ctmAssetId + mapping(address ctmAddress => bytes32 ctmAssetId) public ctmAssetIdFromAddress; + /// @dev used to indicate the currently active settlement layer for a given chainId mapping(uint256 chainId => uint256 activeSettlementLayerChainId) public settlementLayer; @@ -320,6 +323,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus bytes32 assetInfo = keccak256(abi.encode(L1_CHAIN_ID, sender, _additionalData)); ctmAssetIdToAddress[assetInfo] = _assetAddress; + ctmAssetIdFromAddress[_assetAddress] = assetInfo; emit AssetRegistered(assetInfo, _assetAddress, _additionalData, msg.sender); } @@ -422,10 +426,10 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus if (ctmAddress == address(0)) { revert ChainIdNotRegistered(_chainId); } - return ctmAssetId(chainTypeManager[_chainId]); + return ctmAssetIdFromAddress[chainTypeManager[_chainId]]; } - function ctmAssetId(address _ctmAddress) public view override returns (bytes32) { + function calculateCtmAssetId(address _ctmAddress) internal view returns (bytes32) { return keccak256(abi.encode(L1_CHAIN_ID, address(l1CtmDeployer), bytes32(uint256(uint160(_ctmAddress))))); } @@ -700,7 +704,9 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus bridgehubData.ctmData ); bytes memory chainMintData = IZKChain(zkChain).forwardedBridgeBurn( - zkChainMap.get(_settlementChainId), + _settlementChainId == L1_CHAIN_ID + ? L1_SETTLEMENT_LAYER_VIRTUAL_ADDRESS + : zkChainMap.get(_settlementChainId), _originalCaller, bridgehubData.chainData ); diff --git a/l1-contracts/contracts/bridgehub/IBridgehub.sol b/l1-contracts/contracts/bridgehub/IBridgehub.sol index f5732b98e..12e353904 100644 --- a/l1-contracts/contracts/bridgehub/IBridgehub.sol +++ b/l1-contracts/contracts/bridgehub/IBridgehub.sol @@ -215,7 +215,7 @@ interface IBridgehub is IAssetHandler, IL1AssetHandler { function ctmAssetIdFromChainId(uint256 _chainId) external view returns (bytes32); - function ctmAssetId(address _ctmAddress) external view returns (bytes32); + function ctmAssetIdFromAddress(address _ctmAddress) external view returns (bytes32); function l1CtmDeployer() external view returns (ICTMDeploymentTracker); diff --git a/l1-contracts/contracts/common/Config.sol b/l1-contracts/contracts/common/Config.sol index beebcd00c..a1e58f464 100644 --- a/l1-contracts/contracts/common/Config.sol +++ b/l1-contracts/contracts/common/Config.sol @@ -120,6 +120,11 @@ address constant SETTLEMENT_LAYER_RELAY_SENDER = address(uint160(0x1111111111111 /// @dev The metadata version that is supported by the ZK Chains to prove that an L2->L1 log was included in a batch. uint256 constant SUPPORTED_PROOF_METADATA_VERSION = 1; +/// @dev The virtual address of the L1 settlement layer. +address constant L1_SETTLEMENT_LAYER_VIRTUAL_ADDRESS = address( + uint160(uint256(keccak256("L1_SETTLEMENT_LAYER_VIRTUAL_ADDRESS")) - 1) +); + struct PriorityTreeCommitment { uint256 nextLeafIndex; uint256 startIndex; diff --git a/l1-contracts/deploy-scripts/DeployL1.s.sol b/l1-contracts/deploy-scripts/DeployL1.s.sol index 868fdbc47..679c95113 100644 --- a/l1-contracts/deploy-scripts/DeployL1.s.sol +++ b/l1-contracts/deploy-scripts/DeployL1.s.sol @@ -620,7 +620,7 @@ contract DeployL1Script is Script { vm.stopBroadcast(); console.log("CTM registered in CTMDeploymentTracker"); - bytes32 assetId = bridgehub.ctmAssetId(addresses.stateTransition.stateTransitionProxy); + bytes32 assetId = bridgehub.ctmAssetIdFromAddress(addresses.stateTransition.stateTransitionProxy); // console.log(address(bridgehub.ctmDeployer()), addresses.bridgehub.ctmDeploymentTrackerProxy); // console.log(address(bridgehub.ctmDeployer().BRIDGE_HUB()), addresses.bridgehub.bridgehubProxy); console.log( From cd16762267f76979872f9bc2caccc15f24bab1eb Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 00:15:11 +0100 Subject: [PATCH 07/23] kl/m02-2gw priority tree issues --- .../contracts/state-transition/chain-deps/facets/Admin.sol | 2 +- .../contracts/state-transition/libraries/PriorityTree.sol | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) 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 27bbe3155..1f25dc6df 100644 --- a/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol +++ b/l1-contracts/contracts/state-transition/chain-deps/facets/Admin.sol @@ -320,7 +320,7 @@ contract AdminFacet is ZKChainBase, IAdmin { ); require(_contractAlreadyDeployed, "Af: contract not deployed"); require(s.settlementLayer != address(0), "Af: not migrated"); - s.priorityTree.checkL1Reinit(_commitment.priorityTree); + s.priorityTree.l1Reinit(_commitment.priorityTree); } else if (_contractAlreadyDeployed) { require(s.settlementLayer != address(0), "Af: not migrated 2"); s.priorityTree.checkGWReinit(_commitment.priorityTree); diff --git a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol index 71d6d9df1..f28dbc5da 100644 --- a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol +++ b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol @@ -98,10 +98,12 @@ library PriorityTree { } /// @notice Reinitialize the tree from a commitment on L1. - function checkL1Reinit(Tree storage _tree, PriorityTreeCommitment memory _commitment) internal view { + function l1Reinit(Tree storage _tree, PriorityTreeCommitment memory _commitment) internal view { require(_tree.startIndex == _commitment.startIndex, "PT: invalid start index"); - require(_tree.unprocessedIndex >= _commitment.unprocessedIndex, "PT: invalid unprocessed index"); + require(_tree.unprocessedIndex <= _commitment.unprocessedIndex, "PT: invalid unprocessed index"); require(_tree.tree._nextLeafIndex >= _commitment.nextLeafIndex, "PT: invalid next leaf index"); + + _tree.unprocessedIndex = _commitment.unprocessedIndex; } /// @notice Reinitialize the tree from a commitment on GW. From 3ad15fb5e1bf3e619fca0b711c5035c2f393be87 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 00:17:13 +0100 Subject: [PATCH 08/23] kl/m03-2gw --- l1-contracts/contracts/bridgehub/Bridgehub.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index 8e23a9f3d..eeb3a80f7 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -762,7 +762,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus ) external payable override onlyAssetRouter onlyL1 { BridgehubBurnCTMAssetData memory bridgehubData = abi.decode(_data, (BridgehubBurnCTMAssetData)); - delete settlementLayer[bridgehubData.chainId]; + settlementLayer[bridgehubData.chainId] = block.chainid; IChainTypeManager(chainTypeManager[bridgehubData.chainId]).forwardedBridgeRecoverFailedTransfer({ _chainId: bridgehubData.chainId, From 4dca48cbd605ee4ed1862d20cd326b212ba957a5 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 00:20:08 +0100 Subject: [PATCH 09/23] kl/m04-2gw --- l1-contracts/contracts/bridgehub/Bridgehub.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index 8e23a9f3d..708347180 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -216,7 +216,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus /// @notice Used for the upgrade to set the baseTokenAssetId previously stored as baseToken. /// @param _chainId the chainId of the chain. function setLegacyBaseTokenAssetId(uint256 _chainId) external override { - if (baseTokenAssetId[_chainId] == bytes32(0)) { + if (baseTokenAssetId[_chainId] != bytes32(0)) { return; } address token = __DEPRECATED_baseToken[_chainId]; From b1ae6b0c3b637f01e43dcadc4c204641a5306bfc Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 00:21:30 +0100 Subject: [PATCH 10/23] fix: kl/m05-2gw more comments --- l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol | 1 + l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol | 1 + 2 files changed, 2 insertions(+) diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index 6c122ccc6..8ca36726c 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -67,6 +67,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { } /// @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. constructor( uint256 _l1ChainId, diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index e96a6d289..9d9dc7c6f 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -37,6 +37,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { bytes32 internal l2TokenProxyBytecodeHash; /// @notice Initializes the bridge contract for later use. + /// @dev this contract is deployed in the L2GenesisUpgrade, and is meant as direct deployment without a proxy. /// @param _l1ChainId The L1 chain id differs between mainnet and testnets. /// @param _l2TokenProxyBytecodeHash The bytecode hash of the proxy for tokens deployed by the bridge. /// @param _aliasedOwner The address of the governor contract. From 69b020952e8b16941819b73d2ffa54a131b952d3 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 10:47:22 +0100 Subject: [PATCH 11/23] stas fixes --- l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol | 6 ++++-- l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol | 9 ++++++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index d93a4f957..a2e877de9 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -120,6 +120,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } } + /// @notice Ensures that the token is deployed inner for legacy tokens. function _ensureTokenDeployedInnerLegacyToken( bytes32 _assetId, address _originToken, @@ -137,16 +138,17 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { tokenAddress[_assetId] = _expectedToken; } - /// @notice Deploys the beacon proxy for the L2 token, while using ContractDeployer system contract. + /// @notice Deploys the beacon proxy for the L2 token, while using ContractDeployer system contract or the legacy shared bridge. /// @dev This function uses raw call to ContractDeployer to make sure that exactly `l2TokenProxyBytecodeHash` is used /// for the code of the proxy. /// @param _salt The salt used for beacon proxy deployment of L2 bridged token. + /// @param _tokenOriginChainId The origin chain id of the token. /// @return proxy The beacon proxy, i.e. L2 bridged token. function _deployBeaconProxy( bytes32 _salt, uint256 _tokenOriginChainId ) internal virtual override returns (BeaconProxy proxy) { - if (address(L2_LEGACY_SHARED_BRIDGE) == address(0) && _tokenOriginChainId != L1_CHAIN_ID) { + if (address(L2_LEGACY_SHARED_BRIDGE) == address(0) || _tokenOriginChainId != L1_CHAIN_ID) { // Deploy the beacon proxy for the L2 token (bool success, bytes memory returndata) = SystemContractsCaller.systemCallWithReturndata( diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index 2280f2add..08873a562 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -370,6 +370,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 }); } + /// @notice Calculates the bridged token address corresponding to native token counterpart. function _calculateExpectedTokenAddress( address _originToken, bytes memory _erc20Data @@ -378,6 +379,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 expectedToken = calculateCreate2TokenAddress(tokenOriginChainId, _originToken); } + /// @notice Returns the origin chain id from the token data. function tokenDataOriginChainId(bytes calldata _erc20Data) public view returns (uint256 tokenOriginChainId) { (tokenOriginChainId, , , ) = DataEncoding.decodeTokenData(_erc20Data); if (tokenOriginChainId == 0) { @@ -385,6 +387,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 } } + /// @notice Checks that the assetId is correct for the origin token and chain. function _assetIdCheck(uint256 _tokenOriginChainId, bytes32 _assetId, address _originToken) internal view { bytes32 expectedAssetId = DataEncoding.encodeNTVAssetId(_tokenOriginChainId, _originToken); if (_assetId != expectedAssetId) { @@ -429,13 +432,13 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 bytes memory _erc20Data ) internal returns (address) { bytes32 salt = _getCreate2Salt(_tokenOriginChainId, _originToken); - - BeaconProxy l2Token = _deployBeaconProxy(salt, _tokenOriginChainId); - BridgedStandardERC20(address(l2Token)).bridgeInitialize(_originToken, _erc20Data); if (_tokenOriginChainId == block.chainid) { revert DeployingBridgedTokenForNativeToken(); } + BeaconProxy l2Token = _deployBeaconProxy(salt, _tokenOriginChainId); + BridgedStandardERC20(address(l2Token)).bridgeInitialize(_originToken, _erc20Data); + originChainId[_assetId] = _tokenOriginChainId; return address(l2Token); } From bbe17544e24132b07fc899018fb611b2cc7be2ae Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 10:51:41 +0100 Subject: [PATCH 12/23] flip order --- .../contracts/bridge/asset-router/L1AssetRouter.sol | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index 8431b5816..8878725f2 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -378,15 +378,15 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { // Do the transfer if allowance to Shared bridge is bigger than amount // And if there is not enough allowance for the NTV bool _weCanTransfer = false; - if ( + if (l1Token.allowance(address(legacyBridge), address(this)) >= _amount) { + _originalCaller = address(legacyBridge); + _weCanTransfer = true; + } else if ( l1Token.allowance(_originalCaller, address(this)) >= _amount && l1Token.allowance(_originalCaller, address(nativeTokenVault)) < _amount ) { _weCanTransfer = true; - } else if (l1Token.allowance(address(legacyBridge), address(this)) >= _amount) { - _originalCaller = address(legacyBridge); - _weCanTransfer = true; - } + } if (_weCanTransfer) { // slither-disable-next-line arbitrary-send-erc20 l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount); From 85d8d469aa71100df2993bf809937e36e5f084ab Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 11:19:53 +0100 Subject: [PATCH 13/23] Vlad issues --- l1-contracts/contracts/bridge/BridgedStandardERC20.sol | 2 +- l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol | 8 ++++++-- l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol | 4 +++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol index c33111fa0..36b93a506 100644 --- a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol +++ b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol @@ -86,7 +86,7 @@ contract BridgedStandardERC20 is ERC20PermitUpgradeable, IBridgedStandardToken, nativeTokenVault = msg.sender; // We parse the data exactly as they were created on the L1 bridge - (uint256 chainId, bytes memory nameBytes, bytes memory symbolBytes, bytes memory decimalsBytes) = DataEncoding + (uint256, bytes memory nameBytes, bytes memory symbolBytes, bytes memory decimalsBytes) = DataEncoding .decodeTokenData(_data); ERC20Getters memory getters; diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index a2e877de9..a89b0ab02 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -108,7 +108,12 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } if (l1LegacyToken != address(0)) { - _ensureTokenDeployedInnerLegacyToken(_assetId, _originToken, _erc20Data, expectedToken, l1LegacyToken); + _ensureTokenDeployedInnerLegacyToken({ + _assetId: _assetId, + _originToken: _originToken, + _expectedToken: expectedToken, + _l1LegacyToken: l1LegacyToken + }); } else { super._ensureTokenDeployedInner({ _tokenOriginChainId: tokenOriginChainId, @@ -124,7 +129,6 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { function _ensureTokenDeployedInnerLegacyToken( bytes32 _assetId, address _originToken, - bytes memory _erc20Data, address _expectedToken, address _l1LegacyToken ) internal { diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index 08873a562..39149334c 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -414,6 +414,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 } /// @notice Calculates the bridged token address corresponding to native token counterpart. + /// @param _tokenOriginChainId The chain id of the origin token. /// @param _bridgeToken The address of native token. /// @return The address of bridged token. function calculateCreate2TokenAddress( @@ -422,6 +423,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 ) public view virtual override returns (address); /// @notice Deploys and initializes the bridged token for the native counterpart. + /// @param _tokenOriginChainId The chain id of the origin token. /// @param _originToken The address of origin token. /// @param _erc20Data The ERC20 metadata of the token deployed. /// @return The address of the beacon proxy (bridged token). @@ -431,10 +433,10 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 address _originToken, bytes memory _erc20Data ) internal returns (address) { - bytes32 salt = _getCreate2Salt(_tokenOriginChainId, _originToken); if (_tokenOriginChainId == block.chainid) { revert DeployingBridgedTokenForNativeToken(); } + bytes32 salt = _getCreate2Salt(_tokenOriginChainId, _originToken); BeaconProxy l2Token = _deployBeaconProxy(salt, _tokenOriginChainId); BridgedStandardERC20(address(l2Token)).bridgeInitialize(_originToken, _erc20Data); From 57a795a39d5797fd2ea3cb087e6ef73a25bb75c4 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 11:23:18 +0100 Subject: [PATCH 14/23] undocumented param --- l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol | 1 + 1 file changed, 1 insertion(+) diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index a89b0ab02..6c65ca08e 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -191,6 +191,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { //////////////////////////////////////////////////////////////*/ /// @notice Calculates L2 wrapped token address given the currently stored beacon proxy bytecode hash and beacon address. + /// @param _tokenOriginChainId The chain id of the origin token. /// @param _l1Token The address of token on L1. /// @return Address of an L2 token counterpart. function calculateCreate2TokenAddress( From c9b138f436398c194a5892d1a06f0a8cc05552a9 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 12:38:56 +0100 Subject: [PATCH 15/23] encoding issue clean up --- .../bridge/asset-router/L1AssetRouter.sol | 2 +- .../contracts/common/libraries/DataEncoding.sol | 16 +++++++++++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index 8878725f2..f3679d12d 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -522,7 +522,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { // Save the deposited amount to claim funds on L1 if the deposit failed on L2 L1_NULLIFIER.bridgehubConfirmL2TransactionForwarded( ERA_CHAIN_ID, - keccak256(abi.encode(_originalCaller, _l1Token, _amount)), + DataEncoding.encodeLegacyTxDataHash(_originalCaller, _l1Token, _amount), txHash ); diff --git a/l1-contracts/contracts/common/libraries/DataEncoding.sol b/l1-contracts/contracts/common/libraries/DataEncoding.sol index 9df83d67a..e586bd051 100644 --- a/l1-contracts/contracts/common/libraries/DataEncoding.sol +++ b/l1-contracts/contracts/common/libraries/DataEncoding.sol @@ -108,7 +108,7 @@ library DataEncoding { if (_encodingVersion == LEGACY_ENCODING_VERSION) { address tokenAddress = INativeTokenVault(_nativeTokenVault).tokenAddress(_assetId); (uint256 depositAmount, ) = abi.decode(_transferData, (uint256, address)); - txDataHash = keccak256(abi.encode(_originalCaller, tokenAddress, depositAmount)); + txDataHash = encodeLegacyTxDataHash(_originalCaller, tokenAddress, depositAmount); } else if (_encodingVersion == NEW_ENCODING_VERSION) { // Similarly to calldata, the txDataHash is collision-resistant. // In the legacy data hash, the first encoded variable was the address, which is padded with zeros during `abi.encode`. @@ -120,6 +120,20 @@ library DataEncoding { } } + /// @notice Encodes the legacy transaction data hash. + /// @dev the encoding strats with 0t + /// @param _originalCaller The address of the entity that initiated the deposit. + /// @param _l1Token The address of the L1 token. + /// @param _amount The amount of the L1 token. + /// @return txDataHash The resulting encoded transaction data hash. + function encodeLegacyTxDataHash( + address _originalCaller, + address _l1Token, + uint256 _amount + ) internal pure returns (bytes32) { + return keccak256(abi.encode(_originalCaller, _l1Token, _amount)); + } + /// @notice Decodes the token data by combining chain id, asset deployment tracker and asset data. function decodeTokenData( bytes calldata _tokenData From 9138d1ed8b3e54bc84cda26780e5f695b1a21ea6 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 12:57:02 +0100 Subject: [PATCH 16/23] some fixes --- l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol | 4 ++-- l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index 6c65ca08e..db0e8d63b 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -198,11 +198,11 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { uint256 _tokenOriginChainId, address _l1Token ) public view virtual override(INativeTokenVault, NativeTokenVault) returns (address) { - bytes32 constructorInputHash = keccak256(abi.encode(address(bridgedTokenBeacon), "")); - bytes32 salt = _getCreate2Salt(_tokenOriginChainId, _l1Token); if (address(L2_LEGACY_SHARED_BRIDGE) != address(0) && _tokenOriginChainId == L1_CHAIN_ID) { return L2_LEGACY_SHARED_BRIDGE.l2TokenAddress(_l1Token); } else { + bytes32 constructorInputHash = keccak256(abi.encode(address(bridgedTokenBeacon), "")); + bytes32 salt = _getCreate2Salt(_tokenOriginChainId, _l1Token); return L2ContractHelper.computeCreate2Address( address(this), diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index 39149334c..d5e6cd28d 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -375,6 +375,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 address _originToken, bytes memory _erc20Data ) internal view returns (address expectedToken, uint256 tokenOriginChainId) { + /// @dev calling externally to convert from memory to calldata tokenOriginChainId = this.tokenDataOriginChainId(_erc20Data); expectedToken = calculateCreate2TokenAddress(tokenOriginChainId, _originToken); } From c5e5c621d87a87e18753cb7f7acc2ec8efa05293 Mon Sep 17 00:00:00 2001 From: kelemeno <34402761+kelemeno@users.noreply.github.com> Date: Tue, 15 Oct 2024 12:57:49 +0100 Subject: [PATCH 17/23] Update l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol Co-authored-by: Vlad Bochok <41153528+vladbochok@users.noreply.github.com> --- l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index f3679d12d..2992b113b 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -377,7 +377,7 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { // Do the transfer if allowance to Shared bridge is bigger than amount // And if there is not enough allowance for the NTV - bool _weCanTransfer = false; + bool weCanTransfer = false; if (l1Token.allowance(address(legacyBridge), address(this)) >= _amount) { _originalCaller = address(legacyBridge); _weCanTransfer = true; From 15ef6d8c173452fddae497ef88bbd54f4301b7f7 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 13:43:43 +0100 Subject: [PATCH 18/23] renaming ensureTokenDeployed --- l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol | 8 ++++---- l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index db0e8d63b..d6390bb4b 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -95,7 +95,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { /// @param _originToken The origin token address. /// @param _erc20Data The ERC20 data. /// @return expectedToken The token address. - function _ensureTokenDeployed( + function _ensureAndSaveTokenDeployed( bytes32 _assetId, address _originToken, bytes memory _erc20Data @@ -108,14 +108,14 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } if (l1LegacyToken != address(0)) { - _ensureTokenDeployedInnerLegacyToken({ + _ensureAndSaveTokenDeployedInnerLegacyToken({ _assetId: _assetId, _originToken: _originToken, _expectedToken: expectedToken, _l1LegacyToken: l1LegacyToken }); } else { - super._ensureTokenDeployedInner({ + super._ensureAndSaveTokenDeployedInner({ _tokenOriginChainId: tokenOriginChainId, _assetId: _assetId, _originToken: _originToken, @@ -126,7 +126,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } /// @notice Ensures that the token is deployed inner for legacy tokens. - function _ensureTokenDeployedInnerLegacyToken( + function _ensureAndSaveTokenDeployedInnerLegacyToken( bytes32 _assetId, address _originToken, address _expectedToken, diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index d5e6cd28d..de803f888 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -132,7 +132,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 (, receiver, originToken, amount, erc20Data) = DataEncoding.decodeBridgeMintData(_data); if (token == address(0)) { - token = _ensureTokenDeployed(_assetId, originToken, erc20Data); + token = _ensureAndSaveTokenDeployed(_assetId, originToken, erc20Data); } _handleChainBalanceDecrease(_originChainId, _assetId, amount, false); IBridgedStandardToken(token).bridgeMint(receiver, amount); @@ -354,14 +354,14 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 TOKEN DEPLOYER FUNCTIONS //////////////////////////////////////////////////////////////*/ - function _ensureTokenDeployed( + function _ensureAndSaveTokenDeployed( bytes32 _assetId, address _originToken, bytes memory _erc20Data ) internal virtual returns (address expectedToken) { uint256 tokenOriginChainId; (expectedToken, tokenOriginChainId) = _calculateExpectedTokenAddress(_originToken, _erc20Data); - _ensureTokenDeployedInner({ + _ensureAndSaveTokenDeployedInner({ _tokenOriginChainId: tokenOriginChainId, _assetId: _assetId, _originToken: _originToken, @@ -397,7 +397,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 } } - function _ensureTokenDeployedInner( + function _ensureAndSaveTokenDeployedInner( uint256 _tokenOriginChainId, bytes32 _assetId, address _originToken, From cc226f1992f2b06bec989e0c7b039c6d0ae40763 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 14:13:23 +0100 Subject: [PATCH 19/23] vlad issues --- l1-contracts/contracts/bridgehub/Bridgehub.sol | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/l1-contracts/contracts/bridgehub/Bridgehub.sol b/l1-contracts/contracts/bridgehub/Bridgehub.sol index 742a38472..bbc296229 100644 --- a/l1-contracts/contracts/bridgehub/Bridgehub.sol +++ b/l1-contracts/contracts/bridgehub/Bridgehub.sol @@ -426,11 +426,7 @@ contract Bridgehub is IBridgehub, ReentrancyGuard, Ownable2StepUpgradeable, Paus if (ctmAddress == address(0)) { revert ChainIdNotRegistered(_chainId); } - return ctmAssetIdFromAddress[chainTypeManager[_chainId]]; - } - - function calculateCtmAssetId(address _ctmAddress) internal view returns (bytes32) { - return keccak256(abi.encode(L1_CHAIN_ID, address(l1CtmDeployer), bytes32(uint256(uint160(_ctmAddress))))); + return ctmAssetIdFromAddress[ctmAddress]; } /*////////////////////////////////////////////////////////////// From 89247f39d46aef4553e42cde6a789ebc806eea77 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 15:31:54 +0100 Subject: [PATCH 20/23] getting to build --- l1-contracts/contracts/bridge/BridgedStandardERC20.sol | 2 +- .../contracts/bridge/asset-router/L1AssetRouter.sol | 6 +++--- l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol | 2 +- .../contracts/state-transition/libraries/PriorityTree.sol | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol index 198b8dc12..39effff61 100644 --- a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol +++ b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol @@ -100,7 +100,7 @@ contract BridgedStandardERC20 is ERC20PermitUpgradeable, IBridgedStandardToken, nativeTokenVault = msg.sender; // We parse the data exactly as they were created on the L1 bridge - (uint256, bytes memory nameBytes, bytes memory symbolBytes, bytes memory decimalsBytes) = DataEncoding + (, bytes memory nameBytes, bytes memory symbolBytes, bytes memory decimalsBytes) = DataEncoding .decodeTokenData(_data); ERC20Getters memory getters; diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index 6b06113d0..e4131d655 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -407,14 +407,14 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { bool weCanTransfer = false; if (l1Token.allowance(address(legacyBridge), address(this)) >= _amount) { _originalCaller = address(legacyBridge); - _weCanTransfer = true; + weCanTransfer = true; } else if ( l1Token.allowance(_originalCaller, address(this)) >= _amount && l1Token.allowance(_originalCaller, address(nativeTokenVault)) < _amount ) { - _weCanTransfer = true; + weCanTransfer = true; } - if (_weCanTransfer) { + if (weCanTransfer) { // slither-disable-next-line arbitrary-send-erc20 l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount); return true; diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index 7fe944117..e6b3c0298 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -142,7 +142,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } tokenAddress[_assetId] = _expectedToken; - assetId[expectedToken] = _assetId; + assetId[_expectedToken] = _assetId; } /// @notice Deploys the beacon proxy for the L2 token, while using ContractDeployer system contract or the legacy shared bridge. diff --git a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol index f28dbc5da..9b3360f03 100644 --- a/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol +++ b/l1-contracts/contracts/state-transition/libraries/PriorityTree.sol @@ -98,7 +98,7 @@ library PriorityTree { } /// @notice Reinitialize the tree from a commitment on L1. - function l1Reinit(Tree storage _tree, PriorityTreeCommitment memory _commitment) internal view { + function l1Reinit(Tree storage _tree, PriorityTreeCommitment memory _commitment) internal { require(_tree.startIndex == _commitment.startIndex, "PT: invalid start index"); require(_tree.unprocessedIndex <= _commitment.unprocessedIndex, "PT: invalid unprocessed index"); require(_tree.tree._nextLeafIndex >= _commitment.nextLeafIndex, "PT: invalid next leaf index"); From 077746afae0f28dce6b65f4a8cf9731e7f523ad2 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 15:33:41 +0100 Subject: [PATCH 21/23] fmt --- .../contracts/bridge/BridgedStandardERC20.sol | 11 ++++------- .../contracts/bridge/asset-router/L1AssetRouter.sol | 6 +++--- .../contracts/bridge/asset-router/L2AssetRouter.sol | 2 +- .../contracts/bridge/ntv/L2NativeTokenVault.sol | 4 ++-- .../contracts/bridge/ntv/NativeTokenVault.sol | 6 +----- 5 files changed, 11 insertions(+), 18 deletions(-) diff --git a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol index 39effff61..d3715a183 100644 --- a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol +++ b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol @@ -86,11 +86,7 @@ contract BridgedStandardERC20 is ERC20PermitUpgradeable, IBridgedStandardToken, /// @param _originToken Address of the origin token that can be deposited to mint this bridged token /// @param _data The additional data that the L1 bridge provide for initialization. /// In this case, it is packed `name`/`symbol`/`decimals` of the L1 token. - function bridgeInitialize( - bytes32 _assetId, - address _originToken, - bytes calldata _data - ) external initializer { + function bridgeInitialize(bytes32 _assetId, address _originToken, bytes calldata _data) external initializer { if (_originToken == address(0)) { revert ZeroAddress(); } @@ -100,8 +96,9 @@ contract BridgedStandardERC20 is ERC20PermitUpgradeable, IBridgedStandardToken, nativeTokenVault = msg.sender; // We parse the data exactly as they were created on the L1 bridge - (, bytes memory nameBytes, bytes memory symbolBytes, bytes memory decimalsBytes) = DataEncoding - .decodeTokenData(_data); + (, bytes memory nameBytes, bytes memory symbolBytes, bytes memory decimalsBytes) = DataEncoding.decodeTokenData( + _data + ); ERC20Getters memory getters; string memory decodedName; diff --git a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol index e4131d655..bc97e9bfa 100644 --- a/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L1AssetRouter.sol @@ -406,14 +406,14 @@ contract L1AssetRouter is AssetRouterBase, IL1AssetRouter, ReentrancyGuard { // And if there is not enough allowance for the NTV bool weCanTransfer = false; if (l1Token.allowance(address(legacyBridge), address(this)) >= _amount) { - _originalCaller = address(legacyBridge); - weCanTransfer = true; + _originalCaller = address(legacyBridge); + weCanTransfer = true; } else if ( l1Token.allowance(_originalCaller, address(this)) >= _amount && l1Token.allowance(_originalCaller, address(nativeTokenVault)) < _amount ) { weCanTransfer = true; - } + } if (weCanTransfer) { // slither-disable-next-line arbitrary-send-erc20 l1Token.safeTransferFrom(_originalCaller, address(nativeTokenVault), _amount); diff --git a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol index 5a1196e36..2bd0fa748 100644 --- a/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol +++ b/l1-contracts/contracts/bridge/asset-router/L2AssetRouter.sol @@ -68,7 +68,7 @@ contract L2AssetRouter is AssetRouterBase, IL2AssetRouter { } /// @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. + /// @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. constructor( uint256 _l1ChainId, diff --git a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol index e6b3c0298..b896f11df 100644 --- a/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/L2NativeTokenVault.sol @@ -37,7 +37,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { bytes32 internal l2TokenProxyBytecodeHash; /// @notice Initializes the bridge contract for later use. - /// @dev this contract is deployed in the L2GenesisUpgrade, and is meant as direct deployment without a proxy. + /// @dev this contract is deployed in the L2GenesisUpgrade, and is meant as direct deployment without a proxy. /// @param _l1ChainId The L1 chain id differs between mainnet and testnets. /// @param _l2TokenProxyBytecodeHash The bytecode hash of the proxy for tokens deployed by the bridge. /// @param _aliasedOwner The address of the governor contract. @@ -127,7 +127,7 @@ contract L2NativeTokenVault is IL2NativeTokenVault, NativeTokenVault { } } - /// @notice Ensures that the token is deployed inner for legacy tokens. + /// @notice Ensures that the token is deployed inner for legacy tokens. function _ensureAndSaveTokenDeployedInnerLegacyToken( bytes32 _assetId, address _originToken, diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index eb3fe4b30..6cdb89222 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -446,11 +446,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 bytes32 salt = _getCreate2Salt(_tokenOriginChainId, _originToken); BeaconProxy l2Token = _deployBeaconProxy(salt, _tokenOriginChainId); - BridgedStandardERC20(address(l2Token)).bridgeInitialize( - _assetId, - _originToken, - _erc20Data - ); + BridgedStandardERC20(address(l2Token)).bridgeInitialize(_assetId, _originToken, _erc20Data); originChainId[_assetId] = _tokenOriginChainId; return address(l2Token); From 2e80c19987afdd276ae8b3a1643621478affb229 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 18:07:51 +0100 Subject: [PATCH 22/23] slither issues --- .../contracts/bridge/BridgedStandardERC20.sol | 1 + l1-contracts/contracts/bridge/L1ERC20Bridge.sol | 12 +++++++++--- .../contracts/bridge/ntv/NativeTokenVault.sol | 1 + l1-contracts/contracts/common/L1ContractErrors.sol | 2 ++ 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol index d3715a183..bbe0ed8c4 100644 --- a/l1-contracts/contracts/bridge/BridgedStandardERC20.sol +++ b/l1-contracts/contracts/bridge/BridgedStandardERC20.sol @@ -96,6 +96,7 @@ contract BridgedStandardERC20 is ERC20PermitUpgradeable, IBridgedStandardToken, nativeTokenVault = msg.sender; // We parse the data exactly as they were created on the L1 bridge + // slither-disable-next-line unused-return (, bytes memory nameBytes, bytes memory symbolBytes, bytes memory decimalsBytes) = DataEncoding.decodeTokenData( _data ); diff --git a/l1-contracts/contracts/bridge/L1ERC20Bridge.sol b/l1-contracts/contracts/bridge/L1ERC20Bridge.sol index 78726ed47..190c086b4 100644 --- a/l1-contracts/contracts/bridge/L1ERC20Bridge.sol +++ b/l1-contracts/contracts/bridge/L1ERC20Bridge.sol @@ -13,7 +13,7 @@ import {IL1AssetRouter} from "./asset-router/IL1AssetRouter.sol"; import {L2ContractHelper} from "../common/libraries/L2ContractHelper.sol"; import {ReentrancyGuard} from "../common/ReentrancyGuard.sol"; -import {EmptyDeposit, WithdrawalAlreadyFinalized, TokensWithFeesNotSupported, ETHDepositNotSupported} from "../common/L1ContractErrors.sol"; +import {EmptyDeposit, WithdrawalAlreadyFinalized, TokensWithFeesNotSupported, ETHDepositNotSupported, ApprovalFailed} from "../common/L1ContractErrors.sol"; import {ETH_TOKEN_ADDRESS} from "../common/Config.sol"; /// @author Matter Labs @@ -204,7 +204,10 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard { _refundRecipient: _refundRecipient }); // clearing approval - IERC20(_l1Token).approve(address(L1_ASSET_ROUTER), 0); + bool success = IERC20(_l1Token).approve(address(L1_ASSET_ROUTER), 0); + if (!success) { + revert ApprovalFailed(); + } depositAmount[msg.sender][_l1Token][l2TxHash] = _amount; emit DepositInitiated({ l2DepositTxHash: l2TxHash, @@ -224,7 +227,10 @@ contract L1ERC20Bridge is IL1ERC20Bridge, ReentrancyGuard { function _approveFundsToAssetRouter(address _from, IERC20 _token, uint256 _amount) internal returns (uint256) { uint256 balanceBefore = _token.balanceOf(address(this)); _token.safeTransferFrom(_from, address(this), _amount); - _token.approve(address(L1_ASSET_ROUTER), _amount); + bool success = _token.approve(address(L1_ASSET_ROUTER), _amount); + if (!success) { + revert ApprovalFailed(); + } uint256 balanceAfter = _token.balanceOf(address(this)); return balanceAfter - balanceBefore; diff --git a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol index 6cdb89222..805e1e041 100644 --- a/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol +++ b/l1-contracts/contracts/bridge/ntv/NativeTokenVault.sol @@ -387,6 +387,7 @@ abstract contract NativeTokenVault is INativeTokenVault, IAssetHandler, Ownable2 /// @notice Returns the origin chain id from the token data. function tokenDataOriginChainId(bytes calldata _erc20Data) public view returns (uint256 tokenOriginChainId) { + // slither-disable-next-line unused-return (tokenOriginChainId, , , ) = DataEncoding.decodeTokenData(_erc20Data); if (tokenOriginChainId == 0) { tokenOriginChainId = L1_CHAIN_ID; diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index 89fbf482d..debe0d075 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -5,6 +5,8 @@ pragma solidity ^0.8.21; error AccessToFallbackDenied(address target, address invoker); // 0x3995f750 error AccessToFunctionDenied(address target, bytes4 selector, address invoker); +// +error ApprovalFailed(); // 0x6c167909 error OnlySelfAllowed(); // 0x52e22c98 From 5ea47e1001fd6b060a1a7c16b55d9063bc77b6f5 Mon Sep 17 00:00:00 2001 From: kelemeno Date: Tue, 15 Oct 2024 18:11:01 +0100 Subject: [PATCH 23/23] missing selector --- l1-contracts/contracts/common/L1ContractErrors.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/l1-contracts/contracts/common/L1ContractErrors.sol b/l1-contracts/contracts/common/L1ContractErrors.sol index debe0d075..a31ac3f14 100644 --- a/l1-contracts/contracts/common/L1ContractErrors.sol +++ b/l1-contracts/contracts/common/L1ContractErrors.sol @@ -5,7 +5,7 @@ pragma solidity ^0.8.21; error AccessToFallbackDenied(address target, address invoker); // 0x3995f750 error AccessToFunctionDenied(address target, bytes4 selector, address invoker); -// +// 0x8164f842 error ApprovalFailed(); // 0x6c167909 error OnlySelfAllowed();