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

Add experimental GMP Precompile [Moonbase only] #2155

Merged
merged 68 commits into from Apr 12, 2023
Merged

Conversation

notlesh
Copy link
Contributor

@notlesh notlesh commented Mar 9, 2023

Overiew

This PR adds a precompile designed to work with GMP ("General Message Passing") protocols such that a user can perform a single interaction with such a protocol in order to bridge tokens across Moonbeam to other parachains via XCM.


                                                          +------------+
                                                          |  Polkadot  |
                                                          |            |
                                                          |            |
                                                          |            |
                                                          +------------+
                                                                ^
                                                                |
   +------------+                        +------------+         |         +------------+
   |  Non-Polka |                        |  Moonbeam  |                   | Other Para |
   |   chain    | ---> GMP protocol ---> |            | -----> XCM -----> |            |
   |            |                        | * GMP      |                   |            |
   |            |                        |  precompile|                   |            |
   +------------+                        +------------+                   +------------+

In the above diagram, a user on a non-Polkadot related chain could use an existing GMP protocol to interact with any XCM-connected chain of Moonbeam with a single interaction. They do so by:

  1. Specifying Moonbeam as the target chain and the GMP precompile as the target account
  2. Providing a payload which tells the GMP precompile what the next destination is

The result is that the existing GMP protocol will deliver tokens/messages to the GMP precompile which will forward them via XCM to some other para/relay chain.

Initial work

This first push provides a sketch of how things might work with Wormhole's "Contract Controlled Transfers," and is meant for early testing on moonbase.

Specifically, it provides a single function which can be called to complete an inbound Wormhole bridge transfer. There are a lot of parameters which govern how a Wormhole bridge transfer can occur, and this aims to support only one of them. This case is handled in the Test local Wormhole ts test written by @crystalin (thanks!)

Current restrictions

The current release really only supports a "happy path."

  • transferred token must be a foreign chain (its token_chain must not be "our" chain)
  • a Wormhole-wrapped version of the transferred token will be used (both on Moonbeam and XCM destination chains)
    • this wrapper ERC20 will need to be registered with the bridge contract
    • the destination XCM chain will need to accept this token as well
  • any errors result in a revert, which leaves tokens trapped
  • there is no fee mechanism supported; no one is compensated for EVM or XCM fees, nor are they incentivized to relay any messages between networks

Future Work

Some of the larger features planned for the future:

  • Fee mechanism (to compensate for EVM/XCM fees and to incentivize relayers)
  • XCM calls (as opposed to simple token transfers)
  • Improved events and error handling
  • Multiple protocols

TODO

  • More tests
  • Ensure xtokens events are emitted (or some other XCM-send related events)
  • Gas accounting
  • Support configuration of contract addresses
  • Code clean up

Copy link
Collaborator

@librelois librelois left a comment

Choose a reason for hiding this comment

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

I made a first pass on the rust part only, I don't see any security issue, but the current design seems to me very inefficient.

I suggest to replace xtokens by sending directly an XCM message with the following content:

Xcm(vec![
  ReserveAssetDeposited(MultiAssets::from(vec![asset, feeAsset])),
  ClearOrigin,
  BuyExecution {
    fees: feeAsset,
    weight_limit: WeightLimit::Unlimited
  },
  DepositAssets {
    assets: AllCounted(2),
    beneficiary: recipient,
  }
])

// This design should allow users to interact with this precompile with no changes to the
// underlying GMP protocols by simply specifying the correct precompile as the target.

/// Receive a wormhole VAA and process it
Copy link
Collaborator

Choose a reason for hiding this comment

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

Even if the VAA is generic, we expect it to have some shape related to xtokens. Maybe we can specify it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what you mean. Are you referring to the way a user specifies what XCM interaction (e.g. target MultiLocation for xtokens)? That would be defined in types.rs with VersionedUserAction, which does need some better documentation at some point...

@girazoki
Copy link
Collaborator

girazoki commented Apr 12, 2023

@librelois the approach of sending the message would only work with local tokens (ERC20s or local assets or the native token), at least with the message you proposed. I am not sure if xc20s is a use case we want to support here.

If we are clear on the fact that we are only sending local tokens, then the approach suggested by @librelois would be more efficient. We just transfer the ERC20 directly to the sovereign account, then send the aforementioned message. Note this is a bit riskier, as you bypass the limits imposed by the xcm-executor when sending messages. I would like to have a very good reason to do this, e.g., the efficiency gain being significant.

