-
Notifications
You must be signed in to change notification settings - Fork 5
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: get chain using the address #1671
Conversation
WalkthroughThe changes in this pull request enhance the Changes
Possibly related PRs
Suggested labels
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
packages/extension-polkagate/src/popup/import/addWatchOnly/index.tsx (2)
39-43
: Consider enhancing error handling feedback.While the error is logged, consider providing user feedback for failed chain metadata retrieval.
const getChain = useCallback((genesisHash: string) => { getMetadata(genesisHash, true).then(setChain).catch((error): void => { console.error(error); + // Consider adding user feedback + onAction('/error', { error: t('Failed to retrieve chain metadata') }); }); }, []);
56-69
: Consider optimizing state updates.The chain detection logic is sound, but consider memoizing the addressChain result to prevent unnecessary re-renders.
+ const addressChain = useMemo(() => + realAddress ? addressToChain(realAddress) : null, + [realAddress]); useEffect(() => { if (realAddress) { - const addressChain = addressToChain(realAddress); addressChain?.genesisHash && getChain(addressChain.genesisHash); // ... rest of the effect } }, [api, getChain, realAddress]);packages/extension-polkagate/src/popup/import/addWatchOnlyFullScreen/index.tsx (2)
70-74
: Consider adding loading state management.While the chain metadata fetching logic is sound, consider adding a loading state to handle the asynchronous nature of
getChain
and improve user feedback.+ const [isLoadingChain, setIsLoadingChain] = useState(false); const getChain = useCallback((genesisHash: string) => { + setIsLoadingChain(true); getMetadata(genesisHash, true) .then(setChain) .catch((error): void => { console.error(error); }) + .finally(() => setIsLoadingChain(false)); }, []);
90-96
: Consider adding user feedback for account creation process.While the account creation logic is solid, consider providing feedback to users during the process, especially for error cases.
createAccountExternal(name, realAddress, chain.genesisHash as HexString) .then(() => { setStorage('profile', PROFILE_TAGS.WATCH_ONLY).catch(console.error); }) .finally(() => openOrFocusTab(`/accountfs/${substrateAddress}/0`, true)) .catch((error: Error) => { setIsBusy(false); + // Consider using a toast or notification system + console.error('Failed to create account:', error); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
packages/extension-polkagate/src/popup/import/addWatchOnly/index.tsx
(3 hunks)packages/extension-polkagate/src/popup/import/addWatchOnlyFullScreen/index.tsx
(5 hunks)packages/extension-polkagate/src/util/utils.ts
(2 hunks)
🔇 Additional comments (7)
packages/extension-polkagate/src/popup/import/addWatchOnly/index.tsx (2)
22-22
: LGTM!
The new utility imports are appropriate for the enhanced address and chain handling functionality.
24-24
: Verify component integration with related features.
Let's verify the integration with related components and features.
✅ Verification successful
Component integration verified successfully
The component is well-integrated within the codebase:
- Properly shares common utilities (
addressToChain
,getSubstrateAddress
) with other components in the staking, proxy management, and account-related features - Correctly integrated with the watch-only account system, alongside other related components like
addWatchOnlyFullScreen
- Well-connected to the proxy management system through
ProxyTable
component usage - No duplicate or conflicting implementations found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for related components and potential integration points
# Check for other components using the same utilities
echo "Components using addressToChain or getSubstrateAddress:"
rg -l "addressToChain|getSubstrateAddress" --type typescript
# Check for other watch-only account related components
echo "Related watch-only components:"
rg -l "Watch-Only|WatchOnly" --type typescript
# Check for proxy-related components
echo "Related proxy components:"
rg -l "ProxyTable" --type typescript
Length of output: 506
Script:
#!/bin/bash
# Description: Check for related components and potential integration points with correct file type
# Check for other components using the same utilities
echo "Components using addressToChain or getSubstrateAddress:"
rg -l "addressToChain|getSubstrateAddress"
# Check for other watch-only account related components
echo "Related watch-only components:"
rg -l "Watch-Only|WatchOnly"
# Check for proxy-related components
echo "Related proxy components:"
rg -l "ProxyTable"
# Check for direct imports or usage of AddAddressOnly
echo "Components referencing AddAddressOnly:"
rg -l "AddAddressOnly"
Length of output: 5541
packages/extension-polkagate/src/popup/import/addWatchOnlyFullScreen/index.tsx (3)
28-28
: LGTM! New utility imports enhance address processing capabilities.
The addition of addressToChain
and getSubstrateAddress
utilities improves the modularity of address processing logic.
52-56
: LGTM! Well-structured chain metadata fetching utility.
The getChain
function is properly memoized and includes error handling.
28-28
: Verify the usage of new utility functions across the codebase.
Let's ensure the new utility functions are consistently used across the codebase.
Also applies to: 90-91
✅ Verification successful
Based on the shell script results, I can now generate the final response:
The utility functions are consistently used across the codebase
addressToChain
is used in both watch-only import componentsgetSubstrateAddress
is widely used across 40+ files for consistent address handling, particularly in:- Account management components
- Staking operations (solo and pool)
- Proxy management
- Identity handling
- Transaction reviews
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check usage of addressToChain and getSubstrateAddress functions
# Check for addressToChain usage
echo "Checking addressToChain usage:"
rg "addressToChain\(" --type ts
# Check for getSubstrateAddress usage
echo -e "\nChecking getSubstrateAddress usage:"
rg "getSubstrateAddress\(" --type ts
# Check for potential similar implementations
echo -e "\nChecking for potential similar implementations:"
rg "\.encode|\.decode" --type ts
Length of output: 9387
packages/extension-polkagate/src/util/utils.ts (2)
19-20
: LGTM!
The new imports are correctly placed and properly used in the new functionality.
583-605
: Verify comprehensive test coverage for address formats.
Please ensure that the function is tested with:
- Addresses from all supported chains
- Invalid addresses
- Edge cases (empty string, malformed addresses)
- Addresses with different SS58 formats
✅ Verification successful
Based on the comprehensive analysis of the codebase, I can now provide a final response regarding the address format verification concern:
Address format handling is properly implemented and secure
The implementation of addressToChain
function is robust and covers all necessary cases:
- Input validation is handled through
isValidAddress
which properly validates addresses usingencodeAddress/decodeAddress
from@polkadot/util-crypto
- Substrate format is checked via
getSubstrateAddress
- Chain detection uses SS58 format matching against
allChains
which contains the complete chain registry - Default fallback to Westend (42) format when no specific chain is matched
- Proper error handling with null returns for invalid cases
The function is used consistently across the codebase in address input components and identity verification flows, with proper chain-specific SS58 format handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify test coverage and usage of addressToChain function
# Check for test files
echo "Checking for test files..."
fd -e test.ts -e spec.ts
# Look for existing test cases
echo "Searching for existing test cases..."
rg -A 5 "addressToChain" --type ts
# Check for different address format handling
echo "Checking address format handling..."
ast-grep --pattern 'isValidAddress($addr)' --type ts
Length of output: 3657
Script:
#!/bin/bash
# Let's verify the test coverage and implementation details more thoroughly
# Check for test files specifically related to address handling
echo "Checking for address-related test files..."
rg -l "address|isValidAddress" $(fd -e test.ts -e spec.ts)
# Look for the implementation of isValidAddress and its dependencies
echo "Checking isValidAddress implementation..."
rg "isValidAddress" --type ts -A 5
# Check for SS58 format handling across the codebase
echo "Checking SS58 format handling..."
rg "ss58Format|encodeAddress|decodeAddress" --type ts -A 3
# Check for chain definitions to verify coverage
echo "Looking for chain definitions..."
rg "WESTEND_GENESIS_HASH|allChains" --type ts -A 3
Length of output: 70751
packages/extension-polkagate/src/popup/import/addWatchOnly/index.tsx
Outdated
Show resolved
Hide resolved
packages/extension-polkagate/src/popup/import/addWatchOnlyFullScreen/index.tsx
Show resolved
Hide resolved
# [0.33.0](v0.32.1...v0.33.0) (2024-11-25) ### Features * get chain using the address ([#1671](#1671)) ([1fa7798](1fa7798))
#1670
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation