codegen: Generate type aliases for better API ergonomics #1249

lexnv · 2023-11-08T11:08:25Z

This PR generates type aliases for:

storage return types and key parameters
calls function parameters
events
runtime APIs

Check the runtime_api section for a common type that users had to manually define in their application if they wanted to store the type.

Storage

Type aliases are placed under the types module.
In the case of storage entries, inner modules are named by the storage entry.
They contain the type alias of the storage result, as well as type aliases for all parameters needed for partial iteration.
Because those parameters are unnamed, they follow Param {index} naming convention.

/// Example for the system pallet
pub mod system {
/// The storage module of the system pallet.
pub mod storage {


pub mod storage {

   /// Generated by this PR:
   /// The module where alias types are placed.
    pub mod types {

        pub mod key_owner {
            /// The storage result type of the query.
            pub type KeyOwner = ::subxt::utils::AccountId32;

            /// Parameters for iterating over keys.
            pub type Param0 = runtime_types::sp_core::crypto::KeyTypeId;
            pub type Param1 = [::core::primitive::u8];
        }
    }

  pub struct StorageApi;
  impl StorageApi {
      pub fn key_owner_iter(
          &self,
      ) -> ::subxt::storage::address::Address<
          ::subxt::storage::address::StaticStorageMapKey,
          
          /// Return type is aliased.
          types::key_owner::KeyOwner,
      ...
     
      pub fn key_owner_iter1(
          &self,
          
          /// Unnamed parameter is also type aliased.
          _0: impl ::std::borrow::Borrow<types::key_owner::Param0>,
      ) -> ...

Calls

Type aliases are placed under the already generated types module.
The structures generated inside the types module are modified to use the type aliases for each structure field.
A module with the snake case name convention is generated next to the structure name and contains all the type aliased fields.

/// Example for the Babe pallet
pub mod babe {
/// The calls module.
pub mod calls {

    pub mod types {
      pub struct ReportEquivocation {
      
          /// Structure fields are aliased. 
          pub equivocation_proof:
              ::std::boxed::Box<report_equivocation::EquivocationProof>,
          pub key_owner_proof: report_equivocation::KeyOwnerProof,
      }
      
      /// Generated module that stores the type aliases.
      pub mod report_equivocation {
          pub type EquivocationProof =
              runtime_types::sp_consensus_slots::EquivocationProof<
                  runtime_types::sp_runtime::generic::header::Header<
                      ::core::primitive::u32,
                  >,
                  runtime_types::sp_consensus_babe::app::Public,
              >;
          pub type KeyOwnerProof = runtime_types::sp_session::MembershipProof;
      }

   pub struct TransactionApi;
   impl TransactionApi {
     
   pub fn report_equivocation(
       &self,
       
       // Function parameters are using aliased types
       
        equivocation_proof: types::report_equivocation::EquivocationProof,
        key_owner_proof: types::report_equivocation::KeyOwnerProof,
        ) -> ...

Events

The generation is similar to calls, where the type aliases are placed in a module next to the generated structure of the event.

  pub mod events {
  
      pub struct IndexAssigned {
          /// This now points to a type alias.
          pub who: index_assigned::Who,
          pub index: index_assigned::Index,
      }
      
      /// Generated module with snake case-derived naming.
      pub mod index_assigned {
          use super::runtime_types;
          pub type Who = ::subxt::utils::AccountId32;
          pub type Index = ::core::primitive::u32;
      }

Runtime APIs

The types module of the runtime API is extended for each runtime API function with an aliased module containing the runtime API's output, together with individual input structure type aliases.

pub struct ExecuteBlock {
    /// This now points to a type alias.
    pub block: execute_block::Block,
}
pub mod execute_block {
    
    /// The generated type alias for block.
    pub type Block = runtime_types :: sp_runtime :: generic :: block :: Block <
       runtime_types :: sp_runtime :: generic :: header :: Header < :: core :: primitive :: u32 >,
       :: subxt :: utils :: UncheckedExtrinsic <
            :: subxt :: utils :: MultiAddress < :: subxt :: utils :: AccountId32 , () > ,
            runtime_types :: rococo_runtime :: RuntimeCall ,
            runtime_types :: sp_runtime :: MultiSignature ,
            (runtime_types :: frame_system :: extensions :: check_non_zero_sender :: CheckNonZeroSender , runtime_types :: frame_system :: extensions :: check_spec_version :: CheckSpecVersion , runtime_types :: frame_system :: extensions :: check_tx_version :: CheckTxVersion , runtime_types :: frame_system :: extensions :: check_genesis :: CheckGenesis , runtime_types :: frame_system :: extensions :: check_mortality :: CheckMortality , runtime_types :: frame_system :: extensions :: check_nonce :: CheckNonce , runtime_types :: frame_system :: extensions :: check_weight :: CheckWeight , runtime_types :: pallet_transaction_payment :: ChargeTransactionPayment ,)
             >
        > ;
    
    
    
