Fix audittens m01 (#22)

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 {AssetIdAlreadyRegistered, EmptyDeposit, Unauthorized, TokensWithFeesNotSupported, TokenNotSupported, NonEmptyMsgValue, ValueMismatch, AddressMismatch, AssetIdMismatch, AmountMustBeGreaterThanZero, ZeroAddress, DeployingBridgedTokenForNativeToken} from "../../common/L1ContractErrors.sol";
+import {BurningNativeWETHNotSupported, AssetIdAlreadyRegistered, EmptyDeposit, Unauthorized, TokensWithFeesNotSupported, TokenNotSupported, NonEmptyMsgValue, ValueMismatch, AddressMismatch, AssetIdMismatch, AmountMustBeGreaterThanZero, ZeroAddress, DeployingBridgedTokenForNativeToken} from "../../common/L1ContractErrors.sol";
 import {AssetHandlerModifiers} from "../interfaces/AssetHandlerModifiers.sol";
 
 /// @author Matter Labs
@@ -95,7 +95,11 @@ abstract contract NativeTokenVault is
     }
 
     function _registerToken(address _nativeToken) internal virtual {
-        if (_nativeToken == WETH_TOKEN) {
+        // We allow registering `WETH_TOKEN` inside `NativeTokenVault` only for L1 native token vault.
+        // It is needed to allow withdrawing such assets. We restrict all WETH-related
+        // operations to deposits from L1 only to be able to upgrade their logic more easily in the
+        // future.
+        if (_nativeToken == WETH_TOKEN && block.chainid != L1_CHAIN_ID) {
             revert TokenNotSupported(WETH_TOKEN);
         }
         require(_nativeToken.code.length > 0, "NTV: empty token");
@@ -268,6 +272,11 @@ abstract contract NativeTokenVault is
         (uint256 _depositAmount, address _receiver) = abi.decode(_data, (uint256, address));
 
         address nativeToken = tokenAddress[_assetId];
+        if (nativeToken == WETH_TOKEN) {
+            // This ensures that WETH_TOKEN can never be bridged from chains it is native to.
+            // It can only be withdrawn from the chain where it has already gotten.
+            revert BurningNativeWETHNotSupported();
+        }
         if (_assetId == BASE_TOKEN_ASSET_ID) {
             if (_depositAmount != msg.value) {
                 revert ValueMismatch(_depositAmount, msg.value);

l1-contracts/contracts/common/L1ContractErrors.sol

@@ -430,6 +430,8 @@ error TokenIsLegacy();
 error LegacyBridgeUsesNonNativeToken();
 // 0x11832de8
 error AssetRouterAllowanceNotZero();
+// 0xaa5f6180
+error BurningNativeWETHNotSupported();
 // 0xb20b58ce
 error NoLegacySharedBridge();
 

l1-contracts/contracts/dev-contracts/test/DummySharedBridge.sol

@@ -58,13 +58,16 @@ contract DummySharedBridge is PausableUpgradeable {
     function depositLegacyErc20Bridge(
         address, //_msgSender,
         address, //_l2Receiver,
-        address, //_l1Token,
-        uint256, //_amount,
+        address _l1Token,
+        uint256 _amount,
         uint256, //_l2TxGasLimit,
         uint256, //_l2TxGasPerPubdataByte,
         address //_refundRecipient
     ) external payable returns (bytes32 txHash) {
         txHash = dummyL2DepositTxHash;
+
+        // Legacy bridge requires this logic to work properly
+        IERC20(_l1Token).transferFrom(msg.sender, address(this), _amount);
     }
 
     function claimFailedDepositLegacyErc20Bridge(

l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1Erc20Bridge/Deposit.t.sol

@@ -125,30 +125,14 @@ contract DepositTest is L1Erc20BridgeTest {
 
     function test_depositSuccessfully() public {
         uint256 amount = 8;
-        bytes32 l2TxHash = keccak256("txHash");
-
-        vm.mockCall(
-            sharedBridgeAddress,
-            abi.encodeWithSelector(
-                IL1AssetRouter.depositLegacyErc20Bridge.selector,
-                alice,
-                randomSigner,
-                address(token),
-                amount,
-                0,
-                0,
-                address(0)
-            ),
-            abi.encode(l2TxHash)
-        );
 
         vm.prank(alice);
         token.approve(address(bridge), amount);
         vm.prank(alice);
         // solhint-disable-next-line func-named-parameters
         vm.expectEmit(true, true, true, true, address(bridge));
         // solhint-disable-next-line func-named-parameters
-        emit DepositInitiated(l2TxHash, alice, randomSigner, address(token), amount);
+        emit DepositInitiated(dummyL2DepositTxHash, alice, randomSigner, address(token), amount);
         bytes32 txHash = bridge.deposit({
             _l2Receiver: randomSigner,
             _l1Token: address(token),
@@ -157,51 +141,35 @@ contract DepositTest is L1Erc20BridgeTest {
             _l2TxGasPerPubdataByte: 0,
             _refundRecipient: address(0)
         });
-        assertEq(txHash, l2TxHash);
+        assertEq(txHash, dummyL2DepositTxHash);
 
-        uint256 depositedAmount = bridge.depositAmount(alice, address(token), l2TxHash);
+        uint256 depositedAmount = bridge.depositAmount(alice, address(token), dummyL2DepositTxHash);
         assertEq(amount, depositedAmount);
     }
 
     function test_legacyDepositSuccessfully() public {
         uint256 amount = 8;
-        bytes32 l2TxHash = keccak256("txHash");
 
-        uint256 depositedAmountBefore = bridge.depositAmount(alice, address(token), l2TxHash);
+        uint256 depositedAmountBefore = bridge.depositAmount(alice, address(token), dummyL2DepositTxHash);
         assertEq(depositedAmountBefore, 0);
 
-        vm.mockCall(
-            sharedBridgeAddress,
-            abi.encodeWithSelector(
-                IL1AssetRouter.depositLegacyErc20Bridge.selector,
-                alice,
-                randomSigner,
-                address(token),
-                amount,
-                0,
-                0,
-                address(0)
-            ),
-            abi.encode(l2TxHash)
-        );
-
         vm.prank(alice);
         token.approve(address(bridge), amount);
         vm.prank(alice);
         // solhint-disable-next-line func-named-parameters
         vm.expectEmit(true, true, true, true, address(bridge));
         // solhint-disable-next-line func-named-parameters
-        emit DepositInitiated(l2TxHash, alice, randomSigner, address(token), amount);
+        emit DepositInitiated(dummyL2DepositTxHash, alice, randomSigner, address(token), amount);
         bytes32 txHash = bridge.deposit({
             _l2Receiver: randomSigner,
             _l1Token: address(token),
             _amount: amount,
             _l2TxGasLimit: 0,
             _l2TxGasPerPubdataByte: 0
         });
-        assertEq(txHash, l2TxHash);
+        assertEq(txHash, dummyL2DepositTxHash);
 
-        uint256 depositedAmount = bridge.depositAmount(alice, address(token), l2TxHash);
+        uint256 depositedAmount = bridge.depositAmount(alice, address(token), dummyL2DepositTxHash);
         assertEq(amount, depositedAmount);
     }
 }

l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1Erc20Bridge/_L1Erc20Bridge_Shared.t.sol

@@ -35,7 +35,7 @@ contract L1Erc20BridgeTest is Test {
     constructor() {
         randomSigner = makeAddr("randomSigner");
         dummyL2DepositTxHash = Utils.randomBytes32("dummyL2DepositTxHash");
-        sharedBridgeAddress = makeAddr("sharedBridgeAddress");
+        sharedBridgeAddress = address(new DummySharedBridge(dummyL2DepositTxHash));
         alice = makeAddr("alice");
         l1NullifierAddress = makeAddr("l1NullifierAddress");
 

l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/L1SharedBridgeFails.t.sol

@@ -21,7 +21,7 @@ import {INativeTokenVault} from "contracts/bridge/ntv/INativeTokenVault.sol";
 import {L1NativeTokenVault} from "contracts/bridge/ntv/L1NativeTokenVault.sol";
 import {L2_BASE_TOKEN_SYSTEM_CONTRACT_ADDR} from "contracts/common/L2ContractAddresses.sol";
 import {IGetters} from "contracts/state-transition/chain-interfaces/IGetters.sol";
-import {AddressAlreadyUsed, WithdrawFailed, Unauthorized, AssetIdNotSupported, SharedBridgeKey, SharedBridgeValueNotSet, L2WithdrawalMessageWrongLength, InsufficientChainBalance, ZeroAddress, ValueMismatch, NonEmptyMsgValue, DepositExists, ValueMismatch, NonEmptyMsgValue, TokenNotSupported, EmptyDeposit, L2BridgeNotDeployed, InvalidProof, NoFundsTransferred, InsufficientFunds, DepositDoesNotExist, WithdrawalAlreadyFinalized, InsufficientFunds, MalformedMessage, InvalidSelector, TokensWithFeesNotSupported} from "contracts/common/L1ContractErrors.sol";
+import {BurningNativeWETHNotSupported, AddressAlreadyUsed, WithdrawFailed, Unauthorized, AssetIdNotSupported, SharedBridgeKey, SharedBridgeValueNotSet, L2WithdrawalMessageWrongLength, InsufficientChainBalance, ZeroAddress, ValueMismatch, NonEmptyMsgValue, DepositExists, ValueMismatch, NonEmptyMsgValue, TokenNotSupported, EmptyDeposit, L2BridgeNotDeployed, InvalidProof, NoFundsTransferred, InsufficientFunds, DepositDoesNotExist, WithdrawalAlreadyFinalized, InsufficientFunds, MalformedMessage, InvalidSelector, TokensWithFeesNotSupported} from "contracts/common/L1ContractErrors.sol";
 import {StdStorage, stdStorage} from "forge-std/Test.sol";
 
 /// We are testing all the specified revert and require cases.
@@ -169,7 +169,7 @@ contract L1AssetRouterFailTest is L1AssetRouterTest {
 
     function test_bridgehubDeposit_Erc_weth() public {
         vm.prank(bridgehubAddress);
-        vm.expectRevert(abi.encodeWithSelector(TokenNotSupported.selector, l1WethAddress));
+        vm.expectRevert(BurningNativeWETHNotSupported.selector);
         // solhint-disable-next-line func-named-parameters
         sharedBridge.bridgehubDeposit(chainId, alice, 0, abi.encode(l1WethAddress, amount, bob));
     }

l1-contracts/test/foundry/l1/unit/concrete/Bridges/L1SharedBridge/_L1SharedBridge_Shared.t.sol

@@ -6,6 +6,7 @@ import {Test} from "forge-std/Test.sol";
 import "forge-std/console.sol";
 
 import {TransparentUpgradeableProxy} from "@openzeppelin/contracts-v4/proxy/transparent/TransparentUpgradeableProxy.sol";
+import {ERC20} from "@openzeppelin/contracts-v4/token/ERC20/ERC20.sol";
 
 import {L1AssetRouter} from "contracts/bridge/asset-router/L1AssetRouter.sol";
 import {IL1AssetRouter} from "contracts/bridge/asset-router/IL1AssetRouter.sol";
@@ -108,7 +109,7 @@ contract L1AssetRouterTest is Test {
         bridgehubAddress = makeAddr("bridgehub");
         alice = makeAddr("alice");
         // bob = makeAddr("bob");
-        l1WethAddress = makeAddr("weth");
+        l1WethAddress = address(new ERC20("Wrapped ETH", "WETH"));
         l1ERC20BridgeAddress = makeAddr("l1ERC20Bridge");
         l2SharedBridge = makeAddr("l2SharedBridge");