Address RES-NODL-BRD01 (#94)

* feat(L1Bridge): add view helpers to quote ETH needed for L2 execution

Address RES-01 “Unbounded ETH Forwarded to Mailbox” from Resonance’s audit by exposing read-only helpers that return the exact base cost as computed by the zkSync Mailbox. We retain the exact-match ETH forwarding model (no capping/refund) to minimize avoidable failures and align with zkSync BridgeHub’s design, which shifts fee calculation to caller/off-chain code.
The added helpers will let integrators pre-compute msg.value without calling Mailbox directly.

* fix(L1Nodl): address RES-03 Missing Zero Address Validation On L1Nodl.constructor()

* fix(L1Bridge): address RES-04 Missing Zero Address Validation On deposit()

* fix(L1Bridge): follow CEI Pattern more strictly to address RES-05

* test: cover L1Nodl and L1Bridge changes

Author: aliXsed

.cspell.json

@@ -56,6 +56,7 @@
     "addrsSlot",
     "NODLNS",
     "solhint",
-    "mixedcase"
+    "mixedcase",
+    "Frontends"
   ]
 }

src/L1Nodl.sol

@@ -12,7 +12,13 @@ import {Nonces} from "@openzeppelin/contracts/utils/Nonces.sol";
 contract L1Nodl is ERC20, ERC20Burnable, AccessControl, ERC20Permit, ERC20Votes {
     bytes32 private constant MINTER_ROLE = keccak256("MINTER_ROLE");
 
+    /// @dev Zero address supplied where non-zero is required.
+    error ZeroAddress();
+
     constructor(address admin, address minter) ERC20("Nodle Token", "NODL") ERC20Permit("Nodle Token") {
+        if (admin == address(0) || minter == address(0)) {
+            revert ZeroAddress();
+        }
         _grantRole(DEFAULT_ADMIN_ROLE, admin);
         _grantRole(MINTER_ROLE, minter);
     }

src/bridge/L1Bridge.sol

@@ -100,6 +100,41 @@ contract L1Bridge is Ownable2Step, Pausable, IL1Bridge {
         _unpause();
     }
 
+    // =============================
+    // View helpers
+    // =============================
+
+    /**
+     * @notice Quotes the ETH required to cover the L2 execution cost for a deposit at the current tx.gasprice.
+     * @dev This is a convenience helper; the actual base cost is a function of the L1 gas price at inclusion time.
+     *      Frontends may prefer {quoteL2BaseCostAtGasPrice} for deterministic quoting.
+     * @param _l2TxGasLimit Maximum L2 gas the enqueued call can consume.
+     * @param _l2TxGasPerPubdataByte Gas per pubdata byte limit for the enqueued call.
+     * @return baseCost The ETH amount that needs to be supplied alongside {deposit}.
+     */
+    function quoteL2BaseCost(uint256 _l2TxGasLimit, uint256 _l2TxGasPerPubdataByte)
+        external
+        view
+        returns (uint256 baseCost)
+    {
+        baseCost = L1_MAILBOX.l2TransactionBaseCost(tx.gasprice, _l2TxGasLimit, _l2TxGasPerPubdataByte);
+    }
+
+    /**
+     * @notice Quotes the ETH required to cover the L2 execution cost for a deposit at a specified L1 gas price.
+     * @param _l1GasPrice The L1 gas price (wei) to use for the quote.
+     * @param _l2TxGasLimit Maximum L2 gas the enqueued call can consume.
+     * @param _l2TxGasPerPubdataByte Gas per pubdata byte limit for the enqueued call.
+     * @return baseCost The ETH amount that needs to be supplied alongside {deposit}.
+     */
+    function quoteL2BaseCostAtGasPrice(uint256 _l1GasPrice, uint256 _l2TxGasLimit, uint256 _l2TxGasPerPubdataByte)
+        external
+        view
+        returns (uint256 baseCost)
+    {
+        baseCost = L1_MAILBOX.l2TransactionBaseCost(_l1GasPrice, _l2TxGasLimit, _l2TxGasPerPubdataByte);
+    }
+
     // =============================
     // External entrypoints
     // =============================
@@ -121,6 +156,9 @@ contract L1Bridge is Ownable2Step, Pausable, IL1Bridge {
         uint256 _l2TxGasPerPubdataByte,
         address _refundRecipient
     ) public payable override whenNotPaused returns (bytes32 txHash) {
+        if (_l2Receiver == address(0)) {
+            revert ZeroAddress();
+        }
         if (_amount == 0) {
             revert ZeroAmount();
         }
@@ -203,7 +241,6 @@ contract L1Bridge is Ownable2Step, Pausable, IL1Bridge {
         if (isWithdrawalFinalized[_l2BatchNumber][_l2MessageIndex]) {
             revert WithdrawalAlreadyFinalized();
         }
-        isWithdrawalFinalized[_l2BatchNumber][_l2MessageIndex] = true;
 
         (address l1Receiver, uint256 amount) = _parseL2WithdrawalMessage(_message);
         L2Message memory l2ToL1Message =
@@ -218,6 +255,9 @@ contract L1Bridge is Ownable2Step, Pausable, IL1Bridge {
         if (!success) {
             revert InvalidProof();
         }
+
+        isWithdrawalFinalized[_l2BatchNumber][_l2MessageIndex] = true;
+
         L1_NODL.mint(l1Receiver, amount);
         emit WithdrawalFinalized(l1Receiver, _l2BatchNumber, _l2MessageIndex, _l2TxNumberInBatch, amount);
     }

test/L1Nodl.t.sol

@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: BSD-3-Clause-Clear
+
+pragma solidity ^0.8.26;
+
+import "forge-std/Test.sol";
+
+import {L1Nodl} from "src/L1Nodl.sol";
+
+contract L1NodlTest is Test {
+    address internal constant ADMIN = address(0xA11CE);
+    address internal constant MINTER = address(0xBEEF);
+
+    function test_constructor_setsRoles() public {
+        L1Nodl token = new L1Nodl(ADMIN, MINTER);
+
+        assertTrue(token.hasRole(token.DEFAULT_ADMIN_ROLE(), ADMIN), "admin role assigned");
+        assertTrue(token.hasRole(keccak256("MINTER_ROLE"), MINTER), "minter role assigned");
+    }
+
+    function test_constructor_revert_adminZero() public {
+        vm.expectRevert(abi.encodeWithSelector(L1Nodl.ZeroAddress.selector));
+        new L1Nodl(address(0), MINTER);
+    }
+
+    function test_constructor_revert_minterZero() public {
+        vm.expectRevert(abi.encodeWithSelector(L1Nodl.ZeroAddress.selector));
+        new L1Nodl(ADMIN, address(0));
+    }
+}

test/bridge/L1Bridge.t.sol

@@ -22,6 +22,10 @@ contract MockMailbox { /* not inheriting IMailbox on purpose */
 
     bytes32 public lastRequestedTxHash;
     address public lastRefundRecipient;
+    uint256 public baseCostReturn;
+    uint256 public expectedBaseCostGasPrice;
+    uint256 public expectedBaseCostGasLimit;
+    uint256 public expectedBaseCostGasPerPubdata;
 
     // Allow tests to toggle outcomes
     function setL1ToL2Failed(bytes32 txHash, bool failed) external {
@@ -32,6 +36,16 @@ contract MockMailbox { /* not inheriting IMailbox on purpose */
         l2InclusionOk[batch][index] = ok;
     }
 
+    function setBaseCostReturn(uint256 value) external {
+        baseCostReturn = value;
+    }
+
+    function expectBaseCostParams(uint256 gasPrice, uint256 gasLimit, uint256 gasPerPubdata) external {
+        expectedBaseCostGasPrice = gasPrice;
+        expectedBaseCostGasLimit = gasLimit;
+        expectedBaseCostGasPerPubdata = gasPerPubdata;
+    }
+
     // --- Methods used by L1Bridge ---
     function requestL2Transaction(
         address _contractL2,
@@ -71,6 +85,18 @@ contract MockMailbox { /* not inheriting IMailbox on purpose */
     ) external view returns (bool) {
         return l2InclusionOk[_batchNumber][_index];
     }
+
+    function l2TransactionBaseCost(uint256 _l1GasPrice, uint256 _l2GasLimit, uint256 _l2GasPerPubdataByte)
+        external
+        view
+        returns (uint256)
+    {
+        // The gas price of zero is allowed as `forge test --zksync` sets it to zero
+        require(_l1GasPrice == expectedBaseCostGasPrice || _l1GasPrice == 0, "unexpected gas price");
+        require(_l2GasLimit == expectedBaseCostGasLimit, "unexpected gas limit");
+        require(_l2GasPerPubdataByte == expectedBaseCostGasPerPubdata, "unexpected gas per pubdata");
+        return baseCostReturn;
+    }
 }
 
 contract L1BridgeTest is Test {
@@ -161,6 +187,14 @@ contract L1BridgeTest is Test {
         bridge.deposit(address(0x1), 0, 100, 1000, USER);
     }
 
+    function test_Deposit_Revert_L2ReceiverZero() public {
+        vm.startPrank(USER);
+        token.approve(address(bridge), 1 ether);
+        vm.expectRevert(abi.encodeWithSelector(L1Bridge.ZeroAddress.selector));
+        bridge.deposit(address(0), 1 ether, 100, 1000, USER);
+        vm.stopPrank();
+    }
+
     function test_Deposit_Revert_InsufficientBalance() public {
         address l2Receiver = address(0x7777);
         uint256 gasLimit = 1_000_000;
@@ -318,6 +352,31 @@ contract L1BridgeTest is Test {
         bridge.finalizeWithdrawal(1, 1, 1, bad, new bytes32[](0));
     }
 
+    function test_QuoteL2BaseCost_UsesTxGasPrice() public {
+        uint256 gasLimit = 500_000;
+        uint256 gasPerPubdata = 800;
+        uint256 quotedValue = 123;
+        vm.txGasPrice(42 gwei);
+        mailbox.setBaseCostReturn(quotedValue);
+        mailbox.expectBaseCostParams(tx.gasprice, gasLimit, gasPerPubdata);
+
+        uint256 quote = bridge.quoteL2BaseCost(gasLimit, gasPerPubdata);
+
+        assertEq(quote, quotedValue, "returns quoted base cost from mailbox");
+    }
+
+    function test_QuoteL2BaseCostAtGasPrice() public {
+        uint256 gasLimit = 250_000;
+        uint256 gasPerPubdata = 900;
+        uint256 gasPrice = 15 gwei;
+        uint256 quotedValue = 456;
+        mailbox.setBaseCostReturn(quotedValue);
+        mailbox.expectBaseCostParams(gasPrice, gasLimit, gasPerPubdata);
+        uint256 quote = bridge.quoteL2BaseCostAtGasPrice(gasPrice, gasLimit, gasPerPubdata);
+
+        assertEq(quote, quotedValue, "returns mailbox quote");
+    }
+
     function test_Pause_Gates_Functions() public {
         vm.prank(ADMIN);
         bridge.pause();