Skip to content

Get votes from subgraph#337

Open
Olaleye-Blessing wants to merge 1 commit into
developmentfrom
get-votes-from-subgraph
Open

Get votes from subgraph#337
Olaleye-Blessing wants to merge 1 commit into
developmentfrom
get-votes-from-subgraph

Conversation

@Olaleye-Blessing
Copy link
Copy Markdown
Collaborator

@Olaleye-Blessing Olaleye-Blessing commented Oct 6, 2025

closes: #330

@netlify
Copy link
Copy Markdown

netlify Bot commented Oct 6, 2025

Deploy Preview for dapper-sundae-ae0873 ready!

Name Link
🔨 Latest commit e157e05
🔍 Latest deploy log https://app.netlify.com/projects/dapper-sundae-ae0873/deploys/68e84e90035ef30008d97753
😎 Deploy Preview https://deploy-preview-337--dapper-sundae-ae0873.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.

@Olaleye-Blessing Olaleye-Blessing force-pushed the get-votes-from-subgraph branch from a999acb to 3326e74 Compare October 9, 2025 22:41
@Olaleye-Blessing Olaleye-Blessing changed the base branch from main to development October 9, 2025 22:59
@Olaleye-Blessing Olaleye-Blessing force-pushed the get-votes-from-subgraph branch 5 times, most recently from 35f1dba to 00da27b Compare October 10, 2025 00:00
@Olaleye-Blessing Olaleye-Blessing marked this pull request as ready for review October 10, 2025 00:08
@tbsoc
Copy link
Copy Markdown
Member

tbsoc commented Feb 25, 2026

/review

@tbsoc
Copy link
Copy Markdown
Member

tbsoc commented Feb 25, 2026

🤖 AI Code Review (Requested by @tbsoc)

Code Review: Get votes from subgraph

Summary

This PR migrates vote data fetching from direct blockchain event logs to a GraphQL subgraph. It replaces the getContractEvents call with a graphql-request query, simplifying the data parsing and removing the need for viem's public client in this hook.


Code Quality

Architecture & Patterns

  • Good separation of concerns: The parsing logic is cleanly extracted into parseVoteFromSubgraph, making it easy to test and modify independently.
  • Type safety: Proper TypeScript interfaces for the GraphQL response (BreadHolderVoted, QueryResponse) are well-defined.
  • Consistent with React Query patterns: The useQuery hook structure remains intact, preserving caching and refetch behavior.

Readability

  • The GraphQL query is readable and well-formatted.
  • Variable naming is clear (breadHolderVoteds matches the subgraph schema).

Minor issues:

  • The getAddress normalization for projects is good, but consider adding a comment explaining why checksum addresses matter here (consistency with other parts of the app).

Security

Concerns:

  1. API key exposure: process.env.NEXT_PUBLIC_SUBGRAPH_API_KEY suggests this key is meant for the browser. If this is a sensitive key, it should not be prefixed with NEXT_PUBLIC_. Verify:

    • Is this key rate-limited or scoped to read-only operations?
    • Can it be abused if exposed to users?
  2. No input sanitization on lastClaimedBlockNumber: The value is interpolated directly into the GraphQL query. While GraphQL variables would be safer, the current implementation uses template literals. Consider:

    // Safer approach using variables
    const query = gql`
      query GetVotes($blockNumber: String!) {
        breadHolderVoteds(where: { blockNumber_gte: $blockNumber }) {
          ...
        }
      }
    `;
    const data = await client.request<QueryResponse>(query, { 
      blockNumber: lastClaimedBlockNumber?.toString() 
    });
  3. Missing error handling: The query could fail due to network issues, subgraph downtime, or rate limiting. Consider adding retry logic or graceful degradation.


Suggestions

Priority Suggestion
High Use GraphQL variables instead of string interpolation for lastClaimedBlockNumber to prevent potential injection issues
High Verify the NEXT_PUBLIC_SUBGRAPH_API_KEY exposure — if sensitive, move to server-side only
Medium Add error handling with user-friendly fallback (e.g., "Unable to load votes, retrying...")
Medium Consider adding a loading state or skeleton UI while votes are fetching
Low Add unit tests for parseVoteFromSubgraph to ensure parsing edge cases are covered
Low Document the subgraph schema dependency in a README or code comment

Overall Assessment

This is a solid migration that improves data fetching efficiency. The main concerns are around security (API key exposure and query parameterization) and error handling. Addressing the high-priority items would make this production-ready.

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.

2 participants