feat(protocol): add solver support for l2 to l1 eth bridging #18805
Conversation
feat(protocol): add solver support for l2 to l1 eth bridging
🚨 Report Summary
For more details view the full report in OpenZeppelin Code Inspector |
|
@AnshuJalan I may need a few days on something else if that's OK. |
Sure |
| contract EtherBridgeWrapper is EssentialContract { | ||
| using Address for address; | ||
| using LibAddress for address; | ||
|
|
There was a problem hiding this comment.
Lets reserve a gap of 50 slots here, just in case.
| import "src/shared/common/EssentialContract.sol"; | ||
| import "@openzeppelin/contracts/utils/Address.sol"; | ||
|
|
||
| contract EtherBridgeWrapper is EssentialContract { |
There was a problem hiding this comment.
Add Natspac comments please
| } | ||
|
|
||
| /// @dev Represents an operation to solve an Ether bridging intent | ||
| struct SolverOp { |
There was a problem hiding this comment.
Is it possible to reuse the SolverOp defines in ERC20Vault (maybe extract to another file) and use 0x0 for EtherBridgeOp to indicate the asset is Ether.
Same recommendation for EtherBridgeOp.
| /// @notice Sends Ether to another chain. | ||
| /// @param _op Options for sending Ether. | ||
| /// @return message_ The constructed message. | ||
| function sendToken(EtherBridgeOp calldata _op) |
| import "src/shared/common/EssentialContract.sol"; | ||
| import "@openzeppelin/contracts/utils/Address.sol"; | ||
|
|
||
| contract EtherBridgeWrapper is EssentialContract { |
There was a problem hiding this comment.
How about we call it EtherVault? If this contract can be merged into ERC20Vault and we consider sending Ether is just another special instance of sending ERC20 tokens with a special token address 0x0, then I'd strongly prefer that implementation.
This is an extension of #18616 for L2 -> L1 FnS ETH bridging.
Bridge.sol. Thus, the Bridge contract itself acts as the “ether vault”.Bridge.solto incorporate a solver condition, but that would fundamentally change the core of the message passing system. Therefore, a better alternative implemented in this PR is a separateEtherBridgeWrapper.solthat handles just the “Eth in” and “Eth out” part of bridging by including a solver condition, leaving the existing functionality ofBridge.soland surrounding infra untouched.Bridge.solcan still be used for standard slow withdrawals.