-
Notifications
You must be signed in to change notification settings - Fork 75
fix(optimism-spokepool): Need to wrap ETH before distributing any WETH to recipients #49
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
Conversation
…art-contracts-v2 into npai/optimism-spokepool
contracts/SpokePool.sol
Outdated
| bytes32[] memory proof | ||
| ) public nonReentrant { | ||
| // This function potentially is expected to transfer WETH to the recipient so we should wrap all ETH. | ||
| _depositEthToWeth(); |
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 it make sence to only do this if the destinationToken is WETH?
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.
sure done
contracts/SpokePool.sol
Outdated
| bytes32[] memory proof | ||
| ) public nonReentrant { | ||
| // This function potentially is expected to transfer WETH to the recipient so we should wrap all ETH. | ||
| _depositEthToWeth(); |
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.
follow on question: does it make sence to put this in the spoke Pool? would it not make more sence to put this in the Optimism spoke pool only seeing this is only relevant on optimism?
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.
As discussed IRL i'll move to Optimism_SpokePool
mrice32
left a comment
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.
LGTM!
The Optimism token bridge sends ETH (address 0xdeaddead...) to SpokePool, so SpokePool accrues an ETH balance after pool rebalances are executed on L1. We make assumptions that the SpokePool has WETH, not ETH, so we need to wrap WETH before distributing refunds or processing relays.
Originally I wanted to implement the wrapping of ETH in the
fallbackorreceivemethods but these are not triggered by ETH deposits over the token bridge because ETH is an ERC20 token on Optimism, so I opted for this approach.