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

call-runtime: use subxt #1776

Conversation

pmikolajczyk41
Copy link
Member

We tried harnessing subxt for the 'call_runtime' feature. Although we can remove the need of manually crafting call object, there are still some issues due to the subxt's design.

  1. subxt macro assumes that subxt crate is available. This is fine for all standard usecases, where one uses full subxt power. However, in our case, we need only runtime types module, without any other subxt features. Especially, because full subxt is far from being no-std compatible and therefore it cannot be used in smart contracts context. For this, we can depend solely on subxt_macro crate, which is no-std compatible and provides only the required macro. Unfortunately, we still have to provide at least a dummy subxt module for primitive types and reexports.

  2. Macro configuration is not trivial. Also, it still lacks possibility of picking only a subset of pallets.

  3. Passing runtime metadata to the macro has to be done with a running node (runtime_metadata_url). Using runtime_metadata_path fails during contract building phase with cargo contract (Failed IO for /tmp/cargo-contract_mB5KYC/metadata.scale, make sure that you are providing the correct file path for metadata: No such file or directory (os error 2)).

  4. subxt uses std library for boxed, vec, string etc. This fails for no-std context, where SC is built. Unfortunately, we find it very hard to remove all such dependencies (e.g. by pointing to the alloc crate). For now, we prepared a temporary version of subxt that works well in no-std context, but fails in a standard one.

  5. RuntimeCall enum is not self-contained. It depends on a group of pallet-specific type families. This is not a problem for a standard usecase, where one uses full subxt power. However, in our case, this blocks putting RuntimeCall into a single place like Environment trait, so that any contract can conveniently build a call object. Instead, every contract has to have an access to the whole module with runtime types.

This is rather a call for discussion / current best effort for #1675.


Ad 4.
Problematic imports:

  • ::std::borrow: easily convertible to ::core::borrow, no further issues here
  • ::std::collections: easily convertible to ::core::alloc::collections, no further issues here
  • ::std::boxed, ::std::vec: could be taken from sp_core::sp_std:: (available only under specific subxt feature) or somewhere where alloc is always available
  • ::std::string: alloc needed in both std and no-std contexts; notice, that this cannot be feature-gated, because the crate, where the macro is used might not have std feature but not be no-std

cc: @athei @cmichi @jsdw

@ascjones
Copy link
Collaborator

ascjones commented May 11, 2023

RuntimeCall enum is not self-contained. It depends on a group of pallet-specific type families. This is not a problem for a standard usecase, where one uses full subxt power. However, in our case, this blocks putting RuntimeCall into a single place like Environment trait, so that any contract can conveniently build a call object. Instead, every contract has to have an access to the whole module with runtime types.

One approach I would explore is to use subxt_codegen::TypeGenerator directly from the ink macro, then we can generate our own RuntimeCall enum and we have full control over the codegen for that type. The codegen for that type is very trivial, just generating an enum. And we may want to customize it. It would also allow us to have control over the derives which are passed in there.

/cc @jsdw

@jsdw
Copy link

jsdw commented May 11, 2023

I think that's def a good avenue to explore; we already have https://github.com/paritytech/subxt/blob/3f16bb8d521031c7e13749d41711f54f490bb84c/codegen/src/api/mod.rs#L196 which was added specifically for the purpose of generating the code needed for this issue, and it might be a good idea to just take that and put it in its own proc macro crate.

I'd then remove runtime_types_only from Subxt as it's a bit of a cludge, and we could then focus on making the generated code from that new macro no-std friendly, which shouldn't be too difficult :)

I'll have a bit of a play over the next couple of days with this approach and see what happens.

@jsdw
Copy link

jsdw commented May 12, 2023

I spent the day experimenting with this approach and came up with this:

https://github.com/jsdw/subxt-runtime-types-nostd-macro (I left some details in the readme over in that repo.)

This crate basically contains a macro which relies on subxt-codegen directly and spits out all of the types, and then a no_std_test folder/crate which uses the macro, to prove that the macro can function and generate no-std compatible code.

I made a branch with changes like @pmikolajczyk41 did to allow subxt-codegen to generate no-std compatible code (behind a feature flag; I'd love if there was a way to avoid this but offhand didn't see one, because you either need to depend on alloc:: or std:: and either one doesn't cleanly work for the other case).

Anyway, I've not really got the time to take this further right now, but hopefully this gives you guys something to build on, and I'm happy enough to merge the required subxt-codegen changes if this is the direction you guys want to take (for now) :)

(as a note from the readme, I think the ideal future path for this would be either implementing the type generation entirely separately from subxt or factoring the type generation stuff out from subxt-codegen into a new crate that both places can use. But, maybe this can suffice for now!)

@ascjones
Copy link
Collaborator

Thanks James, will take a look at this next week!

@pmikolajczyk41 pmikolajczyk41 deleted the pmikolajczyk41/call-runtime++ branch July 7, 2023 09:03
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.

3 participants