Testnets #116
Testnets #116
Conversation
…ation Profiles keep both registrations from issue on legacy subgraph Pending fix on subgraph This is a temporary workaround for fixing the issue at front-end
fix: vouch icon does not show due to legacy subgraph issue on registr…
WalkthroughThe changes involve a significant reorganization of imports and minor adjustments in the Changes
Poem
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 (
|
✅ Deploy Preview for proof-of-humanity-v2 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
src/app/[pohid]/[chain]/[request]/page.tsx (2)
Line range hint
89-111: Consider refactoring the hasExpired functionThe function contains:
- Commented out code that should be either removed or uncommented
- Complex nested conditions that could be simplified
Consider breaking down the logic into smaller, more focused functions:
- const hasExpired = () => { + const isResolvedRequestExpired = (request, contractData) => { + if (!request.revocation && request.humanity.winnerClaim.length > 0) { + if (request.humanity.winnerClaim[0].index === request.index) { + if (!!contractData.humanityLifespan) { + return !request.humanity.registration || + Number(request.humanity.registration?.expirationTime) < Date.now() / 1000; + } + } + } + return true; + }; + + const isTransferringRequestExpired = (request, contractData) => { + return Number(request.creationTime) + Number(contractData.humanityLifespan) < Date.now() / 1000; + }; + + const hasExpired = () => { if (request.status.id === "resolved") { - if (!request.revocation && request.humanity.winnerClaim.length > 0) { - if (request.humanity.winnerClaim[0].index === request.index) { - if (!!contractData.humanityLifespan) { - return ( - !request.humanity.registration || - Number(request.humanity.registration?.expirationTime) < - Date.now() / 1000 - ); - } - } else return true; - } + return isResolvedRequestExpired(request, contractData); } else if (request.status.id === "transferring") { - return ( - Number(request.creationTime) + Number(contractData.humanityLifespan) < - Date.now() / 1000 - ); + return isTransferringRequestExpired(request, contractData); } return true; };
Line range hint
146-201: Enhance error handling and type safety in prepareVouchDataThe function could benefit from:
- More specific error handling
- Better separation of concerns
- Stronger type safety
Consider these improvements:
+ interface VouchChainData { + claimer?: { + id?: Address; + name?: string; + registration?: { + humanity: { + id?: Address; + winnerClaim: Array<{ + evidenceGroup: { + evidence: Array<{ uri: string }>; + }; + }>; + }; + }; + }; + } + const getVoucherChainData = ( + rawVouches: Record<SupportedChainId, ClaimerQuery>[], + chain: typeof supportedChains[number] + ): VouchChainData => { + const voucherEvidenceChain = supportedChains.find( + (chain) => + rawVouches[chain.id].claimer && + rawVouches[chain.id].claimer?.registration?.humanity.winnerClaim + ); + const relevantChain = voucherEvidenceChain ?? chain; + return rawVouches[relevantChain.id]; + }; const prepareVouchData = async ( rawVouches: Record<SupportedChainId, ClaimerQuery>[], isOnChain: boolean, skipStatusCheck: boolean, ): Promise<VouchData[]> => { return Promise.all( rawVouches.map(async (rawVoucher) => { const out: VouchData = { voucher: undefined, name: undefined, pohId: undefined, photo: undefined, vouchStatus: undefined, isOnChain, }; try { - const voucherEvidenceChain = supportedChains.find( - (chain) => - rawVoucher[chain.id].claimer && - rawVoucher[chain.id].claimer?.registration?.humanity.winnerClaim, - ); - const relevantChain = !!voucherEvidenceChain - ? voucherEvidenceChain - : chain; + const chainData = getVoucherChainData(rawVoucher, chain); + const { claimer } = chainData; - out.name = rawVoucher[relevantChain.id].claimer?.name; - out.voucher = rawVoucher[relevantChain.id].claimer?.id; + out.name = claimer?.name; + out.voucher = claimer?.id; + out.pohId = claimer?.registration?.humanity.id ?? out.voucher; - out.pohId = - rawVoucher[relevantChain.id].claimer?.registration?.humanity.id; - if (!out.pohId) out.pohId = out.voucher; if (!skipStatusCheck) { out.vouchStatus = isOnChain ? isValidOnChainVouch( request.claimer.vouchesReceived.find( (v) => v.from.id === out.voucher!, )! as VouchQuery, ) : await isValidVouch( chain.id, out.voucher!, offChainVouches.find( (vouch) => vouch.voucher === out.voucher, )?.expiration, ); } - const uri = rawVoucher[ - relevantChain.id - ].claimer?.registration?.humanity.winnerClaim - .at(0) - ?.evidenceGroup.evidence.at(0)?.uri; + const uri = claimer?.registration?.humanity.winnerClaim + .at(0) + ?.evidenceGroup.evidence.at(0) + ?.uri; if (!uri) return out; const evFile = await ipfsFetch<EvidenceFile>(uri); if (!evFile?.fileURI) return out; const regFile = await ipfsFetch<RegistrationFile>(evFile.fileURI); out.photo = regFile.photo; return out; - } catch { + } catch (error) { + console.error('Error preparing vouch data:', error); return out; } }), ); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/app/[pohid]/[chain]/[request]/page.tsx (1 hunks)
- src/data/claimer.ts (2 hunks)
🔇 Additional comments (4)
src/data/claimer.ts (2)
8-28:⚠️ Potential issueSeveral improvements needed in the sanitization function.
The function has several areas that could be improved for better maintainability and reliability:
- The function mutates its input parameter without documenting this behavior
- Using
mapfor side effects is an antipattern- Using
varinstead ofletis outdated- The nested conditionals make the logic harder to follow
Consider this refactored version:
// This fixes an error in the legacy subgraph were registration has not // been removed as expected. Once solved the issue at subgraph level this // function should be removed +/** + * @param res - Record of claimer queries by chain ID + * @description Mutates the input parameter to fix legacy subgraph registration issues + */ const sanitizeRegistration = (res: Record<SupportedChainId, ClaimerQuery>) => { - var regChain: SupportedChainId | undefined = undefined; + let regChain: SupportedChainId | undefined = undefined; - supportedChains.map((chain) => { + supportedChains.forEach((chain) => { if (!regChain && res[chain.id].claimer?.registration) regChain = chain.id; else if (regChain && res[chain.id].claimer?.registration) { + const currentTime = res[chain.id].claimer?.registration?.humanity.winnerClaim.at(0)?.resolutionTime; + const regChainTime = res[regChain].claimer?.registration?.humanity.winnerClaim.at(0)?.resolutionTime; + + if (!currentTime || !regChainTime) return; + if ( - res[regChain].claimer?.registration?.humanity.winnerClaim.at(0) - ?.resolutionTime > - res[chain.id].claimer?.registration?.humanity.winnerClaim.at(0) - ?.resolutionTime + regChainTime > currentTime ) { res[chain.id].claimer!.registration = null; } else {Let's verify the usage of this temporary fix:
#!/bin/bash # Check if there are any TODOs or related comments about subgraph fixes rg -i "subgraph.*fix|fix.*subgraph" --type ts
Line range hint
8-41: Verify if this fix affects both testnet and mainnet.Since this PR is related to testnets, we should verify if this subgraph fix is needed for both testnet and mainnet environments.
Let's check for environment-specific code:
✅ Verification successful
Let me gather more information about the chain configurations and supported chains to better understand the scope of this fix.
Based on the gathered information, I can now provide a final response:
The fix is needed for both mainnet and testnet environments
The sanitization fix is required across all environments because:
- The code comment explicitly states it's fixing a legacy subgraph issue that needs to be addressed at the subgraph level
- The implementation in
sanitizeRegistrationis environment-agnostic and works with any supported chains- The same code is present in both mainnet and testnet configurations, as seen in the chain configurations (
chains.mainnets.tsandchains.testnets.ts)- The subgraph URLs in
config/subgraph.tsshow this affects both mainnet and testnet deployments🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for environment-specific handling in chain configurations rg -A 5 "mainnet|testnet" --type ts # Check the subgraph configurations rg -A 5 "subgraph.*url|url.*subgraph" --type tsLength of output: 2342
Script:
#!/bin/bash # Check chain configurations and supported chains rg -A 5 "supportedChains" --type ts # Check for any environment-specific handling in the claimer data rg -A 5 "sanitizeRegistration|sanitizeClaimerData" --type ts # Look for any chain-specific logic in the codebase ast-grep --pattern 'const mainnet = { $$$, id: $_ }'Length of output: 28316
src/app/[pohid]/[chain]/[request]/page.tsx (2)
1-30: LGTM! Well-organized importsThe imports are now logically grouped and well-organized, improving code readability.
Line range hint
203-458: Improve performance and accessibility in the UI renderingSeveral improvements could enhance the component:
- Async operations in rendering could cause performance issues
- Some images lack proper alt text
- Accessibility attributes could be enhanced
Consider these improvements:
- Move data preparation to a separate hook or server action:
// hooks/useVouchData.ts export const useVouchData = (vouches: Array<any>) => { const [vouchData, setVouchData] = useState<VouchData[]>([]); useEffect(() => { const prepareData = async () => { const data = await prepareVouchData(vouches, true, true); setVouchData(data); }; prepareData(); }, [vouches]); return vouchData; };
- Add proper alt text and ARIA labels:
<Image - alt="image" + alt={`Profile photo of ${registrationFile?.name || 'user'}`} src={ipfs(registrationFile.photo)} width={144} height={144} /> <video className="w-full" + aria-label="Registration video" src={ipfs(registrationFile.video)} controls />
- Add proper heading structure and ARIA landmarks:
- <div className="content mx-auto flex w-[84vw] max-w-[1500px]"> + <main className="content mx-auto flex w-[84vw] max-w-[1500px]" role="main"> <h1 className="sr-only">Request Details</h1> ... - </div> + </main>Let's verify the accessibility issues:
| sanitizeRegistration(out); | ||
| return await sanitizeClaimerData(out, id); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider error handling for sanitization step.
The integration of sanitizeRegistration looks correct, but consider adding error handling to prevent issues from propagating.
Consider wrapping the sanitization in a try-catch:
- sanitizeRegistration(out);
+ try {
+ sanitizeRegistration(out);
+ } catch (error) {
+ console.error('Failed to sanitize registration data:', error);
+ // Still proceed with sanitizeClaimerData as the original data might be usable
+ }
return await sanitizeClaimerData(out, id);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sanitizeRegistration(out); | |
| return await sanitizeClaimerData(out, id); | |
| try { | |
| sanitizeRegistration(out); | |
| } catch (error) { | |
| console.error('Failed to sanitize registration data:', error); | |
| // Still proceed with sanitizeClaimerData as the original data might be usable | |
| } | |
| return await sanitizeClaimerData(out, id); |
Summary by CodeRabbit
New Features
Improvements
Requestcomponent's code, maintaining its core functionality.