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 8 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.
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