Skip to content

Commit

Permalink
address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tsnewnami committed Nov 14, 2023
1 parent db47f82 commit 68d1b32
Show file tree
Hide file tree
Showing 16 changed files with 85 additions and 46 deletions.
9 changes: 4 additions & 5 deletions src/child/ChildERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ contract ChildERC20Bridge is
* @notice Fallback function on recieving native IMX from WIMX contract.
*/
receive() external payable {
// Revert if sender if not the wIMX token address
// Or the wIMXToken is not set (contract not initialized)
if (msg.sender != wIMXToken || wIMXToken == address(0)) {
// Revert if sender is not the WIMX token address
if (msg.sender != wIMXToken) {
revert NonWrappedNativeTransfer();
}
}
Expand Down Expand Up @@ -245,7 +244,7 @@ contract ChildERC20Bridge is
revert ZeroAmount();
}
if (msg.value == 0) {
revert ZeroValue();
revert NoGas();
}

address rootToken;
Expand Down Expand Up @@ -385,7 +384,7 @@ contract ChildERC20Bridge is
if (rootToken == NATIVE_ETH) {
childToken = childETHToken;
} else {
childToken = IChildERC20(rootTokenToChildToken[address(rootToken)]);
childToken = IChildERC20(rootTokenToChildToken[rootToken]);
if (address(childToken) == address(0)) {
revert NotMapped();
}
Expand Down
4 changes: 2 additions & 2 deletions src/interfaces/child/IChildERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ interface IChildERC20BridgeErrors {
error InsufficientValue();
/// @notice Error when the withdrawal amount is zero
error ZeroAmount();
/// @notice Error when the msg.value amount is zero
error ZeroValue();
/// @notice Error when a message is sent with no gas payment.
error NoGas();
/// @notice Error when the contract to mint had no bytecode.
error EmptyTokenContract();
/// @notice Error when the mint operation failed.
Expand Down
2 changes: 2 additions & 0 deletions src/interfaces/root/IRootERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ interface IRootERC20BridgeErrors {
error ZeroAmount();
/// @notice Error when a zero address is given when not valid.
error ZeroAddress();
/// @notice Error when a message is sent with no gas payment.
error NoGas();
/// @notice Error when the child chain name is invalid.
error InvalidChildChain();
/// @notice Error when a token is already mapped.
Expand Down
9 changes: 7 additions & 2 deletions src/root/RootERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -170,8 +170,7 @@ contract RootERC20Bridge is
*/
receive() external payable {
// Revert if sender is not the WETH token address
// Or the rootWETHToken is not set (contract not initialized)
if (msg.sender != rootWETHToken || rootWETHToken == address(0)) {
if (msg.sender != rootWETHToken) {
revert NonWrappedNativeTransfer();
}
}
Expand Down Expand Up @@ -289,6 +288,9 @@ contract RootERC20Bridge is
}

function _mapToken(IERC20Metadata rootToken) private returns (address) {
if (msg.value == 0) {
revert NoGas();
}
if (address(rootToken) == address(0)) {
revert ZeroAddress();
}
Expand Down Expand Up @@ -331,6 +333,9 @@ contract RootERC20Bridge is
if (amount == 0) {
revert ZeroAmount();
}
if (msg.value == 0) {
revert NoGas();
}
if (
address(rootToken) == rootIMXToken && imxCumulativeDepositLimit != UNLIMITED_DEPOSIT
&& IERC20Metadata(rootIMXToken).balanceOf(address(this)) + amount > imxCumulativeDepositLimit
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ contract ChildERC20BridgeWithdrawIntegrationTest is
IChildAxelarBridgeAdaptorErrors,
Utils
{
// Define here to avoid error collisions between IChildERC20BridgeErrors and IChildAxelarBridgeAdaptorErrors
error ZeroValue();

address constant CHILD_BRIDGE = address(3);
address constant CHILD_BRIDGE_ADAPTOR = address(4);
string constant CHILD_CHAIN_NAME = "test";
Expand Down Expand Up @@ -134,7 +131,7 @@ contract ChildERC20BridgeWithdrawIntegrationTest is
function test_RevertIf_WithdrawWithNoGas() public {
ChildERC20 childToken = ChildERC20(childBridge.rootTokenToChildToken(rootToken));

vm.expectRevert(ZeroValue.selector);
vm.expectRevert(NoGas.selector);
childBridge.withdraw(childToken, withdrawAmount);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,10 @@ contract ChildERC20BridgeWithdrawWIMXIntegrationTest is
IChildAxelarBridgeAdaptorErrors,
Utils
{
// TODO: consider moving this to a setup function
address constant CHILD_BRIDGE = address(3);
address constant ROOT_IMX_TOKEN = address(555555);
address constant WRAPPED_IMX = address(0xabc);

ChildERC20Bridge public childBridge;
ChildAxelarBridgeAdaptor public axelarAdaptor;
address public rootToken;
address public rootImxToken;
ChildERC20 public childTokenTemplate;
MockAxelarGasService public mockAxelarGasService;
MockAxelarGateway public mockAxelarGateway;
WIMX public wIMXToken;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ contract ChildERC20BridgeWithdrawWIMXToIntegrationTest is
IChildAxelarBridgeAdaptorErrors,
Utils
{
address constant CHILD_BRIDGE = address(3);
address constant ROOT_IMX_TOKEN = address(555555);
address constant WRAPPED_IMX = address(0xabc);

Expand All @@ -47,19 +46,20 @@ contract ChildERC20BridgeWithdrawWIMXToIntegrationTest is
}

function test_WithdrawWIMXTo_CallsBridgeAdaptor() public {
address receiver = address(0xabcd);
uint256 withdrawFee = 1;
uint256 withdrawAmount = 7 ether;

wIMXToken.approve(address(childBridge), withdrawAmount);
bytes memory predictedPayload =
abi.encode(WITHDRAW_SIG, ROOT_IMX_TOKEN, address(this), address(this), withdrawAmount);
abi.encode(WITHDRAW_SIG, ROOT_IMX_TOKEN, address(this), receiver, withdrawAmount);
vm.expectCall(
address(axelarAdaptor),
withdrawFee,
abi.encodeWithSelector(axelarAdaptor.sendMessage.selector, predictedPayload, address(this))
);

childBridge.withdrawWIMXTo{value: withdrawFee}(address(this), withdrawAmount);
childBridge.withdrawWIMXTo{value: withdrawFee}(receiver, withdrawAmount);
}

function test_WithdrawWIMXToWithDifferentAccount_CallsBridgeAdaptor() public {
Expand All @@ -80,12 +80,13 @@ contract ChildERC20BridgeWithdrawWIMXToIntegrationTest is
}

function test_WithdrawWIMXTo_CallsAxelarGateway() public {
address reciever = address(0xabcd);
uint256 withdrawFee = 300;
uint256 withdrawAmount = 7 ether;

wIMXToken.approve(address(childBridge), withdrawAmount);
bytes memory predictedPayload =
abi.encode(WITHDRAW_SIG, ROOT_IMX_TOKEN, address(this), address(this), withdrawAmount);
abi.encode(WITHDRAW_SIG, ROOT_IMX_TOKEN, address(this), reciever, withdrawAmount);
vm.expectCall(
address(mockAxelarGateway),
0,
Expand All @@ -97,11 +98,11 @@ contract ChildERC20BridgeWithdrawWIMXToIntegrationTest is
)
);

childBridge.withdrawWIMXTo{value: withdrawFee}(address(this), withdrawAmount);
childBridge.withdrawWIMXTo{value: withdrawFee}(reciever, withdrawAmount);
}

function test_WithdrawWIMXToWithDifferentAccount_CallsAxelarGateway() public {
address receiver = address(0xabcde);
address receiver = address(0xabcd);
uint256 withdrawFee = 300;
uint256 withdrawAmount = 7 ether;

Expand All @@ -123,12 +124,13 @@ contract ChildERC20BridgeWithdrawWIMXToIntegrationTest is
}

function test_WithdrawWIMXTo_CallsGasService() public {
address receiver = address(0xabcd);
uint256 withdrawFee = 300;
uint256 withdrawAmount = 7 ether;

wIMXToken.approve(address(childBridge), withdrawAmount);
bytes memory predictedPayload =
abi.encode(WITHDRAW_SIG, ROOT_IMX_TOKEN, address(this), address(this), withdrawAmount);
abi.encode(WITHDRAW_SIG, ROOT_IMX_TOKEN, address(this), receiver, withdrawAmount);

vm.expectCall(
address(axelarGasService),
Expand All @@ -143,7 +145,7 @@ contract ChildERC20BridgeWithdrawWIMXToIntegrationTest is
)
);

childBridge.withdrawWIMXTo{value: withdrawFee}(address(this), withdrawAmount);
childBridge.withdrawWIMXTo{value: withdrawFee}(receiver, withdrawAmount);
}

function test_WithdrawWIMXToWithDifferentAccount_CallsGasService() public {
Expand Down Expand Up @@ -172,17 +174,18 @@ contract ChildERC20BridgeWithdrawWIMXToIntegrationTest is
}

function test_WithdrawWIMXTo_EmitsAxelarMessageSentEvent() public {
address receiver = address(0xabcd);
uint256 withdrawFee = 300;
uint256 withdrawAmount = 7 ether;

wIMXToken.approve(address(childBridge), withdrawAmount);
bytes memory predictedPayload =
abi.encode(WITHDRAW_SIG, ROOT_IMX_TOKEN, address(this), address(this), withdrawAmount);
abi.encode(WITHDRAW_SIG, ROOT_IMX_TOKEN, address(this), receiver, withdrawAmount);

vm.expectEmit(address(axelarAdaptor));
emit AxelarMessageSent(childBridge.rootChain(), childBridge.rootERC20BridgeAdaptor(), predictedPayload);

childBridge.withdrawWIMXTo{value: withdrawFee}(address(this), withdrawAmount);
childBridge.withdrawWIMXTo{value: withdrawFee}(receiver, withdrawAmount);
}

function test_WithdrawWIMXToWithDifferentAccount_EmitsAxelarMessageSentEvent() public {
Expand Down
17 changes: 16 additions & 1 deletion test/unit/child/ChildERC20Bridge.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,24 @@ contract ChildERC20BridgeUnitTest is Test, IChildERC20BridgeEvents, IChildERC20B
assertFalse(address(childBridge.childETHToken()).code.length == 0, "childETHToken contract empty");
}

function test_NativeTransferFromWIMX() public {
address caller = address(0x123a);
payable(caller).transfer(2 ether);

uint256 wIMXStorageSlot = 158;
vm.store(address(childBridge), bytes32(wIMXStorageSlot), bytes32(uint256(uint160(caller))));

vm.startPrank(caller);
uint256 bal = address(childBridge).balance;
payable(childBridge).transfer(1 ether);
uint256 postBal = address(childBridge).balance;

assertEq(bal + 1 ether, postBal, "balance not increased");
}

function test_RevertIfNativeTransferIsFromNonWIMX() public {
vm.expectRevert(NonWrappedNativeTransfer.selector);
payable(childBridge).transfer(1);
payable(childBridge).transfer(1 ether);
}

function test_RevertIfInitializeTwice() public {
Expand Down
2 changes: 1 addition & 1 deletion test/unit/child/withdrawals/ChildERC20BridgeWithdraw.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ contract ChildERC20BridgeWithdrawUnitTest is Test, IChildERC20BridgeEvents, IChi
function test_RevertsIf_WithdrawCalledWithZeroFee() public {
uint256 withdrawAmount = 100;

vm.expectRevert(ZeroValue.selector);
vm.expectRevert(NoGas.selector);
childBridge.withdraw(IChildERC20(address(2222222)), withdrawAmount);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ contract ChildERC20BridgeWithdrawIMXUnitTest is Test, IChildERC20BridgeEvents, I
function test_RevertIf_WithdrawIMXCalledWithZeroFee() public {
uint256 withdrawAmount = 300;

vm.expectRevert(ZeroValue.selector);
vm.expectRevert(NoGas.selector);
childBridge.withdrawIMX(withdrawAmount);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ contract ChildERC20BridgeWithdrawToUnitTest is Test, IChildERC20BridgeEvents, IC
function test_RevertsIf_WithdrawToCalledWithZeroFee() public {
uint256 withdrawAmount = 300;

vm.expectRevert(ZeroValue.selector);
vm.expectRevert(NoGas.selector);
childBridge.withdrawTo(IChildERC20(address(2222222)), address(this), withdrawAmount);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ contract ChildERC20BridgeWithdrawIMXToUnitTest is Test, IChildERC20BridgeEvents,
function test_RevertsIf_withdrawIMXToCalledWithZeroFee() public {
uint256 withdrawAmount = 7 ether;

vm.expectRevert(ZeroValue.selector);
vm.expectRevert(NoGas.selector);
childBridge.withdrawIMXTo(address(this), withdrawAmount);
}

Expand Down
10 changes: 4 additions & 6 deletions test/unit/child/withdrawals/ChildERC20BridgeWithdrawWIMX.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@ import {WIMX} from "../../../../src/child/WIMX.sol";
import {Address} from "@openzeppelin/contracts/utils/Address.sol";

contract ChildERC20BridgeWithdrawWIMXUnitTest is Test, IChildERC20BridgeEvents, IChildERC20BridgeErrors, Utils {
address constant ROOT_BRIDGE = address(3);
string public ROOT_BRIDGE_ADAPTOR = Strings.toHexString(address(4));
string constant ROOT_CHAIN_NAME = "test";
address constant ROOT_IMX_TOKEN = address(0xccc);
address constant WIMX_TOKEN_ADDRESS = address(0xabc);
ChildERC20 public childTokenTemplate;
ChildERC20Bridge public childBridge;
MockAdaptor public mockAdaptor;
Expand Down Expand Up @@ -54,11 +52,11 @@ contract ChildERC20BridgeWithdrawWIMXUnitTest is Test, IChildERC20BridgeEvents,
);
}

function test_RevertsIf_withdrawIMXCalledWithZeroFee() public {
function test_RevertsIf_withdrawWIMXCalledWithZeroFee() public {
uint256 withdrawAmount = 7 ether;

vm.expectRevert(ZeroValue.selector);
childBridge.withdrawIMX(withdrawAmount);
vm.expectRevert(NoGas.selector);
childBridge.withdrawWIMX(withdrawAmount);
}

function test_RevertsIf_WithdrawWIMXCalledWithInsufficientFund() public {
Expand All @@ -79,7 +77,7 @@ contract ChildERC20BridgeWithdrawWIMXUnitTest is Test, IChildERC20BridgeEvents,
childBridge.withdrawWIMX{value: withdrawFee}(withdrawAmount);
}

function test_RevertsIf_ZeroAmountIsProvided() public {
function test_RevertsIf_WithdrawWIMXCalledWithZeroAmount() public {
uint256 withdrawFee = 300;

vm.expectRevert(ZeroAmount.selector);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ contract ChildERC20BridgeWithdrawWIMXToUnitTest is Test, IChildERC20BridgeEvents
function test_RevertsIf_withdrawWIMXToCalledWithZeroFee() public {
uint256 withdrawAmount = 7 ether;

vm.expectRevert(ZeroValue.selector);
vm.expectRevert(NoGas.selector);
childBridge.withdrawWIMXTo(address(this), withdrawAmount);
}

Expand All @@ -86,7 +86,7 @@ contract ChildERC20BridgeWithdrawWIMXToUnitTest is Test, IChildERC20BridgeEvents
childBridge.withdrawWIMXTo{value: withdrawFee}(address(this), withdrawAmount);
}

function test_RevertsIf_ZeroAmountIsProvided() public {
function test_RevertsIf_WithdrawWIMXToCalledWithZeroAmount() public {
uint256 withdrawFee = 300;

vm.expectRevert(ZeroAmount.selector);
Expand Down
26 changes: 26 additions & 0 deletions test/unit/root/RootERC20Bridge.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,21 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid
assertEq(rootBridge.rootWETHToken(), WRAPPED_ETH, "rootWETHToken not set");
}

function test_NativeTransferFromWETH() public {
address caller = address(0x123a);
payable(caller).transfer(2 ether);

uint256 wETHStorageSlot = 158;
vm.store(address(rootBridge), bytes32(wETHStorageSlot), bytes32(uint256(uint160(caller))));

vm.startPrank(caller);
uint256 bal = address(rootBridge).balance;
payable(rootBridge).transfer(1 ether);
uint256 postBal = address(rootBridge).balance;

assertEq(bal + 1 ether, postBal, "balance not increased");
}

function test_RevertIfNativeTransferIsFromNonWETH() public {
vm.expectRevert(NonWrappedNativeTransfer.selector);
payable(rootBridge).transfer(1);
Expand Down Expand Up @@ -441,6 +456,11 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid
* MAP TOKEN
*/

function test_RevertsIf_MapTokenCalledWithZeroFee() public {
vm.expectRevert(NoGas.selector);
rootBridge.mapToken(token);
}

function test_mapToken_EmitsTokenMappedEvent() public {
address childToken =
Clones.predictDeterministicAddress(address(token), keccak256(abi.encodePacked(token)), CHILD_BRIDGE);
Expand Down Expand Up @@ -740,6 +760,12 @@ contract RootERC20BridgeUnitTest is Test, IRootERC20BridgeEvents, IRootERC20Brid
* DEPOSIT TOKEN
*/

function test_RevertsIf_DepositTokenWithZeroFee() public {
uint256 amount = 100;
vm.expectRevert(NoGas.selector);
rootBridge.deposit(IERC20Metadata(IMX_TOKEN), amount);
}

function test_RevertsIf_IMXDepositLimitExceeded() public {
uint256 imxCumulativeDepositLimit = 700;

Expand Down
Loading

0 comments on commit 68d1b32

Please sign in to comment.