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

Add Support for all (Viem) Chains #103

Merged
merged 2 commits into from
Sep 2, 2024
Merged

Add Support for all (Viem) Chains #103

merged 2 commits into from
Sep 2, 2024

Conversation

bh2smith
Copy link
Contributor

@bh2smith bh2smith commented Sep 2, 2024

User description

Instead of having a finite supported chain list, we now support all chains provided by viem.


PR Type

enhancement, dependencies


Description

  • Replaced individual chain imports with a wildcard import from viem/chains.
  • Updated SUPPORTED_NETWORKS to include all chains exported by viem, allowing dynamic support for any new chains added by the viem library.

Changes walkthrough 📝

Relevant files
Enhancement
network.ts
Support all chains exported by `viem` in network configuration

src/network.ts

  • Replaced individual chain imports with a wildcard import from
    viem/chains.
  • Updated SUPPORTED_NETWORKS to include all chains exported by viem.
  • +3/-25   

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @bh2smith bh2smith changed the title Add Support for all Chains Exported by viem. Add Support for all (Viem) Chains Sep 2, 2024
    @bh2smith bh2smith merged commit 8ef54cb into main Sep 2, 2024
    1 check passed
    @bh2smith bh2smith deleted the mo-chainz branch September 2, 2024 08:59
    @mintbase-codium-pr-agent
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Dynamic Import Check
    Ensure that the dynamic import of all chains from viem/chains using wildcard import and subsequent usage in SUPPORTED_NETWORKS does not introduce any runtime errors or issues with missing or undefined chain configurations.

    @mintbase-codium-pr-agent
    Copy link

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Add validation for the 'chains' object to ensure it contains valid data

    Consider validating the chains object before using it in createNetworkMap. This
    ensures that the object is not empty and contains valid data, preventing runtime
    errors if the import fails or the data structure is not as expected.

    src/network.ts [5]

    +if (!chains || Object.keys(chains).length === 0) {
    +  throw new Error("No chains found or invalid chains object.");
    +}
     const SUPPORTED_NETWORKS = createNetworkMap(Object.values(chains));
     
    Suggestion importance[1-10]: 9

    Why: This suggestion adds a crucial validation step to ensure that the 'chains' object is not empty or invalid, which can prevent potential runtime errors. This is a significant improvement in terms of robustness and error handling.

    9
    Possible bug
    Handle cases where 'Object.values(chains)' might return an empty array in 'createNetworkMap'

    Ensure that the createNetworkMap function handles cases where Object.values(chains)
    might return an empty array, which could lead to unexpected behavior if not handled
    properly.

    src/network.ts [5]

    -const SUPPORTED_NETWORKS = createNetworkMap(Object.values(chains));
    +const networkValues = Object.values(chains);
    +const SUPPORTED_NETWORKS = networkValues.length > 0 ? createNetworkMap(networkValues) : {};
     
    Suggestion importance[1-10]: 8

    Why: This suggestion ensures that the 'createNetworkMap' function handles empty arrays gracefully, preventing potential issues. It is a valuable addition for robustness.

    8
    Best practice
    Add a type definition for the 'SUPPORTED_NETWORKS' variable to enhance readability and maintain type safety

    To enhance readability and maintainability, consider adding a type definition for
    the SUPPORTED_NETWORKS variable. This will help developers understand what kind of
    data structure they are working with and ensure type safety.

    src/network.ts [5]

    -const SUPPORTED_NETWORKS = createNetworkMap(Object.values(chains));
    +type NetworkMap = Record<string, Chain>;
    +const SUPPORTED_NETWORKS: NetworkMap = createNetworkMap(Object.values(chains));
     
    Suggestion importance[1-10]: 7

    Why: Adding a type definition improves code readability and maintainability by making the data structure explicit. This is a good practice but not as critical as runtime validation.

    7
    Enhancement
    Add a unit test to verify the integrity and completeness of the 'SUPPORTED_NETWORKS' map

    To ensure that all networks are correctly supported and initialized, consider adding
    a unit test that verifies the integrity and completeness of the SUPPORTED_NETWORKS
    map.

    src/network.ts [5]

    -const SUPPORTED_NETWORKS = createNetworkMap(Object.values(chains));
    +// Example unit test
    +describe('Network Map Initialization', () => {
    +  it('should correctly initialize all supported networks', () => {
    +    const expectedNetworks = Object.values(chains);
    +    const actualNetworks = Object.keys(SUPPORTED_NETWORKS);
    +    expect(actualNetworks.sort()).toEqual(expectedNetworks.sort());
    +  });
    +});
     
    Suggestion importance[1-10]: 6

    Why: Adding a unit test is beneficial for ensuring the integrity of the 'SUPPORTED_NETWORKS' map. While it is an enhancement for testing, it is not as critical as runtime checks or type definitions.

    6

    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.

    1 participant