[v0.50] Convert historic metadata to subxt::Metadata #2120
Conversation
c9852b5 to
d668fa4
Compare
| /// Should type paths be sanitized to make them more amenable to things like codegen? | ||
| /// | ||
| /// Default: false | ||
| pub fn sanitize_paths(&mut self, sanitize: bool) { | ||
| self.sanitize_paths = sanitize; | ||
| } |
There was a problem hiding this comment.
Mostly for my own education, but what exactly is the purpose of this or how did it come about? What does sanitizing actually do and why is the default false?
There was a problem hiding this comment.
In modern metadatas, paths are expected to be the module that some type comes from.
Eg for "builtin types" we have no path eg:
Option
Result
u32
bool
And for non-builtin types we have paths to their crates like:
sp_runtime::AccountId32
parity_scale_codec::Compact
The names given to historic types in most cases don't include such paths, or include paths that would be invalid like <Foo as Bar>::SomeType (where <Foo as Bar> is the path).
This is fine on the whole, but our codegen actually uses this path information to put types in the right places.
sanitize_paths thus does the job of converting all paths to something "sensible" that we could use in codegen or whatever (mostly this means adding a path, and sometimes it means throwing away naff paths)
| // This only started existing in V14+ metadata, but in any case, | ||
| // we don't need to know how to decode the signed payload for | ||
| // historic blocks (hopefully), so set to unknown. | ||
| additional_ty: unknown_type_id.into() |
There was a problem hiding this comment.
This should probably be explicitly documented somewhere other than just here.
There was a problem hiding this comment.
Mmm, I think when the new Subxt takes form we should document any potential pitfalls and how to resolve them, eg around unknown types and such. In this particular case the APIs should prevent it from ever being needed ideally!
andrew-ifrita
left a comment
There was a problem hiding this comment.
Looks good. There is quite a lot here so I hope I didn't miss anything.
| /// Options to configure the legacy translating. | ||
| pub(crate) struct Opts { | ||
| pub sanitize_paths: bool, | ||
| pub ignore_not_found: bool, |
There was a problem hiding this comment.
Any suggestions on best practice for unknown types here. We should probably log any related info so we can get the type squared away in frame_decode, but do you think we should throw an error saying failed to decode in the polkadot-rest-api?
There was a problem hiding this comment.
That's atleast how PJS did it in the past but it caused some serious issue for folks.
There was a problem hiding this comment.
Yeah unknown types were a definite issue here. Since modern type information cannot contain an "unknown type" in the way that historic metadata + types can, the conversion currently maps all unknown types to a special empty variant type (called "special::Unknown").
The idea here is that, hopefully, any type decoding that relies on unknown types will fail in a way that can be easily understood (I would need to double check the errors to see if that'll be the case automatically or not though!)
Internally it is possible to not ignore "not found" / "unknown" types and instead error. It could be that a future iteration of this strips out anything from the metadata entirely where an unknown type exists, or that we scan through and find all of the unknown types so that it's easy to know what needs populating. In the first version it was easier to just ignore this eventuality!
TarikGul
left a comment
There was a problem hiding this comment.
Super solid, just one question.
* v0.50.0: Integrate frame-decode, redo storage APIs and break up Error. (#2100) * WIP integrating new frame-decode and working out new storage APIS * WIP: first pass adding new storage things to subxt-core * Second pass over Address type and start impl in Subxt * WIP new storage APIs * WIP New storage APIs roughly completed, lots of errors still * Remove PlainorMap enum; plain and map values now use same struct to simplify usage * Begin 'fixing' errors * WIP splitting errors and tidying payload/address traits * Get subxt-core compiling * Small fixes in subxt-core and remove metadata mod * subxt-core: cargo check --all-targets passes * Fix test * WIP starting to update subxt from subxt-core changes * WIP splitting up subxt errors into smaller variants * WIP errors: add DispatchError errors * Port new Storage APIs to subxt-core * cargo check -p subxt passes * Quick-fix errors in subxt-cli (explore subcommand) * fmt * Finish fixing codegen up and start fixing examples * get Subxt examples compiling and bytes_at for constants * Add some arcs to limit lifetimes in subxt/subxt-core storage APIs * A little Arcing to allow more method chaining in Storage APIs, aligning with Subxt * Update codegen test * cargo check --all-targets passing * cargo check --features 'unstable-light-client' passing * clippy * Remove unused dep in subxt * use published frame-decode * fix wasm-example * Add new tx extension to fix daily tests * Remove unused subxt_core::dynamic::DecodedValue type * Update book to match changes * Update docs to fix more broken bits * Add missing docs * fmt * allow larger result errs for now * Add missing alloc imports in subxt-core * Fix doc tests and fix bug getting constant info * Fix V14 -> Metadata transform for storage & constants * Fix parachain example * Fix FFI example * BlockLength decodes t ostruct, not u128 * use fetch/iter shorthands rather than entry in most storage tests * Fix some integration tests * Fix Runtime codegen tests * Expose the dynamic custom_value selecter and use in a UI test * Update codegen metadata * Tidy CLI storage query and support (str,str) as a storage address * Add (str,str) as valid constant address too * Show string tuple in constants example * Via the magic of traits, avoid needing any clones of queries/addresses and accept references to them * clippy * [v0.50] update scale-info-legacy and frame-decode to latest (#2119) * bump scale-info-legacy and frame-decode to latest * Remove something we don't need in this PR * Fully remove unused for now dep * [v0.50] Convert historic metadata to subxt::Metadata (#2120) * First pass converting historic metadatas to our subxt::Metadata type * use published frame-decode * fmt and rename legacy metadata macro * Enable legacy feature where needed in subxt_metadata so it compiles on its own * Use cargo hack more in CI and fix subxt-metadata features * Add tests for metadata conversion (need to optimise; some too expensive right now * Address performance and equality issues in metadata conversion testing * fmt * fmt all * clippy * Fix a doc link * Test codegen and fixes to make it work * Remove local frame-decode patch * bump frame-decode to latest * [v0.50.0] Allow visiting extrinsic fields in subxt_historic (#2124) * Allow visiting extrinsic fields * fmt * Don't use local scale-decode dep * Clippy and tidy * Extend 'subxt codegen' CLI to work with legacy metadatas * Simplify historic extrinsics example now that AccountId32s have paths/names * clippy * clippy * clippy.. * Allow visiting storage values, too, and clean up extrinsic visiting a little by narrowing lifetime * Try to fix flaky test * Add custom value decode to extrinsics example * Remove useless else branch ra thought I needed * Simplify examples * Prep to release v0.0.5 (#2126)
Goal
The goal of this PR is to convert historic metadata formats to
subxt_metadata::Metadata. This will make it much easier to extend Subxt to work at historic blocks, as we no longer have to abstract over modern vs "old" metadatas or deal with legacy types. Things like codegen at historic blocks for instance will hopefully "Just Work" (though we will see if that holds up in practice!).The main thing involved here is taking a
scale_info_legacy::TypeRegistrySetwhich contains definitions for historic types, and converting these definitions across to entries in the modernscale_info::PortableRegistryas needed, updating the corresponding type IDs across metadata as we convert it over.The original plan was to convert things to
frame_metadata::v15::Metadata, but there was a blocker: v15 metadata has a single "index" per pallet, but older metadatas have a different "index" for errors, calls and events in each pallet, which cannot be represented in v15 metadata. Thus, the easier initial route is to make the necessary changes to our internalsubxt::Metadatato support this and any other differences.The hope is that we can iterate on this and tweak/refine the
Metadatastruct as needed, and then apply our learnings from that and create an external format which can be shared more widely and act as a target which any metadata version (+ types where needed) can be converted into. PAPI and Subxt could both rely on this new target if they liked.Summary of changes
cargo hackmore to improve our check coverage a bit.subxt_metadata::Metadataformat.pallet_by_call_index,pallet_by_error_indexetc) to allow this conversion to work.frame-decodeand adapt to changes there. Test that codegen of this converted metadata runs without error.The main changes are in
metadata/src/from/legacy/*