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.

Sign up for GitHub

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

Jump to bottom

Add transfer precompile design doc #51

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add transfer precompile design doc #51

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9c0492c
Add transfer precompile design doc
karlb Jul 23, 2024
8d4c656
Apply suggestions from code review
karlb Jul 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions protocol/transfer-precompile.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# Purpose

Since its inception, the Celo blockchain provides a feature called "[Token
Duality](https://specs.celo.org/token_duality.html)", which allows the native
token to be used both in the usual ETH-like way and as an ERC-20 token.

We would like to bring this feature to more OP-Stack based chains and bring the
Celo L2 closer to vanilla OP-stack by adding the underlying transfer precompile to
op-geth.

# Summary

Add a precompile to op-geth that allows transferring native tokens.
This precompile can only be called by a single privileged contract.

# Problem Statement + Context

Many users and contracts have to deal with both ERC20 tokens and native tokens.
Since they can't be accessed by the same interface, they either have to be
handled as two different cases or be explicitly wrapped by an ERC-20 token
wrapping contract.
Wrapping tokens is not an ideal solution either, since the wrapped token is
of limited use (e.g. can't be used to pay gas) and wrapping/unwrapping is
annoying and incurs additional gas charges.

This can be solved by having a contract that provides an ERC-20 interface
Copy link
Contributor

Choose a reason for hiding this comment

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

So the main value prop here is that the unwrap/wrap steps are saved. In a world with smart contract wallets, users can do multicalls and save needing to send multiple transactions, but this is still cheaper as there is no wrap/unwrap step necessary

Copy link
Author

Choose a reason for hiding this comment

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

Yes, but there is plenty of benefit before everyone uses smart contract wallets and every app has implemented perfect wrap/unwrap handling (Should tokens stay wrapped after the action to save gas? Should existing wrapped tokens be used? Even if they are not enough so that you have to wrap anyway? Do you want to bulk wrap to save gas on the next actions? Should the wallet UI show wrapped tokens separately from unwrapped tokens?).

There is also the argument that it makes the app developers lives easier since they don't have to implement all these wrap/unwrap cases. But since most apps are deployed across many chains, most devs will have to do that anyway.

directly to the native token. To implement such a contract, the blockchain must
provide a way to transfer native tokens on behalf of the transaction sender.

# Proposed Solution

Add one new blockchain parameter `TransferPrecompileAllowedCaller` and a new precompile.
The parameter defines the address from which the precompile must be called.

Precompile name: `transfer`
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there any security concerns here? I feel like i've seen bugs based on having a dual interface into balance

Copy link
Author

Choose a reason for hiding this comment

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

One more interface is certainly one more potential attack vector, especially when it provides access to everyone's balances. Fortunately, the interface is quite simple and low on interaction with other features. I don't know of any security issues we had with this feature since the Celo mainnet launch. The biggest danger I see is the privileged contract getting compromised.

Precompile address: 0x0000000000000000000000000000000000000101 (directly after the `P256VERIFY` precompile from Fjord)
Parameters (abi-encoded): `address from, address to, uint256 value`
Gas costs: 9000
Result: `value` native tokens will be transferred from `from` to `to`.
No return value

The precompile can fail for the following reasons:
* `TransferPrecompileAllowedCaller` is not configured for the blockchain
* It is not called from the contract at the `TransferPrecompileAllowedCaller` address
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the TransferPrecompileAllowedCaller address an arbitrary enshrined smart contract? In the context of the OP Stack, this could be a predeploy. Why can't anybody call this contract and instead of msg.sender being the first arg, the native EVM can look at the CALLER (which is msg.sender)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok given the geth code and your comment:

Removing the from parameter from the precompile is not an option because
msg.sender can't be accessed from inside the precompile.
There, the caller is set to the contract doing TRANSFER.call.

I understand this design decision more. It would be a larger diff to send through the evm context into the precompile

Copy link
Author

Choose a reason for hiding this comment

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

I understand this design decision more. It would be a larger diff to send through the evm context into the precompile

Yes, and it would also reduce the chances of the precompile being supported by third party tooling (e.g. foundry's anvil).

* The `from` address has less than `value` native tokens
* The input is not exactly 96 bytes long
* The input parameters can't be parsed correctly

Example usage in solidity:

```solidity
address constant TRANSFER = address(0x0000000000000000000000000000000000000101);

function _transfer(address to, uint256 value) internal {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the design space of possible TransferPrecompileAllowedCaller contracts? In practice would it mostly look like the pseudocode here?

Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like the allowed caller should just be a predeploy that is literally this code, perhaps optimized and its part of the design, the way that i read the design right now makes it sound like the allowed caller can be configured to different contracts

Copy link
Author

Choose a reason for hiding this comment

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

I would expect the contract to implement the all of ERC20 in a way that passes through the native balance. So in addition to the transfer, you would have other ERC20 functions like

  function decimals() external view returns (uint8) {
    return 18;
  }

  function balanceOf(address _owner) public view returns (uint256) {
    return _owner.balance;
  }

and potentially other interfaces (it could be a proxy for upgradibility if you consider that safe enough, other token interfaces, etc). I assume it would be hard to find one contract that fits everyone here, and it would need to be configurable in every case to get the name and symbol right.

Having a predeploy with only the a transfer function that can be called from the full ERC20 contract would not solve anything AFAICS. That function would still need to receive the from since msg.sender can't be used for that due to the additional call. And it would still need to check that the calling contract is the privileged contract.

(success, ) = TRANSFER.call.value(0).gas(gasleft())(abi.encode(msg.sender, to, value));
require(success, "transfer failed");
emit Transfer(msg.sender, to, value);
}
```

The `from` parameter should always be set to `msg.sender` so that each address
can only transfer its own funds.
Removing the `from` parameter from the precompile is not an option because
`msg.sender` can't be accessed from inside the precompile.
There, the caller is set to the contract doing `TRANSFER.call`.

# Risks & Uncertainties

* The privileged contract at `TransferPrecompileAllowedCaller` can execute arbitrary transfers, so it must be a well known and safe contract.
* Like all precompiles, it must be added as part of a hardfork.

# Open questions
* Where is the right place for the `TransferPrecompileAllowedCaller` configuration? Should it be part of `OptimismConfig`?
* If `TransferPrecompileAllowedCaller` is not configured, is it ok to have the precompile, although it will always revert? Or would it be better to remove it from the list of active precompiles?

# History

The concept outlined in this document is in production use on the Celo mainnet since its launch in May 2020.
As part of Celo's L2 migration, support for it has been added to a fork of the OP-stack.

Code examples:
* [The transfer precompile in the op-geth fork](https://github.com/celo-org/op-geth/blob/celo7/core/vm/celo_contracts.go)
* [The CELO token using the precompile](https://github.com/celo-org/celo-monorepo/blob/2ace1d08b9da5d2618e945eef7663cc385917a2d/packages/protocol/contracts/common/GoldToken.sol#L302-L315) to provide access to the native token