-
Notifications
You must be signed in to change notification settings - Fork 18
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
Add transfer precompile design doc #51
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 | ||
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` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok given the geth code and your comment:
I understand this design decision more. It would be a larger diff to send through the evm context into the precompile There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the design space of possible There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
(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 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.