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

[SMR-1727] Map token L1 #103

Closed
wants to merge 27 commits into from
Closed

[SMR-1727] Map token L1 #103

wants to merge 27 commits into from

Conversation

Benjimmutable
Copy link
Contributor

@Benjimmutable Benjimmutable commented Sep 28, 2023

This is the first PR for bridge smart contracts that interface with the Axelar Network.

Motivation

Before a token gets bridged, we want to establish a mapping of L1 Token Address => L2 Token Address on both Root Chain and Child Chain. To do this, we want to send a Map Token message.

  • This pattern is taken from the Edge bridge contracts.
  • This pattern is not necessary. You could likely still get away with using the Clones.predictDeterministicAddress function on every bridge operation, but then complexities are introduced in regards to determining if you need to deploy an ERC20 token to that address (i.e. on the first bridge), if the correct SC is deployed there, etc.
    • Therefore, I believe it to be better to just establish a clear mapping/deploy first and enjoy the guarantees that come with that.

Changes

This PR is only for the L1 message sending. L2 message receiving will be in a follow up PR.

The rough architecture currently will be as follows:
image

Justification for Adaptor

The underlying communication protocol may change. Using the adaptor pattern is a modular way that allows us to replace the underlying message passing logic without having to upgrade the bridge contract directly. It also allows us to decouple vendor-specific messaging logic from the bridge logic.

The Adaptor is not made to be upgradeable. This is because the only state it holds is config details (smart contract addresses), it only acts as a middleman message passer, and it is trivial to redeploy a new one and set the adaptor to the new contract.

Feel free to challenge this.

Also worth mentioning that any logic + data checks that are agnostic to the underlying message passing protocol should be done in the RootERC20Bridge contract, and anything specific to the Axelar message passing protocol should be in the Axelar adaptor.

Prior Art

I copied and took inspiration as much as possible from the Edge Root Bridge contract. The child chain message receiving will be inspired/copied from here.

Pausability

Pausing or anything similar is not introduced in this PR. Given that this is just a PoC I did not spend too much time thinking about this, but it will definitely be something that is scrutinised heavily, and is important to consider, in the near future.

Owner role

There is currently an Ownable2Step role. Again, given that this is just a PoC I did not spend too much time thinking about RBAC but ownership of this contract will definitely be something that is scrutinised heavily in the near future.

@Benjimmutable Benjimmutable changed the title Map token l1 [SMR-1727] Map token L1 Sep 28, 2023
@Benjimmutable Benjimmutable marked this pull request as ready for review September 28, 2023 02:30
@Benjimmutable Benjimmutable requested a review from a team as a code owner September 28, 2023 02:30
@drinkcoffee
Copy link
Contributor

Sentence in description seemed to stop part way through: "This is because the only state it holds is"

@Benjimmutable Benjimmutable marked this pull request as draft September 28, 2023 22:03
@Benjimmutable Benjimmutable changed the title [SMR-1727] Map token L1 [SMR-1727] Map token L1 (WIP) Sep 28, 2023
@Benjimmutable Benjimmutable self-assigned this Sep 29, 2023
@Benjimmutable Benjimmutable marked this pull request as ready for review September 29, 2023 04:19
@Benjimmutable Benjimmutable changed the title [SMR-1727] Map token L1 (WIP) [SMR-1727] Map token L1 Sep 29, 2023
* Initial L2-side commit

* Almost finished adding child erc20 bridge

* Add new child bridge contracts

* Child ERC20Bridge and unit tests

* Add some todos

* Refactor into child and root directories

* Child ERC20 token mapping

* Cleanup
@drinkcoffee
Copy link
Contributor

@Benjimmutable , this PR is stale. Are you still actively working on this? Should this PR be abandoned?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants