Conversation
🦋 Changeset detectedLatest commit: 84acb4f The changes in this PR will be included in the next version bump. This PR includes changesets to release 19 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
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a program Changes
Sequence Diagram(s)sequenceDiagram
participant Request as Request Handler
participant Middleware as Referral Leaderboard<br/>Editions Middleware
participant Indexing as Indexing Status Cache
participant Closeout as Immutability Helper
participant Upgrade as Cache Upgrade Manager
participant EditionCache as Edition Cache Manager
Request->>Middleware: Incoming HTTP request
Middleware->>Middleware: Ensure per-edition caches initialized
Middleware->>Upgrade: checkAndUpgradeImmutableCaches() (async, non-blocking)
loop per edition
Upgrade->>Indexing: read indexing status for edition
Indexing-->>Upgrade: indexing snapshot
Upgrade->>Closeout: assumeReferralProgramEditionImmutablyClosed(rules, referenceTime)
Closeout-->>Upgrade: isImmutable (true/false)
alt immutable
Upgrade->>EditionCache: create infinite-TTL cache & initialize
EditionCache-->>Upgrade: new cache initialized
Upgrade->>EditionCache: atomically swap in new cache
else not immutable
Upgrade->>EditionCache: skip upgrade, keep existing cache
end
end
Middleware-->>Request: Continue handling request
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds referral-program “status” metadata to ENS Referrals v1 responses and introduces logic in ENSAPI to upgrade per-edition leaderboard caches to indefinite storage once an edition is assumed immutably closed.
Changes:
- Add
status(Scheduled/Active/Closed) to leaderboard-page and edition-metrics domain objects, plus serialization and Zod validation. - Add SWRCache helper
isIndefinitelyStored()and implement ENSAPI middleware that opportunistically upgrades closed-edition caches to infinite TTL/no proactive revalidation. - Add closeout heuristic (
ASSUMED_CHAIN_REORG_SAFE_DURATION) and update mocks/tests for the newstatusfield.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensnode-sdk/src/shared/cache/swr-cache.ts | Adds isIndefinitelyStored() helper for identifying “infinite” caches. |
| packages/ens-referrals/src/v1/leaderboard-page.ts | Adds status to ReferrerLeaderboardPage and computes it from rules + accurateAsOf. |
| packages/ens-referrals/src/v1/edition-metrics.ts | Adds status to ranked/unranked edition metrics and computes it from rules + accurateAsOf. |
| packages/ens-referrals/src/v1/api/zod-schemas.ts | Extends response schemas to validate the new status field. |
| packages/ens-referrals/src/v1/api/serialize.ts | Serializes the new status field into API responses. |
| apps/ensapi/src/cache/referral-leaderboard-editions.cache.ts | Exports createEditionLeaderboardBuilder and documents upgrade-to-immutable behavior. |
| apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts | Adds non-blocking per-request check to upgrade caches to immutable storage once closed. |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/closeout.ts | Introduces “immutably closed” heuristic based on end time + safety window. |
| apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/mocks-v1.ts | Updates mock API response to include status. |
| apps/ensapi/src/handlers/ensanalytics-api-v1.test.ts | Updates tests to expect status in responses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/closeout.ts`:
- Around line 26-31: The function assumeReferralProgramEditionImmutablyClosed
should guard against negative durations by returning false when the latest
indexed timestamp precedes the edition end time; add a check at the top of
assumeReferralProgramEditionImmutablyClosed that if referenceTime <
rules.endTime then return false (before calling durationBetween), so
durationBetween/deserializeDuration and the makeNonNegativeIntegerSchema
validation are not invoked and no RangeError is thrown; keep the existing
comparison to ASSUMED_CHAIN_REORG_SAFE_DURATION after this guard.
In
`@apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts`:
- Around line 105-115: Replacing the old SWRCache (variable cache) with a new
SWRCache instance (immutableCache) discards existing cached entries and causes
an immediate fetch (proactivelyInitialize: true) which can block reads during
the transition; before calling cache.destroy() or before replacing it via
caches.set(editionSlug, immutableCache), extract the live cached value(s) from
the old cache and seed them into the new SWRCache created by
createEditionLeaderboardBuilder(editionConfig) (or avoid destroying and reuse
the existing cache store), and if the SWRCache API requires, set
proactivelyInitialize to false while seeding then trigger any needed background
revalidation—update the logic around cache, SWRCache,
createEditionLeaderboardBuilder, immutableCache and caches.set to
preserve/transfer existing entries rather than dropping them.
- Around line 53-119: checkAndUpgradeImmutableCaches has a race where two
concurrent invocations can both upgrade the same edition and one can destroy the
other's new cache; fix by adding a per-edition in-progress guard (e.g., a
Set<string> upgradeInProgress) checked at the start of each loop iteration
(using editionSlug) to skip or wait if an upgrade is already running, mark the
slug in upgradeInProgress before calling cache.destroy() and creating the new
SWRCache, and clear the slug from the set after caches.set(editionSlug,
immutableCache) (also ensure any early continues remove the slug if you choose a
lock-then-check pattern); reference checkAndUpgradeImmutableCaches,
ReferralLeaderboardEditionsCacheMap, isIndefinitelyStored, cache.destroy,
caches.set, and the created SWRCache/immutableCache when applying the guard.
- Around line 68-76: Move the call to indexingStatusCache.read() out of the
per-edition loop: call indexingStatusCache.read() once before iterating, check
if the result is an Error and log/handle it (preserving the existing
logger.debug message but without editionSlug since it's not per-edition), then
use the resulting indexingStatus variable inside the loop for the immutability
check; update references to indexingStatus and remove the in-loop await to avoid
repeated reads and redundant logging per edition.
apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/closeout.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/closeout.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts`:
- Around line 190-213: The background upgrade promise from upgradeEditionCache
is stored in inProgressUpgrades without a catch, causing possible unhandled
promise rejections; fix by attaching a .catch handler to the promise before
storing/returning so all rejections are logged/handled (use logger.error with
context { editionSlug }) and still ensure the existing .finally() removes the
entry from inProgressUpgrades; update the self-invoking block that creates
upgradePromise (and the variable inProgressUpgrades) to use promise.catch(...)
then .finally(...) so errors are consumed and logged while cleanup remains
intact.
In `@packages/ens-referrals/src/v1/api/zod-schemas.ts`:
- Around line 206-210: The z.enum for the status field (using
ReferralProgramStatuses) is missing the consistent custom error message pattern
used elsewhere; update the status schema to pass a valueLabel-derived message to
z.enum (e.g., use valueLabel(ReferralProgramStatuses) or the same helper used by
other fields) so the error text matches other fields, and make the same change
for the other status z.enum instance in this file that also references
ReferralProgramStatuses.
- Around line 153-157: Extract the duplicated enum into a shared Zod schema
(e.g., create a single ReferralProgramStatusSchema using z.enum with
ReferralProgramStatuses) and replace the three inline occurrences of
z.enum([...ReferralProgramStatuses.Scheduled, ...]) with that shared
ReferralProgramStatusSchema; update imports/exports in v1/api/zod-schemas.ts so
all schemas reference the single ReferralProgramStatusSchema (replace the inline
enum in the schemas that currently use "status" so they reuse the new helper).
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| z.enum(ReferralProgramStatuses, { | ||
| message: `${valueLabel} must be "Scheduled", "Active", or "Closed"`, |
There was a problem hiding this comment.
makeReferralProgramStatusSchema is using z.enum(ReferralProgramStatuses, …), but ReferralProgramStatuses is an object map (e.g. { Scheduled: "Scheduled", … }), not a string tuple. This will either fail type-checking or validate incorrectly at runtime. Use z.nativeEnum(ReferralProgramStatuses) (if supported by the zod/v4 build you’re using) or pass an explicit tuple/Object.values(ReferralProgramStatuses) cast to a non-empty string tuple, so the schema actually validates the allowed status strings.
| z.enum(ReferralProgramStatuses, { | |
| message: `${valueLabel} must be "Scheduled", "Active", or "Closed"`, | |
| z.nativeEnum(ReferralProgramStatuses, { | |
| errorMap: (issue, ctx) => { | |
| if (issue.code === z.ZodIssueCode.invalid_enum_value) { | |
| return { message: `${valueLabel} must be "Scheduled", "Active", or "Closed"` }; | |
| } | |
| return { message: ctx.defaultError }; | |
| }, |
There was a problem hiding this comment.
CodeRabbit says it's ok, nativeEnum seems to only work with TypeScript's enums.
There was a problem hiding this comment.
Suggest reviewing how we define Zod schemas for other enum values. For example: https://github.com/namehash/ensnode/blob/main/packages/ponder-sdk/src/deserialize/indexing-metrics.ts#L77 and then repeating those patterns here.
There was a problem hiding this comment.
@Goader I continue to believe the above comment is good for you to review.
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Show resolved
Hide resolved
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Goader Great work here 👍 Shared a few small comments. Please take the lead to merge when ready!
| * Otherwise, it initializes caches for each edition in the config set. | ||
| * | ||
| * Each cache's builder function handles immutability internally - when an edition becomes immutably | ||
| * closed (past the safety window), the builder returns cached data without re-fetching. |
There was a problem hiding this comment.
| * closed (past the safety window), the builder returns cached data without re-fetching. | |
| * closed (past the safety window), the builder returns previously cached data without re-fetching. |
| editionConfig: ReferralProgramEditionConfig, | ||
| ): () => Promise<ReferrerLeaderboard> { | ||
| return async (): Promise<ReferrerLeaderboard> => { | ||
| ): (cachedResult?: CachedResult<ReferrerLeaderboard>) => Promise<ReferrerLeaderboard> { |
There was a problem hiding this comment.
Could you check all the other SWRCache fn implementations? Suggest we add this new param to each of them to nicely keep everything in sync. Of course, appreciate the param would be unused in the other fn implementations, but it seems nice to explicitly identify how it is there.
| * Duration after which we assume a closed edition is safe from chain reorganizations. | ||
| * | ||
| * This is a heuristic value (10 minutes) chosen to provide a reasonable safety margin | ||
| * beyond typical Ethereum finality. It is not a guarantee of immutability. |
There was a problem hiding this comment.
@Goader Suggest to apply this suggestion from Copilot 👍
| z.enum(ReferralProgramStatuses, { | ||
| message: `${valueLabel} must be "Scheduled", "Active", or "Closed"`, |
There was a problem hiding this comment.
@Goader I continue to believe the above comment is good for you to review.
| */ | ||
| export const makeReferralProgramStatusSchema = (valueLabel: string = "status") => | ||
| z.enum(ReferralProgramStatuses, { | ||
| message: `${valueLabel} must be "Scheduled", "Active", or "Closed"`, | ||
| }); |
There was a problem hiding this comment.
@Goader This feedback from Greptile is related to the other related comment thread in this file.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export const indexingStatusCache = new SWRCache({ | ||
| fn: async () => | ||
| export const indexingStatusCache = new SWRCache<CrossChainIndexingStatusSnapshot>({ | ||
| fn: async (_cachedResult) => |
There was a problem hiding this comment.
'_cachedResult' is defined but never used.
| fn: async (_cachedResult) => | |
| fn: async () => |
| async function loadReferralProgramEditionConfigSet( | ||
| _cachedResult?: CachedResult<ReferralProgramEditionConfigSet>, | ||
| ): Promise<ReferralProgramEditionConfigSet> { |
There was a problem hiding this comment.
'_cachedResult' is defined but never used.
| async function loadReferralProgramEditionConfigSet( | |
| _cachedResult?: CachedResult<ReferralProgramEditionConfigSet>, | |
| ): Promise<ReferralProgramEditionConfigSet> { | |
| async function loadReferralProgramEditionConfigSet(): Promise<ReferralProgramEditionConfigSet> { |
| export const referrerLeaderboardCache = new SWRCache({ | ||
| fn: async () => { | ||
| export const referrerLeaderboardCache = new SWRCache<ReferrerLeaderboard>({ | ||
| fn: async (_cachedResult) => { |
There was a problem hiding this comment.
'_cachedResult' is defined but never used.
| fn: async (_cachedResult) => { | |
| fn: async () => { |
| export const makeReferralProgramStatusSchema = (_valueLabel: string = "status") => | ||
| z.enum(ReferralProgramStatuses); |
There was a problem hiding this comment.
'_valueLabel' is assigned a value but never used.
| export const makeReferralProgramStatusSchema = (_valueLabel: string = "status") => | |
| z.enum(ReferralProgramStatuses); | |
| export const makeReferralProgramStatusSchema = (valueLabel: string = "status") => | |
| z.enum(ReferralProgramStatuses, { | |
| invalid_type_error: `${valueLabel} must be one of: ${ReferralProgramStatuses.join(", ")}`, | |
| }); |
Referral Program Statuses
closes: #1523
Summary
statusfield ("Scheduled", "Active", "Closed") to referral program API responses, calculated from program timing relative toaccurateAsOfWhy
Testing
Notes for Reviewer (Optional)
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.tslines ~190-204), I was not aware of this pattern before, so best if this is confirmed to be correct.Pre-Review Checklist (Blocking)