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

Refactor XCM Simulator Example #4220

Merged

Conversation

shawntabrizi
Copy link
Member

This PR does a "developer experience" refactor of the XCM Simulator Example.

I was looking for existing code / documentation where developers could better learn about working with and configuring XCM.

The XCM Simulator was a natural starting point due to the fact that it can emulate end to end XCM scenarios, without needing to spawn multiple real chains.

However, the XCM Simulator Example was just 3 giant files with a ton of configurations, runtime, pallets, and tests mashed together.

This PR breaks down the XCM Simulator Example in a way that I believe is more approachable by a new developer who is looking to navigate the various components of the end to end example, and modify it themselves.

The basic structure is:

  • xcm simulator example
    • lib (tries to only use the xcm simulator macros)
    • tests
    • relay-chain
      • mod (basic runtime that developers should be familiar with)
      • xcm-config
        • mod (contains the XcmConfig type
        • various files for each custom configuration
    • parachain
      • mock_msg_queue (custom pallet for simulator example)
      • mod (basic runtime that developers should be familiar with)
      • xcm-config
        • mod (contains the XcmConfig type
        • various files for each custom configuration

I would like to add more documentation to this too, but I think this is a first step to be accepted which will affect how documentation is added to the example

@shawntabrizi shawntabrizi requested review from a team as code owners April 19, 2024 18:09
@shawntabrizi
Copy link
Member Author

@shawntabrizi
Copy link
Member Author

In general, I am not super confident about a file named locations and limits, but was the best I could think of.

Copy link
Contributor

@franciscoaguirre franciscoaguirre left a comment

Choose a reason for hiding this comment

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

The example serves its purpose better with this, to be a basic example for XCM but mostly of how to use the simulator.

@shawntabrizi
Copy link
Member Author

shawntabrizi commented Apr 19, 2024

In 8518b05, I removed:

pub type SovereignAccountOf = (
	SiblingParachainConvertsVia<Sibling, AccountId>,
	AccountId32Aliases<RelayNetwork, AccountId>,
	ParentIsPreset<AccountId>,
);

As it seemed to be a duplicate definition of LocationConverter, but missing Account32Hash, which it probably should have.

Instances where SovereignAccountOf was used, i put LocationConverter instead.

In general, my feedback is also that SovereignAccountOf is a bad name for the config on the XCM Pallet, and should be something more like SovereignAccountIdConverter

@franciscoaguirre franciscoaguirre added R0-silent Changes should not be mentioned in any release notes T6-XCM This PR/Issue is related to XCM. labels Apr 22, 2024
@shawntabrizi
Copy link
Member Author

@franciscoaguirre ping to help me get this in. thank you


pub type TrustedLockers = TrustedLockerCase<RelayTokenForRelay>;

impl pallet_xcm::Config for Runtime {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would give you a dinner and a hug in Singapore if as a part of the XCM tutorials, we can have an simple version of TestDefaultConfig for this as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Although, #1959

Copy link
Member Author

Choose a reason for hiding this comment

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

i will take a look for sure

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Looks good, I think adding more inline-documentation + integration with #2633 would be great.

In general, I think if you and @franciscoaguirre can make sure the XCM @ PBA5 is reflected in #2633, it would be a great outcome.

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

very nice!

Comment on lines +29 to +30
// Generated from `decl_test_network!`
pub type XcmRouter = EnsureDecodableXcm<crate::RelayChainXcmRouter>;
Copy link
Contributor

Choose a reason for hiding this comment

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

very nit, what about extracting XcmRouter to the standalone routing.rs? E.g. barrier is also one-liner.

I think, also it makes sense putting a barrier and routing stuff together. The barrier validates what was routed/sent by some SendXcm impl, e.g. good example is WithUniqueTopic vs TrailingSetTopicAsId

Copy link
Member Author

@shawntabrizi shawntabrizi Apr 29, 2024

Choose a reason for hiding this comment

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

Thanks for the suggestion! I am totally open to the idea, but let me provide some larger context.

I would like to use this refactor for part of a tutorial which teaches readers how to configure XCM for various situations.

Barrier I expect will be a spot which could be configured in multiple ways, which is why it is extracted into one file even if it is a one liner. That file will be used to explore the various possibilities for barrier.

I don't know much about XCM router. Are there other sensible configurations for it? Or usually it is just plugged into the same pallet?

Copy link
Contributor

Choose a reason for hiding this comment

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

The router is usually one of the available transport options, not much configuration there.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the suggestion! I am totally open to the idea, but let me provide some larger context.

Barrier I expect will be a spot which could be configured in multiple ways, which is why it is extracted into one file even if it is a one liner. That file will be used to explore the various possibilities for barrier.

Ok, thank you for a context, so I agree mixing a barrier and router is not a good idea for this kind of tutorial.

I would like to use this refactor for part of a tutorial which teaches readers how to configure XCM for various situations.

I don't know much about XCM router. Are there other sensible configurations for it? Or usually it is just plugged into the same pallet?

E.g. we have a quite complex XcmRouter for AssetHubKusama: https://github.com/polkadot-fellows/runtimes/blob/main/system-parachains/asset-hubs/asset-hub-kusama/src/xcm_config.rs#L619-L643, simplified:

pub XcmRouter = (
    ParentAsUmp,
    XcmpQueue,
    ToPolkadotXcmRouter,
    ToEthereumXcmRouter
);

I can imagine a standalone chapter for XCM routing explaining Location, SendXcm, ExportMessage or creating custom implementation/feature like WithUniqueTopic ... so yes, for that chapter, I would extract routers to standalone files, e.g.:

example/src/parachain/xcm_config/routing.rs:

pub type XcmRouter = EnsureDecodableXcm<
       // XCMP - `(1, [$crate::Parachain(id)])` part of ParachainXcmRouter
       crate::ToSiblingParachainXcmRouter<MsgQueue>,   
       // UMP - `(1, [])` part of ParachainXcmRouter  
       crate::ToRelayChainXcmRouter<MsgQueue>, 
>;

example/src/relay_chain/xcm_config/routing.rs:

pub type XcmRouter = EnsureDecodableXcm<
       // DMP - `(0, [$crate::Parachain(id)])` part of RelayChainXcmRouter
       crate::ToChildParachainXcmRouter<MsgQueue>,
>;

Anyway, what tutorial do you mean exactly? I remember I saw your twitter post DotCodeSchool some time ago?

Copy link
Member Author

Choose a reason for hiding this comment

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

@bkontur tutorial will live here, but still in progress: https://github.com/shawntabrizi/polkadot-sdk-workshop-xcm/tree/master

will be ready by the next academy.

I will spend some time to look more into XCM router config, and if it makes sense, i will push more updates to this example in Polkadot, and continue to use it as the base template for the tutorial.

@kianenigma kianenigma added this pull request to the merge queue Apr 29, 2024
Merged via the queue into paritytech:master with commit 4875ea1 Apr 29, 2024
142 of 144 checks passed
@shawntabrizi shawntabrizi deleted the shawntabrizi-xcm-simulator-refactor branch April 30, 2024 17:27
Morganamilo pushed a commit that referenced this pull request May 2, 2024
This PR does a "developer experience" refactor of the XCM Simulator
Example.

I was looking for existing code / documentation where developers could
better learn about working with and configuring XCM.

The XCM Simulator was a natural starting point due to the fact that it
can emulate end to end XCM scenarios, without needing to spawn multiple
real chains.

However, the XCM Simulator Example was just 3 giant files with a ton of
configurations, runtime, pallets, and tests mashed together.

This PR breaks down the XCM Simulator Example in a way that I believe is
more approachable by a new developer who is looking to navigate the
various components of the end to end example, and modify it themselves.

The basic structure is:

- xcm simulator example
    - lib (tries to only use the xcm simulator macros)
    - tests
    - relay-chain
        - mod (basic runtime that developers should be familiar with)
        - xcm-config
            - mod (contains the `XcmConfig` type
            - various files for each custom configuration  
    - parachain
        - mock_msg_queue (custom pallet for simulator example)
        - mod (basic runtime that developers should be familiar with)
        - xcm-config
            - mod (contains the `XcmConfig` type
            - various files for each custom configuration

I would like to add more documentation to this too, but I think this is
a first step to be accepted which will affect how documentation is added
to the example

---------

Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
TarekkMA pushed a commit to moonbeam-foundation/polkadot-sdk that referenced this pull request Aug 2, 2024
This PR does a "developer experience" refactor of the XCM Simulator
Example.

I was looking for existing code / documentation where developers could
better learn about working with and configuring XCM.

The XCM Simulator was a natural starting point due to the fact that it
can emulate end to end XCM scenarios, without needing to spawn multiple
real chains.

However, the XCM Simulator Example was just 3 giant files with a ton of
configurations, runtime, pallets, and tests mashed together.

This PR breaks down the XCM Simulator Example in a way that I believe is
more approachable by a new developer who is looking to navigate the
various components of the end to end example, and modify it themselves.

The basic structure is:

- xcm simulator example
    - lib (tries to only use the xcm simulator macros)
    - tests
    - relay-chain
        - mod (basic runtime that developers should be familiar with)
        - xcm-config
            - mod (contains the `XcmConfig` type
            - various files for each custom configuration  
    - parachain
        - mock_msg_queue (custom pallet for simulator example)
        - mod (basic runtime that developers should be familiar with)
        - xcm-config
            - mod (contains the `XcmConfig` type
            - various files for each custom configuration

I would like to add more documentation to this too, but I think this is
a first step to be accepted which will affect how documentation is added
to the example

---------

Co-authored-by: Francisco Aguirre <[email protected]>
Co-authored-by: Kian Paimani <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
R0-silent Changes should not be mentioned in any release notes T6-XCM This PR/Issue is related to XCM.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants