Skip to content
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

fix: H-02 Forwarder and Withdrawal Helper Contracts do not Handle ETH Transfers #725

Merged
merged 13 commits into from
Nov 18, 2024

Conversation

bmzig
Copy link
Contributor

@bmzig bmzig commented Nov 6, 2024

The WithdrawalHelperBase and ForwarderBase contracts are designed to be deployed on L2s and assist in moving tokens and messages to and from L3 chains. These contracts are inherited by the chain-specific contracts, currently designed for Arbitrum and OVM-based blockchains. However, none of these contracts handle ETH transfers correctly. This is because they do not contain the receive function, which causes any attempt to transfer ETH to these contracts to fail.

In the case of forwarder contracts, the lack of the receive function means that WETH transfers, which rely on unwrapping before bridging such as the transfers made through the Optimism_Adapter, will fail, leaving the ETH in the bridge until the contract can be upgraded. In the case of withdrawal helper contracts, the lack of the receive function implies that they will not be capable of unwrapping WETH in an attempt to transfer it to Ethereum. Additionally, they will not be capable of receiving ETH bridged from L3s. Moreover, while the withdrawal helper contracts contain token-bridging logic, they do not support bridging ETH. This means that even if they were capable of receiving ETH and ETH was bridged to them from L3s, it could not be routed to L1.

Consider adding a receive function to the ForwarderBase and WithdrawalHelperBase contracts to facilitate incoming ETH transfers and the unwrapping of WETH tokens during bridging. As the contracts do not support bridging ETH directly, the receive function should include logic to ensure that incoming ETH is handled correctly and can be sent on to the target chain.

This PR adds receive to both ForwarderBase and WithdrawalHelperBase so that it can receive native token transfers from L1 or L3 bridges. Both the ForwarderBase and WithdrawalHelperBase now contain a hook named _wrapNativeToken which is called whenever a L2-L3 bridge or withdrawal which includes the L2's native token is initiated on the respective contracts. Both the ForwarderBase and WithdrawalHelperBase now take an additional argument _wrappedNativeToken in their constructor, which is used to deposit the native token in the _wrapNativeToken hook.

bmzig added 8 commits October 9, 2024 13:56
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig bmzig changed the title fix: support native token bridging in the forwarder/withdrawal helper fix: H-02 Forwarder and Withdrawal Helper Contracts do not Handle ETH Transfers Nov 8, 2024
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig bmzig marked this pull request as ready for review November 14, 2024 01:33
@pxrl
Copy link
Contributor

pxrl commented Nov 14, 2024

@bmzig If you merge in the latest change from master you'll get a fix from Reinis that addresses the SVM test failure.

Signed-off-by: bennett <bennett@umaproject.org>
Signed-off-by: bennett <bennett@umaproject.org>
@bmzig bmzig merged commit 068c525 into master Nov 18, 2024
9 checks passed
@bmzig bmzig deleted the 1124oz/h02 branch November 18, 2024 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants