Skip to content

Commit

Permalink
Document risk of locked funds if receiver lacks fallback
Browse files Browse the repository at this point in the history
  • Loading branch information
ermyas committed Dec 11, 2023
1 parent 1e81ee7 commit 95f896b
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 6 deletions.
18 changes: 16 additions & 2 deletions src/child/ChildERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,16 @@ import {BridgeRoles} from "../common/BridgeRoles.sol";
* - An account with an UNPAUSER_ROLE can unpause the contract.
* - An account with an ADAPTOR_MANAGER_ROLE can update the root bridge adaptor address.
* - An account with a DEFAULT_ADMIN_ROLE can grant and revoke roles.
* @dev Note:
*
* @dev Caution:
* - When withdrawing ETH (L2 -> L1), it's crucial to make sure that the receiving address on the root chain,
* if it's a contract, has a receive or fallback function that allows it to accept native ETH on the root chain.
* If this isn't the case, the transaction on the root chain could revert, potentially locking the user's funds indefinitely.
* - There is undefined behaviour for bridging non-standard ERC20 tokens (e.g. rebasing tokens). Please approach such cases with great care.
* - This is an upgradeable contract that should be operated behind OpenZeppelin's TransparentUpgradeableProxy.
* - The initialize function is susceptible to front running, so precautions should be taken to account for this scenario.
*
* @dev Note:
* - This is an upgradeable contract that should be operated behind OpenZeppelin's TransparentUpgradeableProxy.
*/
contract ChildERC20Bridge is
BridgeRoles,
Expand Down Expand Up @@ -238,13 +244,21 @@ contract ChildERC20Bridge is

/**
* @inheritdoc IChildERC20Bridge
* @dev Caution:
* When withdrawing ETH, it's crucial to make sure that the receiving address (`msg.sender`) on the root chain,
* if it's a contract, has a receive or fallback function that allows it to accept native ETH.
* If this isn't the case, the transaction on the root chain could revert, potentially locking the user's funds indefinitely.
*/
function withdrawETH(uint256 amount) external payable {
_withdraw(childETHToken, msg.sender, amount);
}

/**
* @inheritdoc IChildERC20Bridge
* @dev Caution:
* When withdrawing ETH, it's crucial to make sure that the receiving address (`receiver`) on the root chain,
* if it's a contract, has a receive or fallback function that allows it to accept native ETH.
* If this isn't the case, the transaction on the root chain could revert, potentially locking the user's funds indefinitely.
*/
function withdrawETHTo(address receiver, uint256 amount) external payable {
_withdraw(childETHToken, receiver, amount);
Expand Down
22 changes: 18 additions & 4 deletions src/root/RootERC20Bridge.sol
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,16 @@ import {BridgeRoles} from "../common/BridgeRoles.sol";
* - An account with a VARIABLE_MANAGER_ROLE can update the cumulative IMX deposit limit.
* - An account with an ADAPTOR_MANAGER_ROLE can update the root bridge adaptor address.
* - An account with a DEFAULT_ADMIN_ROLE can grant and revoke roles.
* @dev Note:
*
* @dev Caution:
* - When depositing IMX (L1 -> L2) it's crucial to make sure that the receiving address on the child chain,
* if it's a contract, has a receive or fallback function that allows it to accept native IMX on the child chain.
* If this isn't the case, the transaction on the child chain could revert, potentially locking the user's funds indefinitely.
* - There is undefined behaviour for bridging non-standard ERC20 tokens (e.g. rebasing tokens). Please approach such cases with great care.
* - This is an upgradeable contract that should be operated behind OpenZeppelin's TransparentUpgradeableProxy.
* - The initialize function is susceptible to front running, so precautions should be taken to account for this scenario.
*
* @dev Note:
* - This is an upgradeable contract that should be operated behind OpenZeppelin's TransparentUpgradeableProxy.
*/
contract RootERC20Bridge is
BridgeRoles,
Expand Down Expand Up @@ -281,15 +287,23 @@ contract RootERC20Bridge is

/**
* @inheritdoc IRootERC20Bridge
* @dev Note that there is undefined behaviour for bridging non-standard ERC20 tokens (e.g. rebasing tokens). Please approach such cases with great care.
* @dev Caution:
* - When depositing IMX, it's crucial to make sure that the receiving address (`msg.sender`) on the child chain,
* if it's a contract, has a receive or fallback function that allows it to accept native IMX.
* If this isn't the case, the transaction on the child chain could revert, potentially locking the user's funds indefinitely.
* - Note that there is undefined behaviour for bridging non-standard ERC20 tokens (e.g. rebasing tokens). Please approach such cases with great care.
*/
function deposit(IERC20Metadata rootToken, uint256 amount) external payable override {
_depositToken(rootToken, msg.sender, amount);
}

/**
* @inheritdoc IRootERC20Bridge
* @dev Note that there is undefined behaviour for bridging non-standard ERC20 tokens (e.g. rebasing tokens). Please approach such cases with great care.
* @dev Caution:
* - When depositing IMX, it's crucial to make sure that the receiving address (`receiver`) on the child chain,
* if it's a contract, has a receive or fallback function that allows it to accept native IMX.
* If this isn't the case, the transaction on the child chain could revert, potentially locking the user's funds indefinitely.
* - Note that there is undefined behaviour for bridging non-standard ERC20 tokens (e.g. rebasing tokens). Please approach such cases with great care.
*/
function depositTo(IERC20Metadata rootToken, address receiver, uint256 amount) external payable override {
_depositToken(rootToken, receiver, amount);
Expand Down

0 comments on commit 95f896b

Please sign in to comment.