Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

interop: interoperable ether transfers design doc #146

Merged
merged 15 commits into from
Nov 26, 2024
Merged
279 changes: 279 additions & 0 deletions protocol/interoperable-ether-transfers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,279 @@
# Purpose

This document exists to align on a design for simplifying the process for sending `ETH` between two interoperable chains.

# Summary

New functionality is introduced to the `ETHLiquidity` contract that enables it to facilitate cross-chain `ETH` transfers to a specified recipient.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't love adding new functionality to the ETHLiquidity contract as it has a ton of virtual ether and makes reasoning about it more difficult. Its previous design only allowed one possible msg.sender. I definitely prefer to put the logic in the bridge contract to keep the liquidity contract more simple

Copy link
Contributor

@skeletor-spaceman skeletor-spaceman Nov 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 let's do this.
But have this be on the SuperWETH as originally planned :)


# Problem Statement + Context

Currently, L2-to-L2 `ETH` transfers between two interoperable chains require four separate transactions:

1. Wrap `ETH` to `SuperchainWETH`.
2. Call `SuperchainTokenBridge#SendERC20` to burn `SuperchainWETH` on source chain and relay a message to the destination chain that mints `SuperchainWETH` to the recipient.
3. Execute the message on the destination chain, triggering `SuperchainTokenBridge#RelayERC20` to mint `SuperchainWETH` to the recipient.
4. Unwrap the received `SuperchainWETH` to `ETH`.

The goal is to reduce the transaction count from four to two, enabling users to send `ETH` to a destination chain directly:

1. Burn `ETH` on source chain and relay a message to destination chain that mints `ETH` to recipient on destination chain.
2. Execute the relayed message on the destination chain that mints `ETH` to the recipient.

# Proposed Solution

## Integrate `ETH` transfer into `SuperchainWETH`

Add two new functions to the `SuperchainWETH` contract: `SendETH` and `RelayETH`.

### `SendETH`

The `SendETH` function combines the first two transactions as follows:

1. Burns `ETH` within the `ETHLiquidity` contract equivalent to the `ETH` sent.
tremarkley marked this conversation as resolved.
Show resolved Hide resolved
2. Sends a message to the destination chain encoding a call to `RelayETH`.

### `RelayETH`

The `RelayETH` function combines the last two transactions as follows:

1. Mints the specified `_amount` of `ETH` from the `ETHLiquidity` contract.
tremarkley marked this conversation as resolved.
Show resolved Hide resolved
2. Sends the minted `ETH` to the specified recipient.

### Contract changes

```solidity
/// @notice Emitted when ETH is sent from one chain to another.
/// @param from Address of the sender.
/// @param to Address of the recipient.
/// @param amount Amount of ETH sent.
/// @param destination Chain ID of the destination chain.
event SendETH(address indexed from, address indexed to, uint256 amount, uint256 destination);

/// @notice Emitted whenever ETH is successfully relayed on this chain.
/// @param from Address of the msg.sender of sendETH on the source chain.
/// @param to Address of the recipient.
/// @param amount Amount of ETH relayed.
/// @param source Chain ID of the source chain.
event RelayETH(address indexed from, address indexed to, uint256 amount, uint256 source);

/// @notice Sends ETH to some target address on another chain.
/// @param _to Address to send ETH to.
/// @param _chainId Chain ID of the destination chain.
function sendETH(address _to, uint256 _chainId) external payable returns (bytes32 msgHash_) {
tremarkley marked this conversation as resolved.
Show resolved Hide resolved
if (_to == address(0)) revert ZeroAddress();

if (IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).isCustomGasToken()) {
revert NotCustomGasToken();
}

// Burn to ETHLiquidity contract.
IETHLiquidity(Predeploys.ETH_LIQUIDITY).burn{ value: msg.value }();

// Send message to other chain.
msgHash_ = IL2ToL2CrossDomainMessenger(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER).sendMessage({
_destination: _chainId,
_target: address(this),
_message: abi.encodeCall(this.relayETH, (msg.sender, _to, msg.value))
});

// Emit event.
emit SendETH(msg.sender, _to, msg.value, _chainId);
}

/// @notice Relays ETH received from another chain.
/// @param _from Address of the msg.sender of sendETH on the source chain.
/// @param _to Address to relay ETH to.
/// @param _amount Amount of ETH to relay.
function relayETH(address _from, address _to, uint256 _amount) external {
if (msg.sender != Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER) revert Unauthorized();

(address crossDomainMessageSender, uint256 source) =
IL2ToL2CrossDomainMessenger(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER).crossDomainMessageContext();

if (crossDomainMessageSender != address(this)) revert InvalidCrossDomainSender();
Comment on lines +89 to +94
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NTH: it'd be cool to have this "am I being called by myself?" abstracted into a "CDM library"


if (IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).isCustomGasToken()) {
revert NotCustomGasToken();
}

IETHLiquidity(Predeploys.ETH_LIQUIDITY).mint(_amount);

new SafeSend{ value: _amount }(payable(_to));

emit RelayETH(_from, _to, _amount, source);
}
```

