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

portable: Retain the provided type IDs #174

Merged
merged 30 commits into from
Mar 21, 2023
Merged

portable: Retain the provided type IDs #174

merged 30 commits into from
Mar 21, 2023

Conversation

lexnv
Copy link
Contributor

@lexnv lexnv commented Mar 8, 2023

This PR implements the retain method on the PortableRegistry.

The retain method allows tools (such as subxt or capi) to interpret the
RuntimeMetadata for only a subset of pallets.

The method retains only the portable types needed to express the provided ids.
A given type ID can be defined by nesting type IDs, such as the case
of a [TypeDef::Composite] and others. To retain a valid [PortableRegistry]
all the types needed to express an ID are included. Therefore, the number of
elements defined by the result equals or exceeds the number of
provided IDs.

This is implemented in two steps:

  • recursively visit and collect all type IDs needed to express the given list of IDs
    • visiting happens in DFS manner
    • while visiting, type IDs are normalized (assigned a new incremental ID starting from zero)
  • modify the nested type IDs to point to the new location of the type in the registry

Testing Done

The following represents the metadata size for only one pallet, based on the PoC branch.
Further work is required to minimize the runtime type present in the Metadata to
reduce the size even more than the PoC.

232K Vesting.scale
235K VoterList.scale
232K Whitelist.scale
451K polkadot_metadata.scale

A more realistic number lies between the two type ID dumps from paritytech/subxt#629 (comment).

This is part of: paritytech/subxt#629

// CC: @paritytech/tools-team

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv requested review from ascjones and a team March 8, 2023 16:26
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
src/portable.rs Outdated Show resolved Hide resolved
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv
Copy link
Contributor Author

lexnv commented Mar 13, 2023

While at it, I've also tried to use the PortableRegistry to build out types without manually updating the nested type ids.

Registry from retain_recursive_ids test

0: u32
1: u64
2: Sequence { ty id 0}
3: MyStruct { primitive: 0, vec_of_u32: 2}
4: MyStructSecond { vec_of_u32: 2, second: 3}

Using the PortableRegistryBuilder without updating nested type IDs

0: MyStructSecond { vec_of_u32: 2, second: 3}
1: Sequence { ty id 0 }
2: u32
3: MyStruct { primitive: 0, vec_of_u32: id 2}

I'm afraid that's not right because MyStruct.primitive should be u32 with ID 2, not ID 0.

Using the current PR

0: MyStructSecond {vec_of_u32: 1, second: 3}
1: Sequence {ty id 2}
2: u32
3: MyStruct { primitive: 2, vec_of_u32: 1 }

Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
@lexnv lexnv requested a review from ascjones March 13, 2023 14:25
src/portable.rs Outdated Show resolved Hide resolved
src/ty/fields.rs Outdated Show resolved Hide resolved
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
src/portable.rs Outdated Show resolved Hide resolved
jsdw and others added 2 commits March 17, 2023 18:04
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
src/ty/mod.rs Outdated Show resolved Hide resolved
src/ty/mod.rs Outdated Show resolved Hide resolved
src/ty/fields.rs Outdated Show resolved Hide resolved
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
src/portable.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@jsdw jsdw left a comment

Choose a reason for hiding this comment

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

Looks great :)

lexnv and others added 2 commits March 21, 2023 15:01
Co-authored-by: James Wilson <james@jsdw.me>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
src/portable.rs Outdated Show resolved Hide resolved
Co-authored-by: Andrew Jones <ascjones@gmail.com>
@lexnv lexnv merged commit ce21de5 into master Mar 21, 2023
@lexnv lexnv deleted the lexnv/portable_retain branch March 21, 2023 14:50
@lexnv lexnv mentioned this pull request Mar 23, 2023
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.

None yet

3 participants