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

Interface to expose Call enum structure from construct_runtime for XCM Transact callers #813

Open
KiChjang opened this issue Jun 3, 2022 · 17 comments
Labels
T6-XCM This PR/Issue is related to XCM.

Comments

@KiChjang
Copy link
Contributor

KiChjang commented Jun 3, 2022

Right now, the way to construct a Transact XCM safely is to import the foreign chain's runtime crate and use its Call enum to encode/decode incoming/outgoing XCM Transact instructions. This is obviously not a practical or scalable solution, since a runtime crate is pretty large in size, and pulling it in as a dependency is going to considerably bloat code sizes, not to mention elongate compilation times.

What teams do nowadays instead is to have a local representation of the SCALE-encoded foreign Call enum. This is not ideal either since it can easily go out of sync with the foreign chain, which would make Transact instructions to the foreign fail.

We'll need to create some kind of interface based on the Call enum without requiring devs to pull in an entire runtime crate.

@KiChjang KiChjang added the T6-XCM This PR/Issue is related to XCM. label Jun 3, 2022
@bkchr
Copy link
Member

bkchr commented Jun 3, 2022

Isn't this what subxt is for? It generates you a Call enum from the metadata?

@xlc
Copy link
Contributor

xlc commented Jun 4, 2022

Duplicated to the request I made more than a year ago: paritytech/substrate#8158

@xlc
Copy link
Contributor

xlc commented Jun 4, 2022

And maybe the proper solutions is polkadot-fellows/xcm-format#22 + PSPs?

@coax1d
Copy link

coax1d commented Jun 6, 2022

Duplicated to the request I made more than a year ago: paritytech/substrate#8158

The example constructed for the xcm simulator in paritytech/polkadot#5598 regarding the origin of this issue was inspired by the Acala team shown here:
https://github.com/AcalaNetwork/Acala/blob/5a8f381a95649eb74385fb7db31421d78118a651/modules/relaychain/src/lib.rs#L39-L100

@KiChjang
Copy link
Contributor Author

Okay so I gave this some more thought, and the kinds of data that we really need is the following:

  1. Pallet index
  2. Call index
  3. Argument types

The pallet index is generated by construct_runtime, whereas the rest is generated by the #[frame_support::pallet] attribute macro.

What this then means is that we can simply modify the two macros and have them generate some sort of interface, and put these data into a new crate. Foreign runtimes can then simply import this newly auto-generated crate. Ideally, this crate will be generated and updated whenever there's a new release -- this would then allow foreign runtimes to stay up-to-date with the latest pallet indices of the XCM Transact recipient's runtime.

@KiChjang
Copy link
Contributor Author

I'm still curious as to whether scale-info or frame-metadata already has tools or functions that make the process easier...

@bkchr
Copy link
Member

bkchr commented Jan 3, 2023

I will repeat myself again: https://docs.rs/subxt-codegen/latest/subxt_codegen/

@coax1d
Copy link

coax1d commented Jan 3, 2023

Just to note..

I have used in a test subxt to pull the runtime meta data which I am interested in and actually use that Runtime to decode a particular xcm Transact message.

@KiChjang
Copy link
Contributor Author

KiChjang commented Jan 3, 2023

It would be much better if the runtime can simply produce artifacts of its own interface as part of its building process. Using an external tool like subxt to extract runtime metadata is pretty brittle and can quickly get out of date as it does not keep track of runtime upgrades.

What seems to be the best solution to me is the ability for substrate-based chains to query other chains for their RuntimeCall structure, and have this metadata stored on-chain along with its runtime version, similar to how we do version negotiation in the XCM pallet.

@coax1d
Copy link

coax1d commented Jan 3, 2023

I do think that is a more robust solution when I look at it at face value.

@bkchr
Copy link
Member

bkchr commented Jan 3, 2023

What seems to be the best solution to me is the ability for substrate-based chains to query other chains for their RuntimeCall structure, and have this metadata stored on-chain along with its runtime version, similar to how we do version negotiation in the XCM pallet.

Why should you store the RuntimeCall of another chain in your state? What does this brings you?

@KiChjang
Copy link
Contributor Author

KiChjang commented Jan 3, 2023

I'm not sure why this question is asked; it's akin to asking "why should we store the XCM version of another chain in the local chain state". We currently do so for versions because we have to figure out what the appropriate XCM version that we want to use; similarly, we should also then store a compressed version of another chain's RuntimeCall locally in order to know how to use Transact correctly.

Frankly speaking, we don't necessarily need to store the RuntimeCall -- all we need are the 3 pieces of data that I mentioned above in order to use Transact correctly (i.e. pallet indices, call indices and argument types).

@bkchr
Copy link
Member

bkchr commented Jan 3, 2023

"why should we store the XCM version of another chain in the local chain state"

There is a huge difference. XCM needs to know the supported XCM version of the remote chain to send a XCM message in the correct format. For transact, XCM doesn't care on what the destination supports expects. It will not interpret or try to "correct" the transact message.

The transact message is constructed in the upper layers, in the chain logic. Why should XCM even try to care what kind of bytes you want to send in transact? Maybe you are speaking with a none Substrate based chain that is expecting quite a different format for the message?

If the runtime call format of the target chain changed, what would you do? A call that your pallet wants to execute requires a new argument? What will you do? Your pallet can not handle this 🤷

@KiChjang
Copy link
Contributor Author

KiChjang commented Jan 4, 2023

We obviously will key on the runtime version of the foreign runtime. Then it would simply check whether the versions match, and if not, we fail the transaction. It doesn't need to try and upgrade or downgrade your transaction.

The only thing this is really protecting against is the change in pallet indices; for the other 2 pieces of data, we can already use ExpectPallet or QueryPallet in XCMv3 to first inquire about the expected version of the pallet. As such, we don't even need to store any of these data either; just the runtime version is enough.

@crystalin
Copy link

Giving my 2 cents:

  • I agree with Basti that XCM "should" not be care about what is used for the transact. I see transact as a shortcut to discuss low level with other chain; but with the intention of removing it (or at least trying to avoid using it) in the future.
  • I also agree with Keith on the requirement for chains to be able to construct properly their transact message (until XCM is properly designed) and being able to maintain those.

Having the possibility to identify other chains pallet/extrinsics/types is important. An external tool could be combined with on-chain storage to allow updating those. I'm not sure a fully on-chain system could work.

@coax1d
Copy link

coax1d commented Jan 4, 2023

I agree with @KiChjang that if you just are storing the runtime version then it isnt so much effort it just works like versioning. Guaranteeing a fail would be nice because in the current paradigm failing is your best case scenario and worst case is something bad is happening and you dont even know it.

@KiChjang
Copy link
Contributor Author

The other solution to this would be to add an ExpectRuntimeVersion instruction, much alike the ExpectPallet instruction. That way, we don't even need to store anything -- the responsibility of ensuring that the Transact instruction works the way it was intended now belongs solely to the sender.

@Sophia-Gold Sophia-Gold transferred this issue from paritytech/polkadot Aug 24, 2023
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
…h#813)

* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
bkchr pushed a commit that referenced this issue Apr 10, 2024
* Nest some crates.

* Alter command execution to make it easier to add new bridges.

* Rename sub-dirs.

* cargo fmt --all

* Address clippy.

* Update relays/substrate/src/rialto_millau/cli.rs

Co-authored-by: Hernando Castano <[email protected]>

Co-authored-by: Hernando Castano <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T6-XCM This PR/Issue is related to XCM.
Projects
None yet
Development

No branches or pull requests

6 participants
@crystalin @xlc @KiChjang @bkchr @coax1d and others