Integrated Nomis Identity #552
Conversation
…r wallet score management
…at-nomis-integration
WalkthroughAdds Nomis score linking, fetching, and point-awarding/deduction across the incentive stack: PointSystem, IncentiveManager hooks, GCR identity routines, IdentityManager, a NomisIdentityProvider, Nomis API client, RPC handlers, and types/models to persist per-chain nomisScores and identities. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Incentive as IncentiveManager
participant PointSystem
participant GCRRepo as GCR Repository
participant NomisProvider as NomisIdentityProvider
rect rgba(200,220,255,0.6)
Note over Client,PointSystem: Award Nomis Score (new flow)
Client->>Incentive: nomisLinked(userId, chain, score, referral?)
Incentive->>PointSystem: awardNomisScorePoints(userId, chain, score, referral?)
PointSystem->>GCRRepo: fetch account & linked wallets
GCRRepo-->>PointSystem: account data (including linked wallets)
PointSystem->>NomisProvider: (optional) getWalletScore / verify identity
alt validation fails
PointSystem-->>Client: 400 error (invalid chain / wallet not linked)
else success
PointSystem->>GCRRepo: apply nomis upsert / addPointsToGCR(type: "nomisScores")
GCRRepo-->>PointSystem: persist result
PointSystem-->>Client: success RPC with updated breakdown.nomisScores
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (9)
src/model/entities/GCRv2/GCR_Main.ts (1)
43-43: Consider adding optional marker for backward compatibility.The
nomisScoresfield should be marked as optional (likeudDomainson line 36) to maintain backward compatibility with existing records that don't have this field yet.🔎 Proposed fix
- nomisScores: { [chain: string]: number } + nomisScores?: { [chain: string]: number }src/libs/identity/tools/nomis.ts (4)
60-60: Remove unused field.The
useMockDatafield is declared but never initialized or used anywhere in the class.🔎 Proposed fix
private readonly http: AxiosInstance private readonly defaultScoreType: number private readonly defaultDeadlineOffset: number - private readonly useMockData: boolean
101-101: Hardcoded timeout overrides configured value.The timeout is hardcoded to 30000ms here, which differs from the configurable
NOMIS_API_TIMEOUT_MS(default 10000ms) set in the constructor on line 80. This inconsistency could cause confusion.🔎 Proposed fix
Consider using the configured timeout or document why this endpoint needs a different timeout:
- const timeout = 30000 + const timeout = this.http.defaults.timeout || 30000
97-99: Consider adding address format validation.The current validation only checks if the address exists but doesn't validate its format. For EVM addresses, consider checking if it's a valid hex address with 0x prefix and 40 hex characters. For Solana, consider validating base58 format.
112-112: Document hardcoded chain IDs.The
scoredChainsarray contains hardcoded chain IDs without documentation. Consider adding a comment explaining what these chains are (e.g., "Ethereum mainnet, Optimism, BSC, Polygon, Mantle, Base, Arbitrum, Linea") or making this configurable.src/model/entities/types/IdentityTypes.ts (1)
31-45: Consider consolidating duplicate interfaces.
SavedNomisIdentityappears to be identical toNomisWalletIdentity. If there's no semantic difference between them (e.g., one for API payloads, one for storage), consider using a single interface or having one extend/reference the other to reduce duplication.src/libs/blockchain/gcr/gcr_routines/identityManager.ts (1)
288-310: Add validation for payload fields beyond presence checks.The
verifyNomisPayloadmethod only validates thatchain,subchain, andaddressare present but doesn't validate the remaining required fields. Consider adding:
- Range validation for
scoreandscoreType- Format validation for
lastSyncedAt(ensure valid timestamp/date string)- Address format validation to ensure it's a valid blockchain address
- Chain/subchain validation against the supported chains (available in the
chainsobject andchainIdsconfig)This would align with the more comprehensive validation pattern used in
verifyPayload.src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (1)
971-980: Consider extracting address normalization to a shared utility.This normalization logic is duplicated in
nomisIdentityProvider.ts(lines 138-148). Extracting to a shared utility would ensure consistent behavior and reduce maintenance burden.src/features/incentive/PointSystem.ts (1)
1465-1472: Add JSDoc clarifying the expected score range and rounding behavior.The function assumes
scoreis in the 0-1 range (multiplied by 100 to produce 0-100 percentage). The implementation is correct, but should be documented with a JSDoc comment explaining the expected input format and howtoFixed(0)rounding affects edge cases (e.g., 0.795 becomes 5 points, while 0.794 becomes 4 points). This prevents future confusion and ensures maintainers understand the dependency on Nomis API score format.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (11)
src/features/incentive/PointSystem.tssrc/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.tssrc/libs/blockchain/gcr/gcr_routines/IncentiveManager.tssrc/libs/blockchain/gcr/gcr_routines/identityManager.tssrc/libs/identity/providers/nomisIdentityProvider.tssrc/libs/identity/tools/nomis.tssrc/libs/network/manageGCRRoutines.tssrc/libs/network/routines/transactions/handleIdentityRequest.tssrc/model/entities/GCRv2/GCR_Main.tssrc/model/entities/types/IdentityTypes.tssrc/types/nomis-augmentations.d.ts
🧰 Additional context used
🧬 Code graph analysis (6)
src/libs/network/routines/transactions/handleIdentityRequest.ts (2)
src/libs/blockchain/gcr/gcr_routines/identityManager.ts (1)
IdentityManager(53-389)src/model/entities/types/IdentityTypes.ts (1)
NomisWalletIdentity(3-19)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (3)
src/libs/blockchain/gcr/gcr_routines/ensureGCRForUser.ts (1)
ensureGCRForUser(8-33)src/model/entities/types/IdentityTypes.ts (2)
SavedNomisIdentity(31-45)NomisWalletIdentity(3-19)src/libs/blockchain/gcr/gcr_routines/IncentiveManager.ts (1)
IncentiveManager(9-196)
src/libs/network/manageGCRRoutines.ts (1)
src/libs/identity/providers/nomisIdentityProvider.ts (1)
NomisIdentityProvider(24-163)
src/libs/blockchain/gcr/gcr_routines/identityManager.ts (1)
src/model/entities/types/IdentityTypes.ts (1)
NomisWalletIdentity(3-19)
src/libs/identity/tools/nomis.ts (1)
src/libs/identity/providers/nomisIdentityProvider.ts (1)
NomisImportOptions(16-22)
src/features/incentive/PointSystem.ts (2)
src/model/entities/types/IdentityTypes.ts (1)
NomisWalletIdentity(3-19)src/libs/blockchain/gcr/gcr_routines/identityManager.ts (1)
IdentityManager(53-389)
🔇 Additional comments (23)
src/libs/identity/tools/nomis.ts (2)
135-154: LGTM!Error handling is well-implemented with proper logging, validation of response structure, and informative error messages.
5-46: LGTM!Type definitions are well-structured with appropriate use of optional fields and generic types.
src/libs/network/routines/transactions/handleIdentityRequest.ts (3)
12-12: LGTM!Import addition is appropriate for the new Nomis identity handling.
95-98: LGTM!The Nomis identity assignment handling follows the same pattern as other identity types, properly delegating to the identity manager for verification.
102-102: LGTM!The Nomis identity removal case is consistent with other identity removal handlers.
src/libs/network/manageGCRRoutines.ts (2)
99-130: LGTM!The
getNomisScorehandler properly validates required inputs, constructs options, and handles errors with appropriate HTTP status codes. Error handling correctly captures both Error instances and other thrown values.
132-145: LGTM!The
getNomisIdentitieshandler follows the same clean error handling pattern asgetNomisScore.src/libs/blockchain/gcr/gcr_routines/IncentiveManager.ts (2)
165-180: LGTM!The
nomisLinkedhook follows the established pattern of other identity linking hooks in this class, properly delegating to the PointSystem.
182-195: LGTM!The
nomisUnlinkedhook is consistent with other unlinking hooks, properly handling the deduction flow.src/types/nomis-augmentations.d.ts (1)
1-33: LGTM!The type augmentations are properly structured and extend the SDK types to support Nomis identities. The module augmentation syntax is correct, and the types align with the Nomis identity structures defined elsewhere in the codebase.
src/model/entities/types/IdentityTypes.ts (2)
3-19: LGTM!The
NomisWalletIdentityinterface is well-structured with appropriate required and optional fields. The metadata object with index signature allows for extensibility.
103-107: LGTM!The
nomisfield properly extendsStoredIdentitieswith the same nested structure asxmidentities, and correctly marked as optional for backward compatibility.src/libs/blockchain/gcr/gcr_routines/identityManager.ts (2)
25-25: LGTM!Import is correctly added for Nomis identity support.
354-364: LGTM!The signature extension to include "nomis" is consistent with the existing pattern for other identity types.
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (2)
940-968: LGTM!The
isFirstConnectionlogic correctly differentiates betweenweb3(usingxmroot key) andnomisidentity types with parameterized JSONB queries.
857-870: LGTM!The new
nomisaddandnomisremoveswitch cases follow the established pattern for other identity operations.src/libs/identity/providers/nomisIdentityProvider.ts (4)
77-95: LGTM!The wallet linkage validation correctly checks the
xmidentities with case-insensitive address comparison for EVM chains.
97-117: LGTM!The identity record construction properly handles optional fields with appropriate fallback values.
119-136: LGTM!The flattening logic correctly transforms the nested chain/subchain structure into a flat array with context preserved.
138-148: LGTM!The address normalization includes proper validation for empty addresses and chain-appropriate transformations.
src/features/incentive/PointSystem.ts (3)
92-115: LGTM!The Nomis identity collection logic correctly mirrors the existing Web3 wallet iteration pattern, properly adding chain/subchain context to each identity.
270-277: LGTM!The
nomisScoreshandling inaddPointsToGCRfollows the established pattern for other breakdown types.
1285-1404: LGTM!The
awardNomisScorePointsmethod has thorough validation: chain allowlist, Twitter linkage requirement, corresponding wallet linkage per chain, and duplicate prevention. The point calculation delegation togetNomisPointsByScoreis clean.
| if (!chain || !subchain || !address || !score) { | ||
| return { success: false, message: "Invalid Nomis identity payload" } | ||
| } |
There was a problem hiding this comment.
Validation rejects valid score of 0.
The condition !score evaluates to true when score === 0, which is a valid Nomis score. This would incorrectly reject identities with a score of zero.
🔎 Proposed fix
- if (!chain || !subchain || !address || !score) {
+ if (!chain || !subchain || !address || typeof score !== "number") {
return { success: false, message: "Invalid Nomis identity payload" }
}📝 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.
| if (!chain || !subchain || !address || !score) { | |
| return { success: false, message: "Invalid Nomis identity payload" } | |
| } | |
| if (!chain || !subchain || !address || typeof score !== "number") { | |
| return { success: false, message: "Invalid Nomis identity payload" } | |
| } |
🤖 Prompt for AI Agents
In src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts around lines 998
to 1000, the validation uses `!score` which incorrectly rejects a legitimate
score of 0; update the condition to explicitly check for missing/invalid score
(e.g., `score === undefined || score === null` or `typeof score !== 'number'`)
instead of `!score`, keeping the other checks for chain, subchain and address
intact so a zero score is accepted as valid.
| identity.address, | ||
| ) | ||
|
|
||
| const accountGCR = await ensureGCRForUser(editOperation.account) |
There was a problem hiding this comment.
Remove operation uses ensureGCRForUser instead of querying existing account.
Using ensureGCRForUser in a remove operation will create a new account if one doesn't exist, which is inconsistent with other remove operations (e.g., applyXmIdentityRemove at line 133 uses findOneBy). Additionally, the score for point deduction is taken from the request payload (identity.score) rather than the stored identity, which could allow manipulation of deducted points.
🔎 Proposed fix
- const accountGCR = await ensureGCRForUser(editOperation.account)
+ const accountGCR = await gcrMainRepository.findOneBy({
+ pubkey: editOperation.account,
+ })
+
+ if (!accountGCR) {
+ return { success: false, message: "Account not found" }
+ }For the score issue, retrieve the stored score before removal:
+ const storedIdentity = chainBucket.find(existing => {
+ const existingAddress = this.normalizeNomisAddress(
+ identity.chain,
+ existing.address,
+ )
+ return existingAddress === normalizedAddress
+ })
+
+ if (!storedIdentity) {
+ return { success: false, message: "Nomis identity not found" }
+ }
+
// ... filter logic ...
if (!simulate) {
await gcrMainRepository.save(accountGCR)
await IncentiveManager.nomisUnlinked(
accountGCR.pubkey,
identity.chain,
- identity.score,
+ storedIdentity.score,
)
}Also applies to: 1110-1114
🤖 Prompt for AI Agents
In src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts around lines 1077
and also 1110-1114, the remove flow is calling ensureGCRForUser which can create
an account instead of querying an existing one and is using identity.score from
the request to compute point deductions; change the logic to query the existing
account and identity (e.g., use findOneBy or the existing repository find
method) and if the account or identity is missing, handle it as a not-found/skip
error instead of creating one; once you have the stored identity record, read
its score from the database and use that stored score for any point deductions,
then proceed with removal; update both occurrences (around 1077 and 1110-1114)
accordingly.
| static async getNomisIdentities( | ||
| address: string, | ||
| chain: string, | ||
| subchain: string, | ||
| ) { | ||
| if (!chain && !subchain) { | ||
| return null | ||
| } | ||
|
|
||
| const data = await this.getIdentities(address, "nomis") | ||
| return (data[chain] || {})[subchain] || [] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/libs/blockchain/gcr/gcr_routines/identityManager.ts | head -50Repository: kynesyslabs/node
Length of output: 1911
🏁 Script executed:
sed -n '320-335p' src/libs/blockchain/gcr/gcr_routines/identityManager.tsRepository: kynesyslabs/node
Length of output: 111
🏁 Script executed:
cat -n src/libs/blockchain/gcr/gcr_routines/identityManager.ts | sed -n '320,335p'Repository: kynesyslabs/node
Length of output: 603
🏁 Script executed:
cat -n src/libs/blockchain/gcr/gcr_routines/identityManager.ts | sed -n '377,390p'Repository: kynesyslabs/node
Length of output: 462
🏁 Script executed:
rg -n "getXmIdentities|getNomisIdentities" --type=ts -B2 -A2 src/Repository: kynesyslabs/node
Length of output: 1643
🏁 Script executed:
cat -n src/libs/network/manageGCRRoutines.ts | sed -n '45,50p'Repository: kynesyslabs/node
Length of output: 283
🏁 Script executed:
cat -n src/libs/network/manageGCRRoutines.ts | sed -n '132,145p'Repository: kynesyslabs/node
Length of output: 625
🏁 Script executed:
rg "IdentityManager\.getNomisIdentities|IdentityManager\.getXmIdentities" --type=ts -B2 -A2Repository: kynesyslabs/node
Length of output: 42
🏁 Script executed:
fd -e test.ts -e spec.ts | head -20Repository: kynesyslabs/node
Length of output: 42
🏁 Script executed:
rg "getNomisIdentities|getXmIdentities" --type=ts -C5Repository: kynesyslabs/node
Length of output: 3407
🏁 Script executed:
rg "\.getNomisIdentities\(|\.getXmIdentities\(" --type=tsRepository: kynesyslabs/node
Length of output: 42
🏁 Script executed:
cat -n src/libs/blockchain/gcr/gcr_routines/identityManager.ts | sed -n '250,290p'Repository: kynesyslabs/node
Length of output: 1759
🏁 Script executed:
cat -n src/libs/blockchain/gcr/gcr_routines/identityManager.ts | sed -n '1,100p'Repository: kynesyslabs/node
Length of output: 3780
🏁 Script executed:
rg "getIdentities\(" --type=ts -B2 -A2 | head -60Repository: kynesyslabs/node
Length of output: 4409
🏁 Script executed:
cat -n src/libs/blockchain/gcr/gcr_routines/identityManager.ts | sed -n '307,340p'Repository: kynesyslabs/node
Length of output: 1418
🏁 Script executed:
cat -n src/libs/blockchain/gcr/gcr_routines/identityManager.ts | sed -n '200,220p'Repository: kynesyslabs/node
Length of output: 934
Guard condition uses AND instead of OR, should validate both chain and subchain are present.
Line 382 checks if (!chain && !subchain) which only returns null when BOTH are missing. This should be if (!chain || !subchain) to properly validate that both parameters are required before accessing the nested object. The same issue exists in getXmIdentities at line 325. Note: these methods do not appear to be directly called in the codebase currently.
🤖 Prompt for AI Agents
In src/libs/blockchain/gcr/gcr_routines/identityManager.ts around lines 377 to
388 (and similarly at line ~325 for getXmIdentities), the guard uses `if (!chain
&& !subchain)` which only returns when both are missing; change the check to `if
(!chain || !subchain)` so the function returns null when either parameter is
absent before accessing nested properties, and update the corresponding
getXmIdentities guard the same way; keep behavior of returning an empty array
when keys are present.
| if (existing) { | ||
| if (options.forceRefresh) { | ||
| log.info( | ||
| `[NomisIdentityProvider] Skipping refresh for ${normalizedWallet} (chain=${chain}/${subchain}) until identity removal`, | ||
| ) | ||
| } | ||
|
|
||
| return existing | ||
| } |
There was a problem hiding this comment.
forceRefresh option has no effect when identity exists.
When existing is found, the code logs "Skipping refresh" but returns the cached identity regardless of forceRefresh value. If refresh is intentionally disabled, the log message is misleading. If refresh should work, the logic needs adjustment.
🔎 Suggested clarification
Either remove the forceRefresh option from NomisImportOptions if refresh is not supported, or implement actual refresh logic:
if (existing) {
- if (options.forceRefresh) {
- log.info(
- `[NomisIdentityProvider] Skipping refresh for ${normalizedWallet} (chain=${chain}/${subchain}) until identity removal`,
- )
- }
-
- return existing
+ if (!options.forceRefresh) {
+ return existing
+ }
+ // Proceed to fetch fresh score from API
}🤖 Prompt for AI Agents
In src/libs/identity/providers/nomisIdentityProvider.ts around lines 45 to 53,
the current logic always returns the cached identity and logs "Skipping refresh"
even when options.forceRefresh is true; update the implementation so
forceRefresh has effect: if options.forceRefresh is true, bypass the cached
return and perform the refresh/fetch flow (and update the cache) instead of
returning existing, and change the log message to reflect when refresh is
actually skipped; alternatively, if refresh is not supported, remove
forceRefresh from NomisImportOptions and callers, and update types/tests
accordingly.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/features/incentive/PointSystem.ts (2)
1285-1404: Consider adding identity existence verification for consistency.Unlike
awardUdDomainPoints(lines 1118-1135) which verifies the domain exists inaccount.identities.udbefore awarding points, this method doesn't verify the Nomis identity exists inaccount.identities.nomisfor the given chain. This verification prevents race conditions where points could be awarded before the identity is properly persisted.🔎 Suggested enhancement
const existingNomisScoreOnChain = userPointsWithIdentities.breakdown.nomisScores?.[chain] if (existingNomisScoreOnChain != null) { const updatedPoints = await this.getUserPointsInternal(userId) return { result: 400, response: { pointsAwarded: 0, totalPoints: updatedPoints.totalPoints, message: nomisScoreAlreadyLinkedMessage, }, require_reply: false, extra: {}, } } + // Verify Nomis identity exists in GCR to prevent race conditions + const account = await ensureGCRForUser(userId) + const nomisIdentityExists = account.identities.nomis?.[chain] && + Object.keys(account.identities.nomis[chain]).some(subchain => { + const identities = account.identities.nomis[chain][subchain] + return Array.isArray(identities) && identities.length > 0 + }) + + if (!nomisIdentityExists) { + return { + result: 400, + response: { + pointsAwarded: 0, + totalPoints: userPointsWithIdentities.totalPoints, + message: `Cannot award points: Nomis identity for ${chain} not found in GCR identities`, + }, + require_reply: false, + extra: {}, + } + } const pointsToAward = this.getNomisPointsByScore(nomisScore)
1485-1492: Consider adding input validation for robustness.The method doesn't validate that
scoreis within an expected range. If the score parameter is negative, greater than 1, or NaN, it could lead to incorrect point calculations.🔎 Suggested defensive check
private getNomisPointsByScore(score: number): number { + // Validate score is in expected range (assuming 0-1) + if (typeof score !== 'number' || isNaN(score)) { + log.warn(`Invalid Nomis score: ${score}, defaulting to minimum points`) + return 1 + } + if (score < 0 || score > 1) { + log.warn(`Nomis score out of range: ${score}, clamping to [0, 1]`) + score = Math.max(0, Math.min(1, score)) + } + const formattedScore = Number((score * 100).toFixed(0)) if (formattedScore >= 80) return 5 if (formattedScore >= 60) return 4 if (formattedScore >= 40) return 3 if (formattedScore >= 20) return 2 return 1 }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/features/incentive/PointSystem.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/features/incentive/PointSystem.ts (3)
src/model/entities/types/IdentityTypes.ts (1)
NomisWalletIdentity(3-19)src/libs/blockchain/gcr/gcr_routines/identityManager.ts (1)
IdentityManager(53-389)src/libs/blockchain/gcr/gcr_routines/ensureGCRForUser.ts (1)
ensureGCRForUser(8-33)
🔇 Additional comments (4)
src/features/incentive/PointSystem.ts (4)
92-115: LGTM: Nomis identity extraction logic is correct.The nested iteration and mapping properly flattens the Nomis identities structure while preserving chain and subchain information.
165-208: LGTM: getUserPointsInternal correctly extended for Nomis.The additions are consistent with existing identity patterns and include proper fallback handling.
221-277: LGTM: addPointsToGCR properly extended for Nomis scores.The implementation follows the established pattern for per-chain point tracking used by web3Wallets and udDomains.
1413-1483: LGTM: Pre-deduction check has been addressed.The implementation now includes the missing check at lines 1432-1449, verifying that Nomis points exist for the chain before deduction. This prevents negative breakdown values and matches the pattern used in other deduct methods like
deductTwitterPointsanddeductUdDomainPoints.
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.