rev-share-limit Referral Program Disqualification#1712
Conversation
🦋 Changeset detectedLatest commit: cab9724 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
|
|
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 admin-disqualification support to the rev-share-limit referral model: new disqualification type and rules, rule validation and builder changes, metrics and schema/serializer extensions to propagate disqualification state and reason, leaderboard logic/tests updated to use qualification function that considers disqualifications. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Leaderboard as Leaderboard
participant Rules as Rules
participant Metrics as MetricsBuilder
participant Serializer as Serializer
Client->>Leaderboard: request leaderboard computation
Leaderboard->>Rules: fetch rules (includes `disqualifications`)
Leaderboard->>Metrics: build metrics(referrer, rules)
Metrics->>Rules: isReferrerQualifiedRevShareLimit(referrer, revenue, rules)
Metrics-->>Leaderboard: return metrics (isAdminDisqualified, reason, isQualified)
Leaderboard->>Serializer: serialize results (include admin disqualification fields)
Serializer-->>Client: respond with serialized leaderboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ 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. Comment |
There was a problem hiding this comment.
Pull request overview
Adds admin-controlled “disqualification” support to the rev-share-limit referral program model so specific referrers can be excluded from qualification/awards and surfaced in metrics + API output.
Changes:
- Extend
rev-share-limitrules with adisqualifications[]list (referrer + reason) and incorporate it into qualification logic. - Add disqualification fields to ranked/unranked referrer metrics (flags + reason) and serialize/validate them via zod schemas.
- Update leaderboard qualification check and edition metrics construction; add tests for disqualification behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ens-referrals/src/v1/edition-metrics.ts | Passes rules into unranked rev-share-limit metrics builder to compute disqualification flags/reason. |
| packages/ens-referrals/src/v1/edition-defaults.ts | Supplies an explicit empty disqualification list for the default rev-share-limit edition. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/rules.ts | Introduces disqualification types/list, validates uniqueness, and updates qualification logic to exclude disqualified referrers. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts | Adds admin-disqualification fields to referrer metrics and validates/derives them from rules. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.ts | Uses shared qualification helper (now includes disqualification) during event processing. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.test.ts | Adds coverage for disqualification scenarios and duplicate-disqualification validation. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts | Extends API schemas for rules + metrics to include disqualification structures/fields. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/api/serialize.ts | Serializes disqualifications and new metric fields into API-friendly shapes. |
| .changeset/proud-wolves-cheer.md | Declares a minor version bump for the new disqualification feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts
Show resolved
Hide resolved
packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts`:
- Around line 48-52: Add a Zod-level uniqueness check to the disqualifications
array schema: update the schema built with
makeReferralProgramAdminDisqualificationSchema (the disqualifications:
z.array(...).default([]) definition) to include a .refine(...) that ensures no
duplicate referrer addresses (or the specific address field used in each
disqualification object) exist in the array, returning a clear error
path/message; this mirrors the runtime duplicate check in
validateReferralProgramRulesRevShareLimit and provides earlier validation
feedback for API inputs.
In `@packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts`:
- Around line 338-339: The lookup for unranked disqualification currently uses
the raw input `referrer`; change it to use the normalized address
`metrics.referrer` instead so mixed-case equivalents match validation.
Specifically, update the `rules.disqualifications.find((d) => d.referrer ===
referrer)` calls (including the occurrences that set `disqualification` and
`isAdminDisqualified`) to compare against `metrics.referrer` (or otherwise
normalizedReferrer) so all checks use the same normalized identifier.
In `@packages/ens-referrals/src/v1/award-models/rev-share-limit/rules.ts`:
- Around line 99-105: The duplicate-address check in rules.ts currently builds
disqualificationAddresses from rules.disqualifications.map(d => d.referrer) and
then compares via uniqueDisqualificationAddresses (a Set), which misses
duplicates that differ only by case; normalize each referrer (e.g., to lowercase
or use a canonical checksum via a utility like ethers.utils.getAddress) when
creating disqualificationAddresses before creating
uniqueDisqualificationAddresses so that addresses with different casing are
treated as identical and the duplicate error is thrown correctly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
.changeset/proud-wolves-cheer.mdpackages/ens-referrals/src/v1/award-models/rev-share-limit/api/serialize.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.test.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/rules.tspackages/ens-referrals/src/v1/edition-defaults.tspackages/ens-referrals/src/v1/edition-metrics.ts
packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts
Show resolved
Hide resolved
packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts
Outdated
Show resolved
Hide resolved
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
packages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts`:
- Around line 29-32: The schema for the "reason" field currently uses
z.string().min(1) which lets whitespace-only values pass but later fails in
rules.ts when checking d.reason.trim().length; update the schema in
zod-schemas.ts (the z.object that uses makeLowercaseAddressSchema and
valueLabel) to perform trimming/empty-check at the schema level — e.g. apply a
.trim() or .transform(s => s.trim()) and then enforce .min(1) or .refine(s =>
s.length > 0) with the same `${valueLabel}.reason must not be empty` message so
whitespace-only strings are rejected consistently before hitting rules.ts.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/rules.ts
packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts`:
- Around line 84-85: The schema currently allows empty or whitespace strings for
adminDisqualificationReason; update the zod schema for
adminDisqualificationReason to require a non-empty trimmed string when present
(mirror the validation used for the reason field) and keep it nullable only for
the null case; ensure the conditional invariant tied to isAdminDisqualified is
enforced (use the same z.string().min(1).transform or .refine pattern used by
reason and/or a .superRefine that checks when isAdminDisqualified === true then
adminDisqualificationReason is a non-empty string) so empty/whitespace values
are rejected.
packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts:76
- JSDoc for
RankedReferrerMetricsRevShareLimit.isQualifiedreferencesawardPoolShareand implies a non-zero award is guaranteed. In rev-share-limit there is noawardPoolSharefield, andisQualifiedcan still be true whileawardPoolApproxValue.amountis 0 (e.g., pool depleted). Update the comment to reference the correct field(s) and describeisQualifiedas eligibility to claim from the pool (not a guarantee of non-zero payout).
/**
* Identifies if the referrer meets the qualifications of the {@link ReferralProgramRulesRevShareLimit} to receive a non-zero `awardPoolShare`.
*
* @invariant true if and only if `totalBaseRevenueContribution` is greater than or equal to
* {@link ReferralProgramRulesRevShareLimit.minQualifiedRevenueContribution} AND
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR introduces admin disqualification support for the Key changes:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[ReferralEvent stream] --> B[Sort by checkpoint ID]
B --> C{For each event}
C --> D[Update referrer state\ntotalReferrals / duration / revenue]
D --> E[Compute totalBaseRevenueAmount]
E --> F[isReferrerQualifiedRevShareLimit]
F --> G{Revenue threshold\nmet?}
G -- No --> J[No pool claim\nwasQualified stays false]
G -- Yes --> H{Admin\ndisqualified?}
H -- Yes --> J
H -- No --> I{wasQualified\nalready?}
I -- No --> K[Claim accumulated award\nfrom pool, set wasQualified=true]
I -- Yes --> L[Claim incremental award\nfrom pool]
J --> C
K --> C
L --> C
C --> M[Sort referrers: pool claim desc,\nduration desc, address desc]
M --> N[buildRankedReferrerMetricsRevShareLimit\nisAdminDisqualified / adminDisqualificationReason]
N --> O[buildAwardedReferrerMetricsRevShareLimit\nawardPoolApproxValue validated = 0 if disqualified]
O --> P[ReferrerLeaderboardRevShareLimit]
Last reviewed commit: 77530ea |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| rules.disqualifications.find((d) => d.referrer === referrer.referrer) ?? null; | ||
|
|
There was a problem hiding this comment.
Inconsistent address normalization vs. isReferrerQualifiedRevShareLimit
In rules.ts, isReferrerQualifiedRevShareLimit explicitly normalizes the referrer before comparing against disqualification entries:
const normalizedReferrer = normalizeAddress(referrer);
const isAdminDisqualified = rules.disqualifications.some(
(d) => d.referrer === normalizedReferrer,
);But the direct disqualification lookups throughout metrics.ts — in buildRankedReferrerMetricsRevShareLimit (line 138), validateRankedReferrerMetricsRevShareLimit (line 116), validateUnrankedReferrerMetricsRevShareLimit (line 266), and buildUnrankedReferrerMetricsRevShareLimit (line 339) — skip normalization and rely on the invariant that metrics.referrer is always lowercase.
This is currently safe because buildReferrerMetrics normalizes via normalizeAddress and validateReferrerMetrics enforces lowercase, so the invariant holds. However, the inconsistency is a potential footgun: if a future code path calls these builders with an unnormalized address (or if the address invariant is ever relaxed), the disqualification lookup would silently return null for a disqualified referrer, incorrectly leaving isAdminDisqualified = false.
Consider normalizing consistently in metrics.ts as well, or adding an explicit comment that normalization is already guaranteed by the ReferrerMetrics invariant.
There was a problem hiding this comment.
We should guarantee we store values in normalized form so that while processing those values we don't need to normalize again or check for normalization.
There was a problem hiding this comment.
Agreed. The current code already follows this pattern - addresses are normalized at creation in buildReferrerMetrics and validated to be lowercase in validateReferrerMetrics, so all downstream processing can safely assume normalized addresses.
The inconsistency I flagged (isReferrerQualifiedRevShareLimit explicitly normalizing while metrics.ts doesn't) actually makes sense now: isReferrerQualifiedRevShareLimit is a public entry point that might receive raw input, while metrics.ts functions operate on already-validated metrics.
For maintainability, consider adding a brief comment at the top of metrics.ts stating the address normalization invariant, like:
| rules.disqualifications.find((d) => d.referrer === referrer.referrer) ?? null; | |
| // All address comparisons in this module assume metrics.referrer is already normalized (lowercase). | |
| // Normalization is enforced by buildReferrerMetrics and validateReferrerMetrics. |
This documents the design decision and prevents future confusion.
Additional Comments (1)
For the expected small sizes of the disqualification list this is harmless, but extracting the disqualification lookup into |
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Goader Nice update! Shared a few small comments. Please take the lead to merge when ready 👍
| /** | ||
| * An admin-imposed disqualification entry for a specific referrer in an edition. | ||
| */ | ||
| export interface ReferralProgramAdminDisqualification { |
There was a problem hiding this comment.
| /** | |
| * An admin-imposed disqualification entry for a specific referrer in an edition. | |
| */ | |
| export interface ReferralProgramAdminDisqualification { | |
| /** | |
| * An admin-imposed disqualification of a specific referrer in an edition. | |
| */ | |
| export interface ReferralProgramEditionDisqualification { |
Goal: Put more emphasis on how this is a disqualification for an edition over it coming from an admin.
| rules: ReferralProgramRulesRevShareLimit, | ||
| ): boolean { | ||
| return totalBaseRevenueContribution.amount >= rules.minQualifiedRevenueContribution.amount; | ||
| const normalizedReferrer = normalizeAddress(referrer); |
There was a problem hiding this comment.
We should guarantee these addresses are stored in normalized form (I assume this means fully lowercase?) so that when we're performing logic on these addresses all formatting is already guaranteed.
There was a problem hiding this comment.
They are normalized by that point in any place this function is being called right now. But since we are exporting it and making it available for anyone to call, I figured it would be nice to have a guarantee on that too instead of stating that invariant in the documentation and putting the responsibility on the caller/user. What do you think?
| * The reason for admin disqualification, or null if not disqualified. | ||
| * | ||
| * @invariant null when {@link isAdminDisqualified} is false. | ||
| * @invariant Non-null string when {@link isAdminDisqualified} is true. |
There was a problem hiding this comment.
| * @invariant Non-null string when {@link isAdminDisqualified} is true. | |
| * @invariant Non-empty string when {@link isAdminDisqualified} is true. |
| rules.disqualifications.find((d) => d.referrer === referrer.referrer) ?? null; | ||
|
|
There was a problem hiding this comment.
We should guarantee we store values in normalized form so that while processing those values we don't need to normalize again or check for normalization.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/ens-referrals/src/v1/award-models/rev-share-limit/rules.ts (1)
158-166:⚠️ Potential issue | 🟠 MajorAvoid linear disqualification scans in the qualification hot path.
Line 164 performs a linear
.some()for every qualification check. During leaderboard construction this can become O(events × disqualifications) and degrade runtime noticeably on larger editions. Prefer a precomputed normalizedSetlookup in event loops.⚙️ Refactor direction
export function isReferrerQualifiedRevShareLimit( referrer: Address, totalBaseRevenueContribution: PriceUsdc, rules: ReferralProgramRulesRevShareLimit, + disqualifiedReferrers: ReadonlySet<Address>, ): boolean { const normalizedReferrer = normalizeAddress(referrer); - const isAdminDisqualified = rules.disqualifications.some( - (d) => d.referrer === normalizedReferrer, - ); + const isAdminDisqualified = disqualifiedReferrers.has(normalizedReferrer); return (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/ens-referrals/src/v1/award-models/rev-share-limit/rules.ts` around lines 158 - 166, The qualification hot path uses rules.disqualifications.some(...) inside isReferrerQualifiedRevShareLimit, which causes O(events × disqualifications); change this to a constant-time Set lookup by precomputing a normalized Set of disqualified referrer addresses (e.g., buildDisqualifiedReferrerSet or disqualifiedReferrersSet) and replace the .some(...) test with disqualifiedReferrersSet.has(normalizedReferrer); ensure the Set is built once (e.g., where rules are loaded or before leaderboard/event loops) using normalizeAddress on each disqualification.referrer so isReferrerQualifiedRevShareLimit only performs a Set.has call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/ens-referrals/src/v1/award-models/rev-share-limit/rules.ts`:
- Around line 158-166: The qualification hot path uses
rules.disqualifications.some(...) inside isReferrerQualifiedRevShareLimit, which
causes O(events × disqualifications); change this to a constant-time Set lookup
by precomputing a normalized Set of disqualified referrer addresses (e.g.,
buildDisqualifiedReferrerSet or disqualifiedReferrersSet) and replace the
.some(...) test with disqualifiedReferrersSet.has(normalizedReferrer); ensure
the Set is built once (e.g., where rules are loaded or before leaderboard/event
loops) using normalizeAddress on each disqualification.referrer so
isReferrerQualifiedRevShareLimit only performs a Set.has call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7b055887-5a01-4123-a6b8-28a1c12927ac
📒 Files selected for processing (4)
packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.test.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/rules.ts
rev-share-limitReferral Program DisqualificationSummary
disqualificationsfield toReferralProgramAdminDisqualificationisReferrerQualifiedRevShareLimitnow also checks for disqualificationisReferrerQualifiedRevShareLimitis now used during the leaderboard construction, never letting a disqualified referrer get "qualified", and therefore preventing from claiming anything from the poolWhy
Testing
Notes for Reviewer (Optional)
disqualificationsfieldPre-Review Checklist (Blocking)