    pub type Output = ();
}

Notes

The function parameters of storage queries are unnamed and are not generated at the moment (ie _2: impl ::std::borrow::Borrow< runtime_types::bounded_collections::bounded_vec::BoundedVec< ::core::primitive::u8>,
Call structures do not reuse the type aliases
No support at the moment for runtime types aliasing

Signed-off-by: Alexandru Vasile <[email protected]>

jsdw · 2023-11-08T11:44:34Z

Something that would be interesting to do if you haven't already is to look at Erin's original sample code (from the issue) and see whether, when using this branch of Subxt, we can simplify the types nicely :)

jsdw · 2023-11-08T11:57:49Z

Looking at some random generated call signature from this:

#[doc = "See [`Pallet::found_society`]."]
pub fn found_society(
    &self,
    founder: alias_types::found_society::Founder,
    max_members: alias_types::found_society::MaxMembers,
    max_intake: alias_types::found_society::MaxIntake,
    max_strikes: alias_types::found_society::MaxStrikes,
    candidate_deposit: alias_types::found_society::CandidateDeposit,
    rules: alias_types::found_society::Rules,
) -> ::subxt::tx::Payload<types::FoundSociety> {

I wonder about the separation between alias_types and types modules. I like what you did with naming each of the input types. It'd be nice though if all of the types referenced came from the same place, so maybe these could be unified into one module, maybe:

pub mod types {
    pub mod found_society {
        mod inputs {
            type Founder = ...;
            type MaxMembers = ...;
        };
        // This instead of `FoundSociety`
        struct Input { ... }
    }
}

And then we'd have

#[doc = "See [`Pallet::found_society`]."]
pub fn found_society(
    &self,
    founder: types::found_society::inputs::Founder,
    max_members: types::found_society::inputs::MaxMembers,
    max_intake: types::found_society::inputs::MaxIntake,
    max_strikes: types::found_society::inputs::MaxStrikes,
    candidate_deposit: types::found_society::inputs::CandidateDeposit,
    rules: types::found_society::inputs::Rules,
) -> ::subxt::tx::Payload<types::found_society::Input> {

Not sure; what do you think?!

jsdw · 2023-11-08T12:02:19Z

I think I like your idea even better, so having eg:

pub mod types {
    struct FoundSociety { ... }
    pub mod found_society {
        type Founder = ...;
        type MaxMembers = ...;
    }
}

Less module indentation and maybe having the name of the extrinsic thing is quite nice actually anyway :)

Signed-off-by: Alexandru Vasile <[email protected]>

…aliases

Signed-off-by: Alexandru Vasile <[email protected]>

…aliases

jsdw · 2023-11-10T17:29:59Z

codegen/src/api/calls.rs

+                            // One downside of the type alias system is that we generate one alias per argument.
+                            // Multiple arguments could potentially have the same type (ie fn call(u32, u32, u32)).


That worried me for a sec, but actually all you mean is that we might have alias like

type Foo = core::primitive::u8; type Bar = core::primitive::u8;

Which is fine I think :)

jsdw · 2023-11-10T17:31:39Z

codegen/src/api/runtime_apis.rs

@@ -11,6 +11,48 @@ use proc_macro2::TokenStream as TokenStream2;
 use quote::{format_ident, quote};

 /// Generates runtime functions for the given API metadata.
+///
+/// # Note


This note is the sort of thing that could fall out of date fairly quickly, so personally I'd just be looking at the generated code anyway to see how things are structured and would avoid such a comment :)

jsdw · 2023-11-10T17:32:34Z

codegen/src/api/runtime_apis.rs

@@ -39,34 +81,53 @@ fn generate_runtime_api(
        let inputs: Vec<_> = method.inputs().enumerate().map(|(idx, input)| {
            // These are method names, which can just be '_', but struct field names can't
            // just be an underscore, so fix any such names we find to work in structs.
-            let name = if input.name == "_" {
-                format_ident!("_{}", idx)
+            let (alias_name, name) = if input.name == "_" {


What about names that start with a _, like _foo? These would also be perfectly valid and we want to make sure they also work ok (a good opportunity for writing a test perhaps if one doesn't already exist)

(and basically, can you guarantee that no combination of valid names will lead to an issue; eg you could transform _foo into s_foo before upper_camel_casing, but what if there is already an s_foo? Unlikely I know, but would be good to be airtight if we can be :))

tadeohepperle

Looks good to me, very nice to have these type aliases. Just one question: The type path of each type alias should always have all generic parameters fully filled in with concrete types, right? Like there are no type aliases that are generic over anything.

jsdw · 2023-11-10T17:54:12Z

I haven't had a chance to properly look over this, but generally the aliases and their locations look good to me!

One thing I'd like you to be sure of is that there are no names of calls/args/whatever that can lead to conflicts/compile errors, but otherwise I'd be happy for this to merge!

…aliases

Signed-off-by: Alexandru Vasile <[email protected]>

…aliases

tadeohepperle

I think it is nice to have the aliases like this for now, however I think a lot of the structure of the code introduced in this PR will have to change, once scale-typegen is introduced into subxt. :)

tadeohepperle · 2023-12-05T17:59:08Z

cli/src/commands/explore/mod.rs

@@ -180,7 +180,7 @@ pub mod tests {
    async fn test_commands() {
        // show pallets:
        let output = simulate_run("").await;
-        assert_eq!(output.unwrap(), "Usage:\n    subxt explore <PALLET>\n        explore a specific pallet\n\nAvailable <PALLET> values are:\n    Balances\n    Multisig\n    ParaInherent\n    Staking\n    System\n    Timestamp\n");
+        assert_eq!(output.unwrap(), "Usage:\n    subxt explore <PALLET>\n        explore a specific pallet\n\nAvailable <PALLET> values are:\n    Balances\n    Multisig\n    ParaInherent\n    System\n    Timestamp\n");
        // if incorrect pallet, error:


I need to remind myself to make these updates in #1290 later, will probably have merge conflicts.

tadeohepperle · 2023-12-05T18:07:36Z

codegen/src/types/type_def.rs

@@ -70,13 +71,16 @@ impl TypeDefGen {
                let docs = should_gen_docs.then_some(&*ty.docs).unwrap_or_default();
                let composite_def = CompositeDef::struct_def(
                    ty,
+                    types_mod_ident,
+                    &type_name,
                    &type_name,


So we pass the type_name as the variant_name parameter here as well, but it is not really used for a normal Composite from the types registry, right?

tadeohepperle · 2023-12-05T18:08:06Z

codegen/src/types/composite_def.rs

        ident: &str,
+        variant_name: &str,


I find it a little bit confusing to add a variant_name field here, which will only have a special meaning for call types etc... I suppose for most types this will be the same as the ident?
Also I think we should maybe introduce a new struct for the type alias functionality instead of cramping it all into a CompositeDef.

But maybe I don't understand enough why these decisions are made.

Is it a bit scary to me that this already long list of arguments has grown by 3. It may be trying to do too many things now.

I wonder how easy it would be to separate the logic to generate the structs with aliases that we need in the "subxt codegen" side, and try to minimise how much we touch this types folder (which will be going away soon when scale-typegen is used instead)?

Yep that makes sense!

I've split the implementation to rely on TypeAliases for generating the type alias module.
On the CompositeDef, we still need a bit of information wrts whatever to generate the structure from scratch (needed for generating all types from the registry); or to generate the structure with type aliases (calls / events).

On that note, we'd probably need to think about integration with scale-typegen wrts:

the current CompositeDefFields has a to_type_aliases_tokens which generates aliases from the fields of a structure

to_struct_field_tokens is adjusted to conditionally use the aliases or the types directly

@tadeohepperle let me know if this is right

jsdw · 2023-12-06T10:02:32Z

I think it is nice to have the aliases like this for now, however I think a lot of the structure of the code introduced in this PR will have to change, once scale-typegen is introduced into subxt. :)

Do you mean that when scale-typegen is used, we won't be able to have the same type aliases? Or will the type aliases stay as they are but just some code structure will change?

tadeohepperle · 2023-12-06T12:59:09Z

Do you mean that when scale-typegen is used, we won't be able to have the same type aliases? Or will the type aliases stay as they are but just some code structure will change?

I am certain that once we use scale-typegen we can still have type aliases like done in here, only the code structure will change. For example, we can amend the output of the generate_structs_from_variants in this PR to also put in the type aliases.

Signed-off-by: Alexandru Vasile <[email protected]>

jsdw · 2023-12-07T16:32:36Z

codegen/src/api/runtime_apis.rs

@@ -160,3 +204,225 @@ pub fn generate_runtime_apis(
        }
    })
 }
+
+#[cfg(test)]
+mod tests {


Nice tests! Def gives me some confidence in the param naming logic :)

codegen/src/types/composite_def.rs

jsdw · 2023-12-07T16:46:23Z

codegen/src/types/composite_def.rs

@@ -210,11 +259,38 @@ impl CompositeDefFields {
        }
    }

+    /// Generate the code for type aliases which will be used to construct the `struct` or `enum`.
+    pub fn to_type_aliases_tokens(&self, visibility: Option<&syn::Visibility>) -> TokenStream {


I still feel uneasy about this and the below alias_module_name additions (which, if provided, basically falls into if branches which do something that takes up most of the logic there).

I wonder whether this alias stuff should be more completely separate from the usual type generation stuff in this types folder, so that the types folder is relatively untouched in this PR (and can be swapped out for scale-typegen fairly easily to do the standard type generation stuff), and the alias stuff lives elsewhere (maybe a separate generate_alias.rs file or something outside of this folder) and is quite independent?

I'm not sure whether that would lead to a bunch of code duplication though, or whether the code for aliases is different enough that there's actually not a lot of overlap (which is just the feeling I get looking at the below).

That all said, @tadeohepperle what are your thoughts on this? Does adding these bits complicate moving to scale-typegen? We can also think about it in a separate PR of course too, since I know that this one has been lingering a while!

I think it will be fine to move the to_type_alias_tokens into a standalone function later when integrating scale-typegen (because CompositeDefFields will no longer exist). Let's merge this and I will figure out the conflicts things out in this PR: #1260

jsdw

I'm inclined to approve this because while I wonder about the alias logic being mixed in with the "normal" type gen logic, we can address that separately, and overall I really like the codegen that this adds, and it'll def make some things much nicer :)

Signed-off-by: Alexandru Vasile <[email protected]>

lexnv added 3 commits November 8, 2023 12:05

codegen: Generate type alias for storage return types

0be80cc

Signed-off-by: Alexandru Vasile <[email protected]>

codegen: Generate type alias for call function arguments

f522e5a

Signed-off-by: Alexandru Vasile <[email protected]>

testing: Update polkadot.rs code from commit 2e2a75f

d050859

Signed-off-by: Alexandru Vasile <[email protected]>

lexnv requested a review from a team as a code owner November 8, 2023 11:08

lexnv added 3 commits November 8, 2023 13:37

codegen: Type aliases for runtime API parameters

bd8f602

Signed-off-by: Alexandru Vasile <[email protected]>

codegen: Type alias for runtime apis output

bfa70d1

Signed-off-by: Alexandru Vasile <[email protected]>

storage: Change path of the aliased module

c5dff9b

Signed-off-by: Alexandru Vasile <[email protected]>

lexnv added 15 commits November 9, 2023 13:40

codegen: Adjust module indentation

c6bb349

Signed-off-by: Alexandru Vasile <[email protected]>

codegen: Do not alias for api::runtime_types with unresolved generics

10fdc75

Signed-off-by: Alexandru Vasile <[email protected]>

codegen: Fix and document runtime API alias generation

8008665

Signed-off-by: Alexandru Vasile <[email protected]>

Update artifacts

2bd89d4

Signed-off-by: Alexandru Vasile <[email protected]>

Update cargo.lock file

a777163

Signed-off-by: Alexandru Vasile <[email protected]>

codegen: Generate composite structs with alias unnamed fields

977b4fb

Signed-off-by: Alexandru Vasile <[email protected]>

testing: Update polkadot.rs file

e353f16

Signed-off-by: Alexandru Vasile <[email protected]>

Merge remote-tracking branch 'origin/master' into lexnv/codegen-type-…

0a316b0

…aliases

codegen: Alias storage unnamed parameters

0798ad9

Signed-off-by: Alexandru Vasile <[email protected]>

Update polkadot.rs

b9398dd

Signed-off-by: Alexandru Vasile <[email protected]>

examples: Change polkadot to rococo runtime

8c51c2a

Signed-off-by: Alexandru Vasile <[email protected]>

codegen: Fix compiling tests in the codegen crate

3ccb99f

Signed-off-by: Alexandru Vasile <[email protected]>

codegen: Extend storage test with alias module

354704e

Signed-off-by: Alexandru Vasile <[email protected]>

cli/tests: Adjust exepcted commands to the latest metadata

8761283

Signed-off-by: Alexandru Vasile <[email protected]>

Merge remote-tracking branch 'origin/master' into lexnv/codegen-type-…

43d9175

…aliases

jsdw reviewed Nov 10, 2023

View reviewed changes

tadeohepperle approved these changes Nov 10, 2023

View reviewed changes

lexnv and others added 9 commits November 17, 2023 11:49

Merge remote-tracking branch 'origin/master' into lexnv/codegen-type-…

c408e97

…aliases

codegen: Remove missleading comment and docs

6ec86d9

Signed-off-by: Alexandru Vasile <[email protected]>

codegen: Ensure unique names for generated runtime API types

17bdaeb

Signed-off-by: Alexandru Vasile <[email protected]>

codegen/tests: Test expected runtime type generation

b8425b9

Signed-off-by: Alexandru Vasile <[email protected]>

codegen/tests: Check duplicate params in runtime APIs

1588869

Signed-off-by: Alexandru Vasile <[email protected]>

codegen/tests: Test colliding names of type aliases and parameters

ab105eb

Signed-off-by: Alexandru Vasile <[email protected]>

Fix clippy

4bf9ac5

Signed-off-by: Alexandru Vasile <[email protected]>

Merge remote-tracking branch 'origin/master' into lexnv/codegen-type-…

5a9f834

…aliases

Merge branch 'master' into lexnv/codegen-type-aliases

6142e2f

tadeohepperle approved these changes Dec 5, 2023

View reviewed changes

lexnv added 2 commits December 7, 2023 17:23

codegen: Separate alias module from struct definition

4352c01

Signed-off-by: Alexandru Vasile <[email protected]>

Update polkadot.rs

d65be15

Signed-off-by: Alexandru Vasile <[email protected]>

jsdw reviewed Dec 7, 2023

View reviewed changes

codegen/src/types/composite_def.rs Outdated Show resolved Hide resolved

jsdw reviewed Dec 7, 2023

View reviewed changes

jsdw approved these changes Dec 7, 2023

View reviewed changes

codegen: Remove outdated docs from composite def

4c0c9f3

Signed-off-by: Alexandru Vasile <[email protected]>

lexnv merged commit c976d0d into master Dec 8, 2023
11 checks passed

lexnv deleted the lexnv/codegen-type-aliases branch December 8, 2023 13:07

tadeohepperle mentioned this pull request Jan 23, 2024

Prep for v0.34.0 release #1395

Merged

lexnv mentioned this pull request Jan 29, 2024

Can we simplify some of the references to generated types, so it's easier to name them? #1242

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

codegen: Generate type aliases for better API ergonomics #1249

codegen: Generate type aliases for better API ergonomics #1249

lexnv commented Nov 8, 2023 •

edited

Loading

jsdw commented Nov 8, 2023

jsdw commented Nov 8, 2023 •

edited

Loading

jsdw commented Nov 8, 2023 •

edited

Loading

jsdw Nov 10, 2023

jsdw Nov 10, 2023

jsdw Nov 10, 2023 •

edited

Loading

jsdw Nov 10, 2023

tadeohepperle left a comment

jsdw commented Nov 10, 2023

tadeohepperle left a comment

tadeohepperle Dec 5, 2023

tadeohepperle Dec 5, 2023

tadeohepperle Dec 5, 2023

jsdw Dec 6, 2023

lexnv Dec 7, 2023

jsdw commented Dec 6, 2023

tadeohepperle commented Dec 6, 2023

jsdw Dec 7, 2023

jsdw Dec 7, 2023

tadeohepperle Dec 8, 2023

jsdw left a comment

		// One downside of the type alias system is that we generate one alias per argument.
		// Multiple arguments could potentially have the same type (ie fn call(u32, u32, u32)).

codegen: Generate type aliases for better API ergonomics #1249

codegen: Generate type aliases for better API ergonomics #1249

Conversation

lexnv commented Nov 8, 2023 • edited Loading

Storage

Calls

Events

Runtime APIs

Notes

jsdw commented Nov 8, 2023

jsdw commented Nov 8, 2023 • edited Loading

jsdw commented Nov 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsdw Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tadeohepperle left a comment

Choose a reason for hiding this comment

jsdw commented Nov 10, 2023

tadeohepperle left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsdw commented Dec 6, 2023

tadeohepperle commented Dec 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsdw left a comment

Choose a reason for hiding this comment

lexnv commented Nov 8, 2023 •

edited

Loading

jsdw commented Nov 8, 2023 •

edited

Loading

jsdw commented Nov 8, 2023 •

edited

Loading

jsdw Nov 10, 2023 •

edited

Loading