Integrated Nomis Identity #552
Conversation
…r wallet score management
…at-nomis-integration
|
Warning Rate limit exceeded@HakobP-Solicy has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 19 minutes and 35 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds Nomis score support across identity and incentive stacks: new Nomis identity types and provider, Nomis API client, GCR upsert/remove handlers, PointSystem award/deduct methods and breakdown persistence, IncentiveManager hooks, RPCs for fetching Nomis score/identities, and type augmentations. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Network as ManageGCRRoutines / API
participant Incentive as IncentiveManager
participant PointSystem
participant GCRRepo as GCR Repository
participant NomisAPI as NomisApiClient / NomisProvider
Client->>Network: RPC: getNomisScore / nomisLinked (user, chain, addr, score?)
Network->>NomisAPI: getWalletScore / listIdentities (optional)
NomisAPI-->>Network: Nomis score payload
Network->>Incentive: nomisLinked(userId, chain, score, referral?)
Incentive->>PointSystem: awardNomisScorePoints(userId, chain, score, referral?)
PointSystem->>GCRRepo: fetch account & identities
GCRRepo-->>PointSystem: account data (including linkedNomis/web3)
PointSystem->>GCRRepo: applyNomisIdentityUpsert / addPointsToGCR(type: "nomisScores")
GCRRepo-->>PointSystem: persist result
PointSystem-->>Incentive: RPCResponse (success or error)
Incentive-->>Network: propagate RPCResponse
Network-->>Client: final RPC response (updated breakdown or error)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
| 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.
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.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/libs/identity/providers/nomisIdentityProvider.ts:
- Around line 143-155: getExistingIdentity currently assumes account.identities
exists and accesses account.identities.nomis which can throw; update
getExistingIdentity to defensively handle missing identities by checking
account.identities before reading nomis (e.g., const nomisIdentities =
account.identities?.nomis || {} or return undefined early if
!account.identities), keeping the rest of the logic using nomisIdentities and
normalizeAddress as before.
- Around line 112-129: The flattenIdentities function accesses
account.identities.nomis without guarding against missing properties; update
flattenIdentities to defensively handle undefined by checking account and
account.identities and defaulting nomisIdentities to an empty object (e.g.,
const nomisIdentities = account?.identities?.nomis || {}) before iterating, so
Object.entries calls never receive undefined; ensure references to
nomisIdentities, chain, subchain and identities remain unchanged.
- Around line 24-61: The method builds a Nomis identity but never persists it to
the GCR account; after creating identityRecord in getWalletScore, add it to
account.identities.nomis (initialize the array if absent), persist the account
(e.g., await account.save() or the appropriate save/update call on the returned
account object), and then return the identityRecord so future calls and
listIdentities/getExistingIdentity see the persisted value.
🧹 Nitpick comments (1)
src/libs/identity/providers/nomisIdentityProvider.ts (1)
70-88: Minor: Redundant address normalization.The
walletAddressparameter is already normalized by the caller (line 31 ingetWalletScore), so Line 76 performs unnecessary work.♻️ Refactor to accept pre-normalized address
private static assertWalletLinked( account: GCRMain, chain: string, subchain: string, walletAddress: string, ) { - const normalizedWallet = this.normalizeAddress(walletAddress, chain) const linked = account.identities?.xm?.[chain]?.[subchain]?.some(identity => { const stored = this.normalizeAddress(identity.address, chain) - return stored === normalizedWallet + return stored === walletAddress }) || false if (!linked) { throw new Error( `Wallet ${walletAddress} is not linked to ${account.pubkey} on ${chain}:${subchain}`, ) } }
📜 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 (3)
src/libs/blockchain/gcr/gcr_routines/identityManager.tssrc/libs/identity/providers/nomisIdentityProvider.tssrc/libs/network/manageGCRRoutines.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/libs/network/manageGCRRoutines.ts
- src/libs/blockchain/gcr/gcr_routines/identityManager.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/libs/identity/providers/nomisIdentityProvider.ts (3)
src/model/entities/types/IdentityTypes.ts (2)
NomisWalletIdentity(3-19)SavedNomisIdentity(31-45)src/libs/identity/tools/nomis.ts (3)
NomisScoreRequestOptions(36-40)NomisApiClient(55-159)NomisWalletScorePayload(5-34)src/libs/blockchain/gcr/gcr_routines/ensureGCRForUser.ts (1)
ensureGCRForUser(8-33)
🔇 Additional comments (4)
src/libs/identity/providers/nomisIdentityProvider.ts (4)
1-21: LGTM!The imports, type alias, and interface definitions are clean and appropriate for the provider's functionality.
63-68: LGTM!The method correctly retrieves the account and delegates to the helper for flattening.
90-110: LGTM!The identity record construction correctly maps the API payload to the SavedNomisIdentity structure with appropriate fallbacks.
131-141: LGTM!The address normalization logic correctly handles EVM and non-EVM chains with appropriate error handling.
| static async getWalletScore( | ||
| pubkey: string, | ||
| walletAddress: string, | ||
| options: NomisImportOptions = {}, | ||
| ): Promise<SavedNomisIdentity> { | ||
| const chain = options.chain || "evm" | ||
| const subchain = options.subchain || "mainnet" | ||
| const normalizedWallet = this.normalizeAddress(walletAddress, chain) | ||
|
|
||
| const account = await ensureGCRForUser(pubkey) | ||
|
|
||
| this.assertWalletLinked(account, chain, subchain, normalizedWallet) | ||
|
|
||
| const existing = this.getExistingIdentity( | ||
| account, | ||
| chain, | ||
| subchain, | ||
| normalizedWallet, | ||
| ) | ||
|
|
||
| if (existing) { | ||
| return existing | ||
| } | ||
|
|
||
| const apiClient = NomisApiClient.getInstance() | ||
| const payload = await apiClient.getWalletScore( | ||
| normalizedWallet, | ||
| options, | ||
| ) | ||
|
|
||
| const identityRecord = this.buildIdentityRecord( | ||
| payload, | ||
| normalizedWallet, | ||
| options, | ||
| ) | ||
|
|
||
| return identityRecord | ||
| } |
There was a problem hiding this comment.
Critical: Fetched identity is not persisted to the GCR account.
The method fetches the wallet score from the Nomis API and builds an identity record, but never saves it to account.identities.nomis. Without persistence, every call will re-fetch from the API, and the identity won't be available for listIdentities or getExistingIdentity.
💾 Proposed fix to persist the identity
const identityRecord = this.buildIdentityRecord(
payload,
normalizedWallet,
options,
)
+
+ // Initialize the nomis identities structure if needed
+ if (!account.identities.nomis) {
+ account.identities.nomis = {}
+ }
+ if (!account.identities.nomis[chain]) {
+ account.identities.nomis[chain] = {}
+ }
+ if (!account.identities.nomis[chain][subchain]) {
+ account.identities.nomis[chain][subchain] = []
+ }
+
+ // Add the new identity
+ account.identities.nomis[chain][subchain].push(identityRecord)
+
+ // Save the account
+ await account.save()
return identityRecord🤖 Prompt for AI Agents
In @src/libs/identity/providers/nomisIdentityProvider.ts around lines 24 - 61,
The method builds a Nomis identity but never persists it to the GCR account;
after creating identityRecord in getWalletScore, add it to
account.identities.nomis (initialize the array if absent), persist the account
(e.g., await account.save() or the appropriate save/update call on the returned
account object), and then return the identityRecord so future calls and
listIdentities/getExistingIdentity see the persisted value.
…at-nomis-integration
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts:
- Around line 1069-1071: The current validation block in GCRIdentityRoutines
that checks identity.chain, identity.subchain, and identity.address is missing a
check for identity.score; add validation that identity.score is present and a
valid number (e.g., typeof identity.score === "number" and isFinite/within
expected range) before proceeding so that the call to
IncentiveManager.nomisUnlinked (which expects nomisScore: number) never receives
undefined; update the existing if condition around identity checks to include
this score validation and return the same { success: false, message: "Invalid
Nomis identity payload" } when it fails.
- Around line 999-1001: The validation currently rejects a legitimate score of 0
because it uses a falsy check; update the condition in the GCR identity payload
check (the block that returns { success: false, message: "Invalid Nomis identity
payload" }) to explicitly test for missing/invalid score rather than
falsiness—e.g., require chain, subchain and address to be truthy but check score
with score === undefined || score === null (or use Number.isFinite(score) /
typeof score !== 'number') so score === 0 is accepted.
- Around line 1024-1056: The code currently computes isFirst by checking other
accounts but misses whether this identity already existed on the same account,
causing duplicate awards on score refresh; before you build filtered, detect if
the identity already existed in the current account (e.g., const
existedInAccount = chainBucket.some(existing =>
this.normalizeNomisAddress(chain, existing.address) === normalizedAddress)),
then after saving only call IncentiveManager.nomisLinked when isFirst is true
AND existedInAccount is false (i.e., a true first-time link for this user); keep
the existing variables chainBucket, normalizedAddress, isFirst, accountGCR,
IncentiveManager.nomisLinked and editOperation.referralCode to locate and update
the condition.
🧹 Nitpick comments (2)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (2)
1078-1078: UsefindOneByinstead ofensureGCRForUserfor remove operations.
ensureGCRForUsercreates an account if it doesn't exist, which is unnecessary overhead for removal. Other remove methods (applyXmIdentityRemoveat line 135,applyUdIdentityRemoveat line 659) usefindOneByand return early if the account doesn't exist.♻️ 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" } + } const chainBucket = accountGCR.identities.nomis?.[identity.chain]?.[identity.subchain]
1111-1115: Score for deduction should come from stored identity, not request payload.Using
identity.scorefrom the request payload for point deduction is fragile. If the client sends an incorrect score, the deduction won't match what was awarded. Retrieve the score from the existing stored identity before removal.♻️ Proposed fix
+ // Find the existing identity to get the stored score for deduction + const existingIdentity = chainBucket.find(existing => { + const existingAddress = this.normalizeNomisAddress( + identity.chain, + existing.address, + ) + return existingAddress === normalizedAddress + }) + if (!exists) { return { success: false, message: "Nomis identity not found" } }Then use
existingIdentity.scorefor the deduction:await IncentiveManager.nomisUnlinked( accountGCR.pubkey, identity.chain, - identity.score, + existingIdentity.score, )
📜 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 (4)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.tssrc/libs/blockchain/gcr/gcr_routines/identityManager.tssrc/libs/network/manageGCRRoutines.tssrc/libs/network/routines/transactions/handleIdentityRequest.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- src/libs/blockchain/gcr/gcr_routines/identityManager.ts
- src/libs/network/manageGCRRoutines.ts
- src/libs/network/routines/transactions/handleIdentityRequest.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (4)
src/libs/blockchain/gcr/handleGCR.ts (1)
GCRResult(78-82)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)
🔇 Additional comments (4)
src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts (4)
14-20: LGTM!Import additions for
NomisWalletIdentityandSavedNomisIdentityare correctly placed and necessary for the new Nomis identity handling methods.
858-871: LGTM!New switch cases for
nomisaddandnomisremovefollow the established pattern and correctly route to the respective handler methods.
941-960: LGTM!The
rootKeyabstraction cleanly reuses the web3 query logic for Nomis identities while correctly differentiating the JSON path (xmvsnomis). Parameterized query is safe.
972-981: LGTM!The
normalizeNomisAddresshelper follows the same normalization pattern used for XM identities and correctly handles EVM chain addresses.
| if (!chain || !subchain || !address || !score) { | ||
| return { success: false, message: "Invalid Nomis identity payload" } | ||
| } |
There was a problem hiding this comment.
Bug: Score validation rejects valid score=0.
The condition !score evaluates to true when score is 0, which is a valid Nomis score representing the lowest reputation tier. This incorrectly rejects legitimate payloads.
🐛 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 999
- 1001, The validation currently rejects a legitimate score of 0 because it uses
a falsy check; update the condition in the GCR identity payload check (the block
that returns { success: false, message: "Invalid Nomis identity payload" }) to
explicitly test for missing/invalid score rather than falsiness—e.g., require
chain, subchain and address to be truthy but check score with score ===
undefined || score === null (or use Number.isFinite(score) / typeof score !==
'number') so score === 0 is accepted.
| const chainBucket = accountGCR.identities.nomis[chain][subchain] | ||
|
|
||
| const filtered = chainBucket.filter(existing => { | ||
| const existingAddress = this.normalizeNomisAddress( | ||
| chain, | ||
| existing.address, | ||
| ) | ||
| return existingAddress !== normalizedAddress | ||
| }) | ||
|
|
||
| const record: SavedNomisIdentity = { | ||
| address: normalizedAddress, | ||
| score, | ||
| scoreType, | ||
| mintedScore: mintedScore ?? null, | ||
| lastSyncedAt: lastSyncedAt || new Date().toISOString(), | ||
| metadata, | ||
| } | ||
|
|
||
| filtered.push(record) | ||
| accountGCR.identities.nomis[chain][subchain] = filtered | ||
|
|
||
| if (!simulate) { | ||
| await gcrMainRepository.save(accountGCR) | ||
|
|
||
| if (isFirst) { | ||
| await IncentiveManager.nomisLinked( | ||
| accountGCR.pubkey, | ||
| chain, | ||
| score, | ||
| editOperation.referralCode, | ||
| ) | ||
| } |
There was a problem hiding this comment.
Double-awarding points on score updates.
isFirstConnection only checks if the address exists in other accounts. When a user updates an existing Nomis identity (score refresh), isFirst remains true because no other user has the address, and points are awarded again.
Capture whether the identity already existed for the current user before filtering to avoid duplicate awards.
🐛 Proposed fix
const chainBucket = accountGCR.identities.nomis[chain][subchain]
+ const alreadyExistsForUser = chainBucket.some(existing => {
+ const existingAddress = this.normalizeNomisAddress(
+ chain,
+ existing.address,
+ )
+ return existingAddress === normalizedAddress
+ })
+
const filtered = chainBucket.filter(existing => {
const existingAddress = this.normalizeNomisAddress(
chain,
existing.address,
)
return existingAddress !== normalizedAddress
})Then update the incentive condition:
- if (isFirst) {
+ if (isFirst && !alreadyExistsForUser) {
await IncentiveManager.nomisLinked(📝 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.
| const chainBucket = accountGCR.identities.nomis[chain][subchain] | |
| const filtered = chainBucket.filter(existing => { | |
| const existingAddress = this.normalizeNomisAddress( | |
| chain, | |
| existing.address, | |
| ) | |
| return existingAddress !== normalizedAddress | |
| }) | |
| const record: SavedNomisIdentity = { | |
| address: normalizedAddress, | |
| score, | |
| scoreType, | |
| mintedScore: mintedScore ?? null, | |
| lastSyncedAt: lastSyncedAt || new Date().toISOString(), | |
| metadata, | |
| } | |
| filtered.push(record) | |
| accountGCR.identities.nomis[chain][subchain] = filtered | |
| if (!simulate) { | |
| await gcrMainRepository.save(accountGCR) | |
| if (isFirst) { | |
| await IncentiveManager.nomisLinked( | |
| accountGCR.pubkey, | |
| chain, | |
| score, | |
| editOperation.referralCode, | |
| ) | |
| } | |
| const chainBucket = accountGCR.identities.nomis[chain][subchain] | |
| const alreadyExistsForUser = chainBucket.some(existing => { | |
| const existingAddress = this.normalizeNomisAddress( | |
| chain, | |
| existing.address, | |
| ) | |
| return existingAddress === normalizedAddress | |
| }) | |
| const filtered = chainBucket.filter(existing => { | |
| const existingAddress = this.normalizeNomisAddress( | |
| chain, | |
| existing.address, | |
| ) | |
| return existingAddress !== normalizedAddress | |
| }) | |
| const record: SavedNomisIdentity = { | |
| address: normalizedAddress, | |
| score, | |
| scoreType, | |
| mintedScore: mintedScore ?? null, | |
| lastSyncedAt: lastSyncedAt || new Date().toISOString(), | |
| metadata, | |
| } | |
| filtered.push(record) | |
| accountGCR.identities.nomis[chain][subchain] = filtered | |
| if (!simulate) { | |
| await gcrMainRepository.save(accountGCR) | |
| if (isFirst && !alreadyExistsForUser) { | |
| await IncentiveManager.nomisLinked( | |
| accountGCR.pubkey, | |
| chain, | |
| score, | |
| editOperation.referralCode, | |
| ) | |
| } |
🤖 Prompt for AI Agents
In @src/libs/blockchain/gcr/gcr_routines/GCRIdentityRoutines.ts around lines
1024 - 1056, The code currently computes isFirst by checking other accounts but
misses whether this identity already existed on the same account, causing
duplicate awards on score refresh; before you build filtered, detect if the
identity already existed in the current account (e.g., const existedInAccount =
chainBucket.some(existing => this.normalizeNomisAddress(chain, existing.address)
=== normalizedAddress)), then after saving only call
IncentiveManager.nomisLinked when isFirst is true AND existedInAccount is false
(i.e., a true first-time link for this user); keep the existing variables
chainBucket, normalizedAddress, isFirst, accountGCR,
IncentiveManager.nomisLinked and editOperation.referralCode to locate and update
the condition.
| if (!identity?.chain || !identity?.subchain || !identity?.address) { | ||
| return { success: false, message: "Invalid Nomis identity payload" } | ||
| } |
There was a problem hiding this comment.
Missing score validation.
The validation doesn't verify identity.score exists. If undefined, IncentiveManager.nomisUnlinked at line 1114 receives undefined for the nomisScore: number parameter, potentially causing incorrect point deductions.
🐛 Proposed fix
- if (!identity?.chain || !identity?.subchain || !identity?.address) {
+ if (
+ !identity?.chain ||
+ !identity?.subchain ||
+ !identity?.address ||
+ typeof identity?.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 (!identity?.chain || !identity?.subchain || !identity?.address) { | |
| return { success: false, message: "Invalid Nomis identity payload" } | |
| } | |
| if ( | |
| !identity?.chain || | |
| !identity?.subchain || | |
| !identity?.address || | |
| typeof identity?.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
1069 - 1071, The current validation block in GCRIdentityRoutines that checks
identity.chain, identity.subchain, and identity.address is missing a check for
identity.score; add validation that identity.score is present and a valid number
(e.g., typeof identity.score === "number" and isFinite/within expected range)
before proceeding so that the call to IncentiveManager.nomisUnlinked (which
expects nomisScore: number) never receives undefined; update the existing if
condition around identity checks to include this score validation and return the
same { success: false, message: "Invalid Nomis identity payload" } when it
fails.
|
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.