Conversation
✅ Deploy Preview for proof-of-humanity-v2 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughThe PR adds explicit chain validation with detailed error handling and logging to the vouch API route, introduces debug logging to the batch write hook for error tracking, and switches the off-chain vouches endpoint from an environment variable to a hardcoded testnet URL. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/data/request.ts`:
- Around line 163-167: The axios GET is using a hardcoded testnet URL which
risks hitting testnet from production; change the call in src/data/request.ts so
the base URL comes from a configurable constant or environment variable (e.g.,
POH_API_BASE or config.api.pohBase) instead of
`testnets--proof-of-humanity-v2.netlify.app`, and build the request URL from
that base with the same path segment
`/api/vouch/${chainId}/for-request/${claimer}/${pohId}`; ensure the code that
calls axios.get (the expression using chainId, claimer, pohId) falls back to a
clearly-documented testnet URL only if the env/config value is missing.
🧹 Nitpick comments (2)
src/app/api/vouch/[chain]/for-request/[claimer]/[pohid]/route.ts (1)
36-43: Debug logs: query params logged after execution; full response dumped.Two observations:
- The DB query params (Lines 36-40) are logged after the query on Lines 29-34 has already executed. Move this log before the query for easier debugging of failures.
- Line 43 logs the entire
dataarray. In a busy environment this can be very noisy and may inadvertently expose vouch details (signatures, expiration) in server logs.Since this targets
testnets, this is acceptable short-term, but consider guarding with a log-level check or removing before any production merge.Suggested reorder
+ console.log("API Route DB Query Params:", { + chainId: chain.id, + pohId: params.pohid.toLowerCase(), + claimer: params.claimer.toLowerCase() + }); + const { data, error } = await datalake .from("poh-vouchdb") .select("*") .eq("chainId", chain.id) .eq("pohId", params.pohid.toLowerCase()) .eq("claimer", params.claimer.toLowerCase()); - console.log("API Route DB Query Params:", { - chainId: chain.id, - pohId: params.pohid.toLowerCase(), - claimer: params.claimer.toLowerCase() - }); - - - console.log("vouches for request:", data); + console.log("vouches for request: count=", data?.length);src/contracts/hooks/useBatchWrite.ts (1)
14-14:console.logon every render — move into auseEffector remove.Lines 14 and 22 execute unconditionally in the component body, so they fire on every re-render (including unrelated state changes from parent components). This adds noise to the console and has a minor perf cost from serializing
capabilitieseach time.Move to a useEffect
- console.log("capabilities:", capabilities, capabilitiesError); + useEffect(() => { + console.log("capabilities:", capabilities, capabilitiesError); + }, [capabilities, capabilitiesError]); ... - console.log("supportsBatchingTransaction:", supportsBatchingTransaction); + useEffect(() => { + console.log("supportsBatchingTransaction:", supportsBatchingTransaction); + }, [supportsBatchingTransaction]);Also applies to: 22-22
Comment on lines
+163
to
167
| //Prod endpoint not work need to debug it, using testnet endpoint for now, they both point to same DB in the backend | ||
| return ( | ||
| await axios.get( | ||
| `${process.env.DEPLOYED_APP}/api/vouch/${chainId}/for-request/${claimer}/${pohId}`, | ||
| `https://testnets--proof-of-humanity-v2.netlify.app/api/vouch/${chainId}/for-request/${claimer}/${pohId}`, | ||
| ) |
Contributor
There was a problem hiding this comment.
Hardcoded testnet URL is a reliability and deployment risk.
Replacing the environment-variable-based endpoint with a hardcoded testnets--proof-of-humanity-v2.netlify.app URL means:
- If this code path is ever reached from a production deployment (or merged to
main), production users will silently hit a testnet endpoint. - The testnet Netlify deployment could go down or change independently of production, breaking vouching.
Even for the testnets branch, prefer an environment variable or at minimum a clearly-scoped constant so it doesn't silently propagate.
Suggested safer approach
- //Prod endpoint not work need to debug it, using testnet endpoint for now, they both point to same DB in the backend
+ // TODO: Revert to env-based URL once prod endpoint issue is resolved
return (
await axios.get(
- `https://testnets--proof-of-humanity-v2.netlify.app/api/vouch/${chainId}/for-request/${claimer}/${pohId}`,
+ `${process.env.NEXT_PUBLIC_VOUCH_API_URL ?? "https://testnets--proof-of-humanity-v2.netlify.app"}/api/vouch/${chainId}/for-request/${claimer}/${pohId}`,
)
).data as { voucher: Address; expiration: number; signature: Hash }[];🤖 Prompt for AI Agents
In `@src/data/request.ts` around lines 163 - 167, The axios GET is using a
hardcoded testnet URL which risks hitting testnet from production; change the
call in src/data/request.ts so the base URL comes from a configurable constant
or environment variable (e.g., POH_API_BASE or config.api.pohBase) instead of
`testnets--proof-of-humanity-v2.netlify.app`, and build the request URL from
that base with the same path segment
`/api/vouch/${chainId}/for-request/${claimer}/${pohId}`; ensure the code that
calls axios.get (the expression using chainId, claimer, pohId) falls back to a
clearly-documented testnet URL only if the env/config value is missing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Bug Fixes
Chores