Fix Runtime V14 scale codec parser

Lohann commented 1 month ago

User description

[x] Fix Runtime V14 codec parser based on the official frame-metadata format.
[x] Parser and Encode metadata from bytes.
[x] Encode metadata as JSON
[ ] Update polkadart-cli generator to use the new metadata format. (in progress)

PR Type

enhancement, other

Description

Implemented a new RuntimeMetadataV14 class to handle runtime metadata version 14.
Added comprehensive codec implementations for metadata components.
Introduced new libraries for scale info and metadata handling.
Updated StateApi to use RuntimeMetadataPrefixed for metadata operations.
Added a new script for fetching and processing chain metadata.
Updated dependencies to include substrate_metadata.

Changes walkthrough 📝

Relevant files

Enhancement

20 files

apis.dart `Update imports for metadata handling` packages/polkadart/lib/apis/apis.dart Added imports from `substrate_metadata` package. Expanded the list of imported classes from `polkadart`.	+19/-4
state.dart `Update state API to use prefixed metadata` packages/polkadart/lib/apis/state.dart Changed `RuntimeMetadata` to `RuntimeMetadataPrefixed`. Updated method return types to use `RuntimeMetadataPrefixed`.	+4/-4
get_metadata.dart `Add script for fetching chain metadata` packages/polkadart_cli/bin/get_metadata.dart Added a new script for fetching and processing chain metadata. Implemented `ChainProperties` class to handle metadata and version.	+90/-0
chain.dart `Update chain generator for metadata v14` packages/polkadart_cli/lib/src/generator/chain.dart Changed import to use `RuntimeMetadataV14` from `substrate_metadata`. Updated type descriptor generation to use `metadata.types`.	+3/-2
field.dart `Add Field class for scale info` packages/polkadart_cli/lib/src/scale_info/field.dart Added a new class `Field` to represent struct-like data fields. Implemented JSON parsing for `Field`.	+58/-0
scale_info.dart `Introduce scale info library` packages/polkadart_cli/lib/src/scale_info/scale_info.dart Added a new library for scale information handling. Implemented parsing and type definition classes.	+711/-0
type_definition.dart `Add type definition handling for scale info` packages/polkadart_cli/lib/src/scale_info/type_definition.dart Added type definition classes for scale info. Implemented JSON serialization and deserialization.	+60/-0
type_parameter.dart `Add TypeParameter class for scale info` packages/polkadart_cli/lib/src/scale_info/type_parameter.dart Added `TypeParameter` class for handling generic type parameters. Implemented JSON parsing for `TypeParameter`.	+14/-0
variant.dart `Add Variant class for scale info` packages/polkadart_cli/lib/src/scale_info/variant.dart Added `Variant` class to represent enum variants. Implemented JSON parsing for `Variant`.	+39/-0
common.dart `Add common metadata handling classes` packages/substrate_metadata/lib/metadata/common.dart Added `RuntimeMetadata` and `RuntimeMetadataPrefixed` classes. Implemented metadata decoding from hex.	+88/-0
metadata.dart `Introduce metadata handling library` packages/substrate_metadata/lib/metadata/metadata.dart Added library for metadata handling. Exported core metadata functionalities.	+21/-0
v14.dart `Implement RuntimeMetadataV14 and codecs` packages/substrate_metadata/lib/metadata/v14.dart Implemented `RuntimeMetadataV14` class for handling v14 metadata. Added codec implementations for metadata components.	+1009/-0
field.dart `Add Field class with codec for scale info` packages/substrate_metadata/lib/scale_info/field.dart Added `Field` class for scale info with codec support. Implemented JSON serialization for `Field`.	+117/-0
portable.dart `Add PortableType class with codec` packages/substrate_metadata/lib/scale_info/portable.dart Added `PortableType` class for portable type representation. Implemented codec for `PortableType`.	+54/-0
scale_info.dart `Introduce scale info library with codecs` packages/substrate_metadata/lib/scale_info/scale_info.dart Added library for scale info handling. Defined type ID codec and utility functions.	+67/-0
type_definition.dart `Add type definition classes with codec` packages/substrate_metadata/lib/scale_info/type_definition.dart Added type definition classes for scale info. Implemented codec support for type definitions.	+763/-0
type_metadata.dart `Add TypeMetadata class for detailed type info` packages/substrate_metadata/lib/scale_info/type_metadata.dart Added `TypeMetadata` class for detailed type information. Implemented JSON serialization for `TypeMetadata`.	+116/-0
type_parameter.dart `Add TypeParameter class with codec` packages/substrate_metadata/lib/scale_info/type_parameter.dart Added `TypeParameter` class for generic type handling. Implemented codec for `TypeParameter`.	+55/-0
variant.dart `Add Variant class with codec` packages/substrate_metadata/lib/scale_info/variant.dart Added `Variant` class for enum variant representation. Implemented codec for `Variant`.	+97/-0
substrate_metadata.dart `Update exports for substrate metadata library` packages/substrate_metadata/lib/substrate_metadata.dart Exported metadata handling functionalities. Added new exports for metadata processing.	+76/-0

Dependencies

1 files

pubspec.yaml

Update dependencies to include substrate_metadata

packages/polkadart_cli/pubspec.yaml - Added `substrate_metadata` as a dependency.

+1/-0

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

CLAassistant commented 1 month ago

All committers have signed the CLA.

github-actions[bot] commented 1 month ago

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review

Code Complexity
The file contains a large amount of code (over 1000 lines), which includes complex classes and methods. This could impact maintainability and readability. Consider refactoring into smaller, more manageable components. Possible Redundancy
The file seems to be defining classes and methods that are potentially duplicated or similar to those in other parts of the project. Review for possible consolidation or removal to avoid redundancy. Inconsistent Naming
There are inconsistencies in naming conventions and potential typographical errors in variable and class names. Standardizing naming conventions would improve code clarity and consistency.

github-actions[bot] commented 1 month ago

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible bug	Correct the method call to pass the output stream in encoding ___ In the `encodeTo` method of `$TypeDefCompositeCodec`, ensure that the `encode` method of `$TypeDefCompositeCodec` is called with the `output` parameter to correctly pass the output stream. [packages/substrate_metadata/lib/scale_info/type_definition.dart [115]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-dfd2daaf996cb19132ec190538042c9be7a1c3c08473db576068e53cadc284b7R115-R115) ```diff -$TypeDefCompositeCodec._().encode(value as TypeDefComposite); +$TypeDefCompositeCodec._().encodeTo(value as TypeDefComposite, output); ``` Suggestion importance[1-10]: 9 Why: This suggestion corrects a potential bug by ensuring the correct method is called with the necessary parameters, which is crucial for the proper functioning of the encoding process.	9
	Improve error handling during hex string decoding ___ Add error handling for potential decoding failures when parsing the hex string to ensure the application does not crash and provides a meaningful error message. [packages/substrate_metadata/lib/metadata/common.dart [18-26]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-744e37ecda229197b2bb42eae309e7de9faac25e2f15536323a7aa91c2f2c4bcR18-R26) ```diff -if (hexString.startsWith('0x')) { - rawData = hex.decode(hexString.substring(2)); -} else { - rawData = hex.decode(hexString); +try { + if (hexString.startsWith('0x')) { + rawData = hex.decode(hexString.substring(2)); + } else { + rawData = hex.decode(hexString); + } + final input = ByteInput.fromBytes(rawData); + return codec.decode(input); +} catch (e) { + throw Exception('Failed to decode hex string: $e'); } -final input = ByteInput.fromBytes(rawData); -return codec.decode(input); ``` Suggestion importance[1-10]: 8 Why: Adding error handling for hex string decoding is crucial for preventing application crashes and providing meaningful error messages. This suggestion significantly enhances the robustness of the code by handling potential exceptions.	8
	Improve error handling for empty or invalid JSON maps in deserialization ___ Consider handling the case where the 'json' map is empty or does not contain expected keys in `TypeDef.fromJson`. This will prevent runtime errors when keys are missing. [packages/substrate_metadata/lib/scale_info/type_definition.dart [11]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-dfd2daaf996cb19132ec190538042c9be7a1c3c08473db576068e53cadc284b7R11-R11) ```diff -final String typeName = json.keys.first; +final String typeName = json.keys.isNotEmpty ? json.keys.first : throw Exception('JSON map is empty or invalid'); ``` Suggestion importance[1-10]: 8 Why: This suggestion improves error handling by checking if the JSON map is empty, which prevents runtime errors and provides a clear exception message. It addresses a potential bug effectively.	8
	Ensure safe casting of `decodedMetadata.metadata` to `RuntimeMetadataV14` ___ Ensure that the `decodedMetadata.metadata` is properly cast to `RuntimeMetadataV14` before using it, as the current implementation assumes the cast is always valid without checking. [packages/polkadart/lib/apis/state.dart [22-28]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-f5ab972bc621b57009d162aa0cb6627bf86b284f72ed51df6970b6c35d616e54R22-R28) ```diff final decodedMetadata = await api.getMetadata(); if (decodedMetadata.metadata is! RuntimeMetadataV14) { await provider.disconnect(); throw Exception('Only metadata version 14 is supported'); } +final metadataV14 = decodedMetadata.metadata as RuntimeMetadataV14; ``` Suggestion importance[1-10]: 7 Why: The suggestion enhances code safety by explicitly casting `decodedMetadata.metadata` to `RuntimeMetadataV14` after verifying its type, which prevents potential runtime errors. This is a good practice for ensuring type safety and improving code reliability.	7
	Prevent potential null dereference by adding null checks or assertions ___ Add null checks or assertions for the `input` parameter in the `decode` method to prevent potential runtime errors when `input` is null. [packages/substrate_metadata/lib/metadata/v14.dart [44-54]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-9d886c70250a98c868358affc2aa9f1c90113c965258dbcfe065bbd693ec37e5R44-R54) ```diff RuntimeMetadataV14 decode(Input input) { + assert(input != null, 'Input cannot be null'); final types = SequenceCodec(PortableType.codec).decode(input); ... } ``` Suggestion importance[1-10]: 7 Why: Adding an assertion to check for null input is a good practice to prevent runtime errors. This change enhances the robustness of the code by ensuring that the input is not null before proceeding with the decode operation.	7
	Ensure non-null JSON input in deserialization methods to prevent errors ___ Add null checks for the 'json' parameter in the `fromJson` methods to ensure that a non-null map is always processed, preventing potential `NullPointerExceptions`. [packages/substrate_metadata/lib/scale_info/type_definition.dart [10]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-dfd2daaf996cb19132ec190538042c9be7a1c3c08473db576068e53cadc284b7R10-R10) ```diff factory TypeDef.fromJson(Map json) { + if (json == null) throw ArgumentError('json cannot be null'); ``` Suggestion importance[1-10]: 7 Why: Adding a null check for the JSON parameter is a good practice to prevent null pointer exceptions. It enhances the robustness of the code by ensuring that the input is valid.	7
Maintainability	Refactor duplicated metadata fetching logic into a separate method ___ Refactor the repeated logic for fetching and validating `RuntimeMetadataV14` into a separate method to avoid code duplication and enhance maintainability. [packages/polkadart_cli/bin/get_metadata.dart [19-32]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-57a3a4cde8ec9ce2a8917067a7e2c5ce10a5f04c64cfd3e3f47046ed1c1da8c6R19-R32) ```diff -final decodedMetadata = await api.getMetadata(); -if (decodedMetadata.metadata is! RuntimeMetadataV14) { - await provider.disconnect(); - throw Exception('Only metadata version 14 is supported'); -} -final version = await api.getRuntimeVersion(); -await provider.disconnect(); -return ChainProperties( - decodedMetadata.metadata as RuntimeMetadataV14, - version, -); +return fetchAndValidateMetadataV14(api); ``` Suggestion importance[1-10]: 6 Why: The suggestion promotes code maintainability by refactoring repeated logic into a separate method. This reduces code duplication and makes future updates easier, although it does not address any critical issues.	6
Enhancement	Enhance error handling in `TypeDef.fromJson` to manage unknown types more gracefully ___ Consider implementing a more robust type checking and error handling for the `TypeDef.fromJson` factory method to manage different or unexpected `typeName` values gracefully. [packages/polkadart_cli/lib/src/scale_info/scale_info.dart [30-70]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-2ed09829411ca55efc21144478ed2c2e1ebd6e39abef6d4bf24ea119a7ca7e74R30-R70) ```diff final String typeName = json.keys.first; switch (typeName) { case 'Composite': { return TypeDefComposite.fromJson(json['Composite']); } case 'Variant': { return TypeDefVariant.fromJson(json['Variant']); } ... default: { - throw Exception('Unknown primitive type $typeName'); + logError('Received unknown type name: $typeName'); + return null; // or a default TypeDef } } ``` Suggestion importance[1-10]: 5 Why: The suggestion aims to improve error handling by logging unknown type names instead of throwing exceptions. While it enhances robustness, the proposed solution may lead to silent failures, which could be problematic if not handled properly.	5
Enhancement	Improve error handling by providing more descriptive error messages ___ Consider handling the case where `input.read()` returns a value outside the expected range by providing a more descriptive error message or a different error handling strategy. [packages/substrate_metadata/lib/metadata/v14.dart [160-178]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-9d886c70250a98c868358affc2aa9f1c90113c965258dbcfe065bbd693ec37e5R160-R178) ```diff final index = input.read(); switch (index) { case 0: return StorageHasher.blake2_128; ... default: - throw Exception('Unknown StorageHasher variant index $index'); + throw Exception('Unknown StorageHasher variant index $index. Please check the input source or data integrity.'); } ``` Suggestion importance[1-10]: 5 Why: The suggestion provides a slightly more descriptive error message, which can help with debugging. However, the improvement is marginal as it only adds a generic suggestion to check input source or data integrity.	5

leonardocustodio commented 4 weeks ago

Also please bump substrate_metadata to v2.0.0 as that will be a breaking change from what we talked about

Category	Suggestion	Score
Possible bug	Correct the method call to pass the output stream in encoding ___ In the `encodeTo` method of `$TypeDefCompositeCodec`, ensure that the `encode` method of `$TypeDefCompositeCodec` is called with the `output` parameter to correctly pass the output stream. [packages/substrate_metadata/lib/scale_info/type_definition.dart [115]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-dfd2daaf996cb19132ec190538042c9be7a1c3c08473db576068e53cadc284b7R115-R115) ```diff -$TypeDefCompositeCodec._().encode(value as TypeDefComposite); +$TypeDefCompositeCodec._().encodeTo(value as TypeDefComposite, output); ``` Suggestion importance[1-10]: 9 Why: This suggestion corrects a potential bug by ensuring the correct method is called with the necessary parameters, which is crucial for the proper functioning of the encoding process.	9
	Improve error handling during hex string decoding ___ Add error handling for potential decoding failures when parsing the hex string to ensure the application does not crash and provides a meaningful error message. [packages/substrate_metadata/lib/metadata/common.dart [18-26]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-744e37ecda229197b2bb42eae309e7de9faac25e2f15536323a7aa91c2f2c4bcR18-R26) ```diff -if (hexString.startsWith('0x')) { - rawData = hex.decode(hexString.substring(2)); -} else { - rawData = hex.decode(hexString); +try { + if (hexString.startsWith('0x')) { + rawData = hex.decode(hexString.substring(2)); + } else { + rawData = hex.decode(hexString); + } + final input = ByteInput.fromBytes(rawData); + return codec.decode(input); +} catch (e) { + throw Exception('Failed to decode hex string: $e'); } -final input = ByteInput.fromBytes(rawData); -return codec.decode(input); ``` Suggestion importance[1-10]: 8 Why: Adding error handling for hex string decoding is crucial for preventing application crashes and providing meaningful error messages. This suggestion significantly enhances the robustness of the code by handling potential exceptions.	8
	Improve error handling for empty or invalid JSON maps in deserialization ___ Consider handling the case where the 'json' map is empty or does not contain expected keys in `TypeDef.fromJson`. This will prevent runtime errors when keys are missing. [packages/substrate_metadata/lib/scale_info/type_definition.dart [11]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-dfd2daaf996cb19132ec190538042c9be7a1c3c08473db576068e53cadc284b7R11-R11) ```diff -final String typeName = json.keys.first; +final String typeName = json.keys.isNotEmpty ? json.keys.first : throw Exception('JSON map is empty or invalid'); ``` Suggestion importance[1-10]: 8 Why: This suggestion improves error handling by checking if the JSON map is empty, which prevents runtime errors and provides a clear exception message. It addresses a potential bug effectively.	8
	Ensure safe casting of `decodedMetadata.metadata` to `RuntimeMetadataV14` ___ Ensure that the `decodedMetadata.metadata` is properly cast to `RuntimeMetadataV14` before using it, as the current implementation assumes the cast is always valid without checking. [packages/polkadart/lib/apis/state.dart [22-28]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-f5ab972bc621b57009d162aa0cb6627bf86b284f72ed51df6970b6c35d616e54R22-R28) ```diff final decodedMetadata = await api.getMetadata(); if (decodedMetadata.metadata is! RuntimeMetadataV14) { await provider.disconnect(); throw Exception('Only metadata version 14 is supported'); } +final metadataV14 = decodedMetadata.metadata as RuntimeMetadataV14; ``` Suggestion importance[1-10]: 7 Why: The suggestion enhances code safety by explicitly casting `decodedMetadata.metadata` to `RuntimeMetadataV14` after verifying its type, which prevents potential runtime errors. This is a good practice for ensuring type safety and improving code reliability.	7
	Prevent potential null dereference by adding null checks or assertions ___ Add null checks or assertions for the `input` parameter in the `decode` method to prevent potential runtime errors when `input` is null. [packages/substrate_metadata/lib/metadata/v14.dart [44-54]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-9d886c70250a98c868358affc2aa9f1c90113c965258dbcfe065bbd693ec37e5R44-R54) ```diff RuntimeMetadataV14 decode(Input input) { + assert(input != null, 'Input cannot be null'); final types = SequenceCodec(PortableType.codec).decode(input); ... } ``` Suggestion importance[1-10]: 7 Why: Adding an assertion to check for null input is a good practice to prevent runtime errors. This change enhances the robustness of the code by ensuring that the input is not null before proceeding with the decode operation.	7
	Ensure non-null JSON input in deserialization methods to prevent errors ___ Add null checks for the 'json' parameter in the `fromJson` methods to ensure that a non-null map is always processed, preventing potential `NullPointerExceptions`. [packages/substrate_metadata/lib/scale_info/type_definition.dart [10]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-dfd2daaf996cb19132ec190538042c9be7a1c3c08473db576068e53cadc284b7R10-R10) ```diff factory TypeDef.fromJson(Map json) { + if (json == null) throw ArgumentError('json cannot be null'); ``` Suggestion importance[1-10]: 7 Why: Adding a null check for the JSON parameter is a good practice to prevent null pointer exceptions. It enhances the robustness of the code by ensuring that the input is valid.	7