That being said, I dont think this change implies any change in the interface, so we could very well merge the PR as is as a first iteration

@librelois
Copy link
Collaborator

@girazoki from my understanding, the current GMP implemenation already deal only with local ERC20 assets, more precisely wrapped assets created by wormhole. And I think that all future GMP protocols will only deal with local ERC20.

@notlesh
Copy link
Contributor Author

notlesh commented Apr 12, 2023

@girazoki from my understanding, the current GMP implemenation already deal only with local ERC20 assets, more precisely wrapped assets created by wormhole. And I think that all future GMP protocols will only deal with local ERC20.

I think this is a good hunch, but I'd rather not limit us to that just yet. It's also possible that we might want to automatically unwrap certain tokens before sending them (much better for fungibility than having yet-another-wrapped-token).

Also, there are other cases (such as transferring GLMR, not supported in this PR) that wouldn't involve a strict ERC20 impl.

That being said, I dont think this change implies any change in the interface, so we could very well merge the PR as is as a first iteration

Agreed -- this is something we can follow up on. I think we need a better understanding of the use cases we want to support as well as the other protocols before doing so.

@notlesh notlesh merged commit 47ccb14 into master Apr 12, 2023
18 checks passed
@notlesh notlesh deleted the notlesh-gmp-precompile branch April 12, 2023 18:45
@crystalin crystalin changed the title GMP Precompiles Add experimental GMP Precompile [Moonbase only] Apr 13, 2023
imstar15 pushed a commit to OAK-Foundation/moonbeam that referenced this pull request May 16, 2023
* Initial sketch with Wormhole

* Stub out design more

* Add gmp precompile to moonbase

* Clean up

* Traits, types

* Make it compile for now

* Fill out contract interaction

* Adds basic test

* Precompile added to runtime, tests

* Rust tests/mock

* decompose VAA

* Use EvmDataWriter, allow one recursion

* Clean up

* Stub out Wormhole VM pre-parse

* Derive EvmData to handle ABI Encoding

* Add notes, add payload fields

* Update Cargo.lock

* Make compiler happy

* Adds local wormhole contracts (moonbeam-foundation#2170)

* Adds wormhole test

* Adds wormhole contracts

* fixes format

* fixes precompile solidity path

* Adds missing test for tracing

* fixes format

* Adds missing xcm

* removes it.only

* and more it only

* Remove unsused test

* Fix new gas

* Adds revert string to contract compilation for tests

* Trim wormhole solidity files

* Modify test to use precompile

* Include payload with VAA

* Use hex for byte blob

* Parse bridge payload

* Clean up ExitReason repetitiveness

* Fill out XCM transfer logic

* Add orml-xtokens dependency

* Add xtokens to mock, clean up

* Update test payload

* Reduce XcmRoutingUserAction to bare minimum

* Add payable fn, clean up chain ids

* Change wormhole bridge impl contract, chain ids

* Provide revert message when underflowing in Wormhole bridge contract

* Don't use tokenChain as emitterChain

* Some DRY for contract calling

* Query for wrapped asset

* Check ERC20 balance before and after to verify bridged assets

* Use foreign chain id, send to relay chain

* Make VersionedUserAction non_exhaustive

* Expect xTokens event

* Restrict subcall depth to 0 and not callable by contract

* Define storage instances for contract addresses

* Call set_storage in test to avoid hard coding contract addresses

* Clean up useless tests

* fmt

* prettier

* Account for extra gas cost

* Bump VersionedMultiLocation to v3

* Resolve compiler warnings

* Clean up

* Avoid long lines

* Missed merge conflict block

* Mock fixes (for 0.9.38 deps update)

* Remove dead code

* toml sort

* clippy

* Use lesser of transferred_amount, msg.amount

* Remove stale comment

* Use Unlimited weight in XCM message

* Rely on EVM's internal fallback to remaining gas

* fix formatting

* Updates contracts

* Migrate new test to compiled new format

* Removes gmp precompile test in favor of wormhole test

* Make OptionQuery explicit

---------

Co-authored-by: crystalin <alan.sapede@gmail.com>
Co-authored-by: Alan Sapede <alan@purestake.com>
@notlesh notlesh added D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited and removed D9-needsaudit👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels May 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A0-pleasereview Pull request needs code review. B7-runtimenoteworthy Changes should be noted in any runtime-upgrade release notes D1-audited👍 PR contains changes to fund-managing logic that has been properly reviewed and externally audited not-breaking Does not need to be mentioned in breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants