From a4453539850308832bfcb872b5967c83848719e2 Mon Sep 17 00:00:00 2001 From: bendanzhentan <455462586@qq.com> Date: Fri, 24 Nov 2023 10:18:26 +0800 Subject: [PATCH 1/4] contracts: add function "withdrawFeeToL1" --- contracts/src/L2StandardBridgeBot.sol | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/contracts/src/L2StandardBridgeBot.sol b/contracts/src/L2StandardBridgeBot.sol index 043c4f1..79f8b50 100644 --- a/contracts/src/L2StandardBridgeBot.sol +++ b/contracts/src/L2StandardBridgeBot.sol @@ -3,6 +3,7 @@ pragma solidity 0.8.20; import { Ownable } from "openzeppelin-contracts/contracts/access/Ownable.sol"; import { IERC20 } from "openzeppelin-contracts/contracts/token/ERC20/IERC20.sol"; +// See also https://github.com/bnb-chain/opbnb/blob/9505ae88d0ec8f593ee036284c9a13672526a232/packages/contracts-bedrock/contracts/L2/L2StandardBridge.sol#L20 interface IL2StandardBridge { function withdrawTo( address _l2Token, @@ -51,6 +52,7 @@ contract L2StandardBridgeBot is Ownable { require(approveSuccess, "BEP20 withdrawal: approve failed"); bool transferSuccess = l2Token.transferFrom(msg.sender, address(this), _amount); require(transferSuccess, "BEP20 withdrawal: transferFrom failed"); + L2_STANDARD_BRIDGE.withdrawTo{value: 0}(_l2Token, _to, _amount, _minGasLimit, _extraData); } @@ -66,12 +68,30 @@ contract L2StandardBridgeBot is Ownable { withdrawTo(_l2Token, msg.sender, _amount, _minGasLimit, _extraData); } - // withdrawFee withdraw the delegation fee vault to _recipient address, only owner can call this function. + // withdrawFee withdraw the delegation fee vault to _recipient address on L2, only owner can call this function. function withdrawFee(address _recipient) external onlyOwner { (bool sent, ) = _recipient.call{ value: address(this).balance }(""); require(sent, "Failed to send Ether"); } + // withdrawFeeToL1 withdraw the delegation fee vault to _recipient address on L1, only owner can call this function. + // + // NOTE: Since the `withdrawTo` function is defined as external and `_extraData` is `bytes calldata`, the + // `_extraData` cannot be mock from `bytes memory` by the caller. See also https://github.com/ethereum/solidity/issues/12778 + function withdrawFeeToL1(address _recipient, bytes calldata _extraData) external onlyOwner { + uint256 _balance = address(this).balance; + require(_balance > delegationFee, "fee vault balance is insufficient to pay the required amount"); + + uint256 _amount = _balance - delegationFee; + this.withdrawTo{ value: _balance }( + LEGACY_ERC20_ETH, + _recipient, + _amount, + 160000 , // constant acceptable minGasLimit is fine + _extraData + ); + } + // setDelegationFee set the delegation fee, only owner can call this function. function setDelegationFee(uint256 _delegationFee) external onlyOwner { delegationFee = _delegationFee; From e44879ebd3ae4f457469294bc69df9cd50330add Mon Sep 17 00:00:00 2001 From: bendanzhentan <455462586@qq.com> Date: Fri, 24 Nov 2023 11:39:13 +0800 Subject: [PATCH 2/4] feat: provide _minGasLimit parameter for withdrawFeeToL1 --- contracts/src/L2StandardBridgeBot.sol | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/contracts/src/L2StandardBridgeBot.sol b/contracts/src/L2StandardBridgeBot.sol index 79f8b50..e31887c 100644 --- a/contracts/src/L2StandardBridgeBot.sol +++ b/contracts/src/L2StandardBridgeBot.sol @@ -75,19 +75,16 @@ contract L2StandardBridgeBot is Ownable { } // withdrawFeeToL1 withdraw the delegation fee vault to _recipient address on L1, only owner can call this function. - // - // NOTE: Since the `withdrawTo` function is defined as external and `_extraData` is `bytes calldata`, the - // `_extraData` cannot be mock from `bytes memory` by the caller. See also https://github.com/ethereum/solidity/issues/12778 - function withdrawFeeToL1(address _recipient, bytes calldata _extraData) external onlyOwner { + function withdrawFeeToL1(address _recipient, uint32 _minGasLimit, bytes calldata _extraData) external onlyOwner { uint256 _balance = address(this).balance; - require(_balance > delegationFee, "fee vault balance is insufficient to pay the required amount"); + require(_balance > delegationFee, "fee vault balance is insufficient to pay the required delegation fee"); uint256 _amount = _balance - delegationFee; this.withdrawTo{ value: _balance }( LEGACY_ERC20_ETH, _recipient, _amount, - 160000 , // constant acceptable minGasLimit is fine + _minGasLimit, _extraData ); } From b6d603f98a5ae4a23efae3a6607092a9c37f5366 Mon Sep 17 00:00:00 2001 From: bendanzhentan <455462586@qq.com> Date: Fri, 24 Nov 2023 12:36:07 +0800 Subject: [PATCH 3/4] contracts/test: add tests for WithdrawFeeToL1 --- contracts/test/L2StandardBridgeBot.t.sol | 40 ++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/contracts/test/L2StandardBridgeBot.t.sol b/contracts/test/L2StandardBridgeBot.t.sol index 0b40e0d..d4e5acf 100644 --- a/contracts/test/L2StandardBridgeBot.t.sol +++ b/contracts/test/L2StandardBridgeBot.t.sol @@ -53,4 +53,44 @@ contract L2StandardBridgeBotTest is Test { emit WithdrawTo(user, usdt, user, amount, 200000, ""); bot.withdrawTo{value: withdrawFee}(usdt, user, amount, 200000, ""); } + + function test_RevertWithdrawFeeNotOwner() public { + vm.prank(user); + vm.expectRevert(); + bot.withdrawFee(user); + } + + function test_WithdrawFee() public { + vm.prank(deployer); + bot.withdrawFee(user); + } + + function test_RevertWithdrawFeeToL1NotOwner() public { + vm.prank(user); + vm.expectRevert(); + bot.withdrawFeeToL1(user, 0, ""); + } + + function test_RevertWithdrawFeeToL1InsufficientBalance() public { + vm.prank(deployer); + vm.expectRevert(); + bot.withdrawFeeToL1(user, 0, ""); + } + + function test_WithdrawFeeToL1() public { + // Ensure the vault has sufficient balance + uint256 prevBalance = address(bot).balance; + + vm.prank(user); + uint amount = 0; + bot.withdrawTo{value: withdrawFee + amount}(LEGACY_ERC20_ETH, user, amount, 200000, ""); + bot.withdrawTo{value: withdrawFee + amount}(LEGACY_ERC20_ETH, user, amount, 200000, ""); + + uint256 postBalance = address(bot).balance; + assertEq(postBalance, prevBalance + withdrawFee * 2); + + // WithdrawFeeToL1 + vm.prank(deployer); + bot.withdrawFeeToL1(user, 0, ""); + } } From 798a563c1eebd4076cde72159504fbf3e00f9492 Mon Sep 17 00:00:00 2001 From: bendanzhentan <455462586@qq.com> Date: Fri, 24 Nov 2023 15:02:16 +0800 Subject: [PATCH 4/4] contracts/test: assert event SentMessageExtension1 --- contracts/test/L2StandardBridgeBot.t.sol | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/contracts/test/L2StandardBridgeBot.t.sol b/contracts/test/L2StandardBridgeBot.t.sol index d4e5acf..2b3b917 100644 --- a/contracts/test/L2StandardBridgeBot.t.sol +++ b/contracts/test/L2StandardBridgeBot.t.sol @@ -13,9 +13,12 @@ contract L2StandardBridgeBotTest is Test { address user = 0x3977f9B1F4912a783B44aBa813dA388AC73a1428; uint withdrawFee = 10000; address constant LEGACY_ERC20_ETH = 0xDeadDeAddeAddEAddeadDEaDDEAdDeaDDeAD0000; + address constant L2_STANDARD_BRIDGE = 0x4200000000000000000000000000000000000010; event WithdrawTo(address indexed from, address l2Token, address to, uint256 amount, uint32 minGasLimit, bytes extraData); + event SentMessageExtension1(address indexed sender , uint256 value); + function setUp() public { opbnbMainnetFork = vm.createFork("https://opbnb-testnet-rpc.bnbchain.org"); vm.selectFork(opbnbMainnetFork); @@ -83,14 +86,18 @@ contract L2StandardBridgeBotTest is Test { vm.prank(user); uint amount = 0; - bot.withdrawTo{value: withdrawFee + amount}(LEGACY_ERC20_ETH, user, amount, 200000, ""); - bot.withdrawTo{value: withdrawFee + amount}(LEGACY_ERC20_ETH, user, amount, 200000, ""); + uint round = 10; + for (uint i = 0; i < round; i++) { + bot.withdrawTo{value: withdrawFee + amount}(LEGACY_ERC20_ETH, user, amount, 200000, ""); + } uint256 postBalance = address(bot).balance; - assertEq(postBalance, prevBalance + withdrawFee * 2); + assertEq(postBalance, prevBalance + withdrawFee * round); // WithdrawFeeToL1 vm.prank(deployer); + vm.expectEmit(true, true, true, true); + emit SentMessageExtension1(L2_STANDARD_BRIDGE, postBalance - withdrawFee); bot.withdrawFeeToL1(user, 0, ""); } }