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

feat: add a new chain #1553

Merged
merged 40 commits into from
Sep 28, 2024
Merged

feat: add a new chain #1553

merged 40 commits into from
Sep 28, 2024

Conversation

Nick-1979
Copy link
Member

@Nick-1979 Nick-1979 commented Sep 22, 2024

closes #1549

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced the ability to add new blockchain chains via the AddNewChain component.
    • Added a new context, UserAddedChainContext, to manage user-defined blockchain endpoints.
    • Enhanced routing with a new /addNewChain/ path for easier navigation.
  • Improvements

    • Updated user interface to include a button for adding new chains in the ChainList component.
    • Improved handling of user-added endpoints and their associated properties.
    • Enhanced user prompts and instructions for entering RPC endpoints and token price IDs.
  • Localization

    • Added multiple new strings to support the new features and improve user guidance in English, French, Hindi, Russian, and Chinese.
  • Bug Fixes

    • Enhanced validation for WebSocket Secure URLs.
    • Improved error handling for parsing user-added endpoints.

@Nick-1979 Nick-1979 added the WIP Work In Progress label Sep 22, 2024
Copy link
Contributor

coderabbitai bot commented Sep 22, 2024

Walkthrough

The changes introduce a new feature in the PolkaGate extension that allows users to add custom blockchain endpoints. Key updates include the addition of a new context for managing user-added chains, the implementation of a component for adding new chains, and the introduction of utility hooks for retrieving and managing these endpoints. The routing system has also been updated to accommodate navigation to the new feature.

Changes

File Path Change Summary
packages/extension-polkagate/src/fullscreen/addNewChain/index.tsx Introduced AddNewChain component for adding blockchain endpoints with user input handling.
packages/extension-polkagate/src/util/types.ts Introduced UserAddedEndpoint interface and UserAddedChains type for user-defined endpoints.
packages/extension-ui/src/Popup/index.tsx Added state for userAddedChainCtx, integrated UserAddedChainContext, and introduced a new route for adding chains.
packages/extension-polkagate/src/fullscreen/homeFullScreen/components/ChainList.tsx Updated ChainList to include a button for adding new chains and adjusted layout and styling.
packages/extension/public/locales/en/translation.json Added new localization strings related to adding new chains and user guidance.
packages/extension/public/locales/fr/translation.json Added French translations for new strings related to adding chains.
packages/extension/public/locales/hi/translation.json Added Hindi translations for new strings related to adding chains.
packages/extension/public/locales/ru/translation.json Added Russian translations for new strings related to adding chains.
packages/extension/public/locales/zh/translation.json Added Chinese translations for new strings related to adding chains.

Assessment against linked issues

