Testnets #132
Testnets #132
Conversation
workaround to be removed after subgraph's update
fix: subgraph's claimer.currentRequest persists after being transferred
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
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/data/user.ts (2)
11-12: Document the significance of the -100 index threshold.The magic number
-100appears to be a special threshold value. Please document why this specific value is significant and what it represents in the business logic.+ // Index values <= -100 indicate invalid or expired requests that should be nullified if (claimer.claimer?.currentRequest.index <= -100) {
8-9: Track the temporary nature of this fix.The comment indicates this is a temporary fix for a legacy subgraph issue, but there's no tracking mechanism.
Would you like me to create a GitHub issue to track the removal of this function once the subgraph issue is resolved?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/data/user.ts(1 hunks)
🔇 Additional comments (1)
src/data/user.ts (1)
10-16: Verify the impact on testnet configurations.
Given that this PR is titled "Testnets", we should verify how this change affects testnet behavior.
✅ Verification successful
Let me gather more specific information about how the testnet configurations are handled.
The index check is not testnet-specific and is safe across all environments
The code change checking for currentRequest.index <= -100 is a generic validation that works consistently across both mainnet and testnet configurations. From analyzing the codebase:
- The chain configuration (
chains.ts) shows proper separation between mainnet and testnet environments through theChainSetenum - The
currentRequesthandling is part of the core data model (as seen ingraphql.ts) and is environment-agnostic - The index check is a pure validation that doesn't interact with any network-specific logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for testnet-specific configurations or handling
rg -l 'testnet|devnet' --type ts
# Look for other index-related conditions in the codebase
ast-grep --pattern 'currentRequest.index'
Length of output: 177
Script:
#!/bin/bash
# Check the content of chain configuration file
cat src/config/chains.ts
# Look for any usage of currentRequest with context
rg "currentRequest" -A 3 --type ts
# Check the contract index file for testnet handling
cat src/contracts/index.ts
Length of output: 15635
| if (claimer.claimer?.currentRequest && claimer.claimer?.registration) { | ||
| if (claimer.claimer?.currentRequest.index <= -100) { | ||
| claimer.claimer.currentRequest = null; | ||
| } else { | ||
| claimer.claimer.registration = null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding type safety and immutable data handling.
The current implementation has several potential issues:
- Direct mutation of input parameters can lead to unexpected side effects
- Unsafe property access without proper null checks
- No type guards for the index property
Consider refactoring to:
const sanitize = (res: MeQuery[]): MeQuery[] => {
return res.map((claimer) => {
if (!claimer.claimer?.currentRequest || !claimer.claimer?.registration) {
return claimer;
}
const sanitizedClaimer = { ...claimer };
if (sanitizedClaimer.claimer) {
if (sanitizedClaimer.claimer.currentRequest?.index <= -100) {
sanitizedClaimer.claimer.currentRequest = null;
} else {
sanitizedClaimer.claimer.registration = null;
}
}
return sanitizedClaimer;
});
};
Summary by CodeRabbit
currentRequestis set tonullunder specific conditions to enhance data integrity.