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

feat: generic L2 forwarder base contract #609

Merged
merged 32 commits into from
Oct 2, 2024
Merged

Conversation

bmzig
Copy link
Contributor

@bmzig bmzig commented Sep 17, 2024

This PR adds the abstract contract ForwarderBase which is meant to be a base for forwarders. Forwarders should relay messages/tokens from an L2 to an L3. A forwarder which inherits ForwarderBase will have to be constructed for each chain to account for an L2's message validation logic, but since ForwarderBase makes use of L2 adapters, it shouldn't matter what the architecture of the L3 is. To be clear, only _requireAdminSender, an internal function which ensures that the caller is the aliased hub pool, must be implemented for each new forwarder.

Copy link
Contributor

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! A few comments

@bmzig bmzig marked this pull request as ready for review September 20, 2024 16:57
@nicholaspai nicholaspai changed the title feat: add a generic L2 forwarder interface feat: add a generic L2 forwarder base contract Sep 24, 2024
contracts/chain-adapters/ForwarderBase.sol Outdated Show resolved Hide resolved
contracts/chain-adapters/Lisk_Adapter.sol Outdated Show resolved Hide resolved
contracts/interfaces/ArbitrumBridgeInterfaces.sol Outdated Show resolved Hide resolved
Signed-off-by: bennett <[email protected]>
@nicholaspai nicholaspai changed the title feat: add a generic L2 forwarder base contract feat: generic L2 forwarder base contract Sep 24, 2024
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments on naming. You shouldn't take naming lightly, it's the difference between code that is easy to review and code that's not, and that's a major difference in code quality.

Signed-off-by: bennett <[email protected]>
@pxrl
Copy link
Contributor

pxrl commented Oct 1, 2024

This PR adds the contract ArbitrumForwarderInterface which is used to send messages/tokens from an arbitrary L2 to an Arbitrum-like L3. The functions which will need to be implemented for the forwarder contracts will be:

  • relayTokens: an external function callable by anybody which bridges tokens to the L3 spoke pool.
  • _relayMessage: an internal function which sends a message to a specified target on L3.
  • _requireAdminSender: an internal function which ensures that the caller is the aliased hub pool.

In general, relayTokens and _relayMessage will be taken from the current Arbitrum_*_Adapter code. Under the assumption that the L2 does aliasing differently than adding 0x11...000...11 to the msg.sender, _requireAdminSender will have to be modified accordingly.

@bmzig This description probably needs an update to reflect the changes that were made during review.

contracts/chain-adapters/ForwarderBase.sol Outdated Show resolved Hide resolved
Signed-off-by: bennett <[email protected]>
Signed-off-by: bennett <[email protected]>
Copy link
Contributor

@pxrl pxrl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK from me!

@bmzig bmzig merged commit 1248f8e into master Oct 2, 2024
9 checks passed
@bmzig bmzig deleted the bz/l2ForwarderInterface branch October 2, 2024 13:24
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.

4 participants