### Advantages
- Since the `SuperchainWETH` contract already has permissions to mint and burn `ETH` with the `ETHLiquidity` contract no changes to the `ETHLiquidity` contract are necessary. Any security risks with this change are limited to `SuperchainWETH` and do not affect other `SuperchainERC20` tokens.
tremarkley marked this conversation as resolved.
Show resolved Hide resolved

### Downsides
- This approach introduces an additional entry point for asset transfers outside of `SuperchainTokenBridge`. While `SuperchainERC20` tokens that implement custom bridging will already have alternative entry points, adding another for native `ETH` transfers could lead to confusion and diverge from a clean separation of concerns. Additionally, by having `SuperchainWETH` serve as both a `SuperchainERC20` and a bridge for native `ETH`, there is an increased risk of ambiguity around its dual function, which could impact usability and clarity for developers.

# Considerations

## SuperchainWETH transfers

This solution does not eliminate the `SuperchainWETH` ERC20 token. `SuperchainWETH` ERC20 transfers would still go through the `SuperchainTokenBridge`.

## Custom Gas Token Chains

To simplify the solution, custom gas token chains will not be supported and must follow the original four-transaction flow, which includes wrapping and unwrapping `SuperchainWETH`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe note that this is no problem in the superchain since standard config requires ETH as the gas token.

And then in our viem extensions we can check this about the destination client prior to calling sendETH

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that does bring up an issue with this, there is not a good way to check at the smart contract level whether the destination is a custom gas token chain or if it is in the dependency set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no guarantee that standard config will always require ether for gas paying and the system was designed to be friendly to that changing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on no guarantee. Just noting the state of the world today but still requires addressing this since the protocol is agnostic to the superchain


# Open Questions
- **Long-term improvements**: Could this functionality eventually extend to custom gas token chains?
- **Rollbacks**: How would rollbacks be handled in this implementation?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume rollbacks wouldn't be an issue as long as relayETH can only be called following the origin chain's finality period has passed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by rollbacks here I am talking about ETH that is sent to a remote chain, but for some reason the relayETH message on the remote chain expires or is not able to be executed and we would like to "rollback" the ETH that was sent so the user can get their funds back

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reorgs are contingent between chains so there is no finality period. The concept of rollback is only relevant if you send to a chain where the call reverts on the remote chain and you cannot replay it or it always reverts. I think people also wanted to solve the problem of sending to a non existent chain using rollbacks, but I don't think that is possible. Now that we require the call on the remote chain to succeed, I think that the rollback design is going to be more difficult. We should think about rollbacks in a different way to solve that problem

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I recommend we use the following design for rollbacks: ethereum-optimism/specs#460

I think there is confusion around what problem we are solving for, there should be no way that the cross chain token send should revert, so we don't need a rollback solution persay, we need to "prevent send to wrong chain" solution


# Alternatives Considered

## Integrate ETH transfers into `ETHLiquidity` contract

