Skip to content

Conversation

@AtelyPham
Copy link
Member

Summary of changes

Provide a detailed description of proposed changes.

  • Network Tab Interface: Added mainnet/testnet toggle in the leaderboard UI with animated tabs
  • Dynamic GraphQL Endpoints: Parameterized GraphQL execution to support network-specific endpoints
  • Query Enhancement: Updated all data queries to accept network parameters
  • Configuration Updates: Changed default GraphQL endpoint from testnet to mainnet
  • Utility Functions: Created network-to-endpoint mapping utility for clean endpoint resolution

Key technical improvements:

  • All GraphQL queries now accept a NetworkType parameter
  • The executeGraphQL utility function is parameterized with endpoint URLs
  • Sync progress indicator shows network-specific indexing status
  • Network state is managed through React state with proper query key invalidation
  • Default environment configuration updated to use mainnet

Proposed area of change

Put an x in the boxes that apply.

  • apps/tangle-dapp
  • apps/tangle-cloud
  • apps/leaderboard
  • libs/tangle-shared-ui
  • libs/ui-components

Associated issue(s)

Specify any issues that can be closed from these changes (e.g. Closes #233).

  • Closes NaN

Screen Recording

If possible provide screenshots and/or a screen recording of proposed change.

CleanShot.2025-08-13.at.23.37.46.mp4

@netlify
Copy link

netlify bot commented Aug 13, 2025

Deploy Preview for tangle-dapp ready!

Name Link
🔨 Latest commit 5e18c9b
🔍 Latest deploy log https://app.netlify.com/projects/tangle-dapp/deploys/689cb16fae0bff0008b464a6
😎 Deploy Preview https://deploy-preview-3052--tangle-dapp.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@netlify
Copy link

netlify bot commented Aug 13, 2025

Deploy Preview for tangle-cloud ready!

Name Link
🔨 Latest commit 5e18c9b
🔍 Latest deploy log https://app.netlify.com/projects/tangle-cloud/deploys/689cb16fae0bff0008b464aa
😎 Deploy Preview https://deploy-preview-3052--tangle-cloud.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@AtelyPham AtelyPham moved this from Not Started 🕧 to Building 🏗️ in Tangle Universe Aug 13, 2025
@claude
Copy link

claude bot commented Aug 13, 2025

Claude finished @AtelyPham's task —— View job


Claude Code Review

I've completed my review of the mainnet/testnet toggle implementation. Here's my comprehensive feedback:

Review Tasks

  • Read repository guidelines (CLAUDE.md)
  • Analyze configuration changes (.env.example, codegen.ts)
  • Review GraphQL utility changes (executeGraphQL.ts)
  • Examine leaderboard component updates
  • Check indexing progress modifications
  • Review points data hook changes
  • Assess overall architecture and design decisions
  • Provide comprehensive feedback and recommendations

✅ Strengths

Good Architecture & Design:

  • Clean separation of concerns with network-specific GraphQL endpoints
  • Proper parameterization of the executeGraphQL utility function
  • Well-structured React Query integration with network-specific cache keys
  • Consistent NetworkType usage across components

UI/UX Implementation:

  • Animated tab interface provides good user experience
  • Network-specific sync progress indicator shows appropriate status
  • Points breakdown tooltip showing mainnet/testnet split is helpful

Code Quality:

  • Good TypeScript usage with proper typing
  • Consistent error handling patterns
  • Proper React hooks usage with appropriate dependencies

🔍 Issues & Concerns

1. Critical Bug in Block Number Logic - LeaderboardTable.tsx:223

const result = latestBlock.testnetBlock - BLOCK_COUNT_IN_SEVEN_DAYS;

This hardcodes testnetBlock regardless of the selected network. Should be:

const result = (networkTab === 'MAINNET' ? latestBlock.mainnetBlock : latestBlock.testnetBlock) - BLOCK_COUNT_IN_SEVEN_DAYS;

2. Incomplete Query Key Migration - useActiveAccountPoints.ts:37

The query key doesn't include the network parameter:

queryKey: [ReactQueryKey.GetAccountPoints, activeAccount],

Should include network to prevent stale data:

queryKey: [ReactQueryKey.GetAccountPoints, activeAccount, network.id],

3. Missing Error Context - executeGraphQL.ts:22-24

The error handling is too generic:

if (!response.ok) {
  throw new Error('Network response was not ok');
}

Consider including response status and network context:

if (!response.ok) {
  throw new Error(`GraphQL request failed: ${response.status} ${response.statusText} (${network})`);
}

4. Performance Concern - LeaderboardTable.tsx:242-253

The leaderboard query runs on every render when blockNumberSevenDaysAgo changes, even if the calculation result is the same. Consider memoizing the query parameters.

5. Configuration Inconsistency

The default endpoint changed from testnet to mainnet in multiple places, but there's no migration guide or backward compatibility consideration for existing deployments.

🚀 Performance Considerations

  1. Query Caching: Good use of React Query with network-specific cache keys
  2. Debounced Search: Proper 300ms debounce on search input
  3. Pagination: Manual pagination implementation looks correct
  4. Unnecessary Re-renders: Consider memoizing the data transformation in LeaderboardTable

🔒 Security Assessment

  • ✅ Safe: No sensitive data exposure in network switching logic
  • ✅ Safe: GraphQL endpoints are properly validated
  • ✅ Safe: Address validation using existing utilities
  • ⚠️ Minor: Consider rate limiting for network switching to prevent API abuse

📚 Documentation Needs

  1. Missing: Documentation for the network switching feature
  2. Missing: Migration guide for the endpoint configuration change
  3. Missing: Comments explaining the network-to-endpoint mapping logic

🛠️ Recommendations

High Priority:

  1. Fix the block number bug in LeaderboardTable.tsx:223
  2. Update query key in useActiveAccountPoints.ts to include network
  3. Add network parameter to block-related queries

Medium Priority:

  1. Improve error messages in executeGraphQL.ts
  2. Add loading states during network switching
  3. Consider memoizing expensive calculations

Low Priority:

  1. Add JSDoc comments for the network utility functions
  2. Consider extracting the network tab logic to a custom hook
  3. Add unit tests for the network switching logic

🎯 Code Quality Score: 7.5/10

The implementation is solid with good architectural decisions, but has a few critical bugs that need immediate attention. The overall approach is well thought out and follows React/TypeScript best practices.


@netlify
Copy link

netlify bot commented Aug 13, 2025

Deploy Preview for tangle-leaderboard ready!

Name Link
🔨 Latest commit 5e18c9b
🔍 Latest deploy log https://app.netlify.com/projects/tangle-leaderboard/deploys/689cb16f7ba25a000800f553
😎 Deploy Preview https://deploy-preview-3052--tangle-leaderboard.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@github-project-automation github-project-automation bot moved this from Building 🏗️ to Completed ✅ in Tangle Universe Aug 14, 2025
@devpavan04 devpavan04 merged commit 533e9fc into develop Aug 14, 2025
22 checks passed
@devpavan04 devpavan04 deleted the trung-tin/add-mainnet-to-leaderboard branch August 14, 2025 14:45
@claude claude bot mentioned this pull request Sep 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Completed ✅

Development

Successfully merging this pull request may close these issues.

3 participants