Maintainability	Refactor duplicated metadata fetching logic into a separate method ___ Refactor the repeated logic for fetching and validating `RuntimeMetadataV14` into a separate method to avoid code duplication and enhance maintainability. [packages/polkadart_cli/bin/get_metadata.dart [19-32]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-57a3a4cde8ec9ce2a8917067a7e2c5ce10a5f04c64cfd3e3f47046ed1c1da8c6R19-R32) ```diff -final decodedMetadata = await api.getMetadata(); -if (decodedMetadata.metadata is! RuntimeMetadataV14) { - await provider.disconnect(); - throw Exception('Only metadata version 14 is supported'); -} -final version = await api.getRuntimeVersion(); -await provider.disconnect(); -return ChainProperties( - decodedMetadata.metadata as RuntimeMetadataV14, - version, -); +return fetchAndValidateMetadataV14(api); ``` Suggestion importance[1-10]: 6 Why: The suggestion promotes code maintainability by refactoring repeated logic into a separate method. This reduces code duplication and makes future updates easier, although it does not address any critical issues.	6
Enhancement	Enhance error handling in `TypeDef.fromJson` to manage unknown types more gracefully ___ Consider implementing a more robust type checking and error handling for the `TypeDef.fromJson` factory method to manage different or unexpected `typeName` values gracefully. [packages/polkadart_cli/lib/src/scale_info/scale_info.dart [30-70]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-2ed09829411ca55efc21144478ed2c2e1ebd6e39abef6d4bf24ea119a7ca7e74R30-R70) ```diff final String typeName = json.keys.first; switch (typeName) { case 'Composite': { return TypeDefComposite.fromJson(json['Composite']); } case 'Variant': { return TypeDefVariant.fromJson(json['Variant']); } ... default: { - throw Exception('Unknown primitive type $typeName'); + logError('Received unknown type name: $typeName'); + return null; // or a default TypeDef } } ``` Suggestion importance[1-10]: 5 Why: The suggestion aims to improve error handling by logging unknown type names instead of throwing exceptions. While it enhances robustness, the proposed solution may lead to silent failures, which could be problematic if not handled properly.	5
Enhancement	Improve error handling by providing more descriptive error messages ___ Consider handling the case where `input.read()` returns a value outside the expected range by providing a more descriptive error message or a different error handling strategy. [packages/substrate_metadata/lib/metadata/v14.dart [160-178]](https://github.com/leonardocustodio/polkadart/pull/487/files#diff-9d886c70250a98c868358affc2aa9f1c90113c965258dbcfe065bbd693ec37e5R160-R178) ```diff final index = input.read(); switch (index) { case 0: return StorageHasher.blake2_128; ... default: - throw Exception('Unknown StorageHasher variant index $index'); + throw Exception('Unknown StorageHasher variant index $index. Please check the input source or data integrity.'); } ``` Suggestion importance[1-10]: 5 Why: The suggestion provides a slightly more descriptive error message, which can help with debugging. However, the improvement is marginal as it only adds a generic suggestion to check input source or data integrity.	5

leonardocustodio / polkadart