diff --git a/src/bridge/ERC20Bridge.sol b/src/bridge/ERC20Bridge.sol index d2d3bffc..86a7a303 100644 --- a/src/bridge/ERC20Bridge.sol +++ b/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 diff --git a/test/foundry/ERC20Outbox.t.sol b/test/foundry/ERC20Outbox.t.sol index 4c385a56..b603e95b 100644 --- a/test/foundry/ERC20Outbox.t.sol +++ b/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 + }); + } } /** diff --git a/test/foundry/RollupCreator.t.sol b/test/foundry/RollupCreator.t.sol index 68b8ebe2..50b09866 100644 --- a/test/foundry/RollupCreator.t.sol +++ b/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 diff --git a/test/foundry/util/NoZeroTransferToken.sol b/test/foundry/util/NoZeroTransferToken.sol new file mode 100644 index 00000000..89af9023 --- /dev/null +++ b/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); + } +} \ No newline at end of file