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

XCMP #263

Closed
wants to merge 59 commits into from
Closed

XCMP #263

wants to merge 59 commits into from

Conversation

4meta5
Copy link
Contributor

@4meta5 4meta5 commented Feb 24, 2021

What does it do?

Adds XCMP transfer logic. There are three new pallets:

  1. hrmp-channels is for proposing, accepting, and closing HRMP channels
  2. xtransfer is for cross-chain transfers (tightly coupled with channels)
  3. token-factory is a wrapper around the EVM for ERC20 minting/burning

The XCMP handler logic is in xtransfer/src/support.rs in the MultiCurrency implementation of TransactAsset. This is used by the LocalAssetTransactor associated type in the runtime implementation of xcm_executor::config.

This PR also removes:

  • parachain-info (to use polkadot version directly instead)
  • token-dealer (replaced by pallets listed above)

What important points reviewers should know?

TODO pallet outlines

Is there something left for follow-up PRs?

What alternative implementations were considered?

Are there relevant PRs or issues in other repositories (Substrate, Polkadot, Frontier, Cumulus)?

paritytech/polkadot#2661

What value does it bring to the blockchain users?

Checklist

  • Does it require a purge of the network?
  • You bumped the runtime version if there are breaking changes in the runtime ?
  • Does it require changes in documentation/tutorials ?

Copy link
Contributor

@JoshOrndorff JoshOrndorff left a comment

Choose a reason for hiding this comment

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

I know it says not ready for review, I was just too excited.

pallets/token-factory/src/lib.rs Outdated Show resolved Hide resolved
/// ERC token identifier
type TokenId: Clone + Copy + AtLeast32BitUnsigned + FullCodec + Debug;
/// Convert from AccountId to H160, should be identity map for Moonbeam
type AccountToH160: Convert<Self::AccountId, H160>;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you couple to pallet evm, you can use the same mapping that is in its config trait without needing this associated type.

Copy link
Contributor Author

@4meta5 4meta5 Feb 24, 2021

Choose a reason for hiding this comment

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

I found this associated type in pallet_evm

/// Mapping from address to account id.
type AddressMapping: AddressMapping<Self::AccountId>;

but I did not find account id to address, which I think I still need. The direction matters because I need to convert from T::AccountId to H160 (and the pallet doesn't have the T::AccountId assignment at compile time)

pallets/token-factory/src/lib.rs Outdated Show resolved Hide resolved
pallets/x-transfer/src/lib.rs Outdated Show resolved Hide resolved
@4meta5 4meta5 marked this pull request as ready for review March 2, 2021 20:24
Copy link
Contributor

@JoshOrndorff JoshOrndorff left a comment

Choose a reason for hiding this comment

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

Just took another quick look. I have some high level questions:

  1. What is the key difference between token dealer and token factory? Is there a clear separation of concerns between the two?

  2. When a user decides to send a foreign asset (eg ACA) to Moonbeam, what kind of destination address do they specify? I think it should be an H160 account, right?

pallets/token-dealer/src/support.rs Outdated Show resolved Hide resolved
pallets/token-dealer/src/lib.rs Outdated Show resolved Hide resolved
pallets/token-factory/contract/contract.sol Outdated Show resolved Hide resolved
pallets/token-factory/contract/contract.sol Show resolved Hide resolved
…version and improve tests and make nonce global for pallet instead of contract specific
@4meta5 4meta5 changed the title Token Factory, DMP XCMP Mar 3, 2021
@4meta5
Copy link
Contributor Author

4meta5 commented Mar 15, 2021

TODO:

  • polkadot-launch tests

@4meta5
Copy link
Contributor Author

4meta5 commented Mar 19, 2021

This PR adds 3 runtime methods in xtransfer for opening, accepting, and closing channels on the relay chain. These channels are what enable HRMP (between parachains).

Each of these methods require the call data as an input for Xcm::Transact because I don't know how to form the call data in the runtime. This Acala demo uses polkadot-js to form the call from the relay chain context, remove the prefix, and then the raw call is used. As of now, we do this and pass the raw call data to our runtime methods, but this is hacky.

There is this PR which includes three new message variants for Xcm to open, accept, and close channels. If these are added, we don't have to use Xcm::Transact, but I have not yet found the polkadot PR to add the messages.

pallets/channels/src/set.rs Outdated Show resolved Hide resolved
joelamouche and others added 6 commits March 24, 2021 11:31
* fix types

* add alice
* fix types

* add alice

* fix call

Co-authored-by: Amar Singh <[email protected]>
* progress

* prog

* it
@4meta5
Copy link
Contributor Author

4meta5 commented Mar 25, 2021

Current error:

2021-03-25 09:07:00        RPC-CORE: getBlock(hash?: BlockHash): SignedBlock:: createType(SignedBlock):: Struct: failed on block: {"header":"Header","extrinsics":"Vec<Extrinsic>"}:: Struct: failed on extrinsics: Vec<Extrinsic>:: createType(ExtrinsicV4):: createType(Call):: Call: failed decoding parachainSystem.setValidationData:: Struct: failed on args: {"data":"SystemInherentData"}:: Number can only safely store up to 53 bits
2021-03-25 09:07:00             DRR: Error: createType(SignedBlock):: Struct: failed on block: {"header":"Header","extrinsics":"Vec<Extrinsic>"}:: Struct: failed on extrinsics: Vec<Extrinsic>:: createType(ExtrinsicV4):: createType(Call):: Call: failed decoding parachainSystem.setValidationData:: Struct: failed on args: {"data":"SystemInherentData"}:: Number can only safely store up to 53 bits)

@4meta5
Copy link
Contributor Author

4meta5 commented Mar 28, 2021

I'll open smaller ones to get this in piece by piece :'(

@4meta5 4meta5 closed this Mar 28, 2021
This was referenced Mar 28, 2021
@4meta5 4meta5 deleted the amar-xc branch February 22, 2022 13:17
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