-
Notifications
You must be signed in to change notification settings - Fork 96
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: token standard #71
Conversation
specs/interop/token-bridging.md
Outdated
between domains. In contrast to [xERC20][xerc20] where the user calls | ||
a bridge that has `mint` and `burn` privileges on the cross chain enabled | ||
tokens, the user instead calls methods on the token contract directly. |
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.
Is this just a UX preference or is there another reason for this?
Also where does xERC20 fit-in, if at all, to this token standard? Mainly asking if it makes sense for this to be xERC20 compatible to reduce fragmentation and ensure superchain tokens are compatible (this assumes xERC20 gains traction, and I'm not up to date on the status there)
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.
I agree that we are now introducing "yet another standard" altho none of the existing standards work for our usecase. xERC20 relies on a bridge contract that can own minting/burning the tokens along with a bunch of config. We don't necessarily need all of the features of xERC20, there is bloat introduced in xERC20 that will hinder its adoption imo. xERC20 should be the minimal feature set for minting/burning then xERC20+ should include the extra features.
Our advantage is that we don't need a bridge contract at all for this standard to work. People can still use xERC20 at the application layer but they would need to wrap existing tokens
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.
wrapping into a new contract would require additional liquidity, right? since you now have to sustain pools for both the xERC20 and its underlying ERC20
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.
I believe it is possible for an ERC20 contract to implement both ISuperchainERC20 and xERC20 at the same time.
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.
Its definitely possible for a contract to support both xERC20 and SuperchainERC20, altho it would be complex
specs/interop/token-bridging.md
Outdated
function bridgeERC20(uint256 _amount, uint256 _chainId) external { | ||
bridgeERC20To(msg.sender, _amount, _chainId); | ||
} |
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.
Not sure if we care about this for the reference implementation, but assuming we want this for aliasing reasons
function bridgeERC20(uint256 _amount, uint256 _chainId) external { | |
bridgeERC20To(msg.sender, _amount, _chainId); | |
} | |
function bridgeERC20(uint256 _amount, uint256 _chainId) external { | |
require(msg.sender == tx.origin, "OptimismSuperchainERC20: function can only be called from an EOA"); | |
bridgeERC20To(msg.sender, _amount, _chainId); | |
} |
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.
Ah yes good call, then its similar to the existing bridge implementation
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.
does this still made sense under eip 3074?
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.
This check exists to prevent a footgun with aliasing between L1 and L2 where you send to the unaliased account or you send to an account that you don't control since there is no guarantee you control the same smart contract on two different domains.
Since there is no aliasing between domains, its possible to argue that this check isn't needed. I would want to see an example of a 4337 bundler - i believe the msg.sender
would be the smart contract wallet. Ideally a user controls the same account between every domain but its not guaranteed. So it would be an implicit footgun if we allowed for it
another idea that might help quite a lot with interoperabolity would be to add an execute instruction after the mint. function bridgeERC20To(address _to, uint256 _amount, uint256 _chainId, bytes calldata _data) public {
_burn(msg.sender, amount);
L2ToL2CrossDomainMessenger.sendMessage({
_destination: chainId,
_target: address(this),
_message: abi.encodeCall(this.finalizeBridgeERC20, (msg.sender, to, amount, _data))
});
}
function finalizeBridgeERC20(address _from, address _to, uint256 _amount, bytes calldata _data) external {
require(msg.sender == address(L2ToL2CrossChainMessenger));
require(L2ToL2CrossChainMessenger.crossDomainMessageSender() == address(this));
_mint(_to, _amount);
if (_data.length > 0) to.call(_from, _amount, _data);
} |
This is definitely good feedback and something that we could include to reduce the number of transactions that users need to send |
glad u liked it @tynes :) also, how do you see these if I wanted to use xERC20 for this. I would only use the canonical do you think that this standard needs to be an actual ERC20 flavor? or can it be a periphery helper with minter/burner roles into a standard ERC20 or xERC20 implementation? I know this adds complexity to a very clean and straightforward implementation. but i wanted to know the reasoning behind the support for creating a new standard. |
From the Frax side (currently operating on a non-superchain), I have a some opinions/questions:
|
hi @pegahcarter nice qs! hope I can give you some useful answers.
100%, this is the intended use-case.
this is a great question, we are currently working on a standard for this specific issue. we'll be able to share more details soon :)
the only thing that would happen here would be that the "superchain L2<>L2 canonical bridge" will be "deprecated". if the xERC20 has no-owner, and only had that superchain minter, it will be effectively bricked to receive or send any further messages. |
To enable the token standard to scale, we are going to want to use a factory that uses a create3 style deployment to ensure that we don't always enforce every contract bytecode to be the same forever. We will likely want to use ERC20 specific identifying information hashed together to create the salt, perhaps with a version to make it flexible longer term. This deterministic address generation scheme would enable superchain enabled ERC20 tokens to not maintain a chain id to address mapping of the same asset on other chains. |
@tynes this is a great idea!
do these points resonate with what you were thinking? |
IMO cross-chain ownership should be delegated to the initial point of token issuance and is a requirement to this standard. If ownership is not desired, it can always be renounced. What would a no-owner token look like on a cross-chain other than the gas token? From my understanding every protocol that would want interoperability would need to approve a non-bridge minter contract for their token. For example, Maker would want to prove some contract they deployed on different superchain for DAI minting rights. It's tough because unless the token and protocol are deployed deterministically across chains (unless there's some other way), there's additional trust required and immutable protocols may not support the opened minting abilities. |
specs/interop/token-bridging.md
Outdated
function bridgeERC20To(address _to, uint256 _amount, uint256 _chainId) public { | ||
_burn(msg.sender, amount); | ||
|
||
L2ToL2CrossDomainMessenger.sendMessage({ |
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.
shouldn't it be l2ToL2CrossDomainMessenger
?
L2ToL2CrossDomainMessenger.sendMessage({ | |
l2ToL2CrossDomainMessenger.sendMessage({ |
otherwise, the variable would collide with the reference contract for L2ToL2CrossDomainMessenger
specs/interop/token-bridging.md
Outdated
|
||
function finalizeBridgeERC20(address to, uint256 amount) external { | ||
require(msg.sender == address(L2ToL2CrossChainMessenger)); | ||
require(L2ToL2CrossChainMessenger.crossDomainMessageSender() == address(this)); |
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.
by above, shouldn't this be
require(L2ToL2CrossChainMessenger.crossDomainMessageSender() == address(this)); | |
require(l2ToL2CrossChainMessenger.crossDomainMessageSender() == address(this)); |
specs/interop/token-bridging.md
Outdated
```solidity | ||
interface ISuperchainERC20 { | ||
function bridgeERC20(uint256 amount, uint256 chainId) external; | ||
function bridgeERC20To(address to, uint256 amount, uint256 chainId) external; |
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.
maybe it could be make sense to instead define methods like transferFrom(address from,..., uint256 chainId)
such that ISuperchainERC20 is seen more clearly as expanding the capabilities of the ERC20
(in this case, by extending the functions with another argument for chainId
)
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.
This was an earlier approach but then we decided towards a model of being very explicit so that its obvious that this is a bridging interface
specs/interop/token-bridging.md
Outdated
|
||
An `OptimismMintableERC20Token` does not natively have the functionality | ||
to be sent between L2s. There are a few approaches to migrating L1 native tokens | ||
to L2 ERC20 tokens that support the `SuperchainERC20` interface. |
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.
It seems like this architecture tightly couples the tokens to the underlying bridge and rollup architecture.
I recently deployed an xERC20 token on two chains, Base and Moonbeam for Moonwell using Wormhole adapters. My guess is that optimism and projects building on top of the superchain would be much better served if instead of rolling a new token standard, the xERC20 standard is used. A superchain xERC20 adapter can be created so that if a project wants to create a natively multichain token across ten different chains, they can leverage the superchain adapters to allow token movements between OP stack chains, and use other adapters to go outside of this ecosystem.
Pros of using xERC20:
- predefined standard, code is already written, audited and in production
- controlled blast radius from rate limits in case of underlying bridge hack
Cons of using xERC20:
- managing rate limits requires governance and external input
- standard is currently not widely adopted, so very few teams have experience or understanding of what it looks like to deal with a token system at scale deployed on 20 chains
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.
If xERC20 is used, the factory contract is already defined, as well as the lockbox contract to migrate another token to this new token standard.
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.
Hey @ElliotFriedman! Thank you for the feedback here. I think no matter what we will be supporting an easy way for xERC20 between L2s on the superchain. The question boils down to product requirements around the GovToken. This standard exists to enable a portable + fungible GovToken where votes can be done between domains within the superchain. It may be possible to use an xERC20 for this but it would be a version of xERC20 where most of the functionality is locked down such that it is 1) governance minimized and 2) only transferrable within the superchain
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.
The use case you are describing sounds totally achievable with code that already exists, you would just need a op-bridge adapter to enable moving tokens within the superchain at high speeds.
To address each point:
- governance minimization can be achieved by setting high rate limits such that no votes or governance actions need to occur to change the rate limits for this token.
- to make this token only transferrable within the superchain, you would only deploy the superchain bridge adapters on supported OP-Stack chains.
- How do you want votes to be tallied across multiple chains? There are patterns off the shelf for a DAO that allows voting in a multichain system with an xERC20 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.
Having something that already just works for cross chain voting with xERC20 would be very helpful and make a better case for using xERC20 here. @kent and @yitongzhang from @voteagora are thinking about this, we hope to have a public design doc soon in our design docs repo, def can follow up with a bump here once its public if you are interested in taking a look :)
A constraint would be that we don't want to have any governance built into the token, its abilities should be set at deploy time with no additional ways to modify things. A lot of the xERC20 interface seems unnecessary for GovToken and I worry about bloating the bytecode with unnecessary stuff
specs/interop/token-bridging.md
Outdated
|
||
Should an overloaded `approve` method be added to the interface so that a user can | ||
approve a contract on a remote domain to bridge on their behalf? A new `allowances` | ||
mapping could be added to the contract to enable this. |
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.
I would encourage some form of permit, which would avoid switching the network in the user wallet for this use case — though the UX of signatures in wallet kinda sucks (it's not immediately clear you're authorizing a transfer of your funds, it looks something like this: https://cdn0.scrvt.com/b095ee27d37b3d7b6b150adba9ac6ec8/7764cb129062498d/2441291776b8/v/42ddfee6f366/uniswap-permit.png
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.
I generally like the idea of using a permit, would need to prototype a crosschain transferFrom
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.
Does the same permit UI exist across wallet providers?
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.
It's standardized as EIP-712 so I would expect most serious wallet to support it, but I haven't verified it.
specs/interop/token-bridging.md
Outdated
function bridgeERC20(uint256 amount, uint256 chainId) external; | ||
function bridgeERC20To(address to, uint256 amount, uint256 chainId) external; | ||
function finalizeBridgeERC20(address to, uint256 amount) external; | ||
} |
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.
TODO: explore the following functions:
approveBridgeERC20(address spender, uint256 amount, uint256 chainId)
which would be the cross chain equivalent ofapprove
bridgeERC20From(address,address,uint256)
which would be the cross chain equivalent oftransferFrom
- Also a cross chain permit could be useful
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.
Also we need to explore the rollback interface being hooked into here
specs/interop/token-bridging.md
Outdated
|
||
```solidity | ||
interface ISuperchainERC20 { | ||
function bridgeERC20(uint256 amount, uint256 chainId) external; |
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.
I like the idea of renaming these to sendERC20
and relayERC20
to be more similar to the cross domain messenger rather than using the bridgeERC20
naming scheme which is used by the standard bridge. The standard bridge is for L1 <> L2, but this is L2 <> L2, so I think using terminology similar to the L2ToL2CrossDomainMessenger is better
Initial pass, needs feedback
Co-authored-by: Skeletor Spaceman <[email protected]>
Co-authored-by: Skeletor Spaceman <[email protected]>
Co-authored-by: skeletor <[email protected]> Co-authored-by: ng <[email protected]>
006391c
to
e15b048
Compare
…standard-no-data interop: token standard without data
Description
SuperchainERC20 Spec