Handle "no zero transfer" tokens

src/bridge/ERC20Bridge.sol

@@ -74,7 +74,9 @@ contract ERC20Bridge is AbsBridge, IERC20Bridge {
 
     function _transferFunds(uint256 amount) internal override {
         // fetch native token from Inbox
-        IERC20(nativeToken).safeTransferFrom(msg.sender, address(this), amount);
+        if (amount > 0) {
+            IERC20(nativeToken).safeTransferFrom(msg.sender, address(this), amount);
+        }
     }
 
     function _executeLowLevelCall(
@@ -91,7 +93,9 @@ contract ERC20Bridge is AbsBridge, IERC20Bridge {
         }
 
         // first release native token
-        IERC20(_nativeToken).safeTransfer(to, value);
+        if (value > 0) {
+            IERC20(_nativeToken).safeTransfer(to, value);
+        }
         success = true;
 
         // if there's data do additional contract call. Make sure that call is not used to

test/foundry/ERC20Outbox.t.sol

@@ -6,7 +6,7 @@ import "./ERC20Bridge.t.sol";
 import "../../src/bridge/ERC20Bridge.sol";
 import "../../src/bridge/ERC20Outbox.sol";
 import "@openzeppelin/contracts/token/ERC20/ERC20.sol";
-import "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetFixedSupply.sol";
+import {NoZeroTransferToken} from "./util/NoZeroTransferToken.sol";
 
 contract ERC20OutboxTest is AbsOutboxTest {
     ERC20Outbox public erc20Outbox;
@@ -17,7 +17,7 @@ contract ERC20OutboxTest is AbsOutboxTest {
 
     function setUp() public {
         // deploy token, bridge and outbox
-        nativeToken = new ERC20PresetFixedSupply("Appchain Token", "App", 1_000_000, address(this));
+        nativeToken = new NoZeroTransferToken("Appchain Token", "App", 1_000_000, address(this));
         bridge = IBridge(TestUtil.deployProxy(address(new ERC20Bridge())));
         erc20Bridge = ERC20Bridge(address(bridge));
         outbox = IOutbox(TestUtil.deployProxy(address(new ERC20Outbox())));
@@ -45,90 +45,11 @@ contract ERC20OutboxTest is AbsOutboxTest {
     }
 
     function test_executeTransaction() public {
-        // fund bridge with some tokens
-        vm.startPrank(user);
-        nativeToken.approve(address(bridge), 100);
-        nativeToken.transfer(address(bridge), 100);
-        vm.stopPrank();
-
-        // create msg receiver on L1
-        ERC20L2ToL1Target target = new ERC20L2ToL1Target();
-        target.setOutbox(address(outbox));
-
-        //// execute transaction
-        uint256 bridgeTokenBalanceBefore = nativeToken.balanceOf(address(bridge));
-        uint256 targetTokenBalanceBefore = nativeToken.balanceOf(address(target));
-
-        bytes32[] memory proof = new bytes32[](1);
-        proof[0] = bytes32(0);
-
-        uint256 withdrawalAmount = 15;
-        bytes memory data = abi.encodeWithSignature("receiveHook()");
-
-        uint256 index = 1;
-        bytes32 itemHash = outbox.calculateItemHash({
-            l2Sender: user,
-            to: address(target),
-            l2Block: 300,
-            l1Block: 20,
-            l2Timestamp: 1234,
-            value: withdrawalAmount,
-            data: data
-        });
-        bytes32 root = outbox.calculateMerkleRoot(proof, index, itemHash);
-        // store root
-        vm.prank(rollup);
-        outbox.updateSendRoot(
-            root,
-            bytes32(uint256(1))
-        );
-
-        outbox.executeTransaction({
-            proof: proof,
-            index: index,
-            l2Sender: user,
-            to: address(target),
-            l2Block: 300,
-            l1Block: 20,
-            l2Timestamp: 1234,
-            value: withdrawalAmount,
-            data: data
-        });
-
-        uint256 bridgeTokenBalanceAfter = nativeToken.balanceOf(address(bridge));
-        assertEq(
-            bridgeTokenBalanceBefore - bridgeTokenBalanceAfter,
-            withdrawalAmount,
-            "Invalid bridge token balance"
-        );
-
-        uint256 targetTokenBalanceAfter = nativeToken.balanceOf(address(target));
-        assertEq(
-            targetTokenBalanceAfter - targetTokenBalanceBefore,
-            withdrawalAmount,
-            "Invalid target token balance"
-        );
-
-        /// check context was properly set during execution
-        assertEq(uint256(target.l2Block()), 300, "Invalid l2Block");
-        assertEq(uint256(target.timestamp()), 1234, "Invalid timestamp");
-        assertEq(uint256(target.outputId()), index, "Invalid outputId");
-        assertEq(target.sender(), user, "Invalid sender");
-        assertEq(uint256(target.l1Block()), 20, "Invalid l1Block");
-        assertEq(uint256(target.withdrawalAmount()), withdrawalAmount, "Invalid withdrawalAmount");
+        _happyExecTx(15);
+    }
 
-        vm.expectRevert(abi.encodeWithSignature("AlreadySpent(uint256)", index));
-        outbox.executeTransaction({
-            proof: proof,
-            index: index,
-            l2Sender: user,
-            to: address(target),
-            l2Block: 300,
-            l1Block: 20,
-            l2Timestamp: 1234,
-            value: withdrawalAmount,
-            data: data
-        });
+    function test_executeTransactionZeroValue() public {
+        _happyExecTx(0);
     }
 
     function test_executeTransaction_revert_CallTargetNotAllowed() public {
@@ -467,6 +388,101 @@ contract ERC20OutboxTest is AbsOutboxTest {
             data: data
         });
     }
+
+    struct HappyExecTxStackVars {
+        uint256 bridgeTokenBalanceBefore;
+        uint256 targetTokenBalanceBefore;
+        ERC20L2ToL1Target target;
+        uint256 index;
+        bytes32 itemHash;
+    }
+    function _happyExecTx(uint256 withdrawalAmount) public {
+        HappyExecTxStackVars memory vars;
+
+        // fund bridge with some tokens
+        vm.startPrank(user);
+        nativeToken.approve(address(bridge), 100);
+        nativeToken.transfer(address(bridge), 100);
+        vm.stopPrank();
+
+        // create msg receiver on L1
+        vars.target = new ERC20L2ToL1Target();
+        vars.target.setOutbox(address(outbox));
+
+        //// execute transaction
+        vars.bridgeTokenBalanceBefore = nativeToken.balanceOf(address(bridge));
+        vars.targetTokenBalanceBefore = nativeToken.balanceOf(address(vars.target));
+
+        bytes32[] memory proof = new bytes32[](1);
+        proof[0] = bytes32(0);
+
+        bytes memory data = abi.encodeWithSignature("receiveHook()");
+
+        vars.index = 1;
+        vars.itemHash = outbox.calculateItemHash({
+            l2Sender: user,
+            to: address(vars.target),
+            l2Block: 300,
+            l1Block: 20,
+            l2Timestamp: 1234,
+            value: withdrawalAmount,
+            data: data
+        });
+        bytes32 root = outbox.calculateMerkleRoot(proof, vars.index, vars.itemHash);
+        // store root
+        vm.prank(rollup);
+        outbox.updateSendRoot(
+            root,
+            bytes32(uint256(1))
+        );
+
+        outbox.executeTransaction({
+            proof: proof,
+            index: vars.index,
+            l2Sender: user,
+            to: address(vars.target),
+            l2Block: 300,
+            l1Block: 20,
+            l2Timestamp: 1234,
+            value: withdrawalAmount,
+            data: data
+        });
+
+        uint256 bridgeTokenBalanceAfter = nativeToken.balanceOf(address(bridge));
+        assertEq(
+            vars.bridgeTokenBalanceBefore - bridgeTokenBalanceAfter,
+            withdrawalAmount,
+            "Invalid bridge token balance"
+        );
+
+        uint256 targetTokenBalanceAfter = nativeToken.balanceOf(address(vars.target));
+        assertEq(
+            targetTokenBalanceAfter - vars.targetTokenBalanceBefore,
+            withdrawalAmount,
+            "Invalid target token balance"
+        );
+
+        /// check context was properly set during execution
+        assertEq(uint256(vars.target.l2Block()), 300, "Invalid l2Block");
+        assertEq(uint256(vars.target.timestamp()), 1234, "Invalid timestamp");
+        assertEq(uint256(vars.target.outputId()), vars.index, "Invalid outputId");
+        assertEq(vars.target.sender(), user, "Invalid sender");
+        assertEq(uint256(vars.target.l1Block()), 20, "Invalid l1Block");
+        assertEq(uint256(vars.target.withdrawalAmount()), withdrawalAmount, "Invalid withdrawalAmount");
+
+        vm.expectRevert(abi.encodeWithSignature("AlreadySpent(uint256)", vars.index));
+        outbox.executeTransaction({
+            proof: proof,
+            index: vars.index,
+            l2Sender: user,
+            to: address(vars.target),
+            l2Block: 300,
+            l1Block: 20,
+            l2Timestamp: 1234,
+            value: withdrawalAmount,
+            data: data
+        });
+    }
 }
 
 /**

test/foundry/RollupCreator.t.sol

@@ -19,6 +19,8 @@ import "../../src/rollup/DeployHelper.sol";
 import "@openzeppelin/contracts/access/Ownable.sol";
 import "@openzeppelin/contracts-upgradeable/access/AccessControlUpgradeable.sol";
 import "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetFixedSupply.sol";
+import {NoZeroTransferToken} from "./util/NoZeroTransferToken.sol";
+
 
 contract RollupCreatorTest is Test {
     RollupCreator public rollupCreator;
@@ -241,11 +243,24 @@ contract RollupCreatorTest is Test {
     }
 
     function test_createErc20Rollup() public {
-        vm.startPrank(deployer);
         address nativeToken = address(
             new ERC20PresetFixedSupply("Appchain Token", "App", 1_000_000 ether, deployer)
         );
 
+        _createERC20Rollup(nativeToken);
+    }
+
+    function test_createErc20RollupNoZeroTransfer() public {
+        address nativeToken = address(
+            new NoZeroTransferToken("Appchain Token", "App", 1_000_000 ether, deployer)
+        );
+
+        _createERC20Rollup(nativeToken);
+    }
+
+    function _createERC20Rollup(address nativeToken) internal {
+        vm.startPrank(deployer);
+
         // deployment params
         ISequencerInbox.MaxTimeVariation memory timeVars = ISequencerInbox.MaxTimeVariation(
             ((60 * 60 * 24) / 15),
@@ -297,6 +312,10 @@ contract RollupCreatorTest is Test {
 
         vm.stopPrank();
 
+        _postCreateERC20RollupChecks(rollupAddress, batchPosterManager, nativeToken, validators, batchPosters);
+    }
+
+    function _postCreateERC20RollupChecks(address rollupAddress, address batchPosterManager, address nativeToken, address[] memory validators, address[] memory batchPosters) internal {
         /// common checks
 
         /// rollup creator

test/foundry/util/NoZeroTransferToken.sol

@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity ^0.8.4;
+
+import "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetFixedSupply.sol";
+
+contract NoZeroTransferToken is ERC20PresetFixedSupply {
+    constructor(
+        string memory name_,
+        string memory symbol_,
+        uint256 initialSupply,
+        address owner
+    ) ERC20PresetFixedSupply(name_, symbol_, initialSupply, owner) {}
+
+    function _transfer(
+        address from,
+        address to,
+        uint256 amount
+    ) internal virtual override {
+        require(amount > 0, "NoZeroTransferToken: zero transfer");
+        super._transfer(from, to, amount);
+    }
+}