Conversation
🦋 Changeset detectedLatest commit: 9cf0a7c The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 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 |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR replaces per-edition leaderboard caching with snapshot-based caching and adds per-event accounting trace support for rev-share-cap editions, including CSV export endpoint, enriched referral events, updated builders that return snapshots (leaderboard + accountingRecords), middleware/typing updates, and tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Handler as API Handler
participant Cache as Snapshot Cache
participant DB as Database
participant Builder as AwardModel Builder
participant Formatter as CSV Formatter
Client->>Handler: GET /v1/ensanalytics/accounting?edition=slug
Handler->>Handler: ensure prerequisites supported
Handler->>Cache: cache.read(edition)
alt Cache Hit
Cache-->>Handler: ReferralEditionSnapshot
else Cache Miss
Cache->>DB: getReferralEvents(edition)
DB-->>DB: enrich events with name, actionType,<br/>transactionHash, registrant
DB-->>Builder: ReferralEvent[]
Builder->>Builder: compute leaderboard & per-event accounting
Builder-->>Cache: ReferralEditionSnapshot (leaderboard + accountingRecords)
Cache-->>Handler: ReferralEditionSnapshot
end
Handler->>Handler: assert awardModel === RevShareCap
Handler->>Formatter: formatAccountingCsv(accountingRecords)
Formatter-->>Handler: CSV string
Handler-->>Client: 200 text/csv (CSV payload)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 57 minutes and 39 seconds.Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the ENS referrals “rev-share-cap” pipeline to produce a per-event award accounting trace, exposes that trace via a new ENSApi CSV endpoint, and refactors leaderboard builders to return an edition “snapshot” object.
Changes:
- Introduce
ReferralEditionSnapshotbuilders (buildReferralEditionSnapshot*) bundling leaderboards with additional computed state (rev-share-cap includes per-event accounting records). - Add rev-share-cap accounting types + tests, and extend
ReferralEventto carry audit fields needed for reporting/export. - Add
GET /v1/ensanalytics/accounting?edition={slug}returning a CSV dump of per-event accounting for rev-share-cap editions.
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ens-referrals/src/leaderboard.ts | Exposes new ReferralEditionSnapshot union alongside leaderboards. |
| packages/ens-referrals/src/index.ts | Re-exports rev-share-cap accounting types. |
| packages/ens-referrals/src/award-models/rev-share-cap/referral-event.ts | Extends ReferralEvent with name/action/txHash/registrant for audit/export. |
| packages/ens-referrals/src/award-models/rev-share-cap/leaderboard.ts | Refactors rev-share-cap builder to return a snapshot + emits per-event accounting records. |
| packages/ens-referrals/src/award-models/rev-share-cap/leaderboard.test.ts | Updates tests for the new snapshot-returning builder. |
| packages/ens-referrals/src/award-models/rev-share-cap/accounting.ts | Adds per-event accounting record + award trace interfaces. |
| packages/ens-referrals/src/award-models/rev-share-cap/accounting.test.ts | Adds targeted tests for per-event accounting trace behavior. |
| packages/ens-referrals/src/award-models/pie-split/leaderboard.ts | Refactors pie-split builder to return a snapshot wrapper. |
| apps/ensapi/src/middleware/referral-edition-snapshots-caches.middleware.ts | Renames/rewires middleware to provide snapshot caches instead of leaderboards. |
| apps/ensapi/src/lib/hono-factory.ts | Updates middleware variable typing to new snapshot-caches middleware. |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/get-referral-edition-snapshot.ts | Replaces leaderboard builder dispatcher with snapshot builder dispatcher. |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/get-referral-edition-snapshot.test.ts | Updates tests for snapshot return shape. |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/format-accounting-csv.ts | Adds CSV formatter for rev-share-cap accounting records. |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.ts | Extends rev-share-cap event query to fetch audit fields + domain name. |
| apps/ensapi/src/index.ts | Updates graceful shutdown to destroy snapshot caches. |
| apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.ts | Uses snapshot caches for existing endpoints and adds /accounting CSV endpoint. |
| apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.test.ts | Updates cache mocks for snapshot shape + adds basic accounting endpoint tests. |
| apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.routes.ts | Adds OpenAPI route definition for /accounting CSV endpoint. |
| apps/ensapi/src/cache/referral-edition-snapshots.cache.ts | Refactors cache to store snapshots (and use new builder). |
| .changeset/shiny-pandas-account.md | Changeset for ensapi: adds accounting CSV endpoint. |
| .changeset/purple-frogs-dream.md | Changeset for ens-referrals: adds snapshot + per-event accounting trace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/ensapi/src/index.ts (1)
61-61:⚠️ Potential issue | 🟡 MinorStale log label after rename.
The log message still references the pre-rename
referralLeaderboardEditionsCache. For consistency with the rest of the PR-wide migration to snapshot-based caching (e.g., the import on Line 6 and the variableeditionsCaches), update the label to reflect the new name.📝 Suggested fix
- logger.info(`Destroyed referralLeaderboardEditionsCache for ${editionSlug}`); + logger.info(`Destroyed referralEditionSnapshotsCache for ${editionSlug}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/index.ts` at line 61, The log message uses an outdated label "referralLeaderboardEditionsCache"; update the logger.info call that references editionSlug to use the new snapshot-based name (e.g., "editionsCaches" or "editionSnapshots" consistent with the import and variable `editionsCaches`) so the message matches the PR-wide rename—find the logger.info(`Destroyed referralLeaderboardEditionsCache for ${editionSlug}`) and change the text to reflect the new cache name.apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.ts (1)
134-159:⚠️ Potential issue | 🟠 MajorReplace
innerJoinwithleftJointo detect missing data explicitly instead of silently dropping referral events.The query chains
registrarActions → registrationLifecycles (on node) → subgraph_domain, both viainnerJoin. This means anyregistrarActionrow without a correspondingregistrationLifecyclesorsubgraph_domain.namewill vanish from results without surfacing an error. The runtime checks in the map function (lines 164–172) only validate rows the query returns; they cannot catch rows silently dropped by the joins.While the schema documentation states registrationLifecycles are guaranteed to exist and
subgraph_domain.namewill "in practice never be null," these guarantees are not enforced at the schema level. If an indexer bug or data gap violates these assumptions, the leaderboard will silently omit referral events rather than fail loudly.Consider using
leftJoinon both tables and adding explicit error throws if the joins fail to produce expected rows. This ensures data integrity is validated explicitly and any gaps surface clearly rather than causing silent revenue loss.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.ts` around lines 134 - 159, Change the two innerJoin calls on ensIndexerSchema.registrationLifecycles and ensIndexerSchema.subgraph_domain to leftJoin so registrarActions rows are not silently dropped, then after executing the query (the mapping logic that currently validates rows returned by the query) add explicit null checks for registrationLifecycles and subgraph_domain (e.g., verify the joined lifecycle and domain.name exist for each row) and throw a descriptive error when either is missing so missing joins surface loudly; adjust any downstream logic that assumed non-null values accordingly (references: the query built from ensIndexerSchema.registrarActions, the joins to ensIndexerSchema.registrationLifecycles and ensIndexerSchema.subgraph_domain, and the post-query mapping/validation code that checks decodedReferrer).apps/ensapi/src/cache/referral-edition-snapshots.cache.ts (1)
41-178: 🧹 Nitpick | 🔵 TrivialStale "leaderboard" wording in log messages and JSDoc after the snapshot rename.
The cache now produces and caches
ReferralEditionSnapshot(which contains bothleaderboardandaccountingRecords), but the human-readable strings throughout this file still say "leaderboard". This hurts grep-ability/observability (the logger category isreferral-edition-snapshots-cachebut the messages talk about leaderboards) and the JSDoc no longer accurately describes the cache contents.Affected spots: lines 41–44 (JSDoc), 57 (
@returns), 87, 90, 97, 107, 112, 125, 168, and 177.♻️ Suggested wording updates
/** * The list of {`@link` OmnichainIndexingStatusId} values that are supported for generating - * referrer leaderboards. + * referral edition snapshots. * - * Other values indicate that we are not ready to generate leaderboards yet. + * Other values indicate that we are not ready to generate edition snapshots yet. */ @@ - * `@returns` A function that builds the leaderboard for the given edition + * `@returns` A function that builds the {`@link` ReferralEditionSnapshot} for the given edition @@ - logger.error( - { error: indexingStatus, editionSlug }, - `Failed to read indexing status cache while generating referral leaderboard for ${editionSlug}. Cannot proceed without valid indexing status.`, - ); - throw new Error( - `Unable to generate referral leaderboard for ${editionSlug}. indexingStatusCache must have been successfully initialized.`, - ); + logger.error( + { error: indexingStatus, editionSlug }, + `Failed to read indexing status cache while generating referral edition snapshot for ${editionSlug}. Cannot proceed without valid indexing status.`, + ); + throw new Error( + `Unable to generate referral edition snapshot for ${editionSlug}. indexingStatusCache must have been successfully initialized.`, + ); @@ - `Unable to generate referrer leaderboard for ${editionSlug}. Omnichain indexing status is currently ${omnichainIndexingStatus} but must be ${supportedOmnichainIndexingStatuses.join(" or ")}.`, + `Unable to generate referral edition snapshot for ${editionSlug}. Omnichain indexing status is currently ${omnichainIndexingStatus} but must be ${supportedOmnichainIndexingStatuses.join(" or ")}.`, @@ - `Unable to generate referrer leaderboard for ${editionSlug}. Latest indexed block ref for chain ${editionConfig.rules.subregistryId.chainId} is null.`, + `Unable to generate referral edition snapshot for ${editionSlug}. Latest indexed block ref for chain ${editionConfig.rules.subregistryId.chainId} is null.`, @@ - `Building referrer leaderboard for ${editionSlug} with rules:\n${JSON.stringify( + `Building referral edition snapshot for ${editionSlug} with rules:\n${JSON.stringify( @@ - `Successfully built referrer leaderboard for ${editionSlug} with ${snapshot.leaderboard.referrers.size} referrers`, + `Successfully built referral edition snapshot for ${editionSlug} with ${snapshot.leaderboard.referrers.size} referrers and ${snapshot.accountingRecords.length} accounting records`, @@ - logger.info(`Initialized leaderboard cache for ${editionSlug}`); + logger.info(`Initialized referral edition snapshot cache for ${editionSlug}`); @@ /** - * Gets the cached instance of referral leaderboard editions caches. + * Gets the cached instance of referral edition snapshots caches. * Returns null if not yet initialized.As per coding guidelines: "Do not add JSDoc
@returnstags that merely restate the method summary; remove such redundancy during PR review." If the updated@returnsoncreateEditionSnapshotBuilderends up being just a restatement of the summary, drop it instead of rewording.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/cache/referral-edition-snapshots.cache.ts` around lines 41 - 178, Update wording to refer to "ReferralEditionSnapshot" / "edition snapshot" instead of "leaderboard" in the JSDoc and all log messages: adjust the JSDoc for createEditionSnapshotBuilder to describe that the cache holds ReferralEditionSnapshot (remove the redundant `@returns` if it only repeats the summary), change logger.debug, logger.info and error strings inside createEditionSnapshotBuilder to say "snapshot" or "edition snapshot" (e.g., the debug that currently says "Edition is immutably closed, returning cached data without re-fetching", the info that logs the rules, and the info after building that references snapshot.leaderboard.referrers.size should say snapshot/edition snapshot and can still reference snapshot.leaderboard.referrers.size in code), and change the initializeReferralEditionSnapshotsCaches init log ("Initialized leaderboard cache for") to "Initialized edition snapshot cache for" (or similar); ensure the references to functions/classes remain the same: createEditionSnapshotBuilder, initializeReferralEditionSnapshotsCaches, cachedInstance, and getReferralEditionSnapshot so the replacements are localized to message/JSDoc strings only.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.test.ts`:
- Around line 987-997: The helper mockSnapshotCache returns an object cast to
ReferralEditionSnapshot but omits required fields like leaderboard and
accountingRecords (required by ReferralEditionSnapshotPieSplit and
ReferralEditionSnapshotRevShareCap); update mockSnapshotCache to either (a)
document this limitation with a brief comment stating it intentionally produces
minimal snapshots only valid for early-exit awardModel checks, or (b) extend its
signature to accept optional partial overrides for leaderboard and
accountingRecords and merge them into the returned object so the mock can
produce structurally valid ReferralEditionSnapshot instances when tests exercise
success paths; reference mockSnapshotCache, ReferralEditionSnapshot,
ReferralEditionSnapshotPieSplit, ReferralEditionSnapshotRevShareCap, and
SWRCache when making the change.
- Around line 986-1034: Add tests to cover the /accounting success and
cache-error 503 paths: reuse mockSnapshotCache and setupAccountingMocks to
inject a cache map via
editionsCachesMiddleware.referralEditionSnapshotsCachesMiddleware; add a test
where mockSnapshotCache returns awardModel
ReferralProgramAwardModels.RevShareCap and the mocked
ReferralEditionSnapshot.read() resolves to an object with accountingRecords
(assert response status 200, Content-Type "text/csv; charset=utf-8",
Content-Disposition includes filename "accounting-${edition}.csv", and body
contains CSV header row plus one sample record row produced by
formatAccountingCsv); add another test where setupAccountingMocks injects a map
or cache whose value is an Error or whose read() resolves to an Error and assert
the endpoint returns 503. Ensure you reference the
app.request("/accounting?edition=...") call and the
ReferralEditionSnapshot.read() behavior when stubbing responses.
In `@apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.ts`:
- Around line 338-383: Add response caching headers, operational logging, and
avoid building the entire CSV in memory: after the rev-share check and before
returning the body in getAccountingCsvRoute, log the edition and record count
via logger.info({ edition, recordCount: cached.accountingRecords.length }), and
set a long-lived Cache-Control (e.g. "public, max-age=31536000, immutable") when
the snapshot is known immutable (use the cached snapshot metadata/flag available
on the cached object), by calling c.header("Cache-Control", "..."); finally
replace the immediate c.body(formatAccountingCsv(cached.accountingRecords), 200)
with a streaming response that converts cached.accountingRecords into CSV rows
(e.g. a ReadableStream or Node stream) so you don’t build the whole CSV in
memory — reuse formatAccountingCsv for row formatting or implement an async
iterator that writes rows to the stream and call c.body(stream, 200); keep
existing error handling and headers (Content-Type/Content-Disposition) intact.
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.ts`:
- Around line 174-184: The exhaustiveness check using "const _exhaustive: never
= record.actionType" is unsafe if the DB-typed column widens; instead perform a
runtime validation against the known enum values by checking
Object.values(RegistrarActionTypes).includes(record.actionType as
RegistrarActionType) in the switch/default path (or before the switch) and, if
it returns false, throw the same Error (the one referencing getReferralEvents
and record.id); remove the _exhaustive assignment so the guard is a pure runtime
check independent of static narrowing.
In
`@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/format-accounting-csv.ts`:
- Around line 74-82: The CSV produced by formatAccountingCsv can contain
non-ASCII ENS names and should include a UTF-8 BOM to avoid Excel mojibake;
update formatAccountingCsv so it prepends "\uFEFF" before the header row (i.e.,
before CSV_COLUMNS.map((c) => c.header).join(",")) while leaving the rest of the
join logic intact and still returning the final string with "\r\n" line endings;
reference CSV_COLUMNS and formatAccountingCsv to locate where to add the BOM.
- Around line 4-13: The CSV-escaping helper csvCell should be applied to every
exported cell to avoid future CSV injection/corruption; update the CSV writer so
that all output values (not just name and disqualificationReason) are passed
through csvCell — e.g., wrap actionType, hex/hash/address columns,
integer/string counts, and any other columns the formatter emits with csvCell
before joining/printing — leaving csvCell implementation unchanged and ensuring
the join logic still produces the same column order.
In `@packages/ens-referrals/src/award-models/rev-share-cap/accounting.test.ts`:
- Around line 264-287: Update the test description and inline comment to remove
references to a "wrapper" or "with-accounting function" and reflect that the
test now directly verifies the leaderboard returned by
buildReferralEditionSnapshotRevShareCap; rename the it(...) string to something
like "leaderboard in snapshot matches expected spot-checks" and delete or
replace the misleading comment "// Verify that the wrapper and the
with-accounting function produce the same leaderboard." so the test (using
leaderboard and rules variables and function
buildReferralEditionSnapshotRevShareCap) clearly documents it's spot-checking
fields of the returned leaderboard.
In `@packages/ens-referrals/src/award-models/rev-share-cap/leaderboard.ts`:
- Around line 80-89: The new interface ReferralEditionSnapshotRevShareCap lacks
per-field JSDoc consistent with its sibling ReferrerLeaderboardRevShareCap;
update ReferralEditionSnapshotRevShareCap to add JSDoc on each field: document
awardModel (assert it equals leaderboard.awardModel and rules.awardModel),
document leaderboard (type ReferrerLeaderboardRevShareCap and that its
awardModel must match), and document accountingRecords (assert its length equals
the number of processed onchain events and that entries are in chronological
onchain order), mirroring the detailed `@invariant-style` comments used for
ReferrerLeaderboardRevShareCap so the file’s comment style stays consistent.
In `@packages/ens-referrals/src/leaderboard.ts`:
- Around line 17-22: Add a short doc-comment to ReferralEditionSnapshot
mirroring ReferrerLeaderboard that notes the union is discriminated by the
awardModel field; update the comment above the ReferralEditionSnapshot type to
mention "Discriminated by awardModel" (or similar) so consumers know to switch
on awardModel when handling ReferralEditionSnapshot variants like
ReferralEditionSnapshotPieSplit and ReferralEditionSnapshotRevShareCap.
---
Outside diff comments:
In `@apps/ensapi/src/cache/referral-edition-snapshots.cache.ts`:
- Around line 41-178: Update wording to refer to "ReferralEditionSnapshot" /
"edition snapshot" instead of "leaderboard" in the JSDoc and all log messages:
adjust the JSDoc for createEditionSnapshotBuilder to describe that the cache
holds ReferralEditionSnapshot (remove the redundant `@returns` if it only repeats
the summary), change logger.debug, logger.info and error strings inside
createEditionSnapshotBuilder to say "snapshot" or "edition snapshot" (e.g., the
debug that currently says "Edition is immutably closed, returning cached data
without re-fetching", the info that logs the rules, and the info after building
that references snapshot.leaderboard.referrers.size should say snapshot/edition
snapshot and can still reference snapshot.leaderboard.referrers.size in code),
and change the initializeReferralEditionSnapshotsCaches init log ("Initialized
leaderboard cache for") to "Initialized edition snapshot cache for" (or
similar); ensure the references to functions/classes remain the same:
createEditionSnapshotBuilder, initializeReferralEditionSnapshotsCaches,
cachedInstance, and getReferralEditionSnapshot so the replacements are localized
to message/JSDoc strings only.
In `@apps/ensapi/src/index.ts`:
- Line 61: The log message uses an outdated label
"referralLeaderboardEditionsCache"; update the logger.info call that references
editionSlug to use the new snapshot-based name (e.g., "editionsCaches" or
"editionSnapshots" consistent with the import and variable `editionsCaches`) so
the message matches the PR-wide rename—find the logger.info(`Destroyed
referralLeaderboardEditionsCache for ${editionSlug}`) and change the text to
reflect the new cache name.
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.ts`:
- Around line 134-159: Change the two innerJoin calls on
ensIndexerSchema.registrationLifecycles and ensIndexerSchema.subgraph_domain to
leftJoin so registrarActions rows are not silently dropped, then after executing
the query (the mapping logic that currently validates rows returned by the
query) add explicit null checks for registrationLifecycles and subgraph_domain
(e.g., verify the joined lifecycle and domain.name exist for each row) and throw
a descriptive error when either is missing so missing joins surface loudly;
adjust any downstream logic that assumed non-null values accordingly
(references: the query built from ensIndexerSchema.registrarActions, the joins
to ensIndexerSchema.registrationLifecycles and ensIndexerSchema.subgraph_domain,
and the post-query mapping/validation code that checks decodedReferrer).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ab1fd3ce-fb34-46b8-ab77-4d0356b0aeb9
📒 Files selected for processing (21)
.changeset/purple-frogs-dream.md.changeset/shiny-pandas-account.mdapps/ensapi/src/cache/referral-edition-snapshots.cache.tsapps/ensapi/src/handlers/ensanalytics/ensanalytics-api.routes.tsapps/ensapi/src/handlers/ensanalytics/ensanalytics-api.test.tsapps/ensapi/src/handlers/ensanalytics/ensanalytics-api.tsapps/ensapi/src/index.tsapps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.tsapps/ensapi/src/lib/ensanalytics/referrer-leaderboard/format-accounting-csv.tsapps/ensapi/src/lib/ensanalytics/referrer-leaderboard/get-referral-edition-snapshot.test.tsapps/ensapi/src/lib/ensanalytics/referrer-leaderboard/get-referral-edition-snapshot.tsapps/ensapi/src/lib/hono-factory.tsapps/ensapi/src/middleware/referral-edition-snapshots-caches.middleware.tspackages/ens-referrals/src/award-models/pie-split/leaderboard.tspackages/ens-referrals/src/award-models/rev-share-cap/accounting.test.tspackages/ens-referrals/src/award-models/rev-share-cap/accounting.tspackages/ens-referrals/src/award-models/rev-share-cap/leaderboard.test.tspackages/ens-referrals/src/award-models/rev-share-cap/leaderboard.tspackages/ens-referrals/src/award-models/rev-share-cap/referral-event.tspackages/ens-referrals/src/index.tspackages/ens-referrals/src/leaderboard.ts
Comment on lines
+74
to
+82
| export function formatAccountingCsv( | ||
| records: ReadonlyArray<ReferralAccountingRecordRevShareCap>, | ||
| ): string { | ||
| const lines = [ | ||
| CSV_COLUMNS.map((c) => c.header).join(","), | ||
| ...records.map((r) => CSV_COLUMNS.map((c) => c.value(r)).join(",")), | ||
| ]; | ||
| return `${lines.join("\r\n")}\r\n`; | ||
| } |
Contributor
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider a UTF-8 BOM for Excel compatibility.
Names returned by ENS (r.name is InterpretedName) can contain non-ASCII characters (emoji, Cyrillic, etc.). When users open a UTF-8 CSV directly in Excel without an explicit import step, Excel will mis-decode bytes and show mojibake. Prepending a UTF-8 BOM ("\uFEFF") before the header row is the standard mitigation, with no impact on RFC-4180-compliant parsers. Non-blocking.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/format-accounting-csv.ts`
around lines 74 - 82, The CSV produced by formatAccountingCsv can contain
non-ASCII ENS names and should include a UTF-8 BOM to avoid Excel mojibake;
update formatAccountingCsv so it prepends "\uFEFF" before the header row (i.e.,
before CSV_COLUMNS.map((c) => c.header).join(",")) while leaving the rest of the
join logic intact and still returning the final string with "\r\n" line endings;
reference CSV_COLUMNS and formatAccountingCsv to locate where to add the BOM.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Contributor
Greptile SummaryThis PR introduces a new Previously flagged P1 issues are addressed: INNER JOINs are replaced by LEFT JOINs with explicit post-query null checks that throw with the specific Confidence Score: 4/5Safe to merge; both previously flagged P1s (INNER JOIN silent truncation, unchecked null casts) are resolved and the new accounting pipeline is well-tested. No new P0 or P1 issues found. The two outside-diff P1 comments from the prior review round are addressed: JOINs are now LEFT with explicit null-throw guards, and transactionHash/registrant receive the same null checks as referrer. The remaining P2 items are non-blocking style concerns. Score held at 4 rather than 5 given the complexity of the sequential race accounting algorithm and the financial-correctness implications of the new CSV export. packages/ens-referrals/src/award-models/rev-share-cap/leaderboard.ts (race algorithm + catch-up award math) and apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.ts (LEFT JOIN null-throw guards) Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant EnsApi
participant IndexingStatusMiddleware
participant EnsanalyticsMiddleware
participant SnapshotCachesMiddleware
participant SWRCache
participant DB
Client->>EnsApi: GET /v1/ensanalytics/accounting?edition=slug
EnsApi->>IndexingStatusMiddleware: sets indexingStatus
EnsApi->>EnsanalyticsMiddleware: checks plugin config + indexing status
alt prerequisites not met
EnsanalyticsMiddleware-->>EnsApi: ensAnalyticsPrerequisites {supported:false}
EnsApi-->>Client: 503 text/plain
end
EnsApi->>SnapshotCachesMiddleware: resolves SWRCache map
EnsApi->>SWRCache: cache.read()
alt cache cold or stale
SWRCache->>DB: LEFT JOIN registrarActions + registrationLifecycles + subgraph_domain
DB-->>SWRCache: ReferralEvent[]
SWRCache->>SWRCache: buildReferralEditionSnapshotRevShareCap()
SWRCache-->>EnsApi: ReferralEditionSnapshot {leaderboard, accountingRecords}
else cache warm
SWRCache-->>EnsApi: cached ReferralEditionSnapshot
end
alt cached instanceof Error
EnsApi-->>Client: 503 text/plain
else awardModel != RevShareCap
EnsApi-->>Client: 400 text/plain
end
EnsApi->>EnsApi: formatAccountingCsv(accountingRecords)
EnsApi-->>Client: 200 text/csv (RFC-4180 CRLF)
Reviews (6): Last reviewed commit: "Merge remote-tracking branch 'origin' in..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.test.ts (1)
1124-1137:⚠️ Potential issue | 🟡 MinorMake the rev-share-cap happy-path cache mock a real snapshot.
mockRevShareCapAccountingCache()casts{ awardModel, accountingRecords }toReferralEditionSnapshot, but the rev-share-cap variant also requiresleaderboard. The test currently passes only because/accountingnever reads that field, so this helper can hide snapshot-shape regressions. Prefer returning a minimal validleaderboardinstead of relying on the cast.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.test.ts` around lines 1124 - 1137, The mockRevShareCapAccountingCache helper returns an object cast to ReferralEditionSnapshot but omits the required leaderboard field for the RevShareCap variant; update mockRevShareCapAccountingCache to return a real minimal snapshot including awardModel: ReferralProgramAwardModels.RevShareCap, accountingRecords, and a valid minimal leaderboard (e.g., an empty or default leaderboard structure that satisfies ReferralEditionSnapshot) so the returned SWRCache<ReferralEditionSnapshot> matches the real shape and prevents hidden regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensapi/src/cache/referral-edition-snapshots.cache.ts`:
- Around line 40-42: Remove the redundant JSDoc `@returns` tags that simply
restate the summary: edit the JSDoc for the function that accepts editionConfig
(the factory that "builds the edition snapshot for the given edition") and
delete its `@returns` entry, and also remove the duplicate `@returns` block at the
later JSDoc (the one around lines 167-172) so only concise summary and param
descriptions remain; keep any meaningful `@returns` only if it adds new
information beyond the summary.
In `@apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.ts`:
- Around line 37-43: The ENSAnalytics sub-app is using the global errorResponse
hook via createApp which serializes validation errors into the generic {message,
details} shape; change the createApp invocation in ensanalytics-api.ts to not
install the shared errorResponse hook and instead register a
ENSAnalytics-specific validation/error hook (e.g., ensAnalyticsErrorResponse or
ensAnalyticsValidationHook) that returns the route-specific envelope with
responseCode for endpoints like /referral-leaderboard, /referrer/:referrer,
/editions and /accounting; update the middleware/list of hooks passed to
createApp (and remove or override errorResponse) so validation errors are
transformed into the responseCode-keyed shape expected by ENSReferralsClient.
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.ts`:
- Around line 135-142: The two innerJoin calls joining
ensIndexerSchema.registrationLifecycles and ensIndexerSchema.subgraph_domain in
the query used by getReferralEvents() cause registrar actions without matching
enrichment rows to be dropped; change those innerJoin(...) calls to
leftJoin(...) (the joins on ensIndexerSchema.registrarActions.node →
registrationLifecycles.node and registrationLifecycles.node →
subgraph_domain.id) and add explicit null-handling/fallbacks in
getReferralEvents() (and any downstream use in
buildReferralEditionSnapshotRevShareCap) so missing
registrationLifecycles/subgraph_domain rows don't silently exclude events—either
coalesce missing fields to defaults or gate snapshot construction until
enrichment exists.
In
`@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/format-accounting-csv.ts`:
- Around line 24-25: Rename the CSV header from "referralId" to
"registrarActionId" so the exported column matches the value accessor
r.registrarActionId; update the header entry in format-accounting-csv (the array
that contains { header: "referralId", value: (r) => r.registrarActionId }) to {
header: "registrarActionId", value: (r) => r.registrarActionId } so consumers
can correlate rows back to the
ReferralAccountingRecordRevShareCap/registrarActionId field.
- Around line 7-11: csvCell currently only handles RFC-4180 quoting but doesn't
neutralize spreadsheet formulas; ensure values that start with "=", "+", "-", or
"@" are prefixed to prevent formula injection before applying the existing CSV
quoting. Update csvCell(value: string) to check value[0] for those characters
and, if present, prepend a safe prefix (e.g., a single quote) to the value, then
perform the current quote/escape logic; make sure callers that pass
user-controlled fields (notably name and disqualificationReason) use this
hardened csvCell so exported CSVs can't trigger spreadsheet formula execution.
In `@apps/ensapi/src/middleware/ensanalytics.middleware.ts`:
- Around line 54-75: The middleware currently sets ensAnalyticsPrerequisites
then calls await next(), allowing downstream expensive middlewares (like
referralProgramEditionConfigSetMiddleware and
referralEditionSnapshotsCachesMiddleware) to run even when prerequisites fail;
change the three unsupported branches (where hasEnsAnalyticsConfigSupport,
c.var.indexingStatus instanceof Error, and hasEnsAnalyticsIndexingStatusSupport
are checked) to short-circuit by returning a route-aware 503 response (or
otherwise halting the middleware chain) instead of calling await next(), or
alternatively move those expensive downstream middlewares behind a handler-level
check that only runs when ensAnalyticsPrerequisites.supported is true; reference
the functions hasEnsAnalyticsConfigSupport, hasEnsAnalyticsIndexingStatusSupport
and the ensAnalyticsPrerequisites context key to locate the changes.
---
Duplicate comments:
In `@apps/ensapi/src/handlers/ensanalytics/ensanalytics-api.test.ts`:
- Around line 1124-1137: The mockRevShareCapAccountingCache helper returns an
object cast to ReferralEditionSnapshot but omits the required leaderboard field
for the RevShareCap variant; update mockRevShareCapAccountingCache to return a
real minimal snapshot including awardModel:
ReferralProgramAwardModels.RevShareCap, accountingRecords, and a valid minimal
leaderboard (e.g., an empty or default leaderboard structure that satisfies
ReferralEditionSnapshot) so the returned SWRCache<ReferralEditionSnapshot>
matches the real shape and prevents hidden regressions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3440732d-fdee-4f7b-a1f6-dd49591a679c
📒 Files selected for processing (16)
apps/ensapi/src/cache/referral-edition-snapshots.cache.tsapps/ensapi/src/handlers/ensanalytics/ensanalytics-api.test.tsapps/ensapi/src/handlers/ensanalytics/ensanalytics-api.tsapps/ensapi/src/index.tsapps/ensapi/src/lib/ensanalytics/referrer-leaderboard/database.tsapps/ensapi/src/lib/ensanalytics/referrer-leaderboard/format-accounting-csv.tsapps/ensapi/src/lib/hono-factory.tsapps/ensapi/src/middleware/ensanalytics.middleware.tsdocs/ensnode.io/ensapi-openapi.jsonpackages/ens-referrals/src/api/index.tspackages/ens-referrals/src/api/prerequisites.tspackages/ens-referrals/src/award-models/pie-split/leaderboard.tspackages/ens-referrals/src/award-models/rev-share-cap/accounting.test.tspackages/ens-referrals/src/award-models/rev-share-cap/leaderboard.test.tspackages/ens-referrals/src/award-models/rev-share-cap/leaderboard.tspackages/ens-referrals/src/leaderboard.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Award Accounting
pushes forward: #1797
Summary
rev-share-capeditionrev-share-capeditions)Why
Testing
Notes for Reviewer (Optional)
registrarsandsubgraphplugins activated to be able to gather the required information (otherwise newinnerJoins will drop everything silently). Should we add them as required for all the ENSAnalytics APIs?registrarActionIdto the CSV?Pre-Review Checklist (Blocking)