[WIP] wstETH on Mantle, DON'T MERGE #52
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
TheDZhon
commented
Sep 13, 2023
TheDZhon
commented
Origin/feature/mantle testnet
feat/mantle-testnet
wstETH deploy config for Mantle testnet
implement ERC20BridgedPermit
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
20
to
114
| contract L2ERC20TokenBridge is | ||
| IL2ERC20Bridge, | ||
| BridgingManager, | ||
| BridgeableTokens, | ||
| CrossDomainEnabled | ||
| { | ||
| /// @inheritdoc IL2ERC20Bridge | ||
| address public immutable l1TokenBridge; | ||
|
|
||
| /// @param messenger_ L2 messenger address being used for cross-chain communications | ||
| /// @param l1TokenBridge_ Address of the corresponding L1 bridge | ||
| /// @param l1Token_ Address of the bridged token in the L1 chain | ||
| /// @param l2Token_ Address of the token minted on the L2 chain when token bridged | ||
| constructor( | ||
| address messenger_, | ||
| address l1TokenBridge_, | ||
| address l1Token_, | ||
| address l2Token_ | ||
| ) CrossDomainEnabled(messenger_) BridgeableTokens(l1Token_, l2Token_) { | ||
| l1TokenBridge = l1TokenBridge_; | ||
| } | ||
|
|
||
| /// @inheritdoc IL2ERC20Bridge | ||
| function withdraw( | ||
| address l2Token_, | ||
| uint256 amount_, | ||
| uint32 l1Gas_, | ||
| bytes calldata data_ | ||
| ) external whenWithdrawalsEnabled onlySupportedL2Token(l2Token_) { | ||
| _initiateWithdrawal(msg.sender, msg.sender, amount_, l1Gas_, data_); | ||
| } | ||
|
|
||
| /// @inheritdoc IL2ERC20Bridge | ||
| function withdrawTo( | ||
| address l2Token_, | ||
| address to_, | ||
| uint256 amount_, | ||
| uint32 l1Gas_, | ||
| bytes calldata data_ | ||
| ) external whenWithdrawalsEnabled onlySupportedL2Token(l2Token_) { | ||
| _initiateWithdrawal(msg.sender, to_, amount_, l1Gas_, data_); | ||
| } | ||
|
|
||
| /// @inheritdoc IL2ERC20Bridge | ||
| function finalizeDeposit( | ||
| address l1Token_, | ||
| address l2Token_, | ||
| address from_, | ||
| address to_, | ||
| uint256 amount_, | ||
| bytes calldata data_ | ||
| ) | ||
| external | ||
| whenDepositsEnabled | ||
| onlySupportedL1Token(l1Token_) | ||
| onlySupportedL2Token(l2Token_) | ||
| onlyFromCrossDomainAccount(l1TokenBridge) | ||
| { | ||
| IERC20Bridged(l2Token_).bridgeMint(to_, amount_); | ||
| emit DepositFinalized(l1Token_, l2Token_, from_, to_, amount_, data_); | ||
| } | ||
|
|
||
| /// @notice Performs the logic for withdrawals by burning the token and informing | ||
| /// the L1 token Gateway of the withdrawal | ||
| /// @param from_ Account to pull the withdrawal from on L2 | ||
| /// @param to_ Account to give the withdrawal to on L1 | ||
| /// @param amount_ Amount of the token to withdraw | ||
| /// @param l1Gas_ Unused, but included for potential forward compatibility considerations | ||
| /// @param data_ Optional data to forward to L1. This data is provided | ||
| /// solely as a convenience for external contracts. Aside from enforcing a maximum | ||
| /// length, these contracts provide no guarantees about its content | ||
| function _initiateWithdrawal( | ||
| address from_, | ||
| address to_, | ||
| uint256 amount_, | ||
| uint32 l1Gas_, | ||
| bytes calldata data_ | ||
| ) internal { | ||
| IERC20Bridged(l2Token).bridgeBurn(from_, amount_); | ||
|
|
||
| bytes memory message = abi.encodeWithSelector( | ||
| IL1ERC20Bridge.finalizeERC20Withdrawal.selector, | ||
| l1Token, | ||
| l2Token, | ||
| from_, | ||
| to_, | ||
| amount_, | ||
| data_ | ||
| ); | ||
|
|
||
| sendCrossDomainMessage(l1TokenBridge, l1Gas_, message); | ||
|
|
||
| emit WithdrawalInitiated(l1Token, l2Token, from_, to_, amount_, data_); | ||
| } | ||
| } |
Check failure
Code scanning / Slither
Name reused High
Comment on lines
+21
to
+150
| bytes calldata data_ | ||
| ) | ||
| external | ||
| whenDepositsEnabled | ||
| onlySupportedL1Token(l1Token_) | ||
| onlySupportedL2Token(l2Token_) | ||
| { | ||
| if (Address.isContract(msg.sender)) { | ||
| revert ErrorSenderNotEOA(); | ||
| } | ||
| _initiateERC20Deposit(msg.sender, msg.sender, amount_, l2Gas_, data_); | ||
| } | ||
|
|
||
| /// @inheritdoc IL1ERC20Bridge | ||
| function depositERC20To( | ||
| address l1Token_, | ||
| address l2Token_, | ||
| address to_, | ||
| uint256 amount_, | ||
| uint32 l2Gas_, | ||
| bytes calldata data_ | ||
| ) | ||
| external | ||
| whenDepositsEnabled | ||
| onlyNonZeroAccount(to_) | ||
| onlySupportedL1Token(l1Token_) | ||
| onlySupportedL2Token(l2Token_) | ||
| { | ||
| _initiateERC20Deposit(msg.sender, to_, amount_, l2Gas_, data_); | ||
| } | ||
|
|
||
| /// @inheritdoc IL1ERC20Bridge | ||
| function finalizeERC20Withdrawal( | ||
| address l1Token_, | ||
| address l2Token_, | ||
| address from_, | ||
| address to_, | ||
| uint256 amount_, | ||
| bytes calldata data_ | ||
| ) | ||
| external | ||
| whenWithdrawalsEnabled | ||
| onlySupportedL1Token(l1Token_) | ||
| onlySupportedL2Token(l2Token_) | ||
| onlyFromCrossDomainAccount(l2TokenBridge) | ||
| { | ||
| IERC20(l1Token_).safeTransfer(to_, amount_); | ||
|
|
||
| emit ERC20WithdrawalFinalized( | ||
| l1Token_, | ||
| l2Token_, | ||
| from_, | ||
| to_, | ||
| amount_, | ||
| data_ | ||
| ); | ||
| } | ||
|
|
||
| /// @dev Performs the logic for deposits by informing the L2 token bridge contract | ||
| /// of the deposit and calling safeTransferFrom to lock the L1 funds. | ||
| /// @param from_ Account to pull the deposit from on L1 | ||
| /// @param to_ Account to give the deposit to on L2 | ||
| /// @param amount_ Amount of the ERC20 to deposit. | ||
| /// @param l2Gas_ Gas limit required to complete the deposit on L2. | ||
| /// @param data_ Optional data to forward to L2. This data is provided | ||
| /// solely as a convenience for external contracts. Aside from enforcing a maximum | ||
| /// length, these contracts provide no guarantees about its content. | ||
| function _initiateERC20Deposit( | ||
| address from_, | ||
| address to_, | ||
| uint256 amount_, | ||
| uint32 l2Gas_, | ||
| bytes calldata data_ | ||
| ) internal { | ||
| IERC20(l1Token).safeTransferFrom(from_, address(this), amount_); | ||
|
|
||
| bytes memory message = abi.encodeWithSelector( | ||
| IL2ERC20Bridge.finalizeDeposit.selector, | ||
| l1Token, | ||
| l2Token, | ||
| from_, | ||
| to_, | ||
| amount_, | ||
| data_ | ||
| ); | ||
|
|
||
| sendCrossDomainMessage(l2TokenBridge, l2Gas_, message); | ||
|
|
||
| emit ERC20DepositInitiated( | ||
| l1Token, | ||
| l2Token, | ||
| from_, | ||
| to_, | ||
| amount_, | ||
| data_ | ||
| ); | ||
| } | ||
|
|
||
| error ErrorSenderNotEOA(); | ||
| } |
Check failure
Code scanning / Slither
Name reused High
Comment on lines
+9
to
+46
| contract CrossDomainEnabled { | ||
| /// @notice Messenger contract used to send and receive messages from the other domain | ||
| ICrossDomainMessenger public immutable messenger; | ||
|
|
||
| /// @param messenger_ Address of the CrossDomainMessenger on the current layer | ||
| constructor(address messenger_) { | ||
| messenger = ICrossDomainMessenger(messenger_); | ||
| } | ||
|
|
||
| /// @dev Sends a message to an account on another domain | ||
| /// @param crossDomainTarget_ Intended recipient on the destination domain | ||
| /// @param message_ Data to send to the target (usually calldata to a function with | ||
| /// `onlyFromCrossDomainAccount()`) | ||
| /// @param gasLimit_ gasLimit for the receipt of the message on the target domain. | ||
| function sendCrossDomainMessage( | ||
| address crossDomainTarget_, | ||
| uint32 gasLimit_, | ||
| bytes memory message_ | ||
| ) internal { | ||
| messenger.sendMessage(crossDomainTarget_, message_, gasLimit_); | ||
| } | ||
|
|
||
| /// @dev Enforces that the modified function is only callable by a specific cross-domain account | ||
| /// @param sourceDomainAccount_ The only account on the originating domain which is | ||
| /// authenticated to call this function | ||
| modifier onlyFromCrossDomainAccount(address sourceDomainAccount_) { | ||
| if (msg.sender != address(messenger)) { | ||
| revert ErrorUnauthorizedMessenger(); | ||
| } | ||
| if (messenger.xDomainMessageSender() != sourceDomainAccount_) { | ||
| revert ErrorWrongCrossDomainSender(); | ||
| } | ||
| _; | ||
| } | ||
|
|
||
| error ErrorUnauthorizedMessenger(); | ||
| error ErrorWrongCrossDomainSender(); | ||
| } |
Check failure
Code scanning / Slither
Name reused High
Comment on lines
+6
to
+18
| interface ICrossDomainMessenger { | ||
| function xDomainMessageSender() external view returns (address); | ||
|
|
||
| /// Sends a cross domain message to the target messenger. | ||
| /// @param _target Target contract address. | ||
| /// @param _message Message to send to the target. | ||
| /// @param _gasLimit Gas limit for the provided message. | ||
| function sendMessage( | ||
| address _target, | ||
| bytes calldata _message, | ||
| uint32 _gasLimit | ||
| ) external; | ||
| } |
Check failure
Code scanning / Slither
Name reused High
Comment on lines
+8
to
+84
| interface IL1ERC20Bridge { | ||
| event ERC20DepositInitiated( | ||
| address indexed _l1Token, | ||
| address indexed _l2Token, | ||
| address indexed _from, | ||
| address _to, | ||
| uint256 _amount, | ||
| bytes _data | ||
| ); | ||
|
|
||
| event ERC20WithdrawalFinalized( | ||
| address indexed _l1Token, | ||
| address indexed _l2Token, | ||
| address indexed _from, | ||
| address _to, | ||
| uint256 _amount, | ||
| bytes _data | ||
| ); | ||
|
|
||
| /// @notice get the address of the corresponding L2 bridge contract. | ||
| /// @return Address of the corresponding L2 bridge contract. | ||
| function l2TokenBridge() external returns (address); | ||
|
|
||
| /// @notice deposit an amount of the ERC20 to the caller's balance on L2. | ||
| /// @param l1Token_ Address of the L1 ERC20 we are depositing | ||
| /// @param l2Token_ Address of the L1 respective L2 ERC20 | ||
| /// @param amount_ Amount of the ERC20 to deposit | ||
| /// @param l2Gas_ Gas limit required to complete the deposit on L2. | ||
| /// @param data_ Optional data to forward to L2. This data is provided | ||
| /// solely as a convenience for external contracts. Aside from enforcing a maximum | ||
| /// length, these contracts provide no guarantees about its content. | ||
| function depositERC20( | ||
| address l1Token_, | ||
| address l2Token_, | ||
| uint256 amount_, | ||
| uint32 l2Gas_, | ||
| bytes calldata data_ | ||
| ) external; | ||
|
|
||
| /// @notice deposit an amount of ERC20 to a recipient's balance on L2. | ||
| /// @param l1Token_ Address of the L1 ERC20 we are depositing | ||
| /// @param l2Token_ Address of the L1 respective L2 ERC20 | ||
| /// @param to_ L2 address to credit the withdrawal to. | ||
| /// @param amount_ Amount of the ERC20 to deposit. | ||
| /// @param l2Gas_ Gas limit required to complete the deposit on L2. | ||
| /// @param data_ Optional data to forward to L2. This data is provided | ||
| /// solely as a convenience for external contracts. Aside from enforcing a maximum | ||
| /// length, these contracts provide no guarantees about its content. | ||
| function depositERC20To( | ||
| address l1Token_, | ||
| address l2Token_, | ||
| address to_, | ||
| uint256 amount_, | ||
| uint32 l2Gas_, | ||
| bytes calldata data_ | ||
| ) external; | ||
|
|
||
| /// @notice Complete a withdrawal from L2 to L1, and credit funds to the recipient's balance of the | ||
| /// L1 ERC20 token. | ||
| /// @dev This call will fail if the initialized withdrawal from L2 has not been finalized. | ||
| /// @param l1Token_ Address of L1 token to finalizeWithdrawal for. | ||
| /// @param l2Token_ Address of L2 token where withdrawal was initiated. | ||
| /// @param from_ L2 address initiating the transfer. | ||
| /// @param to_ L1 address to credit the withdrawal to. | ||
| /// @param amount_ Amount of the ERC20 to deposit. | ||
| /// @param data_ Data provided by the sender on L2. This data is provided | ||
| /// solely as a convenience for external contracts. Aside from enforcing a maximum | ||
| /// length, these contracts provide no guarantees about its content. | ||
| function finalizeERC20Withdrawal( | ||
| address l1Token_, | ||
| address l2Token_, | ||
| address from_, | ||
| address to_, | ||
| uint256 amount_, | ||
| bytes calldata data_ | ||
| ) external; | ||
| } |
Check failure
Code scanning / Slither
Name reused High
Comment on lines
+10
to
+90
| interface IL2ERC20Bridge { | ||
| event WithdrawalInitiated( | ||
| address indexed _l1Token, | ||
| address indexed _l2Token, | ||
| address indexed _from, | ||
| address _to, | ||
| uint256 _amount, | ||
| bytes _data | ||
| ); | ||
|
|
||
| event DepositFinalized( | ||
| address indexed _l1Token, | ||
| address indexed _l2Token, | ||
| address indexed _from, | ||
| address _to, | ||
| uint256 _amount, | ||
| bytes _data | ||
| ); | ||
|
|
||
| event DepositFailed( | ||
| address indexed _l1Token, | ||
| address indexed _l2Token, | ||
| address indexed _from, | ||
| address _to, | ||
| uint256 _amount, | ||
| bytes _data | ||
| ); | ||
|
|
||
| /// @notice Returns the address of the corresponding L1 bridge contract | ||
| function l1TokenBridge() external returns (address); | ||
|
|
||
| /// @notice Initiates a withdraw of some tokens to the caller's account on L1 | ||
| /// @param l2Token_ Address of L2 token where withdrawal was initiated. | ||
| /// @param amount_ Amount of the token to withdraw. | ||
| /// @param l1Gas_ Unused, but included for potential forward compatibility considerations. | ||
| /// @param data_ Optional data to forward to L1. This data is provided | ||
| /// solely as a convenience for external contracts. Aside from enforcing a maximum | ||
| /// length, these contracts provide no guarantees about its content. | ||
| function withdraw( | ||
| address l2Token_, | ||
| uint256 amount_, | ||
| uint32 l1Gas_, | ||
| bytes calldata data_ | ||
| ) external; | ||
|
|
||
| /// @notice Initiates a withdraw of some token to a recipient's account on L1. | ||
| /// @param l2Token_ Address of L2 token where withdrawal is initiated. | ||
| /// @param to_ L1 adress to credit the withdrawal to. | ||
| /// @param amount_ Amount of the token to withdraw. | ||
| /// @param l1Gas_ Unused, but included for potential forward compatibility considerations. | ||
| /// @param data_ Optional data to forward to L1. This data is provided | ||
| /// solely as a convenience for external contracts. Aside from enforcing a maximum | ||
| /// length, these contracts provide no guarantees about its content. | ||
| function withdrawTo( | ||
| address l2Token_, | ||
| address to_, | ||
| uint256 amount_, | ||
| uint32 l1Gas_, | ||
| bytes calldata data_ | ||
| ) external; | ||
|
|
||
| /// @notice Completes a deposit from L1 to L2, and credits funds to the recipient's balance of | ||
| /// this L2 token. This call will fail if it did not originate from a corresponding deposit | ||
| /// in L1StandardTokenBridge. | ||
| /// @param l1Token_ Address for the l1 token this is called with | ||
| /// @param l2Token_ Address for the l2 token this is called with | ||
| /// @param from_ Account to pull the deposit from on L2. | ||
| /// @param to_ Address to receive the withdrawal at | ||
| /// @param amount_ Amount of the token to withdraw | ||
| /// @param data_ Data provider by the sender on L1. This data is provided | ||
| /// solely as a convenience for external contracts. Aside from enforcing a maximum | ||
| /// length, these contracts provide no guarantees about its content. | ||
| function finalizeDeposit( | ||
| address l1Token_, | ||
| address l2Token_, | ||
| address from_, | ||
| address to_, | ||
| uint256 amount_, | ||
| bytes calldata data_ | ||
| ) external; | ||
| } |
Check failure
Code scanning / Slither
Name reused High
Comment on lines
45
to
60
| function permit(address owner, address spender, uint256 amount, uint256 deadline, uint8 v, bytes32 r, bytes32 s) | ||
| public | ||
| virtual | ||
| override | ||
| { | ||
| require(deadline >= block.timestamp, "ERC20Permit: expired deadline"); | ||
|
|
||
| bytes32 hashStruct = keccak256(abi.encode(PERMIT_TYPEHASH, owner, spender, amount, nonces[owner]++, deadline)); | ||
|
|
||
| bytes32 hash = keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), hashStruct)); | ||
|
|
||
| address signer = ecrecover(hash, v, r, s); | ||
| require(signer != address(0) && signer == owner, "ERC20Permit: invalid signature"); | ||
|
|
||
| _approve(owner, spender, amount); | ||
| } |
Check warning
Code scanning / Slither
Public function that could be declared external Warning
TheDZhon
commented
Sep 13, 2023
| L1_BRIDGE_ADMIN=0x3e40D73EB977Dc6a537aF587D48316feE66E9C8c | ||
| L1_DEPOSITS_ENABLED=false | ||
| L1_WITHDRAWALS_ENABLED=true | ||
| L1_DEPOSITS_ENABLERS=["0x3e40D73EB977Dc6a537aF587D48316feE66E9C8c","0x3cd9F71F80AB08ea5a7Dca348B5e94BC595f26A0"] |
There was a problem hiding this comment.
Should remove the dev msig
| L1_WITHDRAWALS_ENABLERS=["0x3e40D73EB977Dc6a537aF587D48316feE66E9C8c"] | ||
| L1_WITHDRAWALS_DISABLERS=["0x3e40D73EB977Dc6a537aF587D48316feE66E9C8c","0x73b047fe6337183A454c5217241D780a932777bD"] | ||
|
|
||
| L2_PROXY_ADMIN=0xEfa0dB536d2c8089685630fafe88CF7805966FC3 |
There was a problem hiding this comment.
No Safe on Mantle?
|
|
||
| bytes32 hash = keccak256(abi.encodePacked("\x19\x01", _domainSeparatorV4(), hashStruct)); | ||
|
|
||
| address signer = ecrecover(hash, v, r, s); |
There was a problem hiding this comment.
doesn't support ERC-1271 though
[ERC20BridgedPermit] Suggest adding a helper function to increase nonce in ERC20BridgedPermit.sol
[ERC20BridgedPermit] Domain separator can be cached instead of being constructed each time a permit is executed
[L2ERC20TokenBridge.sol] Withdrawal functions in L2ERC20TokenBridge contract do not verify if the sender is an EOA
update ERC20BridgedPermit deployment config
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.