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

Fixes polkadart examples #515

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Fixes polkadart examples #515

wants to merge 2 commits into from

Conversation

leonardocustodio
Copy link
Owner

@leonardocustodio leonardocustodio commented Jan 31, 2025

User description

@justkawal I suggest doing this for now, seems to be the fastest way and that will not break everything you have done. Refactoring can be done later.


PR Type

Enhancement, Tests


Description

  • Added new example scripts for demonstrating Polkadart functionalities.

  • Introduced support for RuntimeMetadata in Registry class.

  • Updated dependencies to include substrate_metadata.

  • Added dependency overrides for substrate_metadata.


Changes walkthrough 📝

Relevant files
Tests
encointer_extrinsic_demo.dart
Added Encointer extrinsic demo example                                     

examples/lib/encointer_extrinsic_demo.dart

  • Added a new example demonstrating ChargeAssetTxPayment signed
    extension.
  • Showcased usage of custom signed extensions with TypeRegistry.
  • Included detailed steps for setting up and running the example.
  • +141/-0 
    extrinsic_demo.dart
    Added general extrinsic demo example                                         

    examples/lib/extrinsic_demo.dart

  • Added a new example for creating and submitting extrinsics.
  • Demonstrated wallet creation and fund transfer between wallets.
  • Included custom RPC call and signed extension examples.
  • +274/-0 
    get_account_infos_demo.dart
    Added account information retrieval demo                                 

    examples/lib/get_account_infos_demo.dart

  • Added a new example for fetching account information.
  • Demonstrated decoding of account storage keys and fetching
    AccountInfo.
  • +33/-0   
    Enhancement
    core.dart
    Added RuntimeMetadata support in core library                       

    packages/polkadart_scale_codec/lib/core/core.dart

  • Imported RuntimeMetadata from substrate_metadata.
  • Updated core library to support metadata-based registry
    initialization.
  • +2/-0     
    registry.dart
    Enhanced Registry with RuntimeMetadata constructor             

    packages/polkadart_scale_codec/lib/core/registry.dart

  • Added Registry.fromRuntimeMetadata constructor.
  • Enabled initialization of registry using runtime metadata.
  • +7/-0     
    Dependencies
    pubspec.yaml
    Updated dependencies to include substrate_metadata             

    packages/polkadart_scale_codec/pubspec.yaml

    • Added substrate_metadata as a dependency.
    +1/-0     
    Configuration changes
    pubspec_overrides.yaml
    Added dependency override for substrate_metadata                 

    packages/polkadart_scale_codec/pubspec_overrides.yaml

    • Added dependency override for substrate_metadata.
    +4/-0     

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • 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

    Possible Issue

    The ChargeAssetTxPayment signed extension is being used with a custom asset_id. Ensure that the Option.some(paymentAsset.toJson()) implementation is correct and compatible with the expected runtime behavior.

    customSignedExtensions: <String, dynamic>{
      'ChargeAssetTxPayment': {
        "tip": BigInt.zero,
        "asset_id": Option.some(paymentAsset.toJson()),
      }, // A custom Signed Extensions
    },
    Incomplete Implementation

    The Registry.fromRuntimeMetadata constructor currently has a placeholder implementation for processing runtimeMetadata.types. This needs to be reviewed and completed to avoid potential runtime issues.

    Registry.fromRuntimeMetadata(RuntimeMetadata runtimeMetadata) {
      runtimeMetadata.types.forEach((type) => {
            // This needs to be done, so we don't break the other packages for now
            // Remove the type and let it be dynamic so we don't need to depend on substrate_metadata if possible
          });
    Potential Error Handling Gap

    The extrinsic submission process does not include error handling for potential failures during submitExtrinsic. Ensure that errors are caught and logged to prevent silent failures.

    final srHash = await author.submitExtrinsic(srExtrinsic);
    print('Sr25519 extrinsic hash: ${hex.encode(srHash)}');

    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    General
    Replace blocking sleep with async delay

    Ensure the sleep function does not block the main thread by using asynchronous
    alternatives like Future.delayed.

    examples/lib/extrinsic_demo.dart [116]

    -sleep(Duration(seconds: 10));
    +await Future.delayed(Duration(seconds: 10));
    Suggestion importance[1-10]: 9

    Why: Replacing the blocking sleep function with an asynchronous alternative prevents the main thread from being blocked, improving the application's responsiveness and adhering to best practices in asynchronous programming.

    9
    Add error handling for async calls

    Add error handling for the await calls to ensure the application can gracefully
    handle potential failures, such as network issues or invalid responses.

    examples/lib/encointer_extrinsic_demo.dart [57-60]

    -final encointerRuntimeVersion = await encointerState.getRuntimeVersion();
    -final encointerSpecVersion = encointerRuntimeVersion.specVersion;
    -final encointerTransactionVersion = encointerRuntimeVersion.transactionVersion;
    +try {
    +  final encointerRuntimeVersion = await encointerState.getRuntimeVersion();
    +  final encointerSpecVersion = encointerRuntimeVersion.specVersion;
    +  final encointerTransactionVersion = encointerRuntimeVersion.transactionVersion;
    +} catch (e) {
    +  print('Error fetching runtime version: $e');
    +  return;
    +}
    Suggestion importance[1-10]: 8

    Why: Adding error handling for the async calls improves the robustness of the application by ensuring it can handle potential failures gracefully, such as network issues or invalid responses. This is a significant enhancement to the code's reliability.

    8
    Validate response from getKeysPaged

    Validate the response from getKeysPaged to handle cases where the returned keys
    might be null or empty, avoiding potential runtime errors.

    examples/lib/get_account_infos_demo.dart [13-14]

     final keys = await polkadot.rpc.state.getKeysPaged(key: accountMapPrefix, count: 10);
    +if (keys == null || keys.isEmpty) {
    +  print('No keys found.');
    +  return;
    +}
    Suggestion importance[1-10]: 7

    Why: Validating the response from getKeysPaged ensures that the application can handle cases where the returned keys are null or empty, preventing potential runtime errors. This is a useful improvement for error handling.

    7
    Possible issue
    Add null check for metadata types

    Add a safeguard to ensure runtimeMetadata.types is not null before iterating,
    preventing potential null pointer exceptions.

    packages/polkadart_scale_codec/lib/core/registry.dart [21-24]

    -runtimeMetadata.types.forEach((type) => {
    -      // This needs to be done, so we don't break the other packages for now
    -      // Remove the type and let it be dynamic so we don't need to depend on substrate_metadata if possible
    -    });
    +if (runtimeMetadata.types != null) {
    +  runtimeMetadata.types.forEach((type) => {
    +        // This needs to be done, so we don't break the other packages for now
    +        // Remove the type and let it be dynamic so we don't need to depend on substrate_metadata if possible
    +      });
    +} else {
    +  print('Runtime metadata types are null.');
    +}
    Suggestion importance[1-10]: 8

    Why: Adding a null check for runtimeMetadata.types prevents potential null pointer exceptions, enhancing the code's safety and reliability. This is a valuable safeguard for handling unexpected input.

    8

    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.

    2 participants