WETH Bridge Implementation #5
Conversation
Co-authored-by: mpavlovic-txfusion <[email protected]>
|
|
||
| /// @dev The accumulated deposited amount per user. | ||
| /// @dev A mapping user address => the total deposited WETH amount by the user | ||
| mapping(address => uint256) public totalDepositedAmountPerUser; |
There was a problem hiding this comment.
We are going to remove this limit in the next release, so better to remove it here too.
| uint256 _l2TxGasLimit, | ||
| uint256 _l2TxGasPerPubdataByte | ||
| ) external payable nonReentrant returns (bytes32 txHash) { | ||
| // ) external payable nonReentrant senderCanCallFunction(allowList) returns (bytes32 txHash) { |
| ); | ||
| } | ||
|
|
||
| function deposit( |
There was a problem hiding this comment.
FYI: It is a bit of outdated interface, the integrating last one will be easy, so no problem. We have added the address _refundRecipient to the interface. You can find up to date info in the
| require(depositedAmount == _amount, "Incorrect amount of funds deposited"); | ||
|
|
||
| // // verify the deposit amount is allowed | ||
| // _verifyDepositLimit(msg.sender, _amount, false); |
| require(_l2Receiver != address(0), "L2 receiver address can not be zero"); | ||
|
|
||
| // Deposit WETH tokens from the depositor address to the smart contract address | ||
| uint256 depositedAmount = _transferWethFunds(msg.sender, address(this), _amount); |
There was a problem hiding this comment.
We have used the _transferWethFunds function logic, since the token implementation was untrusted. Here the token is static and well-known. So you could use _amount instead of depositedAmount, and replace _transferWethFunds with l1Weth.safeTransferFrom(_from, _to, _amount);
For using the safeTransferFrom you need to import the https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC20/utils/SafeERC20.sol OZ lib.
There was a problem hiding this comment.
Here is the L1 WETH implementation we want to support https://etherscan.io/token/0xc02aaa39b223fe8d0a0e5c4f27ead9083c756cc2#code.
This WETH implementation is not ERC20, so unfortunately we can't use l1Weth.safeTransferFrom(_from, _to, _amount);...
There was a problem hiding this comment.
- it is an ERC20, it is implement all methods from the https://eips.ethereum.org/EIPS/eip-20, so it is ERC20
- The
safeTransferFromis not a function on token itself, but a method from the OZ library
| address public override l2WethAddress; | ||
|
|
||
| /// @dev ETH token address on L2. | ||
| address public constant l2EthAddress = address(0x800a); |
There was a problem hiding this comment.
NIT: maybe use caps snake case for constant variables?
| // Burn WETH on L2. | ||
| IL2StandardToken(l2WethAddress).bridgeBurn(msg.sender, _amount); | ||
| // Withdraw ETH to L1 bridge. | ||
| IEthToken(l2EthAddress).withdraw{value: _amount}(l1WethBridge); |
There was a problem hiding this comment.
Do you really need to withdraw? The bridge already burn tokens and send ether to you
There was a problem hiding this comment.
After calling the bridgeBurn method, ETH is withdrawn to the L2 WETH Bridge. It’s important to transfer the ETH to the L1 WETH Bridge after this step so that the WETH can be wrapped and sent back to the user on L1.
There was a problem hiding this comment.
Sorry, it is me misunderstand the logic. I was confused, because I thought that the contract address that you use for withdrawing is l2WethAddress not l2EthAddress.
| } | ||
|
|
||
| receive() external payable {} | ||
| } |
| return abi.encodePacked(IL1WethBridge.finalizeWithdrawal.selector, _l2Sender, _l1Receiver, _amount); | ||
| } | ||
|
|
||
| receive() external payable {} |
There was a problem hiding this comment.
NIT: event would be nice to have there
|
|
||
| pragma solidity ^0.8.0; | ||
|
|
||
| interface IEthToken { |
There was a problem hiding this comment.
?
| interface IEthToken { | |
| interface IL2WethToken { |
There was a problem hiding this comment.
This is the interface for ETH on L2. We use it to withdraw ETH from the L2 WETH Bridge to the L1 WETH Bridge.
There was a problem hiding this comment.
I see, makes perfect sense then.
|
@mpopovac-txfusion Thanks for your contribution! This PR was merged in the private repo and migrated to this repo already. Closing it because of this reason. |
[SMA-1454]: Open Source Work
Add comment on `MAX_SYSTEM_CONTRACT_ADDRESS` choice
No description provided.