Introduce `SendETH` and `RelayETH` functions to the `ETHLiquidity` contract. By adding these functions directly to `ETHLiquidity`, the contract retains its original purpose of centralizing the minting and burning of `ETH` for cross-chain transfers, ensuring that all liquidity management occurs within a single, dedicated contract.
tremarkley marked this conversation as resolved.
Show resolved Hide resolved

### `SendETH` function

The `SendETH` function combines the first two transactions as follows:

1. Burns `ETH` within the `ETHLiquidity` contract equivalent to the `ETH` sent.
Copy link
Contributor

@hamdiallam hamdiallam Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're not burning WETH, how about we add these functions to the ETHLiquidity contract?

I think it makes intent a bit clearer and keeps WETH completely separate as just an ERC20? No need to overload the ERC20 contract

Copy link
Contributor Author

@tremarkley tremarkley Oct 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it definitely simplifies things and creates better separation of concerns to leave SuperchainWETH as-is and move this into ETHLiquidity, so I'm onboard with that

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ETHLiquidity holds so much ether, we have to be really careful with what functionality is added to it. It should be callable in the most minimal amount of ways. I originally designed it so that there could be only a single useful caller. We need to be very careful with any modifications to it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tynes are you onboard with the suggested solution or do you think it is too risky of a change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

definitely see the concern on additions to ETHLiquidity. But want to note that even if this is in SuperchainWETH, a vulnerability there would also mean a vulnerability in ETHLiquidity since the relay mechanism pulls ETH from this contract

tremarkley marked this conversation as resolved.
Show resolved Hide resolved
2. Sends a message to the destination chain encoding a call to `RelayETH`.

### `RelayETH` function

The `RelayETH` function combines the last two transactions as follows:

1. Mints the specified `_amount` of `ETH` from the `ETHLiquidity` contract.
2. Sends the minted `ETH` to the specified recipient.

### Contract changes

Add an internal `_burn` function that will be called by `ETHLiquidity#SendETH` and update the external `burn` function to call `_burn`:

```solidity
/// @notice Allows an address to lock ETH liquidity into this contract.
function burn() external payable {
if (msg.sender != Predeploys.SUPERCHAIN_WETH) revert Unauthorized();
_burn(msg.sender, msg.value);
}

/// @notice Allows an address to lock ETH liquidity into this contract.
/// @param _sender Address that sent the ETH to be locked.
/// @param _amount The amount of liquidity to burn.
function _burn(address _sender, uint256 _amount) internal {
if (IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).isCustomGasToken()) revert NotCustomGasToken();
emit LiquidityBurned(_sender, _amount);
}
```

Add an internal `_mint` function that will be called by `ETHLiquidity#RelayETH` and update the external `mint` function to call `_mint`:

```solidity
/// @notice Allows an address to unlock ETH liquidity from this contract.
/// @param _amount The amount of liquidity to unlock.
function mint(uint256 _amount) external {
if (msg.sender != Predeploys.SUPERCHAIN_WETH) revert Unauthorized();
_mint(msg.sender, _amount);
}

/// @notice Allows an address to unlock ETH liquidity from this contract.
/// @param _recipient Address to send the unlocked ETH to.
/// @param _amount The amount of ETH liquidity to unlock.
function _mint(address _recipient, uint256 _amount) internal {
if (IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).isCustomGasToken()) revert NotCustomGasToken();
new SafeSend{ value: _amount }(payable(_recipient));
emit LiquidityMinted(_recipient, _amount);
}
```
Comment on lines +154 to +190
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this outdated?
not sure how this fits with the current design.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is up to date. this design is under the Alternatives Considered section and is meant to illustrate a design that was considered for adding this functionality to the ETHLiquidity contract


Add `SendETH` and `RelayETH` function to `ETHLiquidity`:

```solidity
/// @notice Sends ETH to some target address on another chain.
/// @param _to Address to send ETH to.
/// @param _chainId Chain ID of the destination chain.
function sendETH(address _to, uint256 _chainId) public payable {
if (IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).isCustomGasToken()) {
revert NotCustomGasToken();
}

_burn(msg.sender, msg.value);

// Send message to other chain.
IL2ToL2CrossDomainMessenger(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER).sendMessage({
_destination: _chainId,
_target: address(this),
_message: abi.encodeCall(this.relayETH, (msg.sender, _to, msg.value))
});
emit SendETH(msg.sender, _to, msg.value, _chainId);
}

/// @notice Relays ETH received from another chain.
/// @param _from Address of the msg.sender of sendETH on the source chain.
/// @param _to Address to relay ETH to.
/// @param _amount Amount of ETH to relay.
function relayETH(address _from, address _to, uint256 _amount) external {
IL2ToL2CrossDomainMessenger messenger = IL2ToL2CrossDomainMessenger(Predeploys.L2_TO_L2_CROSS_DOMAIN_MESSENGER);
if (msg.sender != address(messenger)) revert Unauthorized();
if (messenger.crossDomainMessageSender() != address(this)) revert Unauthorized();
if (IL1Block(Predeploys.L1_BLOCK_ATTRIBUTES).isCustomGasToken()) {
revert NotCustomGasToken();
}

_mint(_to, _amount);
emit RelayETH(_from, _to, _amount, messenger.crossDomainMessageSource());
}
```

### Advantages
- All ETH liquidity management occurs within a single, dedicated contract.

### Downsides
- Since `ETHLiquidity` holds so much ether, we have to be really careful with what functionality is added to it. It should be callable in the most minimal amount of ways. The original design of `ETHLiquidity` intended there to be only a single useful caller. We need to be very careful with any modifications to it

## Integrate `ETH` transfer into `SuperchainTokenBridge`

One alternative is to add two new functions to the `SuperchainTokenBridge` contract: `SendETH` and `RelayETH`.

```solidity
/// @notice Sends ETH to a target address on another chain.
/// @param _to Address to send ETH to.
/// @param _amount Amount of ETH to send.
/// @param _chainId Chain ID of the destination chain.
/// @return msgHash_ Hash of the message sent.
function sendETH(address _to, uint256 _amount, uint256 _chainId) external payable returns (bytes32 msgHash_) {
if (_to == address(0)) revert ZeroAddress();

ISuperchainWETH(payable(Predeploys.SUPERCHAIN_WETH)).deposit{ value: msg.value }();
ISuperchainWETH(payable(Predeploys.SUPERCHAIN_WETH)).crosschainBurn(msg.sender, msg.value);

bytes memory message = abi.encodeCall(this.relayETH, (msg.sender, _to, _amount));
msgHash_ = IL2ToL2CrossDomainMessenger(MESSENGER).sendMessage(_chainId, address(this), message);

emit SendERC20(Predeploys.SUPERCHAIN_WETH, msg.sender, _to, _amount, _chainId);
}

/// @notice Relays ETH received from another chain.
/// @param _from Address of the msg.sender of sendETH on the source chain.
/// @param _to Address to relay ETH to.
/// @param _amount Amount of ETH to relay.
function relayETH(address _from, address _to, uint256 _amount) external {
if (msg.sender != MESSENGER) revert Unauthorized();

(address crossDomainMessageSender, uint256 source) =
IL2ToL2CrossDomainMessenger(MESSENGER).crossDomainMessageContext();
if (crossDomainMessageSender != address(this)) revert InvalidCrossDomainSender();

ISuperchainWETH(payable(Predeploys.SUPERCHAIN_WETH)).crosschainMint(address(this), _amount);
ISuperchainWETH(payable(Predeploys.SUPERCHAIN_WETH)).withdraw(_amount);

new SafeSend{ value: _amount }(payable(_to));

emit RelayERC20(Predeploys.SUPERCHAIN_WETH, _from, _to, _amount, source);
}
```

### Advantages
- The advantage of this solution is that `SuperchainTokenBridge`would handle both `ETH` transfers and `SuperchainERC20` transfers, simplifying developer integrations.

### Downsides
- This solution creates changes to a highly sensitive contract. `SuperchainTokenBridge` has permissions to `mint` and `burn` standard `SuperchainERC20` tokens, so updates must be treated with extreme caution.