Objective Addressed Explanation
Offering an option for chain developers to easily add their chain specifications and endpoints directly into the extension. (Issue #1549)
Ensure inputs are thoroughly validated to prevent breaking the extension. (Issue #1549) Validation mechanisms are not explicitly detailed in the changes.

Possibly related PRs

Suggested labels

test

Suggested reviewers

  • AMIRKHANEF

🐰 In the meadow of code, new chains take flight,
With endpoints to add, our future is bright.
A context for users, a button to share,
PolkaGate's magic is blooming with care!
Let's hop to the rhythm of changes we make,
Together we build, for innovation's sake! 🌟


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 3cbfe28 and eb9e6fb.

📒 Files selected for processing (1)
  • packages/extension-polkagate/src/fullscreen/addNewChain/index.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/extension-polkagate/src/fullscreen/addNewChain/index.tsx

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

chore: remove unused
@Nick-1979 Nick-1979 added new feature and removed WIP Work In Progress labels Sep 22, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 15

Outside diff range and nitpick comments (3)
packages/extension-polkagate/src/hooks/useTokens.ts (1)

12-12: Nitpick: Space added before the opening parenthesis.

Adding a space before the opening parenthesis in the function signature is a matter of coding style preference and does not affect the functionality.

packages/extension-polkagate/src/fullscreen/addNewChain/index.tsx (2)

125-125: Remove console.log statements from production code

Leaving console.log statements can expose internal state and clutter the console. It's better to remove them or replace them with proper logging mechanisms.

Apply this diff to remove the log:

-          console.log('metadata:', metadata);

135-135: Remove commented-out code to maintain code cleanliness

Commented-out code can make the codebase harder to read and maintain. If the code isn't needed, it's better to remove it.

Apply this diff:

-          // setVerifiedEndpoint((prev) => ([...prev, endpoint]));
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 082c316 and b55caa4.

Files ignored due to path filters (9)
  • packages/extension-polkagate/src/assets/icons/BTC.svg is excluded by !**/*.svg
  • packages/extension-polkagate/src/assets/icons/DAI.svg is excluded by !**/*.svg
  • packages/extension-polkagate/src/assets/icons/LDOT.svg is excluded by !**/*.svg
  • packages/extension-polkagate/src/assets/icons/USDC.svg is excluded by !**/*.svg
  • packages/extension-polkagate/src/assets/icons/USDT.svg is excluded by !**/*.svg
  • packages/extension-polkagate/src/assets/icons/assetHub.svg is excluded by !**/*.svg
  • packages/extension-polkagate/src/assets/icons/lcDOT.svg is excluded by !**/*.svg
  • packages/extension-polkagate/src/assets/img/endpoint.png is excluded by !**/*.png
  • packages/extension-polkagate/src/fullscreen/addNewChain/endpoint.png is excluded by !**/*.png
Files selected for processing (48)
  • packages/extension-polkagate/src/assets/icons/index.ts (0 hunks)
  • packages/extension-polkagate/src/assets/img/index.ts (1 hunks)
  • packages/extension-polkagate/src/assets/logos/index.ts (1 hunks)
  • packages/extension-polkagate/src/components/AssetDualLogo.tsx (1 hunks)
  • packages/extension-polkagate/src/components/AssetLogo.tsx (1 hunks)
  • packages/extension-polkagate/src/components/ChainLogo.tsx (1 hunks)
  • packages/extension-polkagate/src/components/DisplayLogo.tsx (0 hunks)
  • packages/extension-polkagate/src/components/FullscreenChain.tsx (4 hunks)
  • packages/extension-polkagate/src/components/InputWithLabel.tsx (4 hunks)
  • packages/extension-polkagate/src/components/Label.tsx (2 hunks)
  • packages/extension-polkagate/src/components/Select2.tsx (8 hunks)
  • packages/extension-polkagate/src/components/TwoButtons.tsx (1 hunks)
  • packages/extension-polkagate/src/components/index.ts (1 hunks)
  • packages/extension-polkagate/src/fullscreen/accountDetails/components/AOC.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/accountDetails/components/AccountInformationForDetails.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/accountDetails/components/TotalChart.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx (1 hunks)
  • packages/extension-polkagate/src/fullscreen/addNewChain/ShowChainInfo.tsx (1 hunks)
  • packages/extension-polkagate/src/fullscreen/addNewChain/index.tsx (1 hunks)
  • packages/extension-polkagate/src/fullscreen/addNewChain/types.tsx (1 hunks)
  • packages/extension-polkagate/src/fullscreen/addNewChain/utils.tsx (1 hunks)
  • packages/extension-polkagate/src/fullscreen/governance/utils/util.ts (8 hunks)
  • packages/extension-polkagate/src/fullscreen/homeFullScreen/components/ChainItem.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/homeFullScreen/components/ChainList.tsx (5 hunks)
  • packages/extension-polkagate/src/fullscreen/homeFullScreen/partials/TotalBalancePieChart.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/homeFullScreen/partials/WatchList.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/partials/Bread.tsx (4 hunks)
  • packages/extension-polkagate/src/fullscreen/sendFund/Review.tsx (2 hunks)
  • packages/extension-polkagate/src/fullscreen/stake/solo/unstake/index.tsx (1 hunks)
  • packages/extension-polkagate/src/hooks/useAssetsBalances.ts (7 hunks)
  • packages/extension-polkagate/src/hooks/useBalances.ts (2 hunks)
  • packages/extension-polkagate/src/hooks/useDecimal.ts (1 hunks)
  • packages/extension-polkagate/src/hooks/useEndpoints.ts (3 hunks)
  • packages/extension-polkagate/src/hooks/useMetadata.ts (0 hunks)
  • packages/extension-polkagate/src/hooks/useToken.ts (1 hunks)
  • packages/extension-polkagate/src/hooks/useTokens.ts (1 hunks)
  • packages/extension-polkagate/src/partials/FullScreenChainSwitch.tsx (5 hunks)
  • packages/extension-polkagate/src/partials/RecentChains.tsx (2 hunks)
  • packages/extension-polkagate/src/popup/account/index.tsx (8 hunks)
  • packages/extension-polkagate/src/util/getChainName.ts (2 hunks)
  • packages/extension-polkagate/src/util/getLogo.ts (2 hunks)
  • packages/extension-polkagate/src/util/utils.ts (1 hunks)
  • packages/extension-polkagate/src/util/workers/getAssetOnAssetHub.js (3 hunks)
  • packages/extension-polkagate/src/util/workers/getAssetOnMultiAssetChain.js (3 hunks)
  • packages/extension-polkagate/src/util/workers/getAssetOnRelayChain.js (3 hunks)
  • packages/extension-polkagate/src/util/workers/utils/getChainEndpoints.js (1 hunks)
  • packages/extension-ui/src/Popup/index.tsx (2 hunks)
  • packages/extension/public/locales/en/translation.json (1 hunks)
Files not reviewed due to no reviewable changes (3)
  • packages/extension-polkagate/src/assets/icons/index.ts
  • packages/extension-polkagate/src/components/DisplayLogo.tsx
  • packages/extension-polkagate/src/hooks/useMetadata.ts
Files skipped from review due to trivial changes (6)
  • packages/extension-polkagate/src/components/Label.tsx
  • packages/extension-polkagate/src/components/TwoButtons.tsx
  • packages/extension-polkagate/src/fullscreen/accountDetails/index.tsx
  • packages/extension-polkagate/src/fullscreen/governance/utils/util.ts
  • packages/extension-polkagate/src/fullscreen/stake/solo/unstake/index.tsx
  • packages/extension-polkagate/src/popup/account/index.tsx
Additional comments not posted (108)
packages/extension-polkagate/src/assets/img/index.ts (1)

1-5: LGTM!

The file follows a standard structure for exporting assets and includes the necessary copyright and license information. The // @ts-nocheck directive is used appropriately to disable TypeScript checking for the image import.

packages/extension-polkagate/src/fullscreen/addNewChain/types.tsx (1)

4-9: LGTM!

The UserAddedEndpoint interface is well-defined and provides a clear contract for the data structure expected when users add new blockchain endpoints. The property names are clear and concise, and their types are appropriate for their intended use.

This interface promotes type safety, code reuse, and maintainability.

packages/extension-polkagate/src/assets/logos/index.ts (1)

4-9: LGTM!

The changes look good:

  • The reordering of the exports does not affect the functionality.
  • Enabling TypeScript type checking by commenting out the //@ts-nocheck directive is a good practice to ensure type safety.
packages/extension-polkagate/src/hooks/useTokens.ts (1)

7-7: LGTM!

Reordering the import statements improves code organization and readability. Good practice to group related imports together.

packages/extension-polkagate/src/hooks/useDecimal.ts (1)

25-26: LGTM! The changes improve the reliability of the useDecimal hook.

The updated return logic provides a fallback value for the token decimals if the decimals array is empty or doesn't exist in the network object. This ensures that the hook always returns a value for the token decimals, either from the network object or the chain object.

The addition of chain?.tokenDecimals to the dependency array of the useMemo hook is also a good change. It ensures that the memoized value is recomputed whenever the token decimals change in the chain object, keeping the returned value up to date.

Overall, these changes improve the reliability and correctness of the useDecimal hook.

packages/extension-polkagate/src/util/getChainName.ts (4)

4-4: LGTM!

The import statement is correct and the imported type is used in the updated function signature.


22-23: LGTM!

The logic to find the chain name from allChains using the provided _genesisHash is correct. Assigning the result to a separate variable chainName allows for implementing fallback logic in case no match is found.


24-26: LGTM!

The fallback logic to find the chain name from genesisOptions by matching the value property with _genesisHash is correct. This enhances the function's capability to derive the chain name from an additional source, making it more flexible.


28-28: LGTM!

Using the potentially updated chainName variable in the return statement is correct. The sanitizeChainName and toCamelCase functions are correctly applied before returning the final result.

packages/extension-polkagate/src/components/AssetDualLogo.tsx (2)

16-16: LGTM!

The function name change from AssetIcon to AssetDualLogo is more descriptive of the component's purpose.


16-16: Verify the usage of this component in the codebase.

Ensure that all import statements and usages of this component have been updated to reflect the new name.

Run the following script to verify the component usage:

Verification successful

Component AssetDualLogo usage verified successfully. All import statements and usages have been updated to the new name.

  • Verified in:
    • packages/extension-polkagate/src/components/index.ts
    • packages/extension-polkagate/src/components/AssetLogo.tsx
    • packages/extension-polkagate/src/components/AssetDualLogo.tsx

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all import statements and usages of the `AssetDualLogo` component.

# Test: Search for the old component name. Expect: No occurrences.
rg --type typescript $'AssetIcon'

# Test: Search for the new component name. Expect: Only valid usages.
rg --type typescript -A 5 $'AssetDualLogo'

Length of output: 144


Script:

#!/bin/bash
# Description: Verify all import statements and usages of the `AssetDualLogo` component.

# Test: Search for the old component name. Expect: No occurrences.
rg --type ts --type tsx 'AssetIcon'

# Test: Search for the new component name. Expect: Only valid usages.
rg --type ts --type tsx -A 5 'AssetDualLogo'

Length of output: 136


Script:

#!/bin/bash
# Description: Verify all import statements and usages of the `AssetDualLogo` component.

# Test: Search for the old component name. Expect: No occurrences.
rg --glob '*.ts' --glob '*.tsx' 'AssetIcon'

# Test: Search for the new component name. Expect: Only valid usages.
rg --glob '*.ts' --glob '*.tsx' -A 5 'AssetDualLogo'

Length of output: 2463

packages/extension-polkagate/src/util/workers/utils/getChainEndpoints.js (4)

6-7: LGTM!

The import statement is correct and the imported function is used appropriately in the code.


8-13: LGTM!

The JSDoc comment has been updated correctly to reflect the new optional parameter and its type.


16-18: LGTM!

The updated filter logic is correct and handles the case when endpoint.info is undefined. The conditions are logically sound.


19-26: LGTM!

The new logic for handling user-added endpoints is implemented correctly:

  • It searches for a matching endpoint based on the sanitized chain name.
  • If a match is found, it constructs an endpoint object with the correct structure.
  • The return value is updated to accommodate the new logic.

The use of sanitizeChainName ensures a consistent comparison between the chain names.

packages/extension-polkagate/src/components/AssetLogo.tsx (1)

20-44: LGTM!

The AssetLogo component is well-structured and follows best practices:

  • It has a clear separation of concerns, rendering either AssetDualLogo or ChainLogo based on the provided props.
  • It uses the useGenesisHashOptions hook for code reusability and maintainability.
  • It is wrapped in React.memo for performance optimization.
  • It uses TypeScript for type safety.
  • It follows a consistent naming convention and has a clear interface defined using the Props interface.
  • It uses a ternary operator for concise and readable conditional rendering.
  • It provides default values for some props as fallback values.
  • It uses the sanitizeChainName utility function for data validation and consistency.

Great job!

packages/extension-polkagate/src/util/getLogo.ts (1)

Line range hint 15-52: LGTM!

The changes to the getLogo function are primarily formatting and minor simplifications that improve readability and maintainability without altering the functionality. The changes are safe and can be approved.

packages/extension-polkagate/src/fullscreen/addNewChain/utils.tsx (3)

12-26: LGTM!

The custom hook UseUserAddedEndpoints is implemented correctly. It retrieves user-added endpoints from storage, updates the state, and handles the case when no data is found.


28-40: LGTM!

The custom hook UseUserAddedEndpoint is implemented correctly. It utilizes the UseUserAddedEndpoints hook to access the endpoints, searches for a matching endpoint based on the provided genesis hash, and returns a dropdown option if found.


42-53: LGTM!

The custom hook UseUserAddedChainColor is implemented correctly. It utilizes the UseUserAddedEndpoints hook to access the endpoints, searches for a matching endpoint based on the provided _genesisHash, and returns the associated color if found.

packages/extension-polkagate/src/fullscreen/homeFullScreen/components/ChainItem.tsx (3)

7-10: LGTM!

The import statements have been correctly updated to include the ChainLogo component from the appropriate directory.


26-49: LGTM!

The rendering logic for the chain logo and name has been correctly updated:

  • The ChainLogo component is used to display the logo, taking chainName and genesisHash as props.
  • The Grid component's layout has been adjusted to ensure proper alignment and overflow handling of the chain name using flexGrow and other CSS properties.

Line range hint 53-61: LGTM!

The Switch component has been correctly moved to a separate Grid item within the component's layout.

packages/extension-polkagate/src/fullscreen/partials/Bread.tsx (4)

15-15: Formatting change looks good!

The added space before the parentheses in the function signature improves consistency and readability.


26-26: New case for 'addNewChain' path looks good!

The added case 'addNewChain' in the switch statement allows for the correct breadcrumb segment to be returned when this path is active. It is grouped with other top-level paths and maintains consistency.


37-38: Verify the event type change and its impact.

The event type for the backToTopMenu function has been changed from React.MouseEventHandler<HTMLAnchorElement> to React.MouseEvent<HTMLButtonElement, MouseEvent>. This suggests a change in the rendered element from an anchor to a button.

Please ensure that this change is intentional and that the function still behaves as expected with the new event type. Also, verify that the casting of event.target to HTMLElement is sufficient for accessing the required properties.


Line range hint 58-63: Changes to the Link component and breadcrumb rendering look good!

The Link component's component prop has been set to 'button', which aligns with the updated event type in the backToTopMenu function. This change ensures that the link is rendered as a button element, providing a consistent user experience.

The minor adjustment in the rendering logic of the breadcrumb items, specifically the placement of the Typography component, does not significantly impact the functionality and maintains readability.

packages/extension-polkagate/src/hooks/useEndpoints.ts (4)

10-10: LGTM!

The import statement is correctly added and the imported function is used later in the code.


24-24: LGTM!

The userAddedEndpoint variable is correctly initialized using the UseUserAddedEndpoint function with the genesisHash parameter. The variable is used later in the code.


64-67: LGTM!

The conditional check is correctly added to return userAddedEndpoint when the endpointOptions array is empty and userAddedEndpoint is truthy. This ensures that user-added endpoints are considered when no other endpoints are available.


69-69: LGTM!

The userAddedEndpoint variable is correctly added to the dependency array of the useEffect hook. This ensures that the effect re-runs when the value of userAddedEndpoint changes.

packages/extension-polkagate/src/components/ChainLogo.tsx (6)

6-6: LGTM!

The imports are necessary for the changes made in the file.


9-11: LGTM!

The imports are necessary for the changes made in the file.


13-13: LGTM!

The import is necessary for the changes made in the file.


19-23: LGTM!

The changes to the Props interface and ChainLogo function signature are consistent with the AI-generated summary and are necessary to support the new functionality of displaying user-added chain logos.


25-31: LGTM!

The changes improve the logic for determining the chain name and logo, and are consistent with the AI-generated summary. The use of useGenesisHashOptions hook simplifies the logic for retrieving chain options, and the filter variable consolidates the conditions for applying styles based on the theme and chain name.


35-71: LGTM!

The changes to the rendering logic are consistent with the AI-generated summary and improve the handling of the new _logo variable. The use of Avatar component for displaying the first character of the chain name when no logo is available is a good fallback approach, and the application of styles using the filter variable is consistent and improves the visual appearance of the logo.

packages/extension-polkagate/src/components/InputWithLabel.tsx (3)

8-8: LGTM!

The import statement for the isWss function is correctly added and follows the existing pattern.


48-50: LGTM!

The new validation condition for the 'wss' type and the usage of the isWss function are correctly implemented and follow the existing pattern.


31-31: Verify the component usage in the codebase.

The changes in the InputWithLabel function signature, such as the removal of className and withoutMargin props and the addition of new props, might affect the component usage across the codebase. Please ensure that all instances of the component are updated to match the new signature.

Run the following script to verify the component usage:

Verification successful

Adjust the verification script to properly search .tsx files.

Please use the following script to verify all instances of the InputWithLabel component with the updated signature:


Check for deprecated props in all InputWithLabel component instances.

Please run the following script to identify any usages of the className or withoutMargin props in the InputWithLabel component:


Verified that deprecated props are removed from all InputWithLabel usages.

All instances of the InputWithLabel component have been updated to exclude the className and withoutMargin props. No deprecated prop usages were found in the codebase.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all instances of the `InputWithLabel` component match the new signature.

# Test: Search for the component usage. Expect: No occurrences of the old props.
rg --type tsx -A 5 $'<InputWithLabel'

Length of output: 66


Script:

#!/bin/bash
# Description: Verify all instances of the `InputWithLabel` component match the new signature.

# Add support for .tsx files by defining a custom type in rg
rg '<InputWithLabel' --type-add 'tsx:*.tsx' --type tsx -A 5

Length of output: 12089


Script:

#!/bin/bash
# Description: Search for deprecated props `className` and `withoutMargin` in `InputWithLabel` component usages.

# Define the search pattern for deprecated props within InputWithLabel
rg '<InputWithLabel[^>]*\b(className|withoutMargin)\b' --type-add 'tsx:*.tsx' --type tsx

Length of output: 90

packages/extension-polkagate/src/util/workers/getAssetOnMultiAssetChain.js (1)

Line range hint 68-86: LGTM!

The changes to the onmessage event handler are consistent with the update to the getAssets function signature. Extracting the userAddedEndpoints from the received data and passing it to the getAssets function ensures that the user-specified endpoints are properly propagated.

The rest of the logic in the event handler remains unchanged, and there are no apparent issues with the updated function call or the surrounding code.

Tools
Biome

[error] 65-67: A global variable should not be reassigned.

Assigning to a global variable can override essential functionality.

(lint/suspicious/noGlobalAssign)

packages/extension-polkagate/src/util/workers/getAssetOnAssetHub.js (2)

74-74: Looks good! The changes to the onmessage event handler and the getAssetOnAssetHub function call are implemented correctly.

The destructuring of userAddedEndpoints from the event data in the onmessage event handler and the subsequent passing of this parameter to the getAssetOnAssetHub function call ensure the proper utilization of the new parameter.

These changes support the function signature update and are necessary for the correct functioning of the asset fetching process with the inclusion of user-defined endpoints.

Also applies to: 89-89


13-13: LGTM! Ensure all calls to this function are updated with the new parameter.

The addition of the userAddedEndpoints parameter enhances the flexibility of the getAssetOnAssetHub function by allowing users to provide their own endpoints. This change aligns with the PR objective of adding a new chain.

Please ensure that all calls to this function are updated to pass the new userAddedEndpoints parameter to avoid potential runtime errors.

Run the following script to verify the function usage:

Verification successful

Verified! All calls to getAssetOnAssetHub have been updated with the new userAddedEndpoints parameter.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `getAssetOnAssetHub` have been updated with the new parameter.

# Test: Search for the function usage. Expect: Only occurrences with the new parameter.
rg --type js -A 5 $'getAssetOnAssetHub'

Length of output: 2853

packages/extension-polkagate/src/fullscreen/homeFullScreen/partials/WatchList.tsx (3)

6-7: LGTM!

The reorganization of the import statement improves code clarity.


12-12: Verify the component change.

Please ensure that the AssetLogo component is thoroughly tested as a replacement for the DisplayLogo component to confirm that it behaves as expected and renders the asset logos correctly.

Do you want me to generate the unit testing code or open a GitHub issue to track this task?


37-37: Verify the component change.

Please ensure that the AssetLogo component is thoroughly tested as a replacement for the DisplayLogo component to confirm that it behaves as expected and renders the asset logos correctly.

packages/extension-polkagate/src/util/workers/getAssetOnRelayChain.js (4)

52-52: LGTM!

The userAddedEndpoints parameter is correctly passed to the getChainEndpoints function to retrieve the chain endpoints considering user-specified values. This change is implemented correctly.


88-91: LGTM!

The getAssetOnRelayChain function has been updated to:

  • Accept the userAddedEndpoints parameter.
  • Pass the userAddedEndpoints parameter to the getBalances function.

These changes maintain consistency with the updates made to the getBalances function and ensure that user-specified endpoints are considered during asset retrieval. The changes are implemented correctly.


122-130: LGTM!

The onmessage event handler has been updated to:

  • Extract the userAddedEndpoints data from the received message.
  • Pass the userAddedEndpoints value to the getAssetOnRelayChain function.

These changes ensure that the user-specified endpoints are correctly propagated to the asset retrieval process. The changes are implemented correctly and maintain consistency with the updates made to the getAssetOnRelayChain function.


51-51: Verify the function signature change in the codebase.

The addition of the userAddedEndpoints parameter to the getBalances function provides flexibility for users to specify custom endpoints. This change looks good.

However, ensure that all function calls to getBalances have been updated to pass the userAddedEndpoints argument to avoid potential runtime errors.

Run the following script to verify the function usage:

Verification successful

Function signature change for getBalances verified successfully.

All calls to getBalances now include the userAddedEndpoints argument, ensuring consistency and preventing potential runtime errors.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `getBalances` pass the `userAddedEndpoints` argument.

# Test: Search for the function usage. Expect: Only occurrences with 3 arguments.
rg --type js -A 5 $'getBalances\('

Length of output: 871

packages/extension-polkagate/src/fullscreen/addNewChain/ShowChainInfo.tsx (5)

1-40: LGTM!

The imports and interfaces are properly defined and used.


44-69: LGTM!

The component is properly defined and the hooks are correctly used to manage and update the state based on prop changes.


73-108: LGTM!

The rendering logic is properly implemented using Material-UI components and conditional rendering based on the state and props.


109-127: LGTM!

The color box rendering is properly implemented using conditional rendering and absolute positioning with a pseudo-element for the triangular shape.


132-132: LGTM!

The component is properly exported as the default export.

packages/extension-polkagate/src/components/index.ts (4)

17-17: LGTM!

The export of AssetDualLogo has been added correctly.


18-18: LGTM!

The export of AssetLogo has been added correctly.


Line range hint 1-16: Verify the removal of DisplayLogo references from the codebase.

The export of DisplayLogo has been removed. Please ensure that all references to DisplayLogo have been removed from the codebase to avoid any potential runtime errors.

Run the following script to verify the removal of DisplayLogo references:

#!/bin/bash
# Description: Verify that `DisplayLogo` is not used in the codebase.

# Test: Search for the usage of `DisplayLogo`. Expect: No occurrences.
rg --type typescript $'DisplayLogo'

Line range hint 1-16: Verify the removal of AssetIcon references from the codebase.

The export of AssetIcon has been removed. Please ensure that all references to AssetIcon have been removed from the codebase to avoid any potential runtime errors.

Run the following script to verify the removal of AssetIcon references:

packages/extension-polkagate/src/fullscreen/accountDetails/components/TotalChart.tsx (2)

15-15: Verify AssetLogo is a valid replacement for DisplayLogo.

Ensure that the AssetLogo component has equivalent or intended functionality to replace the DisplayLogo component. Verify it doesn't adversely affect the logo display.


140-140: AssetLogo usage change is linked to the previous import change.

This change in the JSX is consistent with the earlier import statement change from DisplayLogo to AssetLogo. Verify both together to ensure AssetLogo is a valid replacement.

packages/extension-polkagate/src/fullscreen/sendFund/Review.tsx (1)

92-92: LGTM!

The addition of the genesisHash prop to the ChainLogo component is a good change. It allows the component to uniquely identify the chain based on the genesis hash, even if multiple chains have the same name.

packages/extension-polkagate/src/partials/FullScreenChainSwitch.tsx (2)

69-87: LGTM!

The changes to the NetworkList component look good. Using the ChainLogo component is a nice improvement as it encapsulates the logo rendering logic and makes the code more readable.


99-99: Looks good!

The changes to the FullScreenChainSwitch component are solid:

  • Extracting genesisHash from useInfo is necessary to pass it to the ChainLogo component.
  • Using a Box to wrap the ChainLogo allows for more flexible styling while maintaining the visual appearance.

The component's functionality remains intact with these changes.

Also applies to: 128-141

packages/extension-polkagate/src/fullscreen/accountDetails/components/AOC.tsx (2)

16-16: LGTM!

The import change from DisplayLogo to AssetLogo looks good.


101-101: LGTM!

The change from the DisplayLogo component to the AssetLogo component looks good. The parameters passed to the AssetLogo component remain the same, suggesting that it is a compatible replacement.

packages/extension-polkagate/src/components/Select2.tsx (4)

6-7: LGTM!

The import reorganization and removal of unused imports improve code quality. The DropdownOption type import is relevant to the component.


14-14: LGTM!

The ChainLogo component import is a relevant addition for displaying chain logos in the component.


26-26: LGTM!

The update to the disabledItems prop type is more concise and improves type safety by explicitly defining the array elements as a union type.


Line range hint 52-197: LGTM!

The modifications to the CustomizedSelect component improve type safety, user experience, and code readability:

  • The explicit type casting in setSelectedValue and defaultValue ensures type safety.
  • The conditional IconComponent prop displays a loading indicator when items are loading, enhancing the user experience.
  • The use of ChainLogo in renderValue is consistent with the component import change and improves the rendering of chain logos.
  • The simplified logic for the disabled state of MenuItem improves code readability.
  • The conditional open prop prevents the menu from opening in certain scenarios, providing a better user experience.

Overall, the changes streamline the component's logic and improve its functionality.

packages/extension-polkagate/src/components/FullscreenChain.tsx (5)

55-55: LGTM!

The addition of the genesisHash prop to the Item component aligns with the PR objective to add a new chain. Making it a required prop ensures that it will always be provided when rendering the component.


58-60: LGTM!

Passing the genesisHash directly to the ChainLogo component is a more reliable way to ensure the correct logo is rendered for the specified chain. This change aligns with the PR objective to add a new chain.


72-72: LGTM!

The adjustment to the spacing in the FullscreenChain function signature improves code consistency and readability. While it doesn't affect the functionality, maintaining a consistent code style is important for long-term maintainability.


90-90: LGTM!

Including isTestnetEnabled in the dependency array of the useMemo hook is the correct approach. It ensures that the _disabledItems array is recalculated whenever the value of isTestnetEnabled changes, preventing stale values and keeping the disabled items in sync with the testnet configuration.


167-167: LGTM!

Setting the genesisHash prop of the Item component to selectedValue within the renderValue prop of the Select component is the correct approach. It ensures that the rendered Item component displays the logo of the currently selected chain based on the genesisHash. This change aligns with the PR objective to add a new chain.

packages/extension-polkagate/src/fullscreen/homeFullScreen/components/ChainList.tsx (5)

6-6: LGTM!

The refactor to import SavedAssets type only for local use is a good improvement.


9-9: LGTM!

The import statement has been correctly updated to include the AddCircle icon component as AddIcon.


16-19: LGTM!

The refactors to the import statements are correct and improve the code organization.


Line range hint 28-153: Great work on enhancing the ChainList component!

The changes made to the ChainList component significantly improve its functionality and user interface:

  • The addition of the "Add a new chain" button provides a convenient way for users to navigate to the '/addNewChain' route using the openOrFocusTab function.
  • The button is properly styled with an icon and positioned alongside the "Reset to default" button, maintaining a consistent look and feel.
  • The layout adjustments to the button container and typography component enhance the overall visual appearance and spacing of the elements.

These enhancements align well with the PR objectives and contribute positively to the user experience.


64-66: LGTM!

The usage of the locally imported SavedAssets type is correct and consistent with the refactor made in the import statement.

packages/extension-polkagate/src/partials/RecentChains.tsx (2)

15-15: LGTM!

The import statement for the ChainLogo component is correct.


234-234: Excellent refactoring!

The usage of the ChainLogo component simplifies the rendering logic and improves code maintainability. By encapsulating the logo display functionality within the ChainLogo component, the complexity of the RecentChains component is reduced.

This change enhances code readability and is unlikely to introduce any functional issues as long as the ChainLogo component is implemented correctly.

packages/extension-polkagate/src/fullscreen/homeFullScreen/partials/TotalBalancePieChart.tsx (3)

6-7: LGTM!

The import statement for the FetchedBalance type has been correctly uncommented to be used in the code.


167-167: LGTM!

The DisplayLogo component has been correctly replaced with AssetLogo in the JSX, and the props passed to the component remain unchanged.


15-15: Verify the component change in the codebase.

The import statement has been correctly updated to use the AssetLogo component instead of DisplayLogo.

Run the following script to verify the component usage:

Verification successful

Retry verifying the component change with a corrected script.


Retry verifying the component change with an updated script.


Component change verified successfully.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all component usages of `DisplayLogo` have been replaced with `AssetLogo`.

# Test: Search for the old component usage. Expect: No occurrences of `DisplayLogo`.
rg --type typescript -g '!node_modules' $'DisplayLogo'

# Test: Search for the new component usage. Expect: Only occurrences of `AssetLogo`.
rg --type typescript -g '!node_modules' -A 5 $'AssetLogo'

Length of output: 180


Script:

#!/bin/bash
# Description: Verify all component usages of `DisplayLogo` have been replaced with `AssetLogo`.

# Test: Search for the old component usage. Expect: No occurrences of `DisplayLogo`.
rg -g '*.ts' -g '*.tsx' 'DisplayLogo' --ignore-dir node_modules

# Test: Search for the new component usage. Expect: Only occurrences of `AssetLogo`.
rg -g '*.ts' -g '*.tsx' 'AssetLogo' --ignore-dir node_modules -A 5

Length of output: 1016


Script:

#!/bin/bash
# Description: Verify all component usages of `DisplayLogo` have been replaced with `AssetLogo`.

# Test: Search for the old component usage. Expect: No occurrences of `DisplayLogo`.
rg -g '*.ts' -g '*.tsx' 'DisplayLogo'

# Test: Search for the new component usage. Expect: Only occurrences of `AssetLogo`.
rg -g '*.ts' -g '*.tsx' 'AssetLogo' -A 5

Length of output: 10518

packages/extension-polkagate/src/fullscreen/accountDetails/components/AccountInformationForDetails.tsx (1)

105-105: LGTM! Verify the compatibility and behavior of the AssetLogo component.

The change from DisplayLogo to AssetLogo looks good. The props passed to the AssetLogo component are consistent with the previous usage of DisplayLogo.

However, please ensure that the AssetLogo component is a compatible replacement for DisplayLogo and that it behaves as expected.

packages/extension-polkagate/src/util/utils.ts (1)

366-374: LGTM!

The isWss function looks good:

  • It correctly checks if the input is defined before validating the URL format.
  • The regular expression looks accurate and should match valid WSS URLs.
  • Exporting the function makes it reusable in other parts of the codebase.

Great job adding this utility function to validate WSS URLs!

packages/extension-polkagate/src/hooks/useBalances.ts (1)

28-28: LGTM!

The added space between the function name and the opening parenthesis is a good stylistic change that improves readability.

packages/extension-ui/src/Popup/index.tsx (1)

302-302: LGTM!

The new route for /addNewChain/ looks good. It's rendering the AddNewChain component wrapped in an ErrorBoundary, which is a solid approach for error handling. The route is also placed correctly within the Switch component.

packages/extension/public/locales/en/translation.json (16)

1391-1391: LGTM!

The new string entry for "Voted Abstain" is properly formatted and valid.


1392-1392: LGTM!

The new string entry for "Add New Chain" is properly formatted and valid.


1393-1393: LGTM!

The new string entry for entering an endpoint or RPC node is properly formatted and valid.


1394-1394: LGTM!

The new string entry for "RPC endpoint for your desired chain" is properly formatted and valid.


1395-1395: LGTM!

The new string entry for "Loading chain information, please wait ..." is properly formatted and valid.


1396-1396: LGTM!

The new string entry for "Chain Information" is properly formatted and valid.


1397-1397: LGTM!

The new string entry for "No information found from this RPC node!" is properly formatted and valid.


1398-1398: LGTM!

The new string entry for adding a chain not listed in the PolkaGate chain list is properly formatted and valid.


1399-1399: LGTM!

The new string entry for finding the chain endpoint in Polkadot.js/org/apps is properly formatted and valid.


1400-1400: LGTM!

The new string entry for finding the token price ID from CoinGecko is properly formatted and valid.


1401-1401: LGTM!

The new string entry for finding the token price ID on CoinGecko is properly formatted and valid.


1402-1402: LGTM!

The new string entry for "Chain token price Id" is properly formatted and valid.


1403-1403: LGTM!

The new string entry for "Check" is properly formatted and valid.


1404-1404: LGTM!

The new string entry for "Add a new chain" is properly formatted and valid.


1405-1405: LGTM!

The new string entry for "Reset to default" is properly formatted and valid.


1406-1407: LGTM!

The new string entries for adding a chain not listed in the PolkaGate chain list and the "How?" question are properly formatted and valid.

packages/extension-polkagate/src/hooks/useAssetsBalances.ts (3)

28-29: LGTM

The type aliases using Record are correctly defined and improve the readability of the code.


302-302: Ensure userAddedEndpoints is handled correctly in worker scripts

The userAddedEndpoints parameter is being passed to the worker scripts via worker.postMessage. Verify that the worker scripts (getAssetOnRelayChain.js, getAssetOnAssetHub.js, and getAssetOnMultiAssetChain.js) are updated to accept and handle this new parameter appropriately.

Also applies to: 355-355, 408-408


467-467: Ensure getChainName supports genesisOptions parameter

The function getChainName is now called with genesisOptions as an argument. Verify that the getChainName function is updated to accept this new parameter and handle it correctly.

Copy link
Member

@AMIRKHANEF AMIRKHANEF left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this feature is rarely used (99.99% of our users won't engage with it), the delay in displaying chain logos in the fullscreenChainSwitch component is unacceptable and will likely frustrate users.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (2)
packages/extension-polkagate/src/fullscreen/governance/FullScreenHeader.tsx (2)

115-121: LGTM! Consider simplifying the conditional rendering.

The restructuring of the RemoteNodeSelectorWithSignals component within its own Grid container improves the consistency of the layout. This change maintains the original conditional rendering logic while ensuring the Grid structure is always present.

Consider simplifying the conditional rendering using the logical AND operator:

<Grid container item justifyContent='flex-end' width='50px'>
  {chain && (
    <RemoteNodeSelectorWithSignals
      address={address}
    />
  )}
</Grid>

This change would make the code slightly more concise and easier to read.


Contradiction Between PR Changes and Objectives

The changes in FullScreenHeader.tsx do not address the PR's objective of enabling chain developers to easily add their chain specifications and endpoints.

  • No relevant files implementing chain specifications or endpoints were found in the codebase.
Analysis chain

Line range hint 1-121: Clarify the relationship between these changes and the PR objectives.

While the changes in this file improve the structure of the FullScreenHeader component, it's not immediately clear how they relate to the PR objective of allowing chain developers to easily add their chain specifications and endpoints.

Could you please clarify:

  1. How do these changes in the FullScreenHeader component contribute to the new functionality described in the PR objectives?
  2. Are there other files that implement the core functionality for adding new chain specifications and endpoints?

To help verify this, let's check for any related changes:

This will help us identify other relevant files that might contain the core implementation of the new functionality.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for files that might be related to adding new chain specifications or endpoints
rg --type typescript --type javascript -i "chain.*specification|chain.*endpoint|add.*chain"

Length of output: 128


Script:

#!/bin/bash
# Search for files related to adding new chain specifications or endpoints using correct file types
rg --type ts --type js -i "chain.*specification|chain.*endpoint|add.*chain"

Length of output: 67608

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 50e4c6d and 0379a1b.

Files selected for processing (3)
  • packages/extension-polkagate/src/fullscreen/governance/FullScreenHeader.tsx (1 hunks)
  • packages/extension-polkagate/src/fullscreen/homeFullScreen/components/ChainList.tsx (5 hunks)
  • packages/extension-polkagate/src/partials/FullScreenChainSwitch.tsx (4 hunks)
Files skipped from review as they are similar to previous changes (2)
  • packages/extension-polkagate/src/fullscreen/homeFullScreen/components/ChainList.tsx
  • packages/extension-polkagate/src/partials/FullScreenChainSwitch.tsx
Additional comments not posted (1)
packages/extension-polkagate/src/fullscreen/governance/FullScreenHeader.tsx (1)

Line range hint 1-121: Summary of review

  1. The changes in this file improve the structure of the FullScreenHeader component, particularly around the RemoteNodeSelectorWithSignals component.
  2. While these changes are positive, their direct relationship to the PR objectives (adding functionality for chain developers to add specifications and endpoints) is not immediately clear.
  3. Further clarification is needed on how these changes contribute to the overall PR objectives.
  4. It's recommended to review additional files that may contain the core implementation of the new functionality for adding chain specifications and endpoints.

To ensure a comprehensive review, please provide any additional files that implement the core functionality described in the PR objectives. This will allow for a more thorough assessment of the changes in relation to adding new chain specifications and endpoints.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (6)
packages/extension-polkagate/src/util/utils.ts (1)

329-337: LGTM! Consider a minor enhancement.

The isWss function is a well-implemented utility for validating WebSocket Secure (WSS) URLs. It aligns with the PR objectives by facilitating the addition of chain endpoints. The function correctly handles undefined inputs and uses a comprehensive regular expression for validation.

Consider adding a check for the maximum length of the input string to prevent potential DoS attacks from extremely long inputs. For example:

 export const isWss = (input: string | undefined): boolean => {
   if (!input) {
     return false;
   }
+
+  const MAX_LENGTH = 2083; // Common maximum URL length
+  if (input.length > MAX_LENGTH) {
+    return false;
+  }

   const urlRegex = /^wss:\/\/([\w\d-]+\.)+[\w\d-]{2,}(\/[\w\d-._~:/?#\[\]@!$&'()*+,;=]*)?$/i;

   return urlRegex.test(input);
 };

This addition would enhance the security of the function without affecting its current functionality.

packages/extension/public/locales/zh/translation.json (1)

Line range hint 1376-1378: Translations for delegation status and help

These translations are concise and accurate. However, the translation for "Need Help?" (需要帮助吗?) ends with a question mark, which is inconsistent with the English version that uses both a question mark and an exclamation mark.

Consider changing "需要帮助吗?" to "需要帮助吗?!" to match the emphasis in the English version.

packages/extension/public/locales/hi/translation.json (1)

Line range hint 1201-1392: New feature translations are consistent and well-implemented

The translations for newer features, including chain management and social recovery, are consistent with the style of previous sections and effectively introduce these concepts to Hindi-speaking users.

Consider adding brief explanations in parentheses for new technical terms like "QR-signer" and "RPC endpoint" to enhance user understanding. For example:

- "QR-signer": "क्यूआर-साइनर",
+ "QR-signer (QR code based signing tool)": "क्यूआर-साइनर (क्यूआर कोड आधारित हस्ताक्षर उपकरण)",
- "RPC endpoint": "आरपीसी एंडपॉइंट",
+ "RPC endpoint (Remote Procedure Call connection point)": "आरपीसी एंडपॉइंट (रिमोट प्रोसीजर कॉल कनेक्शन पॉइंट)",
packages/extension/public/locales/fr/translation.json (3)

Line range hint 1-100: Account management and staking translations are good, with a minor suggestion.

The French translations for account management and staking are generally accurate and consistent. Technical terms are appropriately handled. However, there's a minor inconsistency in the translation of "stake":

  • Sometimes it's translated as "mise" (e.g., "Mise en jeu")
  • Other times it's translated as "enjeu" (e.g., "Désenjeu")

Consider standardizing the translation of "stake" throughout the file for better consistency. Choose either "mise" or "enjeu" and use it consistently.


Line range hint 201-300: Account recovery and security translations are good, with a minor grammatical suggestion.

The French translations for account recovery and security-related content are generally accurate, consistent, and complete. Technical terms are handled appropriately, and placeholders are correctly formatted. However, there's a minor grammatical issue:

Line 277: "Unlock date unknown" is translated as "Date de déverrouillage inconnue"

Consider changing "Date de déverrouillage inconnue" to "Date de déverrouillage inconnue." (adding a period at the end) for better grammatical correctness and consistency with other similar entries.


Line range hint 301-1381: UI elements and general messages translations are good, with a few suggestions for improvement.

The French translations for UI elements and general messages are generally accurate, consistent, and complete. Technical terms are handled appropriately, and placeholders are correctly formatted. However, there are a few instances where the translations could be improved:

  1. Line 1116: "Staked Solo" is translated as "Mis en jeu en solo". Consider changing to "Mise en jeu individuelle" for consistency with other translations.

  2. Line 1214: "Stake Extra" is translated as "Mise en jeu". Consider changing to "Mise en jeu supplémentaire" to better convey the meaning of "extra".

  3. Line 1329: "Ledger error: {{errorMessage}}" is translated literally. Consider changing to "Erreur Ledger : {{errorMessage}}" for better readability in French.

Consider implementing the suggested changes to improve clarity and naturalness of the translations.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0379a1b and 6fd6881.

Files selected for processing (7)
  • packages/extension-polkagate/src/fullscreen/homeFullScreen/components/ChainList.tsx (5 hunks)
  • packages/extension-polkagate/src/util/utils.ts (2 hunks)
  • packages/extension/public/locales/en/translation.json (1 hunks)
  • packages/extension/public/locales/fr/translation.json (1 hunks)
  • packages/extension/public/locales/hi/translation.json (1 hunks)
  • packages/extension/public/locales/ru/translation.json (1 hunks)
  • packages/extension/public/locales/zh/translation.json (1 hunks)
Files skipped from review as they are similar to previous changes (2)
  • packages/extension-polkagate/src/fullscreen/homeFullScreen/components/ChainList.tsx
  • packages/extension/public/locales/en/translation.json
Additional comments not posted (103)
packages/extension-polkagate/src/util/utils.ts (1)

329-337: Changes align well with PR objectives and enhance module utility.

The addition of the isWss function:

  1. Supports the PR objective of allowing chain developers to add their endpoints by providing a specific validation for WSS URLs.
  2. Complements existing URL-related functions in the file, enhancing the overall utility of the module.
  3. Is well-integrated and doesn't introduce any breaking changes.

This change is a valuable addition to the utility functions and aligns perfectly with the goal of improving the extensibility of the application for chain developers.

packages/extension/public/locales/zh/translation.json (88)

1387-1399: New translations added for chain management functionality

These new translations cover the functionality for adding new chains to the extension. The translations appear accurate and consistent with the existing style. However, there are a few points to consider:

  1. The translation for "How?" (如何?) is very brief. Consider expanding it to "如何操作?" for clarity.
  2. The translation for the CoinGecko URL might benefit from additional context. Consider adding a note that this is an English website.
  3. The term "RPC endpoint" is translated literally. Consider adding a brief explanation in parentheses for users who might not be familiar with this technical term.

1384-1386: New translations for voting actions

The translations for "Voted Aye", "Voted Nay", and "Voted Abstain" are accurate and concise. They maintain consistency with the existing translation style.


Line range hint 1382-1383: Translations for account removal and default chain switching

These translations are clear and accurately convey the intended messages. The use of placeholders ({{accountName}} and {{chainName}}) is correct and consistent with other translations in the file.


Line range hint 1379-1381: New translations for profile visibility and social media

These translations are accurate and maintain consistency with the existing style. The use of placeholders ({{profileName}} and {{visibility}}) is correct.


Line range hint 1373-1375: Translations for account balance updates and removal

These translations accurately convey the intended messages and are consistent with the existing style. The use of exclamation marks for emphasis is appropriate in Chinese.


Line range hint 1370-1372: Translations for sending funds and existential deposit

These translations are accurate and maintain consistency with the existing style. The use of a colon in "最低余额: {{ED}}" is appropriate in Chinese.


Line range hint 1367-1369: Translations for account visibility and connection management

These translations accurately convey the intended messages and are consistent with the existing style. The use of parentheses in "对网站隐藏(不可见)" is appropriate for providing additional clarity.


Line range hint 1363-1366: Translations for account creation and import options

These translations are accurate and consistent with the existing style. They provide clear instructions for various account-related actions.


Line range hint 1360-1362: Translation for nomination pools evolution notice

This translation accurately conveys the complex message about the evolution of nomination pools. It maintains the technical terminology while being clear in Chinese.


Line range hint 1357-1359: Translations for profile management and portfolio

These translations are concise and accurate. They maintain consistency with the existing translation style.


Line range hint 1354-1356: Translations for Ledger account migration

These translations accurately convey the technical instructions for Ledger account migration. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1351-1353: Translations for version information and dismissal actions

These translations are accurate and consistent with the existing style. The use of colon in "版本: {{version}}" is appropriate in Chinese.


Line range hint 1348-1350: Translations for Ledger device instructions

These translations accurately convey the instructions for using a Ledger device. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1345-1347: Translations for Ledger account import options

These translations are accurate and consistent with the existing style. They provide clear instructions for importing Ledger accounts.


Line range hint 1342-1344: Translations for Ledger connection types

These translations accurately describe different Ledger connection types. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1339-1341: Translations for profile management actions

These translations are concise and accurate. They maintain consistency with the existing translation style.


Line range hint 1336-1338: Translations for account visibility settings

These translations accurately convey the account visibility settings. They maintain consistency with the existing style.


Line range hint 1333-1335: Translations for Ledger-related error messages

These translations accurately convey error messages related to Ledger devices. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1330-1332: Translations for Ledger connection instructions

These translations accurately convey the instructions for connecting a Ledger device. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1327-1329: Translations for migration-related actions

These translations accurately describe migration-related actions. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1324-1326: Translations for Ledger app types

These translations accurately describe different types of Ledger apps. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1321-1323: Translation for Ledger connection type explanation

This translation accurately conveys the complex explanation for choosing a Ledger connection type. It maintains the technical terminology while being clear in Chinese.


Line range hint 1318-1320: Translations for account import modes

These translations accurately describe different modes for importing accounts. They maintain consistency with the existing style.


Line range hint 1315-1317: Translations for Ledger account selection instructions

These translations accurately convey the instructions for selecting Ledger accounts to import. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1312-1314: Translations for Ledger connection instructions

These translations accurately convey the instructions for connecting a Ledger device. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1309-1311: Translations for proxied account import

These translations accurately describe the process of importing proxied accounts. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1306-1308: Translations for account management actions

These translations accurately describe various account management actions. They maintain consistency with the existing style.


Line range hint 1303-1305: Translations for welcome messages

These translations accurately convey the welcome messages. They maintain a friendly tone consistent with the English version.


Line range hint 1300-1302: Translations for transaction confirmation and API connection status

These translations accurately convey the confirmation message and API connection status. They maintain consistency with the existing style.


Line range hint 1297-1299: Translations for QR code signing instructions

These translations accurately convey the instructions for signing with a QR code. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1294-1296: Translations for chart-related messages

These translations accurately convey messages related to chart data availability and reset actions. They maintain consistency with the existing style.


Line range hint 1291-1293: Translations for pool management actions

These translations accurately describe various pool management actions. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1288-1290: Translations for staking-related messages

These translations accurately convey messages related to staking actions and rewards. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1285-1287: Translations for staking options

These translations accurately describe different staking options. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1282-1284: Translations for pool creation and joining

These translations accurately describe the processes of creating and joining pools. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1279-1281: Translations for staking instructions

These translations accurately convey instructions related to staking. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1276-1278: Translations for chain selection and address copying

These translations accurately describe actions related to chain selection and address copying. They maintain consistency with the existing style.


Line range hint 1273-1275: Translations for transaction warnings

These translations accurately convey warnings related to transaction balances. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1270-1272: Translations for proxy management

These translations accurately describe proxy management actions. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1267-1269: Translations for adding proxy accounts

These translations accurately convey messages related to adding proxy accounts. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1264-1266: Translations for account management actions

These translations accurately describe various account management actions. They maintain consistency with the existing style.


Line range hint 1261-1263: Translations for wallet reset instructions

These translations accurately convey instructions for resetting a wallet. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1258-1260: Translations for password management

These translations accurately describe password management actions. They maintain consistency with the existing style.


Line range hint 1255-1257: Translations for account deletion warnings

These translations accurately convey warnings related to account deletion. They maintain consistency with the existing style and use appropriate cautionary language.


Line range hint 1252-1254: Translations for password-related actions

These translations accurately describe various password-related actions. They maintain consistency with the existing style.


Line range hint 1249-1251: Translations for welcome messages and account creation

These translations accurately convey welcome messages and instructions for account creation. They maintain a friendly tone consistent with the English version.


Line range hint 1246-1248: Translations for privacy policy

These translations accurately convey the privacy policy information. They maintain consistency with the existing style and use appropriate formal language.


Line range hint 1243-1245: Translations for account balance information

These translations accurately describe different types of account balances. They maintain consistency with the existing style and use appropriate financial terms.


Line range hint 1240-1242: Translations for proxy-related messages

These translations accurately convey messages related to proxy accounts. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1237-1239: Translations for account import instructions

These translations accurately convey instructions for importing accounts. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1234-1236: Translations for profile management

These translations accurately describe profile management actions. They maintain consistency with the existing style.


Line range hint 1231-1233: Translations for reasons and waiting messages

These translations accurately convey messages related to reasons and waiting periods. They maintain consistency with the existing style.


Line range hint 1228-1230: Translations for chart-related messages

These translations accurately describe chart-related information. They maintain consistency with the existing style.


Line range hint 1225-1227: Translations for reward-related messages

These translations accurately convey messages related to rewards. They maintain consistency with the existing style and use appropriate financial terms.


Line range hint 1222-1224: Translations for validator-related messages

These translations accurately describe validator-related information. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1219-1221: Translations for staking limit messages

These translations accurately convey messages related to staking limits. They maintain consistency with the existing style and use appropriate financial terms.


Line range hint 1216-1218: Translations for account exploration options

These translations accurately describe account exploration options. They maintain consistency with the existing style.


Line range hint 1213-1215: Translations for validator selection instructions

These translations accurately convey instructions for selecting validators. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1210-1212: Translations for reward destination configuration

These translations accurately describe the process of configuring reward destinations. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1207-1209: Translations for active validator information

These translations accurately convey information about active validators. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1204-1206: Translations for staking-related actions

These translations accurately describe various staking-related actions. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1201-1203: Translations for asset-related messages

These translations accurately convey messages related to assets and tokens. They maintain consistency with the existing style.


Line range hint 1198-1200: Translations for common tasks and account details

These translations accurately describe common tasks and account details. They maintain consistency with the existing style.


Line range hint 1195-1197: Translations for account import instructions

These translations accurately convey instructions for importing accounts. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1192-1194: Translations for chain search and selection

These translations accurately describe chain search and selection actions. They maintain consistency with the existing style.


Line range hint 1189-1191: Translations for account export instructions

These translations accurately convey instructions for exporting accounts. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1186-1188: Translations for QR code scanning instructions

These translations accurately convey instructions for scanning QR codes. They maintain consistency with the existing style.


Line range hint 1183-1185: Translations for proxy type selection

These translations accurately describe proxy type selection. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1180-1182: Translations for account connection messages

These translations accurately convey messages related to account connections. They maintain consistency with the existing style.


Line range hint 1177-1179: Translations for authentication request messages

These translations accurately describe authentication request processes. They maintain consistency with the existing style and use appropriate cautionary language.


Line range hint 1174-1176: Translations for account access management

These translations accurately convey instructions for managing account access. They maintain consistency with the existing style.


Line range hint 1171-1173: Translations for account visibility messages

These translations accurately describe account visibility settings. They maintain consistency with the existing style.


Line range hint 1168-1170: Translations for demo exploration and account creation

These translations accurately convey options for exploring a demo and creating accounts. They maintain consistency with the existing style.


Line range hint 1165-1167: Translations for Ledger connection instructions

These translations accurately convey instructions for connecting Ledger devices. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1162-1164: Translations for portfolio and staking messages

These translations accurately describe portfolio and staking-related information. They maintain consistency with the existing style and use appropriate financial terms.


Line range hint 1159-1161: Translations for account management actions

These translations accurately describe various account management actions. They maintain consistency with the existing style.


Line range hint 1156-1158: Translations for new feature announcements

These translations accurately convey announcements about new features. They maintain consistency with the existing style and use appropriate emoji.


Line range hint 1153-1155: Translations for Ledger account migration

These translations accurately describe the process of Ledger account migration. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1150-1152: Translations for advanced mode options

These translations accurately describe advanced mode options. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1147-1149: Translations for Ledger app selection

These translations accurately convey instructions for selecting Ledger apps. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1144-1146: Translations for Ledger connection types

These translations accurately describe different Ledger connection types. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1141-1143: Translations for profile removal actions

These translations accurately describe profile removal actions. They maintain consistency with the existing style.


Line range hint 1138-1140: Translations for profile creation instructions

These translations accurately convey instructions for creating profiles. They maintain consistency with the existing style.


Line range hint 1135-1137: Translations for account visibility settings

These translations accurately describe account visibility settings. They maintain consistency with the existing style.


Line range hint 1132-1134: Translations for profile-related actions

These translations accurately describe various profile-related actions. They maintain consistency with the existing style.


Line range hint 1129-1131: Translations for reserved balance reasons

These translations accurately describe reasons for reserved balances. They maintain consistency with the existing style and use appropriate financial terms.


Line range hint 1126-1128: Translations for chain-related messages

These translations accurately convey messages related to blockchain operations. They maintain consistency with the existing style and use appropriate technical terms.


Line range hint 1123-1125: Translations for social media platform names

These translations accurately represent social media platform names. They maintain consistency by keeping the original English names, which is appropriate for well-known global platforms.

packages/extension/public/locales/hi/translation.json (5)

Line range hint 1-300: General UI translations look good

The translations for common UI elements and messages in this section are consistent and accurate. They appropriately convey the meaning of the original English text in Hindi.


Line range hint 301-600: Staking and governance translations are well-handled

The translations related to staking, governance, and account management are accurate and consistent. It's noteworthy that technical terms are often kept in English (e.g., "staking", "governance"), which is a good practice for maintaining clarity and avoiding confusion.


Line range hint 601-900: Account and transaction translations effectively convey complex concepts

The translations for account management, transaction details, and various blockchain operations are accurate and consistent with previous sections. It's commendable how complex blockchain concepts are well-explained in Hindi, making them accessible to Hindi-speaking users while maintaining technical accuracy.


Line range hint 901-1200: Advanced features and error messages are clearly translated

The translations for advanced features, error messages, and specific blockchain operations are consistent with previous sections. It's particularly noteworthy that error messages are clearly translated, which is crucial for helping Hindi-speaking users understand and resolve issues they may encounter.


Line range hint 1-1392: Overall, excellent quality Hindi translations

This translation file provides comprehensive and high-quality Hindi translations for a blockchain-related application. The translations are consistent throughout the file, effectively conveying both simple UI elements and complex blockchain concepts. The practice of keeping technical terms in English while providing Hindi context is commendable, as it maintains clarity while making the application accessible to Hindi-speaking users.

The introduction of new features and concepts is handled well, maintaining the style and quality of existing translations. The clear translation of error messages and advanced features will greatly aid user understanding and troubleshooting.

To further enhance the file:

  1. Consider adding brief explanations for new technical terms as suggested earlier.
  2. Regularly review and update translations as new features are added to the application.
  3. Consider getting feedback from native Hindi speakers who are also familiar with blockchain technology to ensure the translations remain both technically accurate and naturally expressed in Hindi.
packages/extension/public/locales/ru/translation.json (6)

Line range hint 1-1390: JSON structure looks valid and well-formatted.

The overall structure of the JSON file appears to be correct. All key-value pairs are properly formatted with quotes around both keys and values. There are no apparent syntax errors in the JSON structure.


1378-1390: New translations for adding a chain are present and appear correct.

The newly added translations for the "Add New Chain" functionality, as mentioned in the AI summary, are present and seem to be correctly translated. These include:

  • "Add New Chain": "Добавить новую сеть"
  • "If your chain is not listed in the PolkaGate chain list...": "Если вашей сети нет в списке сетей PolkaGate..."
  • "How?": "Как?"
  • "Find the chain endpoint in the Polkadot.js/org/apps...": "Найдите эндпоинт сети на Polkadot.js/org/apps..."
  • "RPC endpoint for your desired chain:": "RPC-эндпоинт для вашей сети:"
  • "Loading chain information, please wait ...": "Загрузка информации о сети, пожалуйста, подождите..."
  • "We couldn't find the token price ID on-chain...": "Мы не смогли найти ID цены токена в сети..."
  • "Find your token on CoinGecko...": "Найдите свой токен на CoinGecko..."
  • "Chain token price Id": "ID цены токена сети"
  • "Check": "Проверить"
  • "Chain Information": "Информация о сети"
  • "No information found from this RPC node!": "Информация с этого узла RPC не найдена!"
  • "Add a new chain": "Добавить новую сеть"

The translations appear to be consistent with the English versions and maintain the necessary placeholders.


Line range hint 1-1390: Placeholder consistency maintained throughout the file.

After reviewing the entire file, I can confirm that placeholders such as {{chainName}}, {{token}}, {{count}}, etc., are consistently used throughout the translations. This ensures that dynamic content can be properly inserted into the translated strings.


Line range hint 1-1390: No missing or incomplete translations detected.

All keys appear to have corresponding translated values. There are no empty strings or obvious placeholders left untranslated.


Line range hint 1-1390: Special characters and formatting appear correct.

The file correctly uses Unicode characters for Russian text. Quotation marks and other special characters are properly escaped where necessary. The formatting of longer strings, including those with line breaks, appears to be correct.


Line range hint 1-1390: LGTM! Russian translations file is well-structured and complete.

The translation.json file for Russian localization is well-formatted, complete, and consistent. It includes the newly added translations for the "Add New Chain" functionality, maintains placeholder consistency, and properly handles special characters. No issues or areas for improvement were identified during this review.

packages/extension/public/locales/fr/translation.json (3)

1382-1395: Translations for adding new chains look good!

The French translations for the feature of adding new chains to the extension are accurate, consistent, and complete. They correctly convey the instructions and UI elements for this process, maintaining the original meaning and proper use of technical terms.


Line range hint 101-200: Governance and voting translations are well done!

The French translations for governance and voting-related content are accurate, consistent, and complete. They correctly convey the meaning of technical terms and processes specific to blockchain governance. Placeholders are properly formatted, and no translations appear to be missing or incomplete in this section.


Line range hint 1-1395: Overall, the French translations are of high quality with only minor improvements needed.

The translations in this file are generally accurate, consistent, and complete. They cover a wide range of features and UI elements for the extension, including account management, staking, governance, security, and general messages. The technical terms are handled appropriately, and placeholders are correctly formatted throughout the file.

Main points for improvement:

  1. Standardize the translation of "stake" throughout the file (choose either "mise" or "enjeu" consistently).
  2. Add a period at the end of "Date de déverrouillage inconnue" for grammatical correctness.
  3. Consider the suggested changes for "Staked Solo", "Stake Extra", and "Ledger error" translations to improve clarity and naturalness.

These minor adjustments will further enhance the overall quality and consistency of the French localization.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range and nitpick comments (7)
packages/extension-polkagate/src/hooks/useApiWithChain2.ts (3)

14-15: Good optimization, consider memoization for further improvement.

Moving createWsEndpoints() outside the function is a good optimization as it avoids redundant calls. This aligns well with the PR objective of improving the extension's functionality for chain developers.

Consider memoizing allEndpoints using useMemo to ensure it's only recalculated when necessary:

const allEndpoints = useMemo(() => createWsEndpoints(), []);

This would prevent unnecessary recalculations if the component re-renders for reasons unrelated to the endpoints.


Line range hint 22-26: Approved change, consider adding error handling.

The use of the allEndpoints constant here is consistent with the earlier change and improves code readability. This modification supports the PR objective of making it easier for chain developers to integrate their specifications.

Consider adding error handling in case allEndpoints is undefined:

const maybeEndpoint = useMemo(() => {
  if (!allEndpoints) {
    console.error('Failed to create WebSocket endpoints');
    return undefined;
  }
  const chainName = sanitizeChainName(chain?.name);
  const endpoints = allEndpoints.filter((e) => String(e.text)?.toLowerCase() === chainName?.toLowerCase() || String(e.info)?.toLowerCase() === chainName?.toLowerCase());
  return endpoints.length ? endpoints[0].value : undefined;
}, [chain?.name, allEndpoints]);

This addition would improve the robustness of the code and provide better debugging information if there's an issue with endpoint creation.


Line range hint 14-31: Overall improvements align with PR objectives, consider additional validation.

The changes made to this file improve code organization and potentially performance, which aligns well with the PR objective of enhancing the extension's functionality for chain developers. The core functionality of useApiWithChain2 remains intact while making it more efficient.

To fully meet the PR objectives, consider adding more robust input validation:

  1. Validate the chain object:
if (!chain || typeof chain !== 'object') {
  console.error('Invalid chain object provided');
  return undefined;
}
  1. Validate the chain name:
const chainName = sanitizeChainName(chain.name);
if (!chainName) {
  console.error('Invalid chain name');
  return undefined;
}
  1. Add type checking for the endpoint object:
if (!endpoints.every(e => typeof e.text === 'string' && typeof e.info === 'string' && typeof e.value === 'string')) {
  console.error('Invalid endpoint object structure');
  return undefined;
}

These additional validations will enhance the robustness of the function and prevent potential issues, as emphasized in the PR description.

packages/extension-polkagate/src/hooks/useApiWithChain.ts (2)

14-15: Approve the change with a suggestion for optimization.

The initialization of allEndpoints outside the useApiWithChain function is a good optimization. It prevents unnecessary re-creation of endpoints on each render.

Consider using useMemo to initialize allEndpoints for better performance in case the component re-renders:

const allEndpoints = useMemo(() => createWsEndpoints(), []);

This ensures that createWsEndpoints() is only called once, even if the component re-renders, while still keeping the initialization at the top level of the component.


Line range hint 21-26: Approve the change with a suggestion for improved clarity.

The use of allEndpoints within the maybeEndpoint useMemo is correct and aligns with the PR objectives. It simplifies the code and potentially improves performance by avoiding unnecessary re-creation of endpoints.

For improved clarity and type safety, consider adding a type annotation to allEndpoints:

const allEndpoints: WsEndpoint[] = createWsEndpoints();

This makes the code more self-documenting and helps catch potential type-related issues early.

packages/extension-polkagate/src/hooks/usePeopleChain.ts (2)

44-44: Function updated to use optimized allEndpoints, but more changes needed.

The usePeopleChain function now uses the optimized allEndpoints variable, which is good. However, to fully meet the PR objectives:

  1. Consider adding a mechanism for chain developers to easily add their chain specifications and endpoints. This could be a separate function or configuration file.
  2. Implement thorough input validation for any new chain data to ensure the extension's stability, as mentioned in the PR description.

To address these points:

  1. Create a new function or configuration file where chain developers can add their chain specs:
// Example: chainConfig.ts
export const customChains = [
  {
    name: 'MyCustomChain',
    genesisHash: '0x...',
    endpoints: ['wss://mycustomchain.example.com']
  }
  // More custom chains can be added here
];
  1. Add input validation in the usePeopleChain function:
function validateChainInput(chain: any): boolean {
  // Implement thorough validation here
  return (
    typeof chain === 'object' &&
    typeof chain.name === 'string' &&
    typeof chain.genesisHash === 'string' &&
    Array.isArray(chain.endpoints) &&
    chain.endpoints.every((endpoint: any) => typeof endpoint === 'string')
  );
}

// Use this in usePeopleChain or wherever custom chain data is processed
if (!validateChainInput(customChain)) {
  throw new Error('Invalid chain configuration');
}

These changes would make it easier for developers to add custom chains while ensuring the stability of the extension.


Line range hint 1-70: Summary: Good optimizations, but PR objectives not fully met

The changes in this file, particularly the optimization of allEndpoints, are beneficial. However, to fully meet the PR objectives:

  1. Implement a mechanism for chain developers to easily add their chain specifications and endpoints.
  2. Add thorough input validation for any new chain data to ensure the extension's stability.

Consider the suggestions in the previous comment to address these points. Once implemented, this PR will better align with its stated objectives of improving extensibility and maintaining stability.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 6fd6881 and 74b83be.

Files selected for processing (9)
  • packages/extension-polkagate/src/hooks/useApiWithChain.ts (1 hunks)
  • packages/extension-polkagate/src/hooks/useApiWithChain2.ts (1 hunks)
  • packages/extension-polkagate/src/hooks/useAssetsBalances.ts (10 hunks)
  • packages/extension-polkagate/src/hooks/useEndpoints.ts (2 hunks)
  • packages/extension-polkagate/src/hooks/usePeopleChain.ts (2 hunks)
  • packages/extension-polkagate/src/hooks/useTeleport.ts (2 hunks)
  • packages/extension-polkagate/src/util/getLogo.ts (2 hunks)
  • packages/extension-polkagate/src/util/getLogo2.ts (1 hunks)
  • packages/extension-polkagate/src/util/workers/utils/getChainEndpoints.js (1 hunks)
Files skipped from review due to trivial changes (1)
  • packages/extension-polkagate/src/hooks/useTeleport.ts
Files skipped from review as they are similar to previous changes (4)
  • packages/extension-polkagate/src/hooks/useAssetsBalances.ts
  • packages/extension-polkagate/src/hooks/useEndpoints.ts
  • packages/extension-polkagate/src/util/getLogo.ts
  • packages/extension-polkagate/src/util/workers/utils/getChainEndpoints.js
Additional comments not posted (4)
packages/extension-polkagate/src/hooks/useApiWithChain.ts (1)

Line range hint 1-53: Overall, the changes improve code organization and align with PR objectives.

The modifications to useApiWithChain.ts enhance code organization and potentially improve performance by moving the allEndpoints initialization outside the function. This change aligns well with the PR objectives of allowing chain developers to easily add their chain specifications and endpoints.

Key points:

  1. The changes support better extensibility for adding new chains.
  2. Code organization is improved without altering the core logic of useApiWithChain.
  3. There are no apparent security issues introduced by these changes.

To ensure that the changes don't introduce any unintended side effects, please run the following verification script:

This script verifies the correct import, initialization, and usage of createWsEndpoints and allEndpoints.

Verification successful

All verification checks passed successfully.

The changes to useApiWithChain.ts have been successfully verified:

  • createWsEndpoints is imported correctly.
  • allEndpoints is initialized properly using createWsEndpoints().
  • allEndpoints is used correctly within maybeEndpoint.

These modifications enhance code organization and align with the PR objectives without introducing any issues.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify that createWsEndpoints is imported and used correctly

# Test 1: Check if createWsEndpoints is imported correctly
if ! rg --type typescript "import \{ createWsEndpoints \} from '@polkagate/apps-config';" packages/extension-polkagate/src/hooks/useApiWithChain.ts; then
    echo "Error: createWsEndpoints is not imported correctly"
    exit 1
fi

# Test 2: Check if allEndpoints is initialized correctly
if ! rg --type typescript "const allEndpoints = createWsEndpoints\(\);" packages/extension-polkagate/src/hooks/useApiWithChain.ts; then
    echo "Error: allEndpoints is not initialized correctly"
    exit 1
fi

# Test 3: Check if allEndpoints is used within maybeEndpoint
if ! rg --type typescript "const endpoints = allEndpoints\?.filter" packages/extension-polkagate/src/hooks/useApiWithChain.ts; then
    echo "Error: allEndpoints is not used correctly within maybeEndpoint"
    exit 1
fi

echo "All checks passed successfully"

Length of output: 305


Script:

#!/bin/bash
# Description: Verify that createWsEndpoints is imported and used correctly

# Test 1: Check if createWsEndpoints is imported correctly
if ! rg -g "*.ts" "import { createWsEndpoints } from '@polkagate/apps-config';" packages/extension-polkagate/src/hooks/useApiWithChain.ts; then
    echo "Error: createWsEndpoints is not imported correctly"
    exit 1
fi

# Test 2: Check if allEndpoints is initialized correctly
if ! rg -g "*.ts" "const allEndpoints = createWsEndpoints\(\);" packages/extension-polkagate/src/hooks/useApiWithChain.ts; then
    echo "Error: allEndpoints is not initialized correctly"
    exit 1
fi

# Test 3: Check if allEndpoints is used within maybeEndpoint
if ! rg -g "*.ts" "const endpoints = allEndpoints\?.filter" packages/extension-polkagate/src/hooks/useApiWithChain.ts; then
    echo "Error: allEndpoints is not used correctly within maybeEndpoint"
    exit 1
fi

echo "All checks passed successfully"

Length of output: 411


Script:

#!/bin/bash
# Description: Verify that createWsEndpoints is imported and used correctly

# Test 1: Check if createWsEndpoints is imported correctly using fixed string search
if ! rg -F "import { createWsEndpoints } from '@polkagate/apps-config';" packages/extension-polkagate/src/hooks/useApiWithChain.ts; then
    echo "Error: createWsEndpoints is not imported correctly"
    exit 1
fi

# Test 2: Check if allEndpoints is initialized correctly using fixed string search
if ! rg -F "const allEndpoints = createWsEndpoints();" packages/extension-polkagate/src/hooks/useApiWithChain.ts; then
    echo "Error: allEndpoints is not initialized correctly"
    exit 1
fi

# Test 3: Check if allEndpoints is used within maybeEndpoint using fixed string search
if ! rg -F "const endpoints = allEndpoints?.filter" packages/extension-polkagate/src/hooks/useApiWithChain.ts; then
    echo "Error: allEndpoints is not used correctly within maybeEndpoint"
    exit 1
fi

echo "All checks passed successfully"

Length of output: 634

packages/extension-polkagate/src/util/getLogo2.ts (1)

13-13: LGTM. Verify createWsEndpoints behavior and update documentation.

The change aligns with the PR objectives of allowing easier integration of chain specifications and endpoints. However, to ensure the change doesn't introduce any unintended side effects:

  1. Please verify that createWsEndpoints functions correctly without arguments.
  2. Update any relevant documentation to reflect this change in usage.

To verify the createWsEndpoints function's behavior, please run the following script:

packages/extension-polkagate/src/hooks/usePeopleChain.ts (2)

6-7: LGTM: Import statements reordered.

The reordering of import statements is a minor change that doesn't affect functionality. It likely aligns with code style or linting rules.


42-42: Optimization: allEndpoints moved outside the function.

Moving allEndpoints = createWsEndpoints() outside the usePeopleChain function is a good optimization. It prevents unnecessary recreation of endpoints on each function call, potentially improving performance. This change also aligns with the PR objective of making it easier to add chain endpoints.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Outside diff range and nitpick comments (4)
packages/extension-polkagate/src/hooks/useAssetsBalances.ts (2)

299-299: LGTM: Integration of new parameters in internal logic

The changes effectively integrate the new userAddedEndpoints and genesisOptions parameters into the hook's logic. This allows for more dynamic handling of chain information and user-defined endpoints.

Consider extracting the chain name retrieval logic into a separate function to improve readability:

const getChainNameSafely = (genesisHash: string): string | undefined => {
  const chainName = getChainName(genesisHash, genesisOptions);
  if (!chainName) {
    console.error('Cannot find chain name by genesis hash!', genesisHash);
  }
  return chainName;
};

Then use it like this:

const chainName = getChainNameSafely(genesisHash);
if (chainName) {
  fetchAssetOnRelayChain(addresses!, chainName);
}

This would make the code more concise and easier to maintain.

Also applies to: 352-352, 405-405, 464-467, 503-507, 512-517


Line range hint 132-517: Overall assessment: Good improvements with minor issues

The changes to useAssetsBalances significantly enhance its flexibility by allowing for user-added endpoints and more dynamic chain handling. The integration of new parameters is well-executed throughout the hook's logic.

However, there are two minor issues to address:

  1. The UserAddedChains type needs to be defined or imported.
  2. A typo in an error message still needs to be corrected.

Once these are resolved, the changes will be ready for approval.

Consider adding unit tests for the new functionality, especially around the handling of userAddedEndpoints and genesisOptions, to ensure the changes work as expected across different scenarios.

packages/extension-polkagate/src/hooks/usePriceIds.ts (2)

25-25: Simplify type annotation for maybePriceIds

Since maybePriceIds will always be an array (possibly empty), you can simplify the type annotation to string[] instead of string[] | undefined.

Apply this diff:

-const maybePriceIds: string[] | undefined = Object.entries(info).map(([_, { priceId }]) => priceId as string).filter(Boolean);
+const maybePriceIds: string[] = Object.entries(info).map(([_, { priceId }]) => priceId as string).filter(Boolean);

37-37: Reminder: Address the TODO comment regarding AssetHub replacement

There's a TODO comment indicating that the replacement of 'AssetHub' in chain names needs to be double-checked. Please ensure this logic is correct and the replacement is necessary. Unaddressed TODOs can lead to overlooked issues in the future.

Do you need assistance in verifying this logic or in determining the correct approach? I'm available to help if needed.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 8704eb1 and 3df180c.

📒 Files selected for processing (6)
  • packages/extension-polkagate/src/hooks/useAssetsBalances.ts (11 hunks)
  • packages/extension-polkagate/src/hooks/usePriceIds.ts (2 hunks)
  • packages/extension-polkagate/src/util/workers/getAssetOnAssetHub.js (3 hunks)
  • packages/extension-polkagate/src/util/workers/getAssetOnMultiAssetChain.js (2 hunks)
  • packages/extension-polkagate/src/util/workers/getAssetOnRelayChain.js (3 hunks)
  • packages/extension-ui/src/Popup/index.tsx (7 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
  • packages/extension-polkagate/src/util/workers/getAssetOnAssetHub.js
  • packages/extension-polkagate/src/util/workers/getAssetOnMultiAssetChain.js
  • packages/extension-polkagate/src/util/workers/getAssetOnRelayChain.js
  • packages/extension-ui/src/Popup/index.tsx
🧰 Additional context used
🪛 GitHub Check: build
packages/extension-polkagate/src/hooks/useAssetsBalances.ts

[failure] 132-132:
Cannot find name 'UserAddedChains'.

🔇 Additional comments (2)
packages/extension-polkagate/src/hooks/useAssetsBalances.ts (1)

9-9: LGTM: Import and type definition updates

The changes to the import statements and type definitions look good. The addition of the DropdownOption import and the updates to AssetsBalancesPerChain and AssetsBalancesPerAddress type aliases improve type safety and readability.

Also applies to: 27-28

packages/extension-polkagate/src/hooks/usePriceIds.ts (1)

39-39: Verify inclusion of userAddedPriceIds in the price IDs set

Including userAddedPriceIds in the nonDuplicatedPriceIds set is essential for incorporating user-added chains. Ensure that any potential duplicates are correctly handled and that the set accurately reflects all unique price IDs.

You can run the following script to verify that nonDuplicatedPriceIds includes all expected price IDs without duplicates:

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (1)
packages/extension-polkagate/src/hooks/usePriceIds.ts (1)

34-39: LGTM: Improved robustness and integration of user-added price IDs

The changes in these lines enhance the code's robustness by filtering out falsy values and correctly incorporate user-added price IDs. This aligns well with the PR objective of integrating custom chain specifications.

For improved clarity, consider using a more explicit filter condition:

let selectedChainsChainName = nonTestNetSelectedChains?.map((genesisHash) => getChainName(genesisHash)).filter((name): name is string => name !== undefined && name !== null);

This makes it clear that we're removing undefined and null values, which might be helpful for future maintenance.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 64d3512 and 6436950.

📒 Files selected for processing (1)
  • packages/extension-polkagate/src/hooks/usePriceIds.ts (2 hunks)
🔇 Additional comments (4)
packages/extension-polkagate/src/hooks/usePriceIds.ts (4)

5-7: LGTM: Necessary imports added

The new imports for useEffect, useState, and getStorage are correctly added and align with the new functionality introduced in this hook.


20-21: LGTM: New state for user-added price IDs

The introduction of userAddedPriceIds state using useState is a good approach for managing user-added price IDs. This aligns well with the PR objective of allowing chain developers to add their chain specifications.


42-42: LGTM: Updated useMemo dependency array

The addition of userAddedPriceIds to the useMemo dependency array is correct and necessary. This ensures that the memoized value is recalculated when user-added price IDs change, maintaining consistency with the new functionality.


Line range hint 1-44: Summary: Successfully implemented custom chain integration with room for minor improvements

The changes in this file successfully implement the feature of allowing chain developers to add their chain specifications, as outlined in the PR objectives. The new functionality is well-integrated with the existing code, and the use of React hooks is appropriate.

Key points:

  1. User-added price IDs are now fetched and incorporated into the final set of price IDs.
  2. The code's robustness has been improved by filtering out potential falsy values.
  3. The useMemo hook has been correctly updated to depend on the new state.

Suggestions for improvement:

  1. Enhance error handling for the getStorage call.
  2. Implement more thorough input validation for user-added price IDs.
  3. Consider using more explicit filtering for clarity.

Overall, this is a solid implementation that meets the PR objectives while maintaining code quality and extensibility.

@Nick-1979 Nick-1979 merged commit 4613722 into main Sep 28, 2024
8 checks passed
@Nick-1979 Nick-1979 deleted the addNewChain branch September 28, 2024 07:33
github-actions bot pushed a commit that referenced this pull request Sep 28, 2024
# [0.13.0](v0.12.3...v0.13.0) (2024-09-28)

### Features

* add a new chain ([#1553](#1553)) ([4613722](4613722))
This was referenced Nov 7, 2024
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.

add cutom chain
3 participants