Mature Price Models for "v1" ENSAnalytics #1562
Conversation
🦋 Changeset detectedLatest commit: 5607554 The changes in this PR will be included in the next version bump. This PR includes changesets to release 18 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
📝 WalkthroughWalkthroughAdds a v1 referrer-leaderboard API and client: new v1 domain models, serialization/deserialization, pagination, aggregation (DB), cache and middleware, renames context key to Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Handler as ENSAPI Handler (v1)
participant Middleware as ReferrerLeaderboardMiddlewareV1
participant Cache as ReferrerLeaderboardCacheV1
participant DB as Database-v1
participant Builder as LeaderboardBuilder (v1)
participant Serializer as v1 Serializer
Client->>Handler: GET /v1/ensanalytics/referrers[?page]
Handler->>Middleware: invoke referrerLeaderboardMiddlewareV1
Middleware->>Cache: read()
Cache->>DB: getReferrerMetrics(rules)
DB-->>Cache: ReferrerMetrics[]
Cache->>Builder: buildReferrerLeaderboard(metrics, rules, accurateAsOf)
Builder-->>Cache: ReferrerLeaderboard
Cache-->>Middleware: ReferrerLeaderboard | Error
Middleware-->>Handler: set c.var.referrerLeaderboardV1
Handler->>Serializer: serializeReferrerLeaderboardPageResponse(leaderboard)
Serializer-->>Handler: SerializedResponse
Handler-->>Client: HTTP 200 JSON
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR introduces a mature v1 API for ENS referral analytics with comprehensive price model support, adding USDC and DAI currencies alongside ETH. The changes establish a parallel v1 API structure that can coexist with the existing v0 API while providing enhanced functionality for the ENS Holiday Awards referral program.
Changes:
- Added USDC and DAI price type support to
@ensnode/ensnode-sdkwith corresponding schemas, serialization, and deserialization functions - Created comprehensive v1 API modules for
@namehash/ens-referralsincluding leaderboard, metrics, rankings, and award calculations - Implemented v1-specific database queries, handlers, and tests in
ensapito support the new API structure
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/shared/zod-schemas.ts | Added Zod schemas for PriceUsdc and PriceDai types |
| packages/ensnode-sdk/src/shared/serialize.ts | Added serialization functions for USDC and DAI prices; included inline copy of uint256ToHex32 to avoid module resolution issues |
| packages/ensnode-sdk/src/shared/deserialize.ts | Added deserialization functions for ETH, USDC, and DAI prices |
| packages/ens-referrals/tsup.config.ts | Added v1.ts as a build entry point to support parallel v1 exports |
| packages/ens-referrals/src/v1.ts | New entry point that exports all v1-specific modules |
| packages/ens-referrals/src/rules-v1.ts | Defines referral program rules including award pool value, qualification thresholds, and time boundaries |
| packages/ens-referrals/src/referrer-metrics-v1.ts | Core metrics types and calculations for referrers including scoring, ranking, and award distribution |
| packages/ens-referrals/src/referrer-detail-v1.ts | Implements referrer detail lookup with support for both ranked and unranked referrers |
| packages/ens-referrals/src/rank-v1.ts | Ranking logic including score boost calculations and comparisons |
| packages/ens-referrals/src/leaderboard-v1.ts | Leaderboard construction from raw metrics with full calculations |
| packages/ens-referrals/src/leaderboard-page-v1.ts | Pagination logic for leaderboard queries |
| packages/ens-referrals/src/api/zod-schemas-v1.ts | Zod schemas for v1 API types including validation rules |
| packages/ens-referrals/src/api/types-v1.ts | TypeScript types for v1 API request/response structures |
| packages/ens-referrals/src/api/serialized-types-v1.ts | Serialized type definitions for v1 API data transfer |
| packages/ens-referrals/src/api/serialize-v1.ts | Serialization functions for converting runtime types to wire format |
| packages/ens-referrals/src/api/deserialize-v1.ts | Deserialization functions with Zod validation for v1 API responses |
| packages/ens-referrals/src/aggregations-v1.ts | Aggregation calculations for leaderboard-wide metrics |
| packages/ens-referrals/package.json | Updated exports to include ./v1 entry point for consumers |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/mocks-v1.ts | Mock data for v1 API testing |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/get-referrer-leaderboard-v1.ts | Database-backed leaderboard construction for v1 API |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database-v1.ts | Drizzle ORM queries for fetching referrer metrics from the database |
| apps/ensapi/src/handlers/ensanalytics-api-v1.ts | Updated import to use v1 entry point |
| apps/ensapi/src/handlers/ensanalytics-api-v1.test.ts | Updated tests to use v1 imports and properly test USDC price objects |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| rules: ReferralProgramRules, | ||
| ): number { | ||
| if (!isReferrerQualified(rank, rules)) return 0; | ||
|
|
There was a problem hiding this comment.
Potential division by zero issue when maxQualifiedReferrers is 1. In line 46, the expression (rules.maxQualifiedReferrers - 1) becomes 0 when maxQualifiedReferrers is 1, resulting in division by zero. This would cause calcReferrerFinalScoreBoost to return Infinity or NaN.
While it may be unlikely that maxQualifiedReferrers would be set to 1 in practice, this should be handled explicitly to avoid runtime errors. Consider adding a check: if maxQualifiedReferrers === 1, return 1 (since rank 1 is the only qualified referrer and should get maximum boost).
| // Avoid division by zero when only a single referrer is qualified. | |
| // In this case, that single referrer (rank 1) should receive the maximum boost. | |
| if (rules.maxQualifiedReferrers === 1) return 1; |
| break; | ||
|
|
||
| case ReferrerDetailResponseCodes.Error: | ||
| return response; | ||
| } |
There was a problem hiding this comment.
The serializeReferrerDetailResponse function has an unreachable fallthrough case. The outer switch statement on line 167 handles both "ok" and "error" response codes, but after the "ok" case's nested switch completes (line 181), there is a break statement on line 182 that prevents any further code from executing. However, the function doesn't have an explicit return statement at the end, which means if somehow the code reaches past line 186 (which shouldn't happen), the function would return undefined.
While this is unlikely to cause issues in practice due to TypeScript's type checking, it would be cleaner to either:
- Remove the break statement on line 182 and let the nested switch return directly
- Add an exhaustive check or throw an error after line 186 to ensure all cases are handled
This could be considered a TypeScript exhaustiveness check issue.
| break; | |
| case ReferrerDetailResponseCodes.Error: | |
| return response; | |
| } | |
| case ReferrerDetailResponseCodes.Error: | |
| return response; | |
| } | |
| throw new Error(`Unexpected ReferrerDetailResponseCode: ${String(response.responseCode)}`); |
| export function deserializeReferrerDetailResponse( | ||
| maybeResponse: SerializedReferrerDetailResponse, | ||
| valueLabel?: string, | ||
| ): ReferrerDetailResponse { | ||
| let deserialized: ReferrerDetailResponse; | ||
| switch (maybeResponse.responseCode) { | ||
| case "ok": { | ||
| switch (maybeResponse.data.type) { | ||
| case "ranked": | ||
| deserialized = { | ||
| responseCode: maybeResponse.responseCode, | ||
| data: deserializeReferrerDetailRanked(maybeResponse.data), | ||
| } as ReferrerDetailResponse; | ||
| break; | ||
|
|
||
| case "unranked": | ||
| deserialized = { | ||
| responseCode: maybeResponse.responseCode, | ||
| data: deserializeReferrerDetailUnranked(maybeResponse.data), | ||
| } as ReferrerDetailResponse; | ||
| break; | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case "error": | ||
| deserialized = maybeResponse; | ||
| break; | ||
| } | ||
|
|
||
| // Then validate the deserialized structure using zod schemas | ||
| const schema = makeReferrerDetailResponseSchema(valueLabel); | ||
| const parsed = schema.safeParse(deserialized); | ||
|
|
||
| if (parsed.error) { | ||
| throw new Error(`Cannot deserialize ReferrerDetailResponse:\n${prettifyError(parsed.error)}\n`); | ||
| } | ||
|
|
||
| return parsed.data; | ||
| } |
There was a problem hiding this comment.
The deserializeReferrerDetailResponse function has the same issue as the serialize variant. The deserialized variable is declared with let but may not be initialized if an unexpected code path is reached. Specifically, after the nested switch statement completes (line 211), there's a break statement (line 212) that exits the outer switch. While TypeScript should catch this at compile time, there's a theoretical code path where deserialized could be undefined when accessed on line 222.
Consider restructuring to ensure the variable is always initialized, or add an exhaustive check that throws an error if all cases aren't handled properly.
| import type { AwardedReferrerMetrics, UnrankedReferrerMetrics } from "../referrer-metrics-v1"; | ||
| import type { ReferralProgramRules } from "../rules-v1"; | ||
| import type { | ||
| ReferrerDetailResponse, |
There was a problem hiding this comment.
'ReferrerDetailResponse' is defined but never used.
| ReferrerDetailResponse, |
| ReferrerDetailResponse, | ||
| ReferrerDetailResponseError, | ||
| ReferrerDetailResponseOk, | ||
| ReferrerLeaderboardPageResponse, |
There was a problem hiding this comment.
'ReferrerLeaderboardPageResponse' is defined but never used.
| ReferrerLeaderboardPageResponse, |
| _valueLabel: string = "ReferrerLeaderboardPageResponseError", | ||
| ) => | ||
| z.object({ | ||
| responseCode: z.literal(ReferrerLeaderboardPageResponseCodes.Error), | ||
| error: z.string(), | ||
| errorMessage: z.string(), |
There was a problem hiding this comment.
'_valueLabel' is assigned a value but never used.
| _valueLabel: string = "ReferrerLeaderboardPageResponseError", | |
| ) => | |
| z.object({ | |
| responseCode: z.literal(ReferrerLeaderboardPageResponseCodes.Error), | |
| error: z.string(), | |
| errorMessage: z.string(), | |
| valueLabel: string = "ReferrerLeaderboardPageResponseError", | |
| ) => | |
| z.object({ | |
| responseCode: z.literal(ReferrerLeaderboardPageResponseCodes.Error), | |
| error: z.string().describe(`${valueLabel}.error`), | |
| errorMessage: z.string().describe(`${valueLabel}.errorMessage`), |
| _valueLabel: string = "ReferrerDetailResponse", | ||
| ) => | ||
| z.object({ | ||
| responseCode: z.literal(ReferrerDetailResponseCodes.Error), | ||
| error: z.string(), | ||
| errorMessage: z.string(), | ||
| }); |
There was a problem hiding this comment.
'_valueLabel' is assigned a value but never used.
| _valueLabel: string = "ReferrerDetailResponse", | |
| ) => | |
| z.object({ | |
| responseCode: z.literal(ReferrerDetailResponseCodes.Error), | |
| error: z.string(), | |
| errorMessage: z.string(), | |
| }); | |
| valueLabel: string = "ReferrerDetailResponse", | |
| ) => | |
| z | |
| .object({ | |
| responseCode: z.literal(ReferrerDetailResponseCodes.Error), | |
| error: z.string(), | |
| errorMessage: z.string(), | |
| }) | |
| .describe(valueLabel); |
| @@ -13,7 +13,14 @@ import { z } from "zod/v4"; | |||
|
|
|||
| import { ENSNamespaceIds, type InterpretedName, Node } from "../ens"; | |||
There was a problem hiding this comment.
'Node' is defined but never used.
| import { ENSNamespaceIds, type InterpretedName, Node } from "../ens"; | |
| import { ENSNamespaceIds, type InterpretedName } from "../ens"; |
| import { | ||
| type CurrencyId, | ||
| CurrencyIds, | ||
| Price, |
There was a problem hiding this comment.
'Price' is defined but never used.
| Price, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ensnode-sdk/src/shared/serialize.ts (1)
2-36: Consider centralizinguint256ToHex32to avoid drift.This is now a local copy of
packages/ensnode-sdk/src/ens/subname-helpers.ts:uint256ToHex32. If that implementation changes, these can diverge. A small shared util imported by both modules would keep behavior in sync while still avoiding the Vite SSR import issue noted in the comment.
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database-v1.ts`:
- Around line 71-89: Add lightweight runtime validation before casting records
to NonNullRecord to avoid runtime crashes if DB shape changes: in the mapping
code around records → NonNullRecord[] (used with buildReferrerMetrics,
deserializeDuration, and priceEth) check each record has a non-null/defined
referrer, a numeric totalReferrals, and stringifiable
totalIncrementalDuration/totalRevenueContribution; if any check fails either
skip the record with a warning log or throw a descriptive error. Alternatively,
replace the manual checks with a small Zod schema that validates/refines {
referrer: string, totalReferrals: number, totalIncrementalDuration: string,
totalRevenueContribution: string } and parse records through it before calling
buildReferrerMetrics.
In `@packages/ens-referrals/src/api/deserialize-v1.ts`:
- Around line 151-181: The switch in deserializeReferrerLeaderboardPageResponse
can leave deserialized undefined if maybeResponse.responseCode is unexpected;
add a default branch (or an exhaustive check) after the existing cases that
throws a clear error including the unexpected maybeResponse.responseCode (and
optionally the serialized payload) so callers fail-fast before calling
makeReferrerLeaderboardPageResponseSchema/safeParse; ensure the thrown message
uses prettifyError or similar context so debugging the malformed response is
easy.
- Around line 190-229: deserializeReferrerDetailResponse lacks default branches
in the outer switch on maybeResponse.responseCode and the inner switch on
maybeResponse.data.type, so add defensive default cases that throw clear Errors
(including the unexpected value) to make the function exhaustive and ensure
deserialized is always assigned; update the outer switch to handle unknown
responseCode by throwing an Error indicating the unexpected responseCode and
include a default in the inner switch to throw an Error indicating the
unexpected data.type (or use a never-exhaustiveness helper if preferred) before
the final zod validation in deserializeReferrerDetailResponse.
In `@packages/ens-referrals/src/api/zod-schemas-v1.ts`:
- Around line 67-86: Update makeUnrankedReferrerMetricsSchema so the eight
fields that must be zero are validated at parse time: totalReferrals,
totalIncrementalDuration, totalRevenueContribution, score, finalScoreBoost,
finalScore, awardPoolShare, and awardPoolApproxValue. Replace their current
permissive schemas (e.g., makeNonNegativeIntegerSchema, makeDurationSchema,
makePriceEthSchema, makeFiniteNonNegativeNumberSchema, makePriceUsdcSchema) with
explicit zero constraints—either z.literal(0) for exact-zero or apply
.min(0).max(0, "<field> must be 0") to the existing schema factories—to ensure
these fields can only be 0 and include a clear error message referencing the
field via the valueLabel.
In `@packages/ens-referrals/src/leaderboard-page-v1.ts`:
- Around line 208-214: Doc invariant for ReferrerLeaderboardPage.referrers
conflicts with ReferrerLeaderboard/getReferrerLeaderboardPage which preserve Map
order (ascending by rank). Fix by aligning docs: update the invariant on
ReferrerLeaderboardPage.referrers to state entries are ordered by `rank`
(ascending) instead of descending (or alternatively, if you prefer descending,
reverse the array returned by `getReferrerLeaderboardPage`/the Map iteration);
reference symbols: ReferrerLeaderboard, getReferrerLeaderboardPage,
ReferrerLeaderboardPage.referrers, AwardedReferrerMetrics.
- Around line 118-148: The validator must also check totalPages and that the
provided startIndex/endIndex match derived values: inside
validateReferrerLeaderboardPageContext compute expectedTotalPages =
Math.ceil(context.totalRecords / context.recordsPerPage) (handle zero records),
compute expectedStartIndex = (context.page - 1) * context.recordsPerPage and
expectedEndIndex = Math.min(expectedStartIndex + context.recordsPerPage,
context.totalRecords), then validate that context.totalPages is a non-negative
integer and equals expectedTotalPages, that context.page <= context.totalPages
(and valid for zero/empty cases), and that context.startIndex ===
expectedStartIndex and context.endIndex === expectedEndIndex (also ensure
endIndex >= startIndex); throw the same style of Error with clear messages when
any of these invariants fail.
In `@packages/ens-referrals/src/leaderboard-v1.ts`:
- Around line 56-61: The duplicate-referrer check is wrong because
uniqueReferrers is just a mapping copy; replace it by creating a Set of referrer
strings from allReferrers (e.g., new Set(allReferrers.map(r => r.referrer))) and
compare set.size to allReferrers.length inside buildReferrerLeaderboard (or
wherever uniqueReferrers is defined); if sizes differ, throw the existing Error
so duplicates are detected before constructing the Map and silently overwriting
entries.
In `@packages/ens-referrals/src/rank-v1.ts`:
- Around line 40-47: calcReferrerFinalScoreBoost currently divides by
(rules.maxQualifiedReferrers - 1) causing divide-by-zero when
maxQualifiedReferrers === 1; update calcReferrerFinalScoreBoost to guard that
case by returning the correct boost (return 1 for the sole qualified referrer)
when rules.maxQualifiedReferrers <= 1 (and keep the existing isReferrerQualified
check), otherwise compute 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1) as
before.
In `@packages/ens-referrals/src/referrer-metrics-v1.ts`:
- Around line 280-303: In buildAwardedReferrerMetrics the code converts
rules.totalAwardPoolValue.amount to Number which can overflow; keep all math in
bigint by using fixed‑point scaling or an integer fraction instead of Number().
Change calcReferrerAwardPoolShare usage so you either obtain a rational
numerator/denominator (or use a fixed SCALE as a BigInt) and compute
awardPoolApproxAmount with bigint arithmetic: (BigInt(numerator) *
rules.totalAwardPoolValue.amount) / BigInt(denominator) (or
(BigInt(Math.round(awardPoolShare*SCALE)) * totalAmount) / SCALE as BigInt),
then set awardPoolApproxValue.amount to that bigint and run
validateAwardedReferrerMetrics as before.
In `@packages/ens-referrals/src/rules-v1.ts`:
- Line 7: validateReferralProgramRules currently omits validation for the
subregistryId field; update the function (and the same checks around lines
67-85) to validate subregistryId alongside price/timestamp/count invariants:
locate the validateReferralProgramRules function and add a check that
subregistryId is present and matches your expected account-id format (e.g.,
non-empty string and the same pattern used elsewhere for ENS account IDs or a
shared validator helper if one exists), and throw or return an error when it
fails so invalid account IDs cannot pass through.
In `@packages/ens-referrals/src/v1.ts`:
- Around line 14-15: Remove the public re-export of the internal Zod schemas by
deleting the export of "./api/zod-schemas-v1" from the v1 public entry (leave
"export * from \"./api/types-v1\""), so the internal module "zod-schemas-v1" is
no longer exposed; ensure any consumers that relied on that export are updated
to import from the internal re-export location (internal.ts) instead and run the
build/tests to confirm no public API leak remains.
| // Type assertion: The WHERE clause in the query above guarantees non-null values for: | ||
| // 1. `referrer` is guaranteed to be non-null due to isNotNull filter | ||
| // 2. `totalIncrementalDuration` is guaranteed to be non-null as it is the sum of non-null bigint values | ||
| // 3. `totalRevenueContribution` is guaranteed to be non-null due to COALESCE with 0 | ||
| interface NonNullRecord { | ||
| referrer: Address; | ||
| totalReferrals: number; | ||
| totalIncrementalDuration: string; | ||
| totalRevenueContribution: string; | ||
| } | ||
|
|
||
| return (records as NonNullRecord[]).map((record) => { | ||
| return buildReferrerMetrics( | ||
| record.referrer, | ||
| record.totalReferrals, | ||
| deserializeDuration(record.totalIncrementalDuration), | ||
| priceEth(BigInt(record.totalRevenueContribution)), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Type assertion is justified but consider adding runtime validation.
The NonNullRecord type assertion is justified by the WHERE clause guarantees (documented in comments). However, if the database schema changes or there's a bug, this could lead to runtime errors. Consider adding minimal validation or using Zod for parsing the records.
🤖 Prompt for AI Agents
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database-v1.ts` around
lines 71 - 89, Add lightweight runtime validation before casting records to
NonNullRecord to avoid runtime crashes if DB shape changes: in the mapping code
around records → NonNullRecord[] (used with buildReferrerMetrics,
deserializeDuration, and priceEth) check each record has a non-null/defined
referrer, a numeric totalReferrals, and stringifiable
totalIncrementalDuration/totalRevenueContribution; if any check fails either
skip the record with a warning log or throw a descriptive error. Alternatively,
replace the manual checks with a small Zod schema that validates/refines {
referrer: string, totalReferrals: number, totalIncrementalDuration: string,
totalRevenueContribution: string } and parse records through it before calling
buildReferrerMetrics.
| export function deserializeReferrerLeaderboardPageResponse( | ||
| maybeResponse: SerializedReferrerLeaderboardPageResponse, | ||
| valueLabel?: string, | ||
| ): ReferrerLeaderboardPageResponse { | ||
| let deserialized: ReferrerLeaderboardPageResponse; | ||
| switch (maybeResponse.responseCode) { | ||
| case "ok": { | ||
| deserialized = { | ||
| responseCode: maybeResponse.responseCode, | ||
| data: deserializeReferrerLeaderboardPage(maybeResponse.data), | ||
| } as ReferrerLeaderboardPageResponse; | ||
| break; | ||
| } | ||
|
|
||
| case "error": | ||
| deserialized = maybeResponse; | ||
| break; | ||
| } | ||
|
|
||
| // Then validate the deserialized structure using zod schemas | ||
| const schema = makeReferrerLeaderboardPageResponseSchema(valueLabel); | ||
| const parsed = schema.safeParse(deserialized); | ||
|
|
||
| if (parsed.error) { | ||
| throw new Error( | ||
| `Cannot deserialize SerializedReferrerLeaderboardPageResponse:\n${prettifyError(parsed.error)}\n`, | ||
| ); | ||
| } | ||
|
|
||
| return parsed.data; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding exhaustiveness check for switch statement.
If maybeResponse.responseCode receives an unexpected value at runtime (e.g., from malformed API response), deserialized would be uninitialized, causing a runtime error at the safeParse call. Adding a default case improves defensive coding.
🛡️ Suggested improvement
switch (maybeResponse.responseCode) {
case "ok": {
deserialized = {
responseCode: maybeResponse.responseCode,
data: deserializeReferrerLeaderboardPage(maybeResponse.data),
} as ReferrerLeaderboardPageResponse;
break;
}
case "error":
deserialized = maybeResponse;
break;
+
+ default:
+ throw new Error(
+ `Unexpected responseCode: ${(maybeResponse as { responseCode: unknown }).responseCode}`,
+ );
}🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/api/deserialize-v1.ts` around lines 151 - 181, The
switch in deserializeReferrerLeaderboardPageResponse can leave deserialized
undefined if maybeResponse.responseCode is unexpected; add a default branch (or
an exhaustive check) after the existing cases that throws a clear error
including the unexpected maybeResponse.responseCode (and optionally the
serialized payload) so callers fail-fast before calling
makeReferrerLeaderboardPageResponseSchema/safeParse; ensure the thrown message
uses prettifyError or similar context so debugging the malformed response is
easy.
| export function deserializeReferrerDetailResponse( | ||
| maybeResponse: SerializedReferrerDetailResponse, | ||
| valueLabel?: string, | ||
| ): ReferrerDetailResponse { | ||
| let deserialized: ReferrerDetailResponse; | ||
| switch (maybeResponse.responseCode) { | ||
| case "ok": { | ||
| switch (maybeResponse.data.type) { | ||
| case "ranked": | ||
| deserialized = { | ||
| responseCode: maybeResponse.responseCode, | ||
| data: deserializeReferrerDetailRanked(maybeResponse.data), | ||
| } as ReferrerDetailResponse; | ||
| break; | ||
|
|
||
| case "unranked": | ||
| deserialized = { | ||
| responseCode: maybeResponse.responseCode, | ||
| data: deserializeReferrerDetailUnranked(maybeResponse.data), | ||
| } as ReferrerDetailResponse; | ||
| break; | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case "error": | ||
| deserialized = maybeResponse; | ||
| break; | ||
| } | ||
|
|
||
| // Then validate the deserialized structure using zod schemas | ||
| const schema = makeReferrerDetailResponseSchema(valueLabel); | ||
| const parsed = schema.safeParse(deserialized); | ||
|
|
||
| if (parsed.error) { | ||
| throw new Error(`Cannot deserialize ReferrerDetailResponse:\n${prettifyError(parsed.error)}\n`); | ||
| } | ||
|
|
||
| return parsed.data; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Same exhaustiveness concern for deserializeReferrerDetailResponse.
Both the outer switch on responseCode and the inner switch on maybeResponse.data.type could benefit from default cases for defensive error handling against malformed inputs.
🛡️ Suggested improvement
switch (maybeResponse.responseCode) {
case "ok": {
switch (maybeResponse.data.type) {
case "ranked":
deserialized = {
responseCode: maybeResponse.responseCode,
data: deserializeReferrerDetailRanked(maybeResponse.data),
} as ReferrerDetailResponse;
break;
case "unranked":
deserialized = {
responseCode: maybeResponse.responseCode,
data: deserializeReferrerDetailUnranked(maybeResponse.data),
} as ReferrerDetailResponse;
break;
+
+ default:
+ throw new Error(
+ `Unexpected detail type: ${(maybeResponse.data as { type: unknown }).type}`,
+ );
}
break;
}
case "error":
deserialized = maybeResponse;
break;
+
+ default:
+ throw new Error(
+ `Unexpected responseCode: ${(maybeResponse as { responseCode: unknown }).responseCode}`,
+ );
}📝 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.
| export function deserializeReferrerDetailResponse( | |
| maybeResponse: SerializedReferrerDetailResponse, | |
| valueLabel?: string, | |
| ): ReferrerDetailResponse { | |
| let deserialized: ReferrerDetailResponse; | |
| switch (maybeResponse.responseCode) { | |
| case "ok": { | |
| switch (maybeResponse.data.type) { | |
| case "ranked": | |
| deserialized = { | |
| responseCode: maybeResponse.responseCode, | |
| data: deserializeReferrerDetailRanked(maybeResponse.data), | |
| } as ReferrerDetailResponse; | |
| break; | |
| case "unranked": | |
| deserialized = { | |
| responseCode: maybeResponse.responseCode, | |
| data: deserializeReferrerDetailUnranked(maybeResponse.data), | |
| } as ReferrerDetailResponse; | |
| break; | |
| } | |
| break; | |
| } | |
| case "error": | |
| deserialized = maybeResponse; | |
| break; | |
| } | |
| // Then validate the deserialized structure using zod schemas | |
| const schema = makeReferrerDetailResponseSchema(valueLabel); | |
| const parsed = schema.safeParse(deserialized); | |
| if (parsed.error) { | |
| throw new Error(`Cannot deserialize ReferrerDetailResponse:\n${prettifyError(parsed.error)}\n`); | |
| } | |
| return parsed.data; | |
| } | |
| export function deserializeReferrerDetailResponse( | |
| maybeResponse: SerializedReferrerDetailResponse, | |
| valueLabel?: string, | |
| ): ReferrerDetailResponse { | |
| let deserialized: ReferrerDetailResponse; | |
| switch (maybeResponse.responseCode) { | |
| case "ok": { | |
| switch (maybeResponse.data.type) { | |
| case "ranked": | |
| deserialized = { | |
| responseCode: maybeResponse.responseCode, | |
| data: deserializeReferrerDetailRanked(maybeResponse.data), | |
| } as ReferrerDetailResponse; | |
| break; | |
| case "unranked": | |
| deserialized = { | |
| responseCode: maybeResponse.responseCode, | |
| data: deserializeReferrerDetailUnranked(maybeResponse.data), | |
| } as ReferrerDetailResponse; | |
| break; | |
| default: | |
| throw new Error( | |
| `Unexpected detail type: ${(maybeResponse.data as { type: unknown }).type}`, | |
| ); | |
| } | |
| break; | |
| } | |
| case "error": | |
| deserialized = maybeResponse; | |
| break; | |
| default: | |
| throw new Error( | |
| `Unexpected responseCode: ${(maybeResponse as { responseCode: unknown }).responseCode}`, | |
| ); | |
| } | |
| // Then validate the deserialized structure using zod schemas | |
| const schema = makeReferrerDetailResponseSchema(valueLabel); | |
| const parsed = schema.safeParse(deserialized); | |
| if (parsed.error) { | |
| throw new Error(`Cannot deserialize ReferrerDetailResponse:\n${prettifyError(parsed.error)}\n`); | |
| } | |
| return parsed.data; | |
| } |
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/api/deserialize-v1.ts` around lines 190 - 229,
deserializeReferrerDetailResponse lacks default branches in the outer switch on
maybeResponse.responseCode and the inner switch on maybeResponse.data.type, so
add defensive default cases that throw clear Errors (including the unexpected
value) to make the function exhaustive and ensure deserialized is always
assigned; update the outer switch to handle unknown responseCode by throwing an
Error indicating the unexpected responseCode and include a default in the inner
switch to throw an Error indicating the unexpected data.type (or use a
never-exhaustiveness helper if preferred) before the final zod validation in
deserializeReferrerDetailResponse.
| export const makeUnrankedReferrerMetricsSchema = (valueLabel: string = "UnrankedReferrerMetrics") => | ||
| z.object({ | ||
| referrer: makeLowercaseAddressSchema(`${valueLabel}.referrer`), | ||
| totalReferrals: makeNonNegativeIntegerSchema(`${valueLabel}.totalReferrals`), | ||
| totalIncrementalDuration: makeDurationSchema(`${valueLabel}.totalIncrementalDuration`), | ||
| totalRevenueContribution: makePriceEthSchema(`${valueLabel}.totalRevenueContribution`), | ||
| score: makeFiniteNonNegativeNumberSchema(`${valueLabel}.score`), | ||
| rank: z.null(), | ||
| isQualified: z.literal(false), | ||
| finalScoreBoost: makeFiniteNonNegativeNumberSchema(`${valueLabel}.finalScoreBoost`).max( | ||
| 1, | ||
| `${valueLabel}.finalScoreBoost must be <= 1`, | ||
| ), | ||
| finalScore: makeFiniteNonNegativeNumberSchema(`${valueLabel}.finalScore`), | ||
| awardPoolShare: makeFiniteNonNegativeNumberSchema(`${valueLabel}.awardPoolShare`).max( | ||
| 1, | ||
| `${valueLabel}.awardPoolShare must be <= 1`, | ||
| ), | ||
| awardPoolApproxValue: makePriceUsdcSchema(`${valueLabel}.awardPoolApproxValue`), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages/ens-referrals -name "*referrer-metrics*" -type fRepository: namehash/ensnode
Length of output: 156
🏁 Script executed:
fd -t f referrer-metrics packages/ens-referrals/Repository: namehash/ensnode
Length of output: 156
🏁 Script executed:
git ls-files packages/ens-referrals/ | grep -i "referrer\|metric"Repository: namehash/ensnode
Length of output: 251
🏁 Script executed:
wc -l packages/ens-referrals/src/referrer-metrics-v1.tsRepository: namehash/ensnode
Length of output: 113
🏁 Script executed:
cat -n packages/ens-referrals/src/referrer-metrics-v1.tsRepository: namehash/ensnode
Length of output: 17799
🏁 Script executed:
wc -l packages/ens-referrals/src/api/zod-schemas-v1.tsRepository: namehash/ensnode
Length of output: 112
🏁 Script executed:
sed -n '67,86p' packages/ens-referrals/src/api/zod-schemas-v1.tsRepository: namehash/ensnode
Length of output: 1120
🏁 Script executed:
sed -n '60,100p' packages/ens-referrals/src/api/zod-schemas-v1.tsRepository: namehash/ensnode
Length of output: 1769
Add zero-value constraints to the UnrankedReferrerMetrics schema.
The Zod schema currently allows non-zero values, but UnrankedReferrerMetrics must have eight fields zeroed according to validation in referrer-metrics-v1.ts. The schema should enforce this at parse time to prevent invalid API payloads.
✅ Suggested refinement
export const makeUnrankedReferrerMetricsSchema = (valueLabel: string = "UnrankedReferrerMetrics") =>
z.object({
referrer: makeLowercaseAddressSchema(`${valueLabel}.referrer`),
totalReferrals: makeNonNegativeIntegerSchema(`${valueLabel}.totalReferrals`),
totalIncrementalDuration: makeDurationSchema(`${valueLabel}.totalIncrementalDuration`),
totalRevenueContribution: makePriceEthSchema(`${valueLabel}.totalRevenueContribution`),
score: makeFiniteNonNegativeNumberSchema(`${valueLabel}.score`),
rank: z.null(),
isQualified: z.literal(false),
finalScoreBoost: makeFiniteNonNegativeNumberSchema(`${valueLabel}.finalScoreBoost`).max(
1,
`${valueLabel}.finalScoreBoost must be <= 1`,
),
finalScore: makeFiniteNonNegativeNumberSchema(`${valueLabel}.finalScore`),
awardPoolShare: makeFiniteNonNegativeNumberSchema(`${valueLabel}.awardPoolShare`).max(
1,
`${valueLabel}.awardPoolShare must be <= 1`,
),
awardPoolApproxValue: makePriceUsdcSchema(`${valueLabel}.awardPoolApproxValue`),
- });
+ }).refine(
+ (value) =>
+ value.totalReferrals === 0 &&
+ value.totalIncrementalDuration === 0 &&
+ value.score === 0 &&
+ value.finalScoreBoost === 0 &&
+ value.finalScore === 0 &&
+ value.awardPoolShare === 0 &&
+ value.totalRevenueContribution.amount === 0n &&
+ value.awardPoolApproxValue.amount === 0n,
+ { message: `${valueLabel} must have zeroed metrics for unranked referrers` },
+ );📝 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.
| export const makeUnrankedReferrerMetricsSchema = (valueLabel: string = "UnrankedReferrerMetrics") => | |
| z.object({ | |
| referrer: makeLowercaseAddressSchema(`${valueLabel}.referrer`), | |
| totalReferrals: makeNonNegativeIntegerSchema(`${valueLabel}.totalReferrals`), | |
| totalIncrementalDuration: makeDurationSchema(`${valueLabel}.totalIncrementalDuration`), | |
| totalRevenueContribution: makePriceEthSchema(`${valueLabel}.totalRevenueContribution`), | |
| score: makeFiniteNonNegativeNumberSchema(`${valueLabel}.score`), | |
| rank: z.null(), | |
| isQualified: z.literal(false), | |
| finalScoreBoost: makeFiniteNonNegativeNumberSchema(`${valueLabel}.finalScoreBoost`).max( | |
| 1, | |
| `${valueLabel}.finalScoreBoost must be <= 1`, | |
| ), | |
| finalScore: makeFiniteNonNegativeNumberSchema(`${valueLabel}.finalScore`), | |
| awardPoolShare: makeFiniteNonNegativeNumberSchema(`${valueLabel}.awardPoolShare`).max( | |
| 1, | |
| `${valueLabel}.awardPoolShare must be <= 1`, | |
| ), | |
| awardPoolApproxValue: makePriceUsdcSchema(`${valueLabel}.awardPoolApproxValue`), | |
| }); | |
| export const makeUnrankedReferrerMetricsSchema = (valueLabel: string = "UnrankedReferrerMetrics") => | |
| z.object({ | |
| referrer: makeLowercaseAddressSchema(`${valueLabel}.referrer`), | |
| totalReferrals: makeNonNegativeIntegerSchema(`${valueLabel}.totalReferrals`), | |
| totalIncrementalDuration: makeDurationSchema(`${valueLabel}.totalIncrementalDuration`), | |
| totalRevenueContribution: makePriceEthSchema(`${valueLabel}.totalRevenueContribution`), | |
| score: makeFiniteNonNegativeNumberSchema(`${valueLabel}.score`), | |
| rank: z.null(), | |
| isQualified: z.literal(false), | |
| finalScoreBoost: makeFiniteNonNegativeNumberSchema(`${valueLabel}.finalScoreBoost`).max( | |
| 1, | |
| `${valueLabel}.finalScoreBoost must be <= 1`, | |
| ), | |
| finalScore: makeFiniteNonNegativeNumberSchema(`${valueLabel}.finalScore`), | |
| awardPoolShare: makeFiniteNonNegativeNumberSchema(`${valueLabel}.awardPoolShare`).max( | |
| 1, | |
| `${valueLabel}.awardPoolShare must be <= 1`, | |
| ), | |
| awardPoolApproxValue: makePriceUsdcSchema(`${valueLabel}.awardPoolApproxValue`), | |
| }).refine( | |
| (value) => | |
| value.totalReferrals === 0 && | |
| value.totalIncrementalDuration === 0 && | |
| value.score === 0 && | |
| value.finalScoreBoost === 0 && | |
| value.finalScore === 0 && | |
| value.awardPoolShare === 0 && | |
| value.totalRevenueContribution.amount === 0n && | |
| value.awardPoolApproxValue.amount === 0n, | |
| { message: `${valueLabel} must have zeroed metrics for unranked referrers` }, | |
| ); |
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/api/zod-schemas-v1.ts` around lines 67 - 86,
Update makeUnrankedReferrerMetricsSchema so the eight fields that must be zero
are validated at parse time: totalReferrals, totalIncrementalDuration,
totalRevenueContribution, score, finalScoreBoost, finalScore, awardPoolShare,
and awardPoolApproxValue. Replace their current permissive schemas (e.g.,
makeNonNegativeIntegerSchema, makeDurationSchema, makePriceEthSchema,
makeFiniteNonNegativeNumberSchema, makePriceUsdcSchema) with explicit zero
constraints—either z.literal(0) for exact-zero or apply .min(0).max(0, "<field>
must be 0") to the existing schema factories—to ensure these fields can only be
0 and include a clear error message referencing the field via the valueLabel.
| export const validateReferrerLeaderboardPageContext = ( | ||
| context: ReferrerLeaderboardPageContext, | ||
| ): void => { | ||
| validateReferrerLeaderboardPageParams(context); | ||
| if (!isNonNegativeInteger(context.totalRecords)) { | ||
| throw new Error( | ||
| `Invalid ReferrerLeaderboardPageContext: total must be a non-negative integer but is ${context.totalRecords}.`, | ||
| ); | ||
| } | ||
| const startIndex = (context.page - 1) * context.recordsPerPage; | ||
| const endIndex = startIndex + context.recordsPerPage; | ||
|
|
||
| if (!context.hasNext && endIndex < context.totalRecords) { | ||
| throw new Error( | ||
| `Invalid ReferrerLeaderboardPageContext: if hasNext is false, endIndex (${endIndex}) must be greater than or equal to total (${context.totalRecords}).`, | ||
| ); | ||
| } else if (context.hasNext && context.page * context.recordsPerPage >= context.totalRecords) { | ||
| throw new Error( | ||
| `Invalid ReferrerLeaderboardPageContext: if hasNext is true, endIndex (${endIndex}) must be less than total (${context.totalRecords}).`, | ||
| ); | ||
| } | ||
| if (!context.hasPrev && context.page !== 1) { | ||
| throw new Error( | ||
| `Invalid ReferrerLeaderboardPageContext: if hasPrev is false, page must be the first page (1) but is ${context.page}.`, | ||
| ); | ||
| } else if (context.hasPrev && context.page === 1) { | ||
| throw new Error( | ||
| `Invalid ReferrerLeaderboardPageContext: if hasPrev is true, page must not be the first page (1) but is ${context.page}.`, | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Context validator misses totalPages/startIndex/endIndex invariants.
validateReferrerLeaderboardPageContext doesn’t verify totalPages, page <= totalPages, or that startIndex/endIndex match derived values. This lets inconsistent contexts pass validation.
✅ Suggested validation additions
export const validateReferrerLeaderboardPageContext = (
context: ReferrerLeaderboardPageContext,
): void => {
validateReferrerLeaderboardPageParams(context);
if (!isNonNegativeInteger(context.totalRecords)) {
throw new Error(
`Invalid ReferrerLeaderboardPageContext: total must be a non-negative integer but is ${context.totalRecords}.`,
);
}
+ if (!isPositiveInteger(context.totalPages)) {
+ throw new Error(
+ `Invalid ReferrerLeaderboardPageContext: totalPages must be a positive integer but is ${context.totalPages}.`,
+ );
+ }
+ const expectedTotalPages = Math.max(
+ 1,
+ Math.ceil(context.totalRecords / context.recordsPerPage),
+ );
+ if (context.totalPages !== expectedTotalPages) {
+ throw new Error(
+ `Invalid ReferrerLeaderboardPageContext: totalPages (${context.totalPages}) must equal ${expectedTotalPages}.`,
+ );
+ }
+ if (context.page > context.totalPages) {
+ throw new Error(
+ `Invalid ReferrerLeaderboardPageContext: page (${context.page}) must be <= totalPages (${context.totalPages}).`,
+ );
+ }
const startIndex = (context.page - 1) * context.recordsPerPage;
const endIndex = startIndex + context.recordsPerPage;
+ const expectedStartIndex = context.totalRecords === 0 ? undefined : startIndex;
+ const expectedEndIndex =
+ context.totalRecords === 0
+ ? undefined
+ : Math.min(endIndex - 1, context.totalRecords - 1);
+ if (context.totalRecords === 0) {
+ if (context.startIndex !== undefined || context.endIndex !== undefined) {
+ throw new Error(
+ `Invalid ReferrerLeaderboardPageContext: startIndex/endIndex must be undefined when totalRecords is 0.`,
+ );
+ }
+ } else if (context.startIndex !== expectedStartIndex || context.endIndex !== expectedEndIndex) {
+ throw new Error(
+ `Invalid ReferrerLeaderboardPageContext: startIndex/endIndex must be ${expectedStartIndex}/${expectedEndIndex} but are ${context.startIndex}/${context.endIndex}.`,
+ );
+ }
if (!context.hasNext && endIndex < context.totalRecords) {
throw new Error(
`Invalid ReferrerLeaderboardPageContext: if hasNext is false, endIndex (${endIndex}) must be greater than or equal to total (${context.totalRecords}).`,
);
} else if (context.hasNext && context.page * context.recordsPerPage >= context.totalRecords) {
throw new Error(
`Invalid ReferrerLeaderboardPageContext: if hasNext is true, endIndex (${endIndex}) must be less than total (${context.totalRecords}).`,
);
}🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/leaderboard-page-v1.ts` around lines 118 - 148,
The validator must also check totalPages and that the provided
startIndex/endIndex match derived values: inside
validateReferrerLeaderboardPageContext compute expectedTotalPages =
Math.ceil(context.totalRecords / context.recordsPerPage) (handle zero records),
compute expectedStartIndex = (context.page - 1) * context.recordsPerPage and
expectedEndIndex = Math.min(expectedStartIndex + context.recordsPerPage,
context.totalRecords), then validate that context.totalPages is a non-negative
integer and equals expectedTotalPages, that context.page <= context.totalPages
(and valid for zero/empty cases), and that context.startIndex ===
expectedStartIndex and context.endIndex === expectedEndIndex (also ensure
endIndex >= startIndex); throw the same style of Error with clear messages when
any of these invariants fail.
| const uniqueReferrers = allReferrers.map((referrer) => referrer.referrer); | ||
| if (uniqueReferrers.length !== allReferrers.length) { | ||
| throw new Error( | ||
| "ReferrerLeaderboard: Cannot buildReferrerLeaderboard containing duplicate referrers", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Duplicate referrer detection is ineffective.
uniqueReferrers is just a copy of the input, so the length comparison always passes and duplicates will silently overwrite when building the Map. Use a Set to detect duplicates reliably.
🐛 Proposed fix
- const uniqueReferrers = allReferrers.map((referrer) => referrer.referrer);
- if (uniqueReferrers.length !== allReferrers.length) {
+ const uniqueReferrers = new Set(allReferrers.map((referrer) => referrer.referrer));
+ if (uniqueReferrers.size !== allReferrers.length) {📝 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 uniqueReferrers = allReferrers.map((referrer) => referrer.referrer); | |
| if (uniqueReferrers.length !== allReferrers.length) { | |
| throw new Error( | |
| "ReferrerLeaderboard: Cannot buildReferrerLeaderboard containing duplicate referrers", | |
| ); | |
| } | |
| const uniqueReferrers = new Set(allReferrers.map((referrer) => referrer.referrer)); | |
| if (uniqueReferrers.size !== allReferrers.length) { | |
| throw new Error( | |
| "ReferrerLeaderboard: Cannot buildReferrerLeaderboard containing duplicate referrers", | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/leaderboard-v1.ts` around lines 56 - 61, The
duplicate-referrer check is wrong because uniqueReferrers is just a mapping
copy; replace it by creating a Set of referrer strings from allReferrers (e.g.,
new Set(allReferrers.map(r => r.referrer))) and compare set.size to
allReferrers.length inside buildReferrerLeaderboard (or wherever uniqueReferrers
is defined); if sizes differ, throw the existing Error so duplicates are
detected before constructing the Map and silently overwriting entries.
| export function calcReferrerFinalScoreBoost( | ||
| rank: ReferrerRank, | ||
| rules: ReferralProgramRules, | ||
| ): number { | ||
| if (!isReferrerQualified(rank, rules)) return 0; | ||
|
|
||
| return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1); | ||
| } |
There was a problem hiding this comment.
Division-by-zero when maxQualifiedReferrers === 1.
calcReferrerFinalScoreBoost divides by (maxQualifiedReferrers - 1), which becomes zero when only one referrer can qualify. This yields Infinity/NaN for the top-ranked referrer.
🛠️ Guard against the edge case
export function calcReferrerFinalScoreBoost(
rank: ReferrerRank,
rules: ReferralProgramRules,
): number {
if (!isReferrerQualified(rank, rules)) return 0;
+ if (rules.maxQualifiedReferrers === 1) return 1;
return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1);
}📝 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.
| export function calcReferrerFinalScoreBoost( | |
| rank: ReferrerRank, | |
| rules: ReferralProgramRules, | |
| ): number { | |
| if (!isReferrerQualified(rank, rules)) return 0; | |
| return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1); | |
| } | |
| export function calcReferrerFinalScoreBoost( | |
| rank: ReferrerRank, | |
| rules: ReferralProgramRules, | |
| ): number { | |
| if (!isReferrerQualified(rank, rules)) return 0; | |
| if (rules.maxQualifiedReferrers === 1) return 1; | |
| return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1); | |
| } |
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/rank-v1.ts` around lines 40 - 47,
calcReferrerFinalScoreBoost currently divides by (rules.maxQualifiedReferrers -
1) causing divide-by-zero when maxQualifiedReferrers === 1; update
calcReferrerFinalScoreBoost to guard that case by returning the correct boost
(return 1 for the sole qualified referrer) when rules.maxQualifiedReferrers <= 1
(and keep the existing isReferrerQualified check), otherwise compute 1 - (rank -
1) / (rules.maxQualifiedReferrers - 1) as before.
| priceUsdc, | ||
| type UnixTimestamp, | ||
| } from "@ensnode/ensnode-sdk"; | ||
| import { makePriceUsdcSchema } from "@ensnode/ensnode-sdk/internal"; |
There was a problem hiding this comment.
subregistryId is never validated.
validateReferralProgramRules checks price, timestamps, and counts, but skips the subregistryId field, allowing invalid account IDs to slip through. Consider validating it alongside the other invariants.
🛠️ Suggested validation
-import { makePriceUsdcSchema } from "@ensnode/ensnode-sdk/internal";
+import { makeAccountIdSchema, makePriceUsdcSchema } from "@ensnode/ensnode-sdk/internal";
@@
const parseResult = priceUsdcSchema.safeParse(rules.totalAwardPoolValue);
if (!parseResult.success) {
throw new Error(
`ReferralProgramRules: totalAwardPoolValue validation failed: ${parseResult.error.message}`,
);
}
+
+ const accountIdSchema = makeAccountIdSchema("ReferralProgramRules.subregistryId");
+ const accountIdResult = accountIdSchema.safeParse(rules.subregistryId);
+ if (!accountIdResult.success) {
+ throw new Error(
+ `ReferralProgramRules: subregistryId validation failed: ${accountIdResult.error.message}`,
+ );
+ }
validateNonNegativeInteger(rules.maxQualifiedReferrers);Also applies to: 67-85
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/rules-v1.ts` at line 7,
validateReferralProgramRules currently omits validation for the subregistryId
field; update the function (and the same checks around lines 67-85) to validate
subregistryId alongside price/timestamp/count invariants: locate the
validateReferralProgramRules function and add a check that subregistryId is
present and matches your expected account-id format (e.g., non-empty string and
the same pattern used elsewhere for ENS account IDs or a shared validator helper
if one exists), and throw or return an error when it fails so invalid account
IDs cannot pass through.
packages/ens-referrals/src/v1.ts
Outdated
| export * from "./api/types-v1"; | ||
| export * from "./api/zod-schemas-v1"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the current state of v1.ts
cat -n packages/ens-referrals/src/v1.tsRepository: namehash/ensnode
Length of output: 1170
🏁 Script executed:
# Check the header and content of zod-schemas-v1.ts
head -40 packages/ens-referrals/src/api/zod-schemas-v1.tsRepository: namehash/ensnode
Length of output: 1481
🏁 Script executed:
# Check if internal.ts exists and its content
cat -n packages/ens-referrals/src/internal.ts 2>/dev/null || echo "internal.ts not found"Repository: namehash/ensnode
Length of output: 723
🏁 Script executed:
# List all files in the src directory to understand the structure
find packages/ens-referrals/src -type f -name "*.ts" | head -20Repository: namehash/ensnode
Length of output: 941
Remove Zod schemas from public v1 entry point.
The header in packages/ens-referrals/src/api/zod-schemas-v1.ts explicitly states that schemas must remain internal and only be re-exported from internal.ts. Re-exporting from the public v1 entry point violates this policy and locks an internal implementation detail into the public API surface. Remove this export from line 15.
🔧 Suggested change
export * from "./api/types-v1";
-export * from "./api/zod-schemas-v1";📝 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.
| export * from "./api/types-v1"; | |
| export * from "./api/zod-schemas-v1"; | |
| export * from "./api/types-v1"; |
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/v1.ts` around lines 14 - 15, Remove the public
re-export of the internal Zod schemas by deleting the export of
"./api/zod-schemas-v1" from the v1 public entry (leave "export * from
\"./api/types-v1\""), so the internal module "zod-schemas-v1" is no longer
exposed; ensure any consumers that relied on that export are updated to import
from the internal re-export location (internal.ts) instead and run the
build/tests to confirm no public API leak remains.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| ): number { | ||
| if (!isReferrerQualified(rank, rules)) return 0; | ||
|
|
||
| return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1); |
There was a problem hiding this comment.
The calcReferrerFinalScoreBoost function will return NaN when maxQualifiedReferrers is 1, because it performs division by (maxQualifiedReferrers - 1) which equals 0.
View Details
📝 Patch Details
diff --git a/packages/ens-referrals/src/rank-v1.ts b/packages/ens-referrals/src/rank-v1.ts
index 7ddc54e2..7535c2f4 100644
--- a/packages/ens-referrals/src/rank-v1.ts
+++ b/packages/ens-referrals/src/rank-v1.ts
@@ -43,6 +43,9 @@ export function calcReferrerFinalScoreBoost(
): number {
if (!isReferrerQualified(rank, rules)) return 0;
+ // Handle edge case: only 1 qualified referrer
+ if (rules.maxQualifiedReferrers === 1) return 1;
+
return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1);
}
diff --git a/packages/ens-referrals/src/rank.ts b/packages/ens-referrals/src/rank.ts
index 093fde9f..e37d6b5e 100644
--- a/packages/ens-referrals/src/rank.ts
+++ b/packages/ens-referrals/src/rank.ts
@@ -43,6 +43,9 @@ export function calcReferrerFinalScoreBoost(
): number {
if (!isReferrerQualified(rank, rules)) return 0;
+ // Handle edge case: only 1 qualified referrer
+ if (rules.maxQualifiedReferrers === 1) return 1;
+
return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1);
}
Analysis
Division by zero in calcReferrerFinalScoreBoost causes NaN when maxQualifiedReferrers is 1
What fails: The calcReferrerFinalScoreBoost() function in packages/ens-referrals/src/rank-v1.ts (and rank.ts) returns NaN when maxQualifiedReferrers === 1 and rank === 1.
How to reproduce:
const rules = {
totalAwardPoolValue: priceUsdc(10_000_000_000n),
maxQualifiedReferrers: 1, // Only 1 qualified referrer
startTime: 1764547200,
endTime: 1767225599,
subregistryId: "0x123"
};
const result = calcReferrerFinalScoreBoost(1, rules);
// Returns: NaNWhat happens vs expected:
- Current behavior: The calculation
1 - (1 - 1) / (1 - 1)evaluates to1 - 0/0 = 1 - NaN = NaN - Expected behavior: The function should return a valid number between 0 and 1 (specifically 1.0, since the only qualified referrer should receive the full boost)
Impact: The NaN value cascades through downstream functions:
calcReferrerFinalScoreMultiplier()receives NaN and returns1 + NaN = NaNcalcReferrerFinalScore()receives NaN and returnsscore * NaN = NaNbuildRankedReferrerMetrics()creates invalid metrics with NaN values- Validation in
validateRankedReferrerMetrics()fails when checking thatfinalScoreBoostmust be between 0 and 1
Fix: Added a special case check for maxQualifiedReferrers === 1 to return 1 (full boost) since there is only one qualified referrer. This is mathematically consistent with the linear interpolation behavior of the original formula for larger values.
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database-v1.ts`:
- Around line 44-88: The SUM(incrementalDuration) can be NULL when all rows are
NULL, so change the select for totalIncrementalDuration to mirror revenue and
wrap the sum in COALESCE to default to 0; specifically replace
sum(schema.registrarActions.incrementalDuration).as("total_incremental_duration")
with sql`COALESCE(SUM(${schema.registrarActions.incrementalDuration}),
0)`.as("total_incremental_duration") so
deserializeDuration(record.totalIncrementalDuration) and the ordering by
total_incremental_duration are safe and non-null.
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/mocks-v1.ts`:
- Around line 949-1540: The referrerLeaderboardPageResponseOk fixture duplicates
the populatedReferrerLeaderboard data; replace the repeated literal by
constructing referrerLeaderboardPageResponseOk from the existing
populatedReferrerLeaderboard fixture (e.g., import/populate
populatedReferrerLeaderboard and then build referrerLeaderboardPageResponseOk by
referencing its properties for data.rules, data.referrers,
data.aggregatedMetrics, data.pageContext and data.accurateAsOf) so changes to
populatedReferrerLeaderboard automatically flow to
referrerLeaderboardPageResponseOk and avoid drift; ensure the top-level
responseCode remains ReferrerLeaderboardPageResponseCodes.Ok and any
BigInt/const types are preserved when mapping.
- Around line 282-947: The populatedReferrerLeaderboard fixture has several
referrer entries (inside populatedReferrerLeaderboard.referrers Map) whose
totalRevenueContribution.amount are incorrectly set to 6_000_000_000_000_000n
but should match the distinct values in dbResultsReferrerLeaderboard (e.g.,
0.007, 0.0065, 0.0075, 0.008, 0.0085 ETH → 7_000_000_000_000_000n,
6_500_000_000_000_000n, 7_500_000_000_000_000n, 8_000_000_000_000_000n,
8_500_000_000_000_000n); locate the affected referrer keys in the Map (those
currently using 6_000_000_000_000_000n) and replace each amount to the exact
big-int values from dbResultsReferrerLeaderboard, then update any dependent
fields in populatedReferrerLeaderboard (scores, awardPoolShare,
awardPoolApproxValue, and aggregatedMetrics like grandTotalRevenueContribution)
so the fixture stays consistent with the source.
In `@packages/ens-referrals/src/aggregations-v1.ts`:
- Around line 69-72: The function buildAggregatedReferrerMetrics assumes
referrers is a complete, globally ranked list starting at rank 1 (the invariant
check expects ranks to be absolute), so add a clear JSDoc on the
buildAggregatedReferrerMetrics export stating that referrers must contain a full
ranked list with ranks starting at 1 (not a paginated/partial slice), and
document the expected shape and consequences (that maxQualifiedReferrers > 0
with no qualified referrers will throw) so callers know to pass the full ranking
or to pre-aggregate before calling.
In `@packages/ens-referrals/src/api/types-v1.ts`:
- Around line 46-50: The ReferrerLeaderboardPageResponseError type (and
ReferrerDetailResponseError) currently carries both error and errorMessage;
decide and implement a single consistent pattern: either consolidate into one
field (e.g., errorMessage: string) by removing the redundant property and
updating all usages of ReferrerLeaderboardPageResponseError and
ReferrerDetailResponseError, or explicitly document and rename to make the
distinction clear (e.g., errorCode: string for machine-readable values and
message: string for human-readable text) and update all references and
serializers/deserializers accordingly; update the type definitions
(ReferrerLeaderboardPageResponseError, ReferrerDetailResponseError) and any code
that constructs or reads these objects to match the chosen shape and ensure
tests/API clients reflect the change.
In `@packages/ens-referrals/src/api/zod-schemas-v1.ts`:
- Around line 110-125: The schema makeReferrerLeaderboardPageContextSchema
currently allows optional startIndex and endIndex but doesn't enforce endIndex
>= startIndex; update the returned z.object to add a refinement (using .refine
or .superRefine on the object) that, when both startIndex and endIndex are
defined, asserts endIndex >= startIndex and returns a clear error path (e.g.,
`${valueLabel}.endIndex` or `${valueLabel}.startIndex`) on failure; reference
the object produced by makeReferrerLeaderboardPageContextSchema and the
startIndex/endIndex fields when implementing the check so the validator triggers
only when both values are present.
In `@packages/ens-referrals/src/leaderboard-v1.ts`:
- Around line 56-61: The duplicate detection is wrong because uniqueReferrers is
created with map which preserves length; replace it with a Set-based check:
compute const uniqueReferrerSet = new Set(allReferrers.map(r => r.referrer)) and
then compare uniqueReferrerSet.size to allReferrers.length (or recreate
uniqueReferrers via Array.from(uniqueReferrerSet)) in the
buildReferrerLeaderboard logic to properly detect duplicates and throw the same
Error when sizes differ; update references to uniqueReferrers accordingly.
In `@packages/ens-referrals/src/referrer-metrics-v1.ts`:
- Around line 287-300: The current calculation for awardPoolApproxAmount loses
precision by converting rules.totalAwardPoolValue.amount (a bigint) to Number;
instead compute the amount using precise integer or big-decimal arithmetic:
either represent awardPoolShare as an integer fraction (numerator/denominator)
and compute awardPoolApproxAmount = (BigInt(rules.totalAwardPoolValue.amount) *
numerator) / denominator, or use a BigNumber/Decimal library to multiply a
Decimal(awardPoolShare) by the bigint total and round/floor to a bigint; update
the code around awardPoolApproxAmount, awardPoolShare and
result.awardPoolApproxValue to use the chosen precise method.
In `@packages/ens-referrals/src/rules-v1.ts`:
- Around line 67-86: The function validateReferralProgramRules currently omits
validation for the subregistryId field; add a call to the existing AccountId
validator (e.g., validateAccountId) to check rules.subregistryId inside
validateReferralProgramRules, placed alongside the other field checks (after
totalAwardPoolValue parsing and before/after maxQualifiedReferrers validation),
and throw a clear Error if validation fails so the code validates that
subregistryId is a valid AccountId.
In `@packages/ensnode-sdk/src/shared/serialize.ts`:
- Around line 23-36: Add a short maintenance note next to the duplicated
uint256ToHex32 definition explaining that this is an intentional inline copy of
the implementation in ../ens/subname-helpers.ts due to Vite SSR resolution, and
include a TODO/link or issue number to track changes so both implementations
stay synchronized; reference the function name uint256ToHex32 and the original
file subname-helpers.ts in the comment to make future updates clear.
♻️ Duplicate comments (9)
packages/ensnode-sdk/src/shared/zod-schemas.ts (1)
16-23: Unused importPriceshould be removed.The
Priceimport on line 19 is not used anywhere in this file. This was previously flagged and remains unresolved.Suggested fix
import { type CurrencyId, CurrencyIds, - Price, type PriceDai, type PriceEth, type PriceUsdc, } from "./currencies";packages/ens-referrals/src/rank-v1.ts (1)
40-47: Division by zero whenmaxQualifiedReferrersis 1.When
rules.maxQualifiedReferrers === 1andrank === 1, line 46 computes(rank - 1) / (rules.maxQualifiedReferrers - 1)which evaluates to0 / 0 = NaN, causing the function to returnNaN.Proposed fix
export function calcReferrerFinalScoreBoost( rank: ReferrerRank, rules: ReferralProgramRules, ): number { if (!isReferrerQualified(rank, rules)) return 0; + // When only one referrer can qualify, that referrer gets the maximum boost + if (rules.maxQualifiedReferrers === 1) return 1; + return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1); }packages/ens-referrals/src/api/types-v1.ts (1)
12-12: Consider using a type alias instead of an empty interface.An interface that declares no additional members is equivalent to its supertype. Using a type alias is more explicit.
packages/ens-referrals/src/api/serialized-types-v1.ts (1)
8-15: Remove unused imports.
ReferrerDetailResponse(line 9) andReferrerLeaderboardPageResponse(line 12) are imported but never used in this file. Only their constituent types (*Okand*Errorvariants) are used.packages/ens-referrals/src/api/zod-schemas-v1.ts (3)
25-25: Remove unused importReferrerDetailRanked.The type
ReferrerDetailRankedis imported but only used in JSDoc comments, not in code. TheReferrerDetailTypeIdsimport is sufficient.
153-160: Remove unused_valueLabelparameter or use it.The
_valueLabelparameter is declared but never used inmakeReferrerLeaderboardPageResponseErrorSchema. Either remove it or use it for field descriptions like the other schema factories.
213-220: Remove unused_valueLabelparameter or use it.Same issue as above -
_valueLabelis declared but unused inmakeReferrerDetailResponseErrorSchema.packages/ens-referrals/src/api/serialize-v1.ts (1)
164-187: Unreachablebreakstatement after nested switch.The
breakon line 182 follows a nested switch where both branches ("ranked"and"unranked") return early. If TypeScript's type narrowing ensures exhaustiveness, thisbreakis dead code. If not, the function could implicitly returnundefined.Consider removing the
breaksince each nested case already returns, or add an exhaustive check after the nested switch for defensive coding:Suggested fix
case "unranked": return { responseCode: response.responseCode, data: serializeReferrerDetailUnranked(response.data), }; } - break; case ReferrerDetailResponseCodes.Error: return response; } }packages/ens-referrals/src/api/deserialize-v1.ts (1)
190-229: Potential uninitialized variable if nested switch is non-exhaustive.The
let deserializedvariable (line 194) may remain unassigned ifmaybeResponse.data.typedoesn't match"ranked"or"unranked". Thebreakon line 212 would exit the outer switch, leavingdeserializedundefined when accessed on line 222.If TypeScript's type system guarantees exhaustiveness of the nested switch, the
breakis dead code. Otherwise, this could cause a runtime error. Consider restructuring to eliminate the intermediate variable or adding an exhaustive guard.Suggested fix
export function deserializeReferrerDetailResponse( maybeResponse: SerializedReferrerDetailResponse, valueLabel?: string, ): ReferrerDetailResponse { - let deserialized: ReferrerDetailResponse; switch (maybeResponse.responseCode) { case "ok": { + let data: ReferrerDetailRanked | ReferrerDetailUnranked; switch (maybeResponse.data.type) { case "ranked": - deserialized = { - responseCode: maybeResponse.responseCode, - data: deserializeReferrerDetailRanked(maybeResponse.data), - } as ReferrerDetailResponse; + data = deserializeReferrerDetailRanked(maybeResponse.data); break; - case "unranked": - deserialized = { - responseCode: maybeResponse.responseCode, - data: deserializeReferrerDetailUnranked(maybeResponse.data), - } as ReferrerDetailResponse; + data = deserializeReferrerDetailUnranked(maybeResponse.data); break; } - break; + const deserialized = { + responseCode: maybeResponse.responseCode, + data, + } as ReferrerDetailResponse; + const schema = makeReferrerDetailResponseSchema(valueLabel); + const parsed = schema.safeParse(deserialized); + if (parsed.error) { + throw new Error(`Cannot deserialize ReferrerDetailResponse:\n${prettifyError(parsed.error)}\n`); + } + return parsed.data; } - - case "error": - deserialized = maybeResponse; - break; + case "error": { + const schema = makeReferrerDetailResponseSchema(valueLabel); + const parsed = schema.safeParse(maybeResponse); + if (parsed.error) { + throw new Error(`Cannot deserialize ReferrerDetailResponse:\n${prettifyError(parsed.error)}\n`); + } + return parsed.data; + } } - - // Then validate the deserialized structure using zod schemas - const schema = makeReferrerDetailResponseSchema(valueLabel); - const parsed = schema.safeParse(deserialized); - - if (parsed.error) { - throw new Error(`Cannot deserialize ReferrerDetailResponse:\n${prettifyError(parsed.error)}\n`); - } - - return parsed.data; }
| totalIncrementalDuration: sum(schema.registrarActions.incrementalDuration).as( | ||
| "total_incremental_duration", | ||
| ), | ||
| // Note: Using raw SQL for COALESCE because Drizzle doesn't natively support it yet. | ||
| // See: https://github.com/drizzle-team/drizzle-orm/issues/3708 | ||
| totalRevenueContribution: | ||
| sql<string>`COALESCE(SUM(${schema.registrarActions.total}), 0)`.as( | ||
| "total_revenue_contribution", | ||
| ), | ||
| }) | ||
| .from(schema.registrarActions) | ||
| .where( | ||
| and( | ||
| // Filter by timestamp range | ||
| gte(schema.registrarActions.timestamp, BigInt(rules.startTime)), | ||
| lte(schema.registrarActions.timestamp, BigInt(rules.endTime)), | ||
| // Filter by decodedReferrer not null | ||
| isNotNull(schema.registrarActions.decodedReferrer), | ||
| // Filter by decodedReferrer not zero address | ||
| ne(schema.registrarActions.decodedReferrer, zeroAddress), | ||
| // Filter by subregistryId matching the provided subregistryId | ||
| eq(schema.registrarActions.subregistryId, formatAccountId(rules.subregistryId)), | ||
| ), | ||
| ) | ||
| .groupBy(schema.registrarActions.decodedReferrer) | ||
| .orderBy(desc(sql`total_incremental_duration`)); | ||
|
|
||
| // Type assertion: The WHERE clause in the query above guarantees non-null values for: | ||
| // 1. `referrer` is guaranteed to be non-null due to isNotNull filter | ||
| // 2. `totalIncrementalDuration` is guaranteed to be non-null as it is the sum of non-null bigint values | ||
| // 3. `totalRevenueContribution` is guaranteed to be non-null due to COALESCE with 0 | ||
| interface NonNullRecord { | ||
| referrer: Address; | ||
| totalReferrals: number; | ||
| totalIncrementalDuration: string; | ||
| totalRevenueContribution: string; | ||
| } | ||
|
|
||
| return (records as NonNullRecord[]).map((record) => { | ||
| return buildReferrerMetrics( | ||
| record.referrer, | ||
| record.totalReferrals, | ||
| deserializeDuration(record.totalIncrementalDuration), | ||
| priceEth(BigInt(record.totalRevenueContribution)), | ||
| ); |
There was a problem hiding this comment.
Guard against NULL totalIncrementalDuration sums.
SUM(incrementalDuration) returns NULL if all rows are NULL for a referrer, but Line 73 assumes non-null and deserializeDuration will then throw. Mirror the COALESCE pattern used for revenue.
🐛 Suggested fix
- totalIncrementalDuration: sum(schema.registrarActions.incrementalDuration).as(
- "total_incremental_duration",
- ),
+ totalIncrementalDuration:
+ sql<string>`COALESCE(SUM(${schema.registrarActions.incrementalDuration}), 0)`.as(
+ "total_incremental_duration",
+ ),🤖 Prompt for AI Agents
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database-v1.ts` around
lines 44 - 88, The SUM(incrementalDuration) can be NULL when all rows are NULL,
so change the select for totalIncrementalDuration to mirror revenue and wrap the
sum in COALESCE to default to 0; specifically replace
sum(schema.registrarActions.incrementalDuration).as("total_incremental_duration")
with sql`COALESCE(SUM(${schema.registrarActions.incrementalDuration}),
0)`.as("total_incremental_duration") so
deserializeDuration(record.totalIncrementalDuration) and the ordering by
total_incremental_duration are safe and non-null.
| export const buildAggregatedReferrerMetrics = ( | ||
| referrers: RankedReferrerMetrics[], | ||
| rules: ReferralProgramRules, | ||
| ): AggregatedReferrerMetrics => { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Document the precondition for the referrers parameter.
The invariant check at lines 97-101 assumes that referrers contains a complete ranked list where ranks start from 1. If this function is called with a partial list (e.g., a page of results with ranks 4-6), the invariant check would incorrectly throw an error when maxQualifiedReferrers > 0 but no qualified referrers are in the slice.
Consider adding a JSDoc comment clarifying this precondition:
Suggested documentation
+/**
+ * Build aggregated metrics from a list of ranked referrer metrics.
+ *
+ * `@param` referrers - Complete list of RankedReferrerMetrics for the leaderboard.
+ * Ranks must start from 1 and be contiguous.
+ * `@param` rules - The referral program rules governing qualification.
+ * `@returns` Aggregated metrics across all referrers.
+ */
export const buildAggregatedReferrerMetrics = (
referrers: RankedReferrerMetrics[],
rules: ReferralProgramRules,
): AggregatedReferrerMetrics => {📝 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.
| export const buildAggregatedReferrerMetrics = ( | |
| referrers: RankedReferrerMetrics[], | |
| rules: ReferralProgramRules, | |
| ): AggregatedReferrerMetrics => { | |
| /** | |
| * Build aggregated metrics from a list of ranked referrer metrics. | |
| * | |
| * `@param` referrers - Complete list of RankedReferrerMetrics for the leaderboard. | |
| * Ranks must start from 1 and be contiguous. | |
| * `@param` rules - The referral program rules governing qualification. | |
| * `@returns` Aggregated metrics across all referrers. | |
| */ | |
| export const buildAggregatedReferrerMetrics = ( | |
| referrers: RankedReferrerMetrics[], | |
| rules: ReferralProgramRules, | |
| ): AggregatedReferrerMetrics => { |
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/aggregations-v1.ts` around lines 69 - 72, The
function buildAggregatedReferrerMetrics assumes referrers is a complete,
globally ranked list starting at rank 1 (the invariant check expects ranks to be
absolute), so add a clear JSDoc on the buildAggregatedReferrerMetrics export
stating that referrers must contain a full ranked list with ranks starting at 1
(not a paginated/partial slice), and document the expected shape and
consequences (that maxQualifiedReferrers > 0 with no qualified referrers will
throw) so callers know to pass the full ranking or to pre-aggregate before
calling.
| export type ReferrerLeaderboardPageResponseError = { | ||
| responseCode: typeof ReferrerLeaderboardPageResponseCodes.Error; | ||
| error: string; | ||
| errorMessage: string; | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider whether both error and errorMessage are necessary.
The error response type has both error and errorMessage fields. This pattern is repeated for ReferrerDetailResponseError as well. Consider documenting the distinction between these fields (e.g., error for machine-readable codes, errorMessage for human-readable text) or consolidating if they serve the same purpose.
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/api/types-v1.ts` around lines 46 - 50, The
ReferrerLeaderboardPageResponseError type (and ReferrerDetailResponseError)
currently carries both error and errorMessage; decide and implement a single
consistent pattern: either consolidate into one field (e.g., errorMessage:
string) by removing the redundant property and updating all usages of
ReferrerLeaderboardPageResponseError and ReferrerDetailResponseError, or
explicitly document and rename to make the distinction clear (e.g., errorCode:
string for machine-readable values and message: string for human-readable text)
and update all references and serializers/deserializers accordingly; update the
type definitions (ReferrerLeaderboardPageResponseError,
ReferrerDetailResponseError) and any code that constructs or reads these objects
to match the chosen shape and ensure tests/API clients reflect the change.
| export const makeReferrerLeaderboardPageContextSchema = ( | ||
| valueLabel: string = "ReferrerLeaderboardPageContext", | ||
| ) => | ||
| z.object({ | ||
| page: makePositiveIntegerSchema(`${valueLabel}.page`), | ||
| recordsPerPage: makePositiveIntegerSchema(`${valueLabel}.recordsPerPage`).max( | ||
| REFERRERS_PER_LEADERBOARD_PAGE_MAX, | ||
| `${valueLabel}.recordsPerPage must not exceed ${REFERRERS_PER_LEADERBOARD_PAGE_MAX}`, | ||
| ), | ||
| totalRecords: makeNonNegativeIntegerSchema(`${valueLabel}.totalRecords`), | ||
| totalPages: makePositiveIntegerSchema(`${valueLabel}.totalPages`), | ||
| hasNext: z.boolean(), | ||
| hasPrev: z.boolean(), | ||
| startIndex: z.optional(makeNonNegativeIntegerSchema(`${valueLabel}.startIndex`)), | ||
| endIndex: z.optional(makeNonNegativeIntegerSchema(`${valueLabel}.endIndex`)), | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider validating startIndex and endIndex relationship.
The schema validates startIndex and endIndex as optional non-negative integers, but doesn't validate that endIndex >= startIndex when both are present. This invariant may be enforced elsewhere, but adding it here would provide earlier validation.
♻️ Proposed refinement to validate index relationship
export const makeReferrerLeaderboardPageContextSchema = (
valueLabel: string = "ReferrerLeaderboardPageContext",
) =>
z.object({
page: makePositiveIntegerSchema(`${valueLabel}.page`),
recordsPerPage: makePositiveIntegerSchema(`${valueLabel}.recordsPerPage`).max(
REFERRERS_PER_LEADERBOARD_PAGE_MAX,
`${valueLabel}.recordsPerPage must not exceed ${REFERRERS_PER_LEADERBOARD_PAGE_MAX}`,
),
totalRecords: makeNonNegativeIntegerSchema(`${valueLabel}.totalRecords`),
totalPages: makePositiveIntegerSchema(`${valueLabel}.totalPages`),
hasNext: z.boolean(),
hasPrev: z.boolean(),
startIndex: z.optional(makeNonNegativeIntegerSchema(`${valueLabel}.startIndex`)),
endIndex: z.optional(makeNonNegativeIntegerSchema(`${valueLabel}.endIndex`)),
- });
+ }).refine(
+ (data) => data.startIndex === undefined || data.endIndex === undefined || data.endIndex >= data.startIndex,
+ { message: `${valueLabel}.endIndex must be >= startIndex` }
+ );📝 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.
| export const makeReferrerLeaderboardPageContextSchema = ( | |
| valueLabel: string = "ReferrerLeaderboardPageContext", | |
| ) => | |
| z.object({ | |
| page: makePositiveIntegerSchema(`${valueLabel}.page`), | |
| recordsPerPage: makePositiveIntegerSchema(`${valueLabel}.recordsPerPage`).max( | |
| REFERRERS_PER_LEADERBOARD_PAGE_MAX, | |
| `${valueLabel}.recordsPerPage must not exceed ${REFERRERS_PER_LEADERBOARD_PAGE_MAX}`, | |
| ), | |
| totalRecords: makeNonNegativeIntegerSchema(`${valueLabel}.totalRecords`), | |
| totalPages: makePositiveIntegerSchema(`${valueLabel}.totalPages`), | |
| hasNext: z.boolean(), | |
| hasPrev: z.boolean(), | |
| startIndex: z.optional(makeNonNegativeIntegerSchema(`${valueLabel}.startIndex`)), | |
| endIndex: z.optional(makeNonNegativeIntegerSchema(`${valueLabel}.endIndex`)), | |
| }); | |
| export const makeReferrerLeaderboardPageContextSchema = ( | |
| valueLabel: string = "ReferrerLeaderboardPageContext", | |
| ) => | |
| z.object({ | |
| page: makePositiveIntegerSchema(`${valueLabel}.page`), | |
| recordsPerPage: makePositiveIntegerSchema(`${valueLabel}.recordsPerPage`).max( | |
| REFERRERS_PER_LEADERBOARD_PAGE_MAX, | |
| `${valueLabel}.recordsPerPage must not exceed ${REFERRERS_PER_LEADERBOARD_PAGE_MAX}`, | |
| ), | |
| totalRecords: makeNonNegativeIntegerSchema(`${valueLabel}.totalRecords`), | |
| totalPages: makePositiveIntegerSchema(`${valueLabel}.totalPages`), | |
| hasNext: z.boolean(), | |
| hasPrev: z.boolean(), | |
| startIndex: z.optional(makeNonNegativeIntegerSchema(`${valueLabel}.startIndex`)), | |
| endIndex: z.optional(makeNonNegativeIntegerSchema(`${valueLabel}.endIndex`)), | |
| }).refine( | |
| (data) => data.startIndex === undefined || data.endIndex === undefined || data.endIndex >= data.startIndex, | |
| { message: `${valueLabel}.endIndex must be >= startIndex` } | |
| ); |
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/api/zod-schemas-v1.ts` around lines 110 - 125, The
schema makeReferrerLeaderboardPageContextSchema currently allows optional
startIndex and endIndex but doesn't enforce endIndex >= startIndex; update the
returned z.object to add a refinement (using .refine or .superRefine on the
object) that, when both startIndex and endIndex are defined, asserts endIndex >=
startIndex and returns a clear error path (e.g., `${valueLabel}.endIndex` or
`${valueLabel}.startIndex`) on failure; reference the object produced by
makeReferrerLeaderboardPageContextSchema and the startIndex/endIndex fields when
implementing the check so the validator triggers only when both values are
present.
| const uniqueReferrers = allReferrers.map((referrer) => referrer.referrer); | ||
| if (uniqueReferrers.length !== allReferrers.length) { | ||
| throw new Error( | ||
| "ReferrerLeaderboard: Cannot buildReferrerLeaderboard containing duplicate referrers", | ||
| ); | ||
| } |
There was a problem hiding this comment.
Bug: Duplicate referrer detection is ineffective.
The current check creates an array of addresses and compares its length to the input array length, but this doesn't actually detect duplicates. If there are duplicate referrer addresses, uniqueReferrers will still have the same length as allReferrers because map preserves array length.
🐛 Proposed fix using Set for proper duplicate detection
export const buildReferrerLeaderboard = (
allReferrers: ReferrerMetrics[],
rules: ReferralProgramRules,
accurateAsOf: UnixTimestamp,
): ReferrerLeaderboard => {
- const uniqueReferrers = allReferrers.map((referrer) => referrer.referrer);
- if (uniqueReferrers.length !== allReferrers.length) {
+ const uniqueReferrers = new Set(allReferrers.map((referrer) => referrer.referrer));
+ if (uniqueReferrers.size !== allReferrers.length) {
throw new Error(
"ReferrerLeaderboard: Cannot buildReferrerLeaderboard containing duplicate referrers",
);
}🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/leaderboard-v1.ts` around lines 56 - 61, The
duplicate detection is wrong because uniqueReferrers is created with map which
preserves length; replace it with a Set-based check: compute const
uniqueReferrerSet = new Set(allReferrers.map(r => r.referrer)) and then compare
uniqueReferrerSet.size to allReferrers.length (or recreate uniqueReferrers via
Array.from(uniqueReferrerSet)) in the buildReferrerLeaderboard logic to properly
detect duplicates and throw the same Error when sizes differ; update references
to uniqueReferrers accordingly.
| export const validateReferralProgramRules = (rules: ReferralProgramRules): void => { | ||
| // Validate totalAwardPoolValue using Zod schema | ||
| const priceUsdcSchema = makePriceUsdcSchema("ReferralProgramRules.totalAwardPoolValue"); | ||
| const parseResult = priceUsdcSchema.safeParse(rules.totalAwardPoolValue); | ||
| if (!parseResult.success) { | ||
| throw new Error( | ||
| `ReferralProgramRules: totalAwardPoolValue validation failed: ${parseResult.error.message}`, | ||
| ); | ||
| } | ||
|
|
||
| validateNonNegativeInteger(rules.maxQualifiedReferrers); | ||
| validateUnixTimestamp(rules.startTime); | ||
| validateUnixTimestamp(rules.endTime); | ||
|
|
||
| if (rules.endTime < rules.startTime) { | ||
| throw new Error( | ||
| `ReferralProgramRules: startTime: ${rules.startTime} is after endTime: ${rules.endTime}.`, | ||
| ); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Validation does not check subregistryId.
The validateReferralProgramRules function validates totalAwardPoolValue, maxQualifiedReferrers, startTime, and endTime, but does not validate the subregistryId field. Consider adding validation for subregistryId to ensure it's a valid AccountId.
♻️ Proposed fix to add subregistryId validation
export const validateReferralProgramRules = (rules: ReferralProgramRules): void => {
// Validate totalAwardPoolValue using Zod schema
const priceUsdcSchema = makePriceUsdcSchema("ReferralProgramRules.totalAwardPoolValue");
const parseResult = priceUsdcSchema.safeParse(rules.totalAwardPoolValue);
if (!parseResult.success) {
throw new Error(
`ReferralProgramRules: totalAwardPoolValue validation failed: ${parseResult.error.message}`,
);
}
validateNonNegativeInteger(rules.maxQualifiedReferrers);
validateUnixTimestamp(rules.startTime);
validateUnixTimestamp(rules.endTime);
if (rules.endTime < rules.startTime) {
throw new Error(
`ReferralProgramRules: startTime: ${rules.startTime} is after endTime: ${rules.endTime}.`,
);
}
+
+ // Consider validating subregistryId format/structure
+ if (!rules.subregistryId || typeof rules.subregistryId !== "string") {
+ throw new Error(`ReferralProgramRules: subregistryId must be a non-empty string.`);
+ }
};🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/rules-v1.ts` around lines 67 - 86, The function
validateReferralProgramRules currently omits validation for the subregistryId
field; add a call to the existing AccountId validator (e.g., validateAccountId)
to check rules.subregistryId inside validateReferralProgramRules, placed
alongside the other field checks (after totalAwardPoolValue parsing and
before/after maxQualifiedReferrers validation), and throw a clear Error if
validation fails so the code validates that subregistryId is a valid AccountId.
| /** | ||
| * Encodes a uint256 bigint as hex string sized to 32 bytes. | ||
| * Uses include, in the context of ENS, decoding the uint256-encoded tokenId of NFT-issuing contracts | ||
| * into Node or LabelHash, which is a common behavior in the ENS ecosystem. | ||
| * (see NameWrapper, ETHRegistrarController) | ||
| * | ||
| * @remarks | ||
| * This is an inline copy of the function from `../ens/subname-helpers.ts` to avoid | ||
| * cross-module imports that cause Vite SSR module resolution issues. When Vitest loads | ||
| * TypeScript sources in a pnpm workspace, importing from ../ens creates a complex module | ||
| * graph that Vite SSR cannot properly resolve, resulting in imported functions appearing | ||
| * as undefined at runtime. | ||
| */ | ||
| const uint256ToHex32 = (num: bigint): Hex => toHex(num, { size: 32 }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify that both uint256ToHex32 implementations are consistent
echo "=== Searching for uint256ToHex32 in subname-helpers.ts ==="
rg -n 'uint256ToHex32' --glob '**/subname-helpers.ts' -A 2
echo ""
echo "=== Searching for uint256ToHex32 in serialize.ts ==="
rg -n 'uint256ToHex32' --glob '**/serialize.ts' -A 2
echo ""
echo "=== Checking if subname-helpers.ts exists and checking its exports ==="
find . -name 'subname-helpers.ts' -type f 2>/dev/null
echo ""
echo "=== Checking relative path from serialize.ts ==="
find . -path '*/shared/serialize.ts' -type f 2>/dev/nullRepository: namehash/ensnode
Length of output: 950
Documentation is thorough; code duplication requires maintenance tracking.
The inline copy of uint256ToHex32 from ../ens/subname-helpers.ts is intentional and well-documented due to Vite SSR module resolution constraints. Both implementations are currently identical. However, this duplication creates a maintenance burden: if the original function in subname-helpers.ts is modified, this private copy must be updated in parallel to maintain consistency.
Consider adding a comment or tracking mechanism (e.g., a maintenance note or linked issue) to ensure the implementations remain synchronized if changes occur to either version.
🤖 Prompt for AI Agents
In `@packages/ensnode-sdk/src/shared/serialize.ts` around lines 23 - 36, Add a
short maintenance note next to the duplicated uint256ToHex32 definition
explaining that this is an intentional inline copy of the implementation in
../ens/subname-helpers.ts due to Vite SSR resolution, and include a TODO/link or
issue number to track changes so both implementations stay synchronized;
reference the function name uint256ToHex32 and the original file
subname-helpers.ts in the comment to make future updates clear.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // We need to convert the share (a number between 0 and 1) to a bigint amount | ||
| const awardPoolApproxAmount = BigInt( | ||
| Math.floor(awardPoolShare * Number(rules.totalAwardPoolValue.amount)), | ||
| ); |
There was a problem hiding this comment.
The calculation on lines 289-290 converts the totalAwardPoolValue.amount (a bigint) to a Number before multiplying by awardPoolShare. This conversion could cause precision loss if totalAwardPoolValue.amount exceeds Number.MAX_SAFE_INTEGER (2^53 - 1, approximately 9 quadrillion). Given that USDC has 6 decimals, this would happen with amounts above 9 million USDC, which is possible for large award pools. Consider using bigint arithmetic throughout: multiply the bigint by a scaled integer representation of the share, then divide by the scaling factor.
| // We need to convert the share (a number between 0 and 1) to a bigint amount | |
| const awardPoolApproxAmount = BigInt( | |
| Math.floor(awardPoolShare * Number(rules.totalAwardPoolValue.amount)), | |
| ); | |
| // Use bigint arithmetic for the large amount to avoid precision loss. We represent the | |
| // share as a scaled integer, multiply in bigint space, then divide by the scale. | |
| const SHARE_SCALE = 1_000_000_000n; | |
| const scaledShare = BigInt(Math.floor(awardPoolShare * Number(SHARE_SCALE))); | |
| const awardPoolApproxAmount = | |
| (rules.totalAwardPoolValue.amount * scaledShare) / SHARE_SCALE; |
| export function deserializeReferrerDetailResponse( | ||
| maybeResponse: SerializedReferrerDetailResponse, | ||
| valueLabel?: string, | ||
| ): ReferrerDetailResponse { | ||
| let deserialized: ReferrerDetailResponse; | ||
| switch (maybeResponse.responseCode) { | ||
| case "ok": { | ||
| switch (maybeResponse.data.type) { | ||
| case "ranked": | ||
| deserialized = { | ||
| responseCode: maybeResponse.responseCode, | ||
| data: deserializeReferrerDetailRanked(maybeResponse.data), | ||
| } as ReferrerDetailResponse; | ||
| break; | ||
|
|
||
| case "unranked": | ||
| deserialized = { | ||
| responseCode: maybeResponse.responseCode, | ||
| data: deserializeReferrerDetailUnranked(maybeResponse.data), | ||
| } as ReferrerDetailResponse; | ||
| break; | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case "error": | ||
| deserialized = maybeResponse; | ||
| break; | ||
| } |
There was a problem hiding this comment.
The function deserializeReferrerDetailResponse has a bug where the variable deserialized may be used uninitialized. If the response.data.type doesn't match "ranked" or "unranked" (which shouldn't happen with proper typing but could with malformed input), the inner switch falls through without assigning to deserialized, and then the variable is used on line 222. This will cause a runtime error. Either add a default case to the inner switch that throws an error, or ensure TypeScript's exhaustiveness checking catches this by restructuring the code.
| * Schema for {@link ReferrerLeaderboardPageResponseError} | ||
| */ | ||
| export const makeReferrerLeaderboardPageResponseErrorSchema = ( | ||
| _valueLabel: string = "ReferrerLeaderboardPageResponseError", |
There was a problem hiding this comment.
'_valueLabel' is assigned a value but never used.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 79 out of 79 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async getReferrerLeaderboardPage( | ||
| request?: ReferrerLeaderboardPageRequest, | ||
| ): Promise<ReferrerLeaderboardPageResponse> { | ||
| const url = new URL(`/ensanalytics/v1/referrers`, this.options.url); |
There was a problem hiding this comment.
The client URL path /ensanalytics/v1/referrers is inconsistent with the server endpoint path /v1/ensanalytics configured in the ENSApi. The server routes the v1 API at /v1/ensanalytics (line 71 in apps/ensapi/src/index.ts), which means the endpoint would be /v1/ensanalytics/referrers, not /ensanalytics/v1/referrers.
The client URL should be updated to /v1/ensanalytics/referrers to match the server endpoint path.
| const url = new URL(`/ensanalytics/v1/referrers`, this.options.url); | |
| const url = new URL(`/v1/ensanalytics/referrers`, this.options.url); |
| */ | ||
| async getReferrerDetail(request: ReferrerDetailRequest): Promise<ReferrerDetailResponse> { | ||
| const url = new URL( | ||
| `/api/ensanalytics/referrers/${encodeURIComponent(request.referrer)}`, |
There was a problem hiding this comment.
The client URL path /api/ensanalytics/referrers/${referrer} appears incorrect. Based on the server endpoint configuration and the pattern in getReferrerLeaderboardPage, this should likely be /v1/ensanalytics/referrers/${referrer} to match the v1 API path structure.
| `/api/ensanalytics/referrers/${encodeURIComponent(request.referrer)}`, | |
| `/v1/ensanalytics/referrers/${encodeURIComponent(request.referrer)}`, |
| ): number { | ||
| if (!isReferrerQualified(rank, rules)) return 0; | ||
|
|
||
| return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1); |
There was a problem hiding this comment.
Division by zero causes NaN return value when maxQualifiedReferrers is 1 and rank is 1
| } | ||
|
|
||
| // Then validate the deserialized structure using zod schemas | ||
| const schema = makeReferrerDetailResponseSchema(valueLabel); |
There was a problem hiding this comment.
Uninitialized variable causes potential runtime error when deserializing responses with unexpected types
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 81 out of 81 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ): number { | ||
| if (!isReferrerQualified(rank, rules)) return 0; | ||
|
|
||
| return 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1); |
There was a problem hiding this comment.
Potential division by zero when maxQualifiedReferrers is 1. The calculation 1 - (rank - 1) / (rules.maxQualifiedReferrers - 1) will divide by zero when maxQualifiedReferrers equals 1, causing NaN results. Consider adding a check to handle this edge case, perhaps returning 1.0 for the boost when there's only one qualified referrer slot.
| const currencyInfo = getCurrencyInfo(rules.totalAwardPoolValue.currency); | ||
| const precisionScale = 10n ** BigInt(currencyInfo.decimals); | ||
| const scaledShare = BigInt(Math.floor(awardPoolShare * Number(precisionScale))); | ||
| const awardPoolApproxAmount = (rules.totalAwardPoolValue.amount * scaledShare) / precisionScale; |
There was a problem hiding this comment.
Potential precision loss in award pool calculation. Converting awardPoolShare to a Number for scaling (BigInt(Math.floor(awardPoolShare * Number(precisionScale)))) may lose precision for very small shares. Consider using bigint arithmetic throughout to maintain precision. For example, convert awardPoolShare to a scaled bigint by multiplying the numerator before division: (rules.totalAwardPoolValue.amount * referrerFinalScore * precisionScale) / grandTotalScore / precisionScale.
| const uniqueReferrers = allReferrers.map((referrer) => referrer.referrer); | ||
| if (uniqueReferrers.length !== allReferrers.length) { |
There was a problem hiding this comment.
Inefficient duplicate detection using array length comparison. The check uniqueReferrers.length !== allReferrers.length only validates that there are duplicates, but doesn't identify which referrer is duplicated. Consider using a Set for efficient duplicate detection: const uniqueReferrers = new Set(allReferrers.map(r => r.referrer)); if (uniqueReferrers.size !== allReferrers.length) to make the intent clearer and improve performance.
| const uniqueReferrers = allReferrers.map((referrer) => referrer.referrer); | |
| if (uniqueReferrers.length !== allReferrers.length) { | |
| const uniqueReferrers = new Set(allReferrers.map((referrer) => referrer.referrer)); | |
| if (uniqueReferrers.size !== allReferrers.length) { |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/ens-referrals/src/v1/client.ts`:
- Around line 149-151: responseData is being unsafely cast to
SerializedReferrerLeaderboardPageResponse before deserialization; change the
call to pass the raw responseData (typed as unknown) into
deserializeReferrerLeaderboardPageResponse and update that function to accept
unknown, perform runtime validation of required fields/types, and throw or
return a clear error when the schema is invalid so unexpected server responses
are surfaced instead of masked by a cast; reference the variable responseData
and the function deserializeReferrerLeaderboardPageResponse for the change.
| return deserializeReferrerLeaderboardPageResponse( | ||
| responseData as SerializedReferrerLeaderboardPageResponse, | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Unsafe type assertion on unvalidated JSON.
Casting responseData to SerializedReferrerLeaderboardPageResponse without validation could mask deserialization errors if the server returns an unexpected schema. Consider adding runtime validation or letting the deserializer handle unknown input with proper error messages.
♻️ Suggested approach
- return deserializeReferrerLeaderboardPageResponse(
- responseData as SerializedReferrerLeaderboardPageResponse,
- );
+ // Let deserializer validate structure and provide meaningful errors
+ return deserializeReferrerLeaderboardPageResponse(responseData);This requires updating the deserializer to accept unknown and perform validation, which provides better error messages when the API schema changes unexpectedly.
🤖 Prompt for AI Agents
In `@packages/ens-referrals/src/v1/client.ts` around lines 149 - 151, responseData
is being unsafely cast to SerializedReferrerLeaderboardPageResponse before
deserialization; change the call to pass the raw responseData (typed as unknown)
into deserializeReferrerLeaderboardPageResponse and update that function to
accept unknown, perform runtime validation of required fields/types, and throw
or return a clear error when the schema is invalid so unexpected server
responses are surfaced instead of masked by a cast; reference the variable
responseData and the function deserializeReferrerLeaderboardPageResponse for the
change.
Greptile OverviewGreptile SummaryThis PR migrates ENS Referrals API to use mature price data models ( Key Changes:
Award Pool Calculation: Risk Assessment: Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as ENSReferralsClient (v1)
participant API as ENSApi (/v1/ensanalytics)
participant MW as referrerLeaderboardMiddlewareV1
participant Cache as referrerLeaderboardCacheV1
participant DB as Database (getReferrerMetrics)
participant Pkg as ens-referrals/v1
Note over Client,Pkg: V1 API Request Flow with PriceEth/PriceUsdc
Client->>API: GET /v1/ensanalytics/referrers
API->>MW: middleware processes request
MW->>Cache: read() leaderboard data
alt Cache Hit
Cache-->>MW: Return cached ReferrerLeaderboard
else Cache Miss or Stale
Cache->>DB: Query referrer metrics
DB-->>Cache: Raw metrics (RevenueContribution as bigint)
Cache->>Pkg: buildReferrerMetrics (creates PriceEth)
Pkg->>Pkg: buildReferrerLeaderboard
Pkg->>Pkg: buildAwardedReferrerMetrics (calculates PriceUsdc)
Note over Pkg: Calculate awardPoolApproxAmount:<br/>scaledShare = floor(awardPoolShare * 10^decimals)<br/>amount = (totalAwardPool * scaledShare) / 10^decimals
Pkg-->>Cache: ReferrerLeaderboard with PriceEth/PriceUsdc
Cache-->>MW: Return ReferrerLeaderboard
end
MW->>API: Set c.var.referrerLeaderboardV1
API->>Pkg: getReferrerLeaderboardPage(request, leaderboard)
Pkg->>Pkg: Paginate and filter metrics
Pkg-->>API: ReferrerLeaderboardPage
API->>Pkg: serializeReferrerLeaderboardPageResponse
Note over Pkg: Serialize PriceEth/PriceUsdc:<br/>Convert bigint amounts to string<br/>Keep currency metadata
Pkg-->>API: SerializedReferrerLeaderboardPageResponse
API-->>Client: JSON response with stringified bigints
Client->>Client: deserializeReferrerLeaderboardPageResponse
Note over Client: Deserialize:<br/>Parse string amounts back to bigint<br/>Validate with Zod schemas
Client-->>Client: Typed ReferrerLeaderboardPageResponse
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
ENS Referrals API v1: Price Types Migration
closes: #1521
Reviewer Focus (Read This First)
What reviewers should focus on
packages/ens-referrals/src/v1/*/v1subpath exportawardPoolApproxAmountError I got during testing:
Location:
apps/ensapi/src/handlers/ensanalytics-api-v1.test.tsError: Tests fail with:
{ "responseCode": "error", "error": "Internal server error", "errorMessage": "(0 , __vite_ssr_import_0__.serializePriceUsdc) is not a function" }In my various experiments, I found it to be also failing with
serializePriceEth, which was implemented before this PR. Tried untangling imports inensnode-sdk/ensandensnode-sdk/sharedto make this dependence a single-direction one, but the problem persisted. Importing this directly from ENSApi seemed to work, and importing it from thedistversion worked as well. The current workaround is to export it frominternaland use it for importing inens-referrals, which is not what I intended, but from the fixes I found seemed to be the most benign.Fixed through: #1572
Problem & Motivation
Why this exists
ens-referralspackage was first implemented, we took a few temporary shortcuts to ensure ENS Holiday Awards could be shipped before Dec 1. These temporary shortcuts included quick fixes for theUSDQuantityandRevenueContributionprice data models currently defined inens-referrals.PriceUsdcandPriceEthare defined inensnode-sdk, however we were previously blocked on using these more mature data models because of issue 1519 and our need to ensure ENS Holiday Awards was shipped on schedule.What Changed (Concrete)
What actually changed
ens-referralstov1directoryv1andv1/internalUSDQuantityandRevenueContributiontypes to use mature price types fromensnode-sdkV1middleware to ENSApi to properly handle new types that appear in Referrer Leaderboardensnode-sdk/sharedto be explicit to avoid Vitest issues/ensanalytics/v1to/v1/ensanalyticsDesign & Planning
How this approach was chosen
v1/versionSelf-Review
What you caught yourself
bigintapi. It was a bug introduced previously inensnode-sdkand migrated toens-referrals. Fixed as of nowCross-Codebase Alignment
Related code you checked
PriceEthand its serialization implementation for consistencyPriceUsdc,PriceEth,awardPoolApproxValue,totalAwardPoolValue,RevenueContribution,USDQuantityUSDQuantityandRevenueContribution)maxQualifiedReferrersis 1, using set foruniqueReferrers)Downstream & Consumer Impact
Who this affects and how
v1path for importsTesting Evidence
How this was validated
v1for importing, Vite issues, proper currency calculationsScope Reductions
What you intentionally didn't do
No changes to v0 implementation.
Risk Analysis
How this could go wrong
Low risk - v0 API completely unchanged, v1 is additive only.
Pre-Review Checklist (Blocking)