-
Notifications
You must be signed in to change notification settings - Fork 15
Referral Program Statuses #1621
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: d2beb23 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)
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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"`, |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CodeRabbit says it's ok, nativeEnum seems to only work with TypeScript's enums.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.test.ts`:
- Around line 22-40: The test repeats identical objects across mockBuilder,
builderWithImmutableData, builderWithStaleData and each editionConfig; extract
shared fixtures like baseRules and baseAggregatedMetrics constants at top of the
test file and then compose per-case variations by spreading those bases (e.g.,
const baseRules = { totalAwardPoolValue: ..., maxQualifiedReferrers: ...,
startTime: ..., endTime: ..., subregistryId: ..., rulesUrl: ... }; const
baseAggregatedMetrics = { grandTotalReferrals: 0, ... };) and update
mockBuilder, builderWithImmutableData, builderWithStaleData and editionConfig
constructions to use { ...baseRules, <overrides> } and {
...baseAggregatedMetrics, <overrides> } so duplication is removed and only
differences remain.
- Around line 78-255: Add a unit test that exercises the concurrency guard by
calling checkAndUpgradeImmutableCaches (or directly invoking the external entry
that triggers the IIFE-based in-progress check) twice concurrently for the same
edition and asserting the second call is skipped; to implement this, mock
createEditionLeaderboardBuilder to return a builder that awaits a controllable
promise (so the first upgrade stays in-progress), invoke
checkAndUpgradeImmutableCaches/upgradeEditionCache for the same edition in
parallel, then resolve the first builder and assert that caches.set or
upgradeEditionCache was only started once (or that the second call returned
early and the original cache was the only one replaced), referencing symbols
createEditionLeaderboardBuilder, checkAndUpgradeImmutableCaches, and
upgradeEditionCache to locate the logic to test.
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.ts`:
- Around line 115-119: The function checkAndUpgradeImmutableCaches is declared
async but never awaits; remove the async keyword from its declaration and keep
the explicit Promise<void> return type, and after dispatching the non-awaited
upgrade promises ensure the function returns a resolved promise (e.g., return
Promise.resolve()) so callers still see a Promise<void> while the upgrades run
in background; update the signature and add the resolved Promise return to the
end of the function body in cache-upgrade.ts where
checkAndUpgradeImmutableCaches is defined.
- Around line 89-93: The map update and cache destruction are in the wrong
order: call caches.set(editionSlug, newCache) before calling oldCache.destroy()
so the map never temporarily points to a destroyed cache; specifically, after
logger.info({ editionSlug }, ...), assign the new cache into caches via
caches.set(editionSlug, newCache) and only then call oldCache.destroy(), keeping
the same editionSlug, newCache, and oldCache variables.
apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.test.ts
Outdated
Show resolved
Hide resolved
apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.test.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.test.ts`:
- Around line 60-63: Tests rely on module-level Map inProgressUpgrades in
cache-upgrade.ts which persists across tests; update the test setup to reset
that state between tests by either exporting and calling a test-only
resetInProgressUpgrades() from cache-upgrade.ts in beforeEach, or by using
vi.resetModules() / vi.isolateModules() in beforeEach to reload the module and
get a fresh inProgressUpgrades Map; reference the inProgressUpgrades symbol and
the cache-upgrade.ts module when making the change so each test starts with a
clean inProgressUpgrades.
- Around line 353-402: The test should also verify that the failed upgrade
removed the edition from the internal inProgressUpgrades map by invoking
checkAndUpgradeImmutableCaches a second time and asserting the builder is
invoked again; update the test that uses checkAndUpgradeImmutableCaches,
createEditionLeaderboardBuilder (mocked as failingBuilder) and
caches/editionConfigSet to call checkAndUpgradeImmutableCaches twice and assert
createEditionLeaderboardBuilder call count increments to 2, confirming the
inProgressUpgrades entry was cleaned up after the failure.
- Around line 65-102: Add an assertion that the old SWRCache instance is
destroyed when upgradeEditionCache swaps caches: spy/mock the oldCache.destroy
method (on the SWRCache created in the test) before calling
upgradeEditionCache(editionSlug, oldCache, editionConfig, caches) and assert
that oldCache.destroy() was called after the upgrade; reference the SWRCache
instance created in the test and the upgradeEditionCache function to locate
where to add the spy and expect call.
- Around line 309-351: Update the test to compute recentEndTime relative to the
reorg safety constant instead of using a fixed 60s so it remains correct if
ASSUMED_CHAIN_REORG_SAFE_DURATION changes: replace the literal recentEndTime =
now - 60 with recentEndTime = now - Math.floor(ASSUMED_CHAIN_REORG_SAFE_DURATION
/ 2) (or another clear fraction) and import or reference
ASSUMED_CHAIN_REORG_SAFE_DURATION at the top of the test file; keep the rest of
the test (variables like notImmutableTimestamp, notReadyCache, and the call to
checkAndUpgradeImmutableCaches) unchanged so the behavior asserts the edition is
still within the safety window.
In `@apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.ts`:
- Around line 115-119: The function checkAndUpgradeImmutableCaches currently
declares a Promise<void> return but is not async, so synchronous throws (e.g.,
from getLatestIndexedBlockRef or future changes) will escape instead of becoming
rejected promises; fix by making checkAndUpgradeImmutableCaches async (add the
async keyword to the function) so any thrown errors are converted to rejected
promises, or alternatively keep it synchronous and change the signature to
return void and wrap the loop body in a try/catch that logs errors and returns
Promise.resolve(); prefer restoring async for the simplest, least invasive
change.
- Around line 22-26: The module-level Map inProgressUpgrades retains entries
across tests causing leakage for repeated slugs; add an explicit cleanup so
tests can't leak state: either export a test-only reset function (e.g.,
resetInProgressUpgrades) that clears inProgressUpgrades and call it in the test
suite's afterEach, or export the Map itself and call .clear() in afterEach of
the checkAndUpgradeImmutableCaches tests; update the tests to invoke that
cleanup to ensure each test starts with an empty inProgressUpgrades.
apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.test.ts
Show resolved
Hide resolved
apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.test.ts
Show resolved
Hide resolved
apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.test.ts
Show resolved
Hide resolved
apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.test.ts
Show resolved
Hide resolved
apps/ensapi/src/lib/ensanalytics/referrer-leaderboard/cache-upgrade.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/ensapi/src/middleware/referral-leaderboard-editions-caches.middleware.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /** | ||
| * Returns true if this cache stores data indefinitely without revalidation. | ||
| * | ||
| * A cache is considered indefinitely stored when it has infinite TTL and no | ||
| * proactive revalidation interval configured. | ||
| */ | ||
| public isIndefinitelyStored(): boolean { | ||
| return ( | ||
| this.options.ttl === Number.POSITIVE_INFINITY && !this.options.proactiveRevalidationInterval | ||
| ); | ||
| } |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isIndefinitelyStored() docstring says the cache stores data indefinitely "without revalidation", but a cache configured with ttl = Infinity can still revalidate in some cases (e.g., when the cached result is an Error and errorTtl is finite, or due to an initial proactivelyInitialize revalidate). Consider rewording the comment to clarify that this check is about configuration for successful values (infinite TTL + no proactive interval), not a guarantee of zero revalidation in all states.
| * 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. |
Copilot
AI
Feb 10, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment for ASSUMED_CHAIN_REORG_SAFE_DURATION references "typical Ethereum finality", but this helper is keyed off rules.subregistryId.chainId and may apply to non-Ethereum chains/L2s as well. Consider making the wording chain-agnostic (e.g., "typical chain finality") or explicitly scoping this heuristic to the supported chains.
| * beyond typical Ethereum finality. It is not a guarantee of immutability. | |
| * beyond typical chain finality assumptions on supported networks. It is not a guarantee | |
| * of immutability. |
Greptile OverviewGreptile SummaryThis PR adds a derived The cache upgrade is triggered non-blockingly from the referral leaderboard caches middleware, checks indexing status, and for eligible editions swaps in a new SWRCache configured with infinite TTL/no proactive revalidation only after the replacement cache initializes successfully and returns data that is itself considered immutable. Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
autonumber
participant Client
participant EnsApi as ensapi (Hono)
participant EditionsMW as referralLeaderboardEditionsCachesMiddleware
participant IndexCache as indexingStatusCache
participant Upgrade as checkAndUpgradeImmutableCaches
participant SWR as SWRCache
Client->>EnsApi: GET /v1/ensanalytics/... (leaderboard)
EnsApi->>EditionsMW: run middleware
EditionsMW->>SWR: initializeReferralLeaderboardEditionsCaches()
EditionsMW-->>Client: continue request (await next)
Note over EditionsMW,Upgrade: Non-blocking upgrade check
EditionsMW->>IndexCache: read()
IndexCache-->>EditionsMW: indexingStatus snapshot
EditionsMW->>Upgrade: checkAndUpgradeImmutableCaches(caches, configSet, snapshot)
loop for each edition cache
Upgrade->>SWR: isIndefinitelyStored()
alt cache not indefinite && edition immutably closed
Upgrade->>SWR: new SWRCache(proactivelyInitialize=true, ttl=Infinity)
SWR->>SWR: read() to initialize
alt initialization ok && data immutable
Upgrade->>SWR: caches.set(slug, newCache)
Upgrade->>SWR: oldCache.destroy()
else init failed or data not immutable
Upgrade->>SWR: newCache.destroy()
end
end
end
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
6 files reviewed, 5 comments
| referenceTime: UnixTimestamp, | ||
| ): boolean { | ||
| const immutabilityThreshold = addDuration(rules.endTime, ASSUMED_CHAIN_REORG_SAFE_DURATION); | ||
| return referenceTime > immutabilityThreshold; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Closeout check off-by-one
assumeReferralProgramEditionImmutablyClosed uses referenceTime > immutabilityThreshold, so an edition whose accurateAsOf equals the threshold (endTime + safety window) will not be considered immutable. If the intent is “ended at least safety window ago”, this should be >= to avoid skipping upgrades exactly at the threshold.
| */ | ||
| export const makeReferralProgramStatusSchema = (valueLabel: string = "status") => | ||
| z.enum(ReferralProgramStatuses, { | ||
| message: `${valueLabel} must be "Scheduled", "Active", or "Closed"`, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
z.enum passed object
makeReferralProgramStatusSchema calls z.enum(ReferralProgramStatuses, ...), but z.enum expects a tuple/array of string literals (or you should use z.nativeEnum for an enum-like object). Passing an object here will throw at runtime or fail typing depending on Zod version. Fix by using z.nativeEnum(ReferralProgramStatuses) or z.enum(["Scheduled","Active","Closed"] as const).
| if (!isImmutable) { | ||
| continue; | ||
| } | ||
|
|
||
| // Atomic check-and-set: prevent concurrent upgrades of the same edition | ||
| const upgradePromise = (() => { | ||
| // Check if upgrade already in progress | ||
| if (inProgressUpgrades.has(editionSlug)) { | ||
| return null; | ||
| } | ||
|
|
||
| // Start upgrade and register promise immediately (no await in between) | ||
| const promise = upgradeEditionCache(editionSlug, cache, editionConfig, caches) | ||
| .catch((error) => { | ||
| logger.error({ editionSlug, error }, "Unexpected error during cache upgrade"); | ||
| }) | ||
| .finally(() => { | ||
| // Always clean up the in-progress tracking | ||
| inProgressUpgrades.delete(editionSlug); | ||
| }); | ||
|
|
||
| inProgressUpgrades.set(editionSlug, promise); | ||
| return promise; | ||
| })(); | ||
|
|
||
| if (!upgradePromise) { | ||
| // Another request is already upgrading this edition | ||
| logger.debug({ editionSlug }, "Upgrade already in progress, skipping"); | ||
| } | ||
|
|
||
| // Don't await - let upgrade run in background | ||
| // Errors are logged inside upgradeEditionCache | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Upgrade promise never awaited
checkAndUpgradeImmutableCaches builds upgradePromise but then doesn’t do anything with it (no void upgradePromise / await / .catch on that specific promise). If upgradeEditionCache rejects before its internal .catch is attached (or throws synchronously), this can surface as an unhandled rejection. Consider immediately attaching handling (e.g., void promise;) and/or wrapping the upgradeEditionCache(...) call in a try/catch inside the IIFE to guarantee the rejection is observed.
Additional Comments (2)
|
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)