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

Fix Runtime V14 scale codec parser #487

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Lohann
Copy link
Collaborator

@Lohann Lohann commented Oct 10, 2024

User description

  • Fix Runtime V14 codec parser based on the official frame-metadata format.
  • Parser and Encode metadata from bytes.
  • 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
    Copy link

    CLAassistant commented Oct 10, 2024

    CLA assistant check
    All committers have signed the CLA.

    Copy link

    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.

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    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]

    -$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]

    -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]

    -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]

     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]

     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]

     factory TypeDef.fromJson(Map<String, dynamic> 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]

    -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]

     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
    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]

     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
    Copy link
    Owner

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

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    3 participants