Add transfer precompile design doc #51
Conversation
karlb
force-pushed
the
transfer-precompile
branch
from
504989b
to
9c0492c
Compare
Co-authored-by: Paul Lange <[email protected]>
karlb
force-pushed
the
transfer-precompile
branch
from
ce9d555
to
8d4c656
Compare
|
|
||
| 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.
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)?
There was a problem hiding this comment.
Ok given the geth code and your comment:
Removing the
fromparameter from the precompile is not an option because
msg.sendercan't be accessed from inside the precompile.
There, the caller is set to the contract doingTRANSFER.call.
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.
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).
| ```solidity | ||
| address constant TRANSFER = address(0x0000000000000000000000000000000000000101); | ||
|
|
||
| function _transfer(address to, uint256 value) internal { |
There was a problem hiding this comment.
What is the design space of possible TransferPrecompileAllowedCaller contracts? In practice would it mostly look like the pseudocode here?
There was a problem hiding this comment.
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.
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.
| 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.
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.
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.
| 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 |
There was a problem hiding this comment.
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.
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.
|
I think this could make sense given the chain operator could turn on/off the smart contract that is authorized to interact with the predeploy. There is a non zero chance that an existing application has a bug due to this, so chains that want to be more conservative in adopting this would be able to not use it. Then chains that do want this feature could turn it on |
Description
Bring Celo's
transferprecompile to OP-stack to allow Token Duality (native token via ERC20) without requiring an op-geth fork.Additional context