paritytech / subxt

Interact with Substrate based nodes in Rust or WebAssembly
Other
408 stars 244 forks source link

Types duplicated when upgrading to subxt 0.36.0 #1599

Closed simonsso closed 4 months ago

simonsso commented 4 months ago

With subxt 0.35.3 codegen generates one type called BoundedVec when upgrading to 0.36.0 over 20 different types are generated

Example BoundedVec19

Screenshot_BoundedVec19

Types generated by 0.35.3 and earlier versions of subxt

Screenshot_BoundedVec_subxt0_33

Example of duplicated code.

Screenshot_BoundedVec_subxt0_36

Steps to reproduce

  1. Clone https://github.com/simonsso/subxt-type-duplication
  2. git checkout OK ; cargo doc --open search for BoundedVec
    • Only one BoundedVec is found
  3. git checkout BROKEN ; cargo doc --open search for BoundedVec
    • Several versions of BoundedVec is found
jsdw commented 4 months ago

Oh interesting; thanks for the issue and steps to reproduce; we'll look into it!

The logic to decide when to de-duplicate types (so that there is one BoundedVec and such rather than loads) can be a bit tricky; it's also easy to go the other way and deduplicate things that actually differ in some way that isn't super obvious. I'm guessing we went too far the wrong way!

jsdw commented 4 months ago

Just to give an update; this basically boils down to this issue: https://github.com/paritytech/scale-typegen/issues/19

The way we used to de-duplicate types was just by looking at their path, but that was found to break in some cases (eg see https://github.com/paritytech/subxt/issues/1247#issuecomment-1799018654; basically when multiple pallets exist that have generic types but the generics are associated types that we don't capture in the metadata IIRC, it all sortof breaks).

To counter this, we stopped looking at just the paths and instead at the shapes of types to determine whether they were duplicates or eachother or not (which basically happens in this line of code: https://github.com/paritytech/scale-typegen/blob/e22a9850bbfa6d6faceed27e5d74ed5199922fd8/typegen/src/utils.rs#L44).

An effect of the current approach is that types like:

struct BoundedVec<T> {
    inner: Vec<T>
}

Are an issue, because the generic type T isn't directly used on a field, but instead is used in a child type, and our logic doesn't currently recurse in such a way as to track that sort of thing.

So, TL;DR the current approach is more conservative about which types to merge together (deduplicate) to avoid bugs, but the result is that you end up with multiple of some types even though in theory you could have just 1. The previous version of Subxt was less conservative, but that broke in a couple of edge cases.

We'll look into improving this logic in https://github.com/paritytech/scale-typegen/issues/19!

jsdw commented 4 months ago

@simonsso have a go at updating to Subxt 0.36.1 and see whether that resolves your issue!

simonsso commented 4 months ago

@jsdw

@simonsso have a go at updating to Subxt 0.36.1 and see whether that resolves your issue!

Upgraded to 0.37.0 and it solved all my problems