Skip to content

Conversation

@vutuanlinh2k2
Copy link
Contributor

@vutuanlinh2k2 vutuanlinh2k2 commented Jan 14, 2026

Summary

  • Add substrate wallet selection modal for claim migration with proper wallet connection handling
  • Add unlock configuration display to claim migration page
  • Replace empty function with Function.prototype for lint compliance
  • Code formatting fixes

Images

CleanShot 2026-01-14 at 16 08 30@2x CleanShot 2026-01-14 at 15 34 53@2x

vutuanlinh2k2 and others added 5 commits January 13, 2026 16:25
…ation

- Add SubstrateWalletModal for selecting between Polkadot.js, Talisman, and SubWallet
- Update SubstrateWalletSelector to use the new modal with localStorage persistence
- Store extension reference for signing without re-fetching
- Add wallet icons (PolkadotJsIcon, SubWalletIcon)
- Filter substrate wallets from EVM wallet modal

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Display unlock percentage and lock schedule on the claim page. Shows
unlocked vs locked amounts with unlock date. Also adds explorer link
for transaction hash in success state.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@claude
Copy link

claude bot commented Jan 14, 2026

Claude finished @vutuanlinh2k2's task —— View job


Code Review Complete ✅

I've thoroughly reviewed the PR changes for the substrate wallet connection fixes and unlock configuration display. Here's my comprehensive feedback:

Todo List:

  • Read and analyze all changed files
  • Review SubstrateWalletModal component implementation
  • Check SubstrateWalletSelector modifications
  • Evaluate claim eligibility hook changes
  • Review unlock configuration display logic
  • Assess wallet connection handling
  • Check new wallet icons implementation
  • Provide comprehensive feedback

Overall Assessment

This is a well-structured PR that adds proper substrate wallet management to the claim migration flow with good separation of concerns and user experience improvements.

Detailed Review

Strengths

  1. Clean Architecture: Well-separated modal component (SubstrateWalletModal) with clear responsibilities
  2. Robust Error Handling: Comprehensive error handling with user-friendly messages for various failure modes
  3. Persistent State: Smart localStorage integration to maintain wallet connections across sessions
  4. Good UX: Loading states, proper animations, and clear visual feedback
  5. Type Safety: Strong TypeScript typing throughout
  6. Unlock Configuration: Well-implemented unlock display showing immediate vs locked tokens

🔧 Code Quality Issues

SubstrateWalletModal.tsx (apps/tangle-dapp/src/pages/claim/migration/components/SubstrateWalletModal.tsx)

  1. Function.prototype Usage (Line 39):

    return Function.prototype as Unsubcall;

    While this satisfies the linter, it's semantically incorrect. Consider:

    return (() => {}) as Unsubcall;
  2. Duplicated Code: The ensureAccountsSubscribe and findSubstrateWallet functions are duplicated between SubstrateWalletModal.tsx and SubstrateWalletSelector.tsx. These should be extracted to a shared utility file.

  3. Window Global Access: Direct access to window.injectedWeb3 should be wrapped with proper browser environment checks:

    if (typeof window === 'undefined' || !window.injectedWeb3) return;

SubstrateWalletSelector.tsx (apps/tangle-dapp/src/pages/claim/migration/components/SubstrateWalletSelector.tsx)

  1. Code Duplication: Same utility functions duplicated from the modal component
  2. localStorage Usage: Missing error boundaries for localStorage operations (line 157-158)
  3. Effect Dependencies: The useEffect at line 165 has incomplete dependencies - should include convertAccounts, onAccountSelect, onExtensionChange

useClaimEligibility.ts (apps/tangle-dapp/src/pages/claim/migration/hooks/useClaimEligibility.ts)

  1. Query Configuration: Line 934 sets staleTime: 0, gcTime: 0 which defeats React Query's caching benefits. Consider using refetchOnMount: 'always' instead
  2. Mock Data Security: Dev mode mock data (lines 1048-1064) should have clear warnings that it's for development only

🔒 Security Considerations

  1. Challenge Generation: The challenge generation logic is correct and prevents replay attacks by binding signatures to specific parameters
  2. Input Validation: Good validation of addresses and amounts before processing
  3. No Sensitive Data Exposure: No private keys or sensitive data exposed in logs or UI

🚀 Performance Considerations

  1. React Query Usage: Generally good, but the claim status query could benefit from better caching strategy
  2. Memoization: Good use of useMemo and useCallback to prevent unnecessary re-renders
  3. Component Re-renders: The unlock configuration display efficiently calculates amounts without excessive computation

📝 Documentation & Maintenance

  1. Type Definitions: Well-defined TypeScript interfaces
  2. Comments: Good inline documentation for complex logic
  3. Error Messages: User-friendly error messages throughout

🔄 Suggested Improvements

  1. Extract Shared Utilities: Create a shared file for wallet connection utilities:

    // apps/tangle-dapp/src/pages/claim/migration/utils/walletUtils.ts
    export const ensureAccountsSubscribe = (extension: InjectedExtension) => { ... }
    export const findSubstrateWallet = async (walletName: string) => { ... }
  2. Add Error Boundaries: Wrap localStorage operations in try-catch blocks

  3. Improve Function.prototype Usage: Replace with proper empty function for semantic correctness

  4. Enhanced Loading States: Consider skeleton loading for better UX during wallet detection

📊 Testing Considerations

The PR includes comprehensive test scenarios in the description. Ensure testing covers:

  • ✅ Wallet modal opens and connects
  • ✅ Connection persistence
  • ✅ Unlock configuration display
  • ✅ End-to-end claim flow

📋 Minor Issues

  1. EvmWalletModal.tsx (Line 30): The substrate wallet filtering logic works but could be more maintainable with a configuration object
  2. Icon Files: New wallet icons are well-structured and follow existing patterns

Recommendation

✅ APPROVE with minor cleanup suggested

This PR successfully implements the required functionality with good architecture and user experience. The suggested improvements are non-blocking and can be addressed in follow-up PRs if desired.


Extract duplicated wallet connection code from SubstrateWalletModal
and SubstrateWalletSelector into shared utils/walletUtils.ts file.

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@vutuanlinh2k2 vutuanlinh2k2 self-assigned this Jan 14, 2026
@tangletools
Copy link

Likely needs some small more improvements indicating that that is a tx hash?

@tangletools tangletools merged commit 619adb4 into v2 Jan 14, 2026
1 of 2 checks passed
@tangletools tangletools deleted the linh/fix/claim-wallet-connection branch January 14, 2026 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants