Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
interop: interoperable ether transfers design doc #146
Changes from 10 commits
b2c87e9
81c3097
657dc6d
d9c3efc
6f892ac
04e3f9e
fb46063
9948477
f77848f
5178c74
35f577c
9bbc779
6e8a7e9
d469ea1
6f5b008
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
NTH: it'd be cool to have this "am I being called by myself?" abstracted into a "CDM library"
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 assume rollbacks wouldn't be an issue as long as relayETH can only be called following the origin chain's finality period has passed?
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 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
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.
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
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 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
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.
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 contractThere 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 definitely simplifies things and creates better separation of concerns to leave
SuperchainWETH
as-is and move this intoETHLiquidity
, so I'm onboard with thatThere 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.
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 itThere 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.
@tynes are you onboard with the suggested solution or do you think it is too risky of a change?
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.
definitely see the concern on additions to
ETHLiquidity
. But want to note that even if this is inSuperchainWETH
, a vulnerability there would also mean a vulnerability inETHLiquidity
since the relay mechanism pulls ETH from this contractThere 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 outdated?
not sure how this fits with the current design.
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 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 theETHLiquidity
contract