Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 5b08c84 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 |
|
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:
📝 WalkthroughWalkthroughRename and reshape rev-share-limit and pie-split API fields: replace reward-pool keys ( Changes
Sequence Diagram(s)(silently omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Greptile SummaryThis PR is a pure rename/refactoring across the
Confidence Score: 5/5Safe to merge — all renames are applied consistently with no stale references or logic changes. All 10 changed files are internally consistent with the new field names. No logic was altered — only identifier names. The sole open question is the semver bump level, which is a P2 style concern that does not block merge. Only .changeset/bright-foxes-dance.md warrants a second look regarding whether Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[ReferralEvent input] --> B[buildReferrerLeaderboardRevShareLimit]
B --> C{isReferrerQualifiedRevShareLimit}
C -->|threshold check| D[rules.minBaseRevenueContribution]
C -->|renamed from minQualifiedRevenueContribution| D
B --> E[scalePrice]
E -->|share fraction| F[rules.maxBaseRevenueShare]
F -->|renamed from qualifiedRevenueShare| F
B --> G[buildAwardedReferrerMetricsRevShareLimit]
G --> H[uncappedAwardValue\nmaxBaseRevenueShare × totalBaseRevenueContribution\nrenamed from standardAwardValue]
G --> I[cappedAwardValue\npool-capped actual award\nrenamed from awardPoolApproxValue]
G --> J[ReferrerLeaderboardRevShareLimit output]
J --> K[rules: minBaseRevenueContribution\nrules: maxBaseRevenueShare]
J --> L[referrers: uncappedAwardValue\nreferrers: cappedAwardValue]
Reviews (1): Last reviewed commit: "renamed variables" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Renames rev-share-limit rule/metric field names across the v1 referrals API to better reflect meaning (per #1786).
Changes:
- Renamed RevShareLimit rule fields:
minQualifiedRevenueContribution→minBaseRevenueContribution,qualifiedRevenueShare→maxBaseRevenueShare. - Renamed RevShareLimit awarded metric fields:
standardAwardValue→uncappedAwardValue,awardPoolApproxValue→cappedAwardValue. - Updated validation, serialization/Zod schemas, tests, and README examples to use the new names.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/ens-referrals/src/v1/award-models/rev-share-limit/rules.ts | Renames rule fields and updates validation/qualification logic to match. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts | Renames awarded metric fields and updates invariants/validation/builders. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.ts | Updates award computation to use renamed rule/metric fields. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.test.ts | Updates RevShareLimit leaderboard tests for renamed fields. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts | Updates RevShareLimit Zod schemas and refinements for renamed fields. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/api/serialized-types.ts | Updates serialized type names for renamed fields. |
| packages/ens-referrals/src/v1/award-models/rev-share-limit/api/serialize.ts | Updates RevShareLimit serializers to emit renamed fields. |
| packages/ens-referrals/src/v1/api/zod-schemas.test.ts | Updates v1 API schema tests to use renamed RevShareLimit fields. |
| packages/ens-referrals/README.md | Updates public docs/examples to the renamed fields. |
| .changeset/bright-foxes-dance.md | Declares a release bump for the API field renames. |
💡 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}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts (1)
72-77:⚠️ Potential issue | 🟡 MinorReplace the stale
awardPoolShareterm in this invariant.That name is no longer exposed by this type or the rest of the renamed API, so the doc now points readers at the wrong concept.
Doc-only cleanup
- * Identifies if the referrer meets the qualifications of the {`@link` ReferralProgramRulesRevShareLimit} to receive a non-zero `awardPoolShare`. + * Identifies if the referrer meets the qualifications of the {`@link` ReferralProgramRulesRevShareLimit} to receive a non-zero award.🤖 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/metrics.ts` around lines 72 - 77, Update the JSDoc invariant to use the current API term instead of the stale `awardPoolShare` identifier; replace `awardPoolShare` with the renamed property `awardShare` in the comment that describes the invariant for ReferralProgramRulesRevShareLimit (the block referencing `totalBaseRevenueContribution` and `isAdminDisqualified`) so the docs point at the correct concept.
🤖 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 126-137: The Zod schema must mirror
validateUnrankedReferrerMetricsRevShareLimit by rejecting any "unranked" record
whose uncappedAwardValue.amount or cappedAwardValue.amount is non-zero; update
the schema block that defines uncappedAwardValue, cappedAwardValue,
isAdminDisqualified, adminDisqualificationReason (the schema using valueLabel)
to add an additional .refine predicate that checks "if this record is unranked
(use the same unranked condition used in
validateUnrankedReferrerMetricsRevShareLimit) then uncappedAwardValue.amount ===
0 && cappedAwardValue.amount === 0", and return a clear message/path (e.g.,
path: ["uncappedAwardValue","cappedAwardValue"]) when violated so the serialized
API shape matches the runtime validator.
In `@packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts`:
- Around line 188-210: The validator in metrics.ts enforces
cappedAwardValue.amount == 0n only for admin-disqualified referrers but misses
the contract that when metrics.isQualified === false (below-threshold)
cappedAwardValue must also be zero; update the validation logic after the
makePriceUsdcSchema parses to add a check for metrics.isQualified === false that
throws a descriptive Error if metrics.cappedAwardValue.amount !== 0n (similar to
the existing isAdminDisqualified check), referencing the same properties
(metrics.isQualified, metrics.cappedAwardValue.amount) and keeping the other
comparisons against rules.totalAwardPoolValue.amount and
metrics.uncappedAwardValue.amount intact.
---
Outside diff comments:
In `@packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts`:
- Around line 72-77: Update the JSDoc invariant to use the current API term
instead of the stale `awardPoolShare` identifier; replace `awardPoolShare` with
the renamed property `awardShare` in the comment that describes the invariant
for ReferralProgramRulesRevShareLimit (the block referencing
`totalBaseRevenueContribution` and `isAdminDisqualified`) so the docs point at
the correct concept.
🪄 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: 8b1dd26f-f231-448b-a5c1-88e14a85bd69
📒 Files selected for processing (10)
.changeset/bright-foxes-dance.mdpackages/ens-referrals/README.mdpackages/ens-referrals/src/v1/api/zod-schemas.test.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/api/serialize.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/api/serialized-types.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.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.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 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/api/zod-schemas.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 139-145: The current .refine call incorrectly sets path to
["uncappedAwardValue","cappedAwardValue"] (which points to a nested path);
replace the .refine with .superRefine on the same Zod schema and, inside the
callback, check the same condition and call ctx.addIssue separately for each
offending sibling field (use paths ["uncappedAwardValue"] and
["cappedAwardValue"] respectively) with the appropriate message; use
ZodIssueCode.custom for the issues so each field gets its own validation error
instead of a nonexistent nested path.
In `@packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts`:
- Around line 189-195: The current validator for
AwardedReferrerMetricsRevShareLimit.uncappedAwardValue only parses a PriceUsdc
but should enforce the documented invariant; compute expectedUncapped =
rules.maxBaseRevenueShare × metrics.totalBaseRevenueContribution inside the
validation logic (using the same numeric types/scale as makePriceUsdcSchema
expects) and either assert parsed uncappedAwardValue equals expectedUncapped or,
better, derive and set uncappedAwardValue from that computation instead of
trusting caller input; update
makePriceUsdcSchema("AwardedReferrerMetricsRevShareLimit.cappedAwardValue")
usage to still validate cappedAwardValue <= expectedUncapped and adjust
buildAwardedReferrerMetricsRevShareLimit() to derive uncappedAwardValue from
rules.maxBaseRevenueShare and metrics.totalBaseRevenueContribution.
🪄 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: 46afcbc6-f698-4608-9a08-24db3381105a
📒 Files selected for processing (2)
packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts
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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts (1)
120-146:⚠️ Potential issue | 🟠 MajorMirror the rest of the unranked zero-metric contract in this schema.
validateUnrankedReferrerMetricsRevShareLimit()still rejects non-zerototalReferrals,totalIncrementalDuration,totalRevenueContribution.amount, andtotalBaseRevenueContribution.amount, but this schema only encodes the zero-award checks. That leaves the serialized API shape looser than the runtime model and allows invalid unranked payloads to parse here.Suggested schema tightening
.object({ referrer: makeLowercaseAddressSchema(`${valueLabel}.referrer`), totalReferrals: makeNonNegativeIntegerSchema(`${valueLabel}.totalReferrals`), totalIncrementalDuration: makeDurationSchema(`${valueLabel}.totalIncrementalDuration`), totalRevenueContribution: makePriceEthSchema(`${valueLabel}.totalRevenueContribution`), totalBaseRevenueContribution: makePriceUsdcSchema( `${valueLabel}.totalBaseRevenueContribution`, ), rank: z.null(), isQualified: z.literal(false), uncappedAwardValue: makePriceUsdcSchema(`${valueLabel}.uncappedAwardValue`), cappedAwardValue: makePriceUsdcSchema(`${valueLabel}.cappedAwardValue`), isAdminDisqualified: z.boolean(), adminDisqualificationReason: z .string() .trim() .min(1, `${valueLabel}.adminDisqualificationReason must not be empty`) .nullable(), }) + .refine((data) => data.totalReferrals === 0, { + message: `${valueLabel}.totalReferrals must be 0 for unranked referrers`, + path: ["totalReferrals"], + }) + .refine((data) => data.totalIncrementalDuration === 0, { + message: `${valueLabel}.totalIncrementalDuration must be 0 for unranked referrers`, + path: ["totalIncrementalDuration"], + }) + .refine((data) => data.totalRevenueContribution.amount === 0n, { + message: `${valueLabel}.totalRevenueContribution must be 0 for unranked referrers`, + path: ["totalRevenueContribution"], + }) + .refine((data) => data.totalBaseRevenueContribution.amount === 0n, { + message: `${valueLabel}.totalBaseRevenueContribution must be 0 for unranked referrers`, + path: ["totalBaseRevenueContribution"], + }) .refine((data) => data.uncappedAwardValue.amount === 0n, {🤖 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/api/zod-schemas.ts` around lines 120 - 146, The schema for unranked referrers currently only enforces zero award values but must also mirror the runtime validator validateUnrankedReferrerMetricsRevShareLimit: add checks to the zod object (around the same chain where rank is z.null() and isQualified is z.literal(false)) to require totalReferrals === 0, totalIncrementalDuration === 0 (or duration.amount === 0 depending on the duration shape), totalRevenueContribution.amount === 0n, and totalBaseRevenueContribution.amount === 0n; implement these as additional .refine() calls (or a single .superRefine()) referencing the fields totalReferrals, totalIncrementalDuration, totalRevenueContribution, and totalBaseRevenueContribution so the parsed API shape matches the runtime validation.
♻️ Duplicate comments (1)
packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts (1)
189-195:⚠️ Potential issue | 🟠 MajorDon't accept an arbitrary
uncappedAwardValuehere.This field is documented as
rules.maxBaseRevenueShare × totalBaseRevenueContribution, but the validator only parses it as a genericPriceUsdcand the builder still takes it from callers. Any inflateduncappedAwardValuecurrently passes as long ascappedAwardValue <= uncappedAwardValue, which weakens the renamed contract. Recompute the expected uncapped amount fromrules.maxBaseRevenueShareandtotalBaseRevenueContributionhere, or derive it insidebuildAwardedReferrerMetricsRevShareLimit()instead of accepting it as input. Based on learnings: Inpackages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts,AwardedReferrerMetricsRevShareLimit.standardAwardValueshould not duplicate the genericPriceUsdcnon-negative invariant; keep only the context-specific formula (“rules.qualifiedRevenueShare × totalBaseRevenueContribution”) and notes about pool independence.Also applies to: 222-231
🤖 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/metrics.ts` around lines 189 - 195, The code currently accepts an arbitrary uncappedAwardValue and only validates it as a generic PriceUsdc; instead recompute and validate uncappedAwardValue from the formula rules.maxBaseRevenueShare × totalBaseRevenueContribution (or move that derivation into buildAwardedReferrerMetricsRevShareLimit()) and validate the computed value against cappedAwardValue, rather than trusting caller input for AwardedReferrerMetricsRevShareLimit.uncappedAwardValue; likewise remove the redundant generic non-negative PriceUsdc check for AwardedReferrerMetricsRevShareLimit.standardAwardValue and keep only the context-specific invariant/description that it equals rules.qualifiedRevenueShare × totalBaseRevenueContribution and is pool-independent. Ensure references to makePriceUsdcSchema, cappedAwardValue, standardAwardValue, buildAwardedReferrerMetricsRevShareLimit, rules.maxBaseRevenueShare, rules.qualifiedRevenueShare, and totalBaseRevenueContribution are used to locate and update the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.ts`:
- Around line 120-146: The schema for unranked referrers currently only enforces
zero award values but must also mirror the runtime validator
validateUnrankedReferrerMetricsRevShareLimit: add checks to the zod object
(around the same chain where rank is z.null() and isQualified is
z.literal(false)) to require totalReferrals === 0, totalIncrementalDuration ===
0 (or duration.amount === 0 depending on the duration shape),
totalRevenueContribution.amount === 0n, and totalBaseRevenueContribution.amount
=== 0n; implement these as additional .refine() calls (or a single
.superRefine()) referencing the fields totalReferrals, totalIncrementalDuration,
totalRevenueContribution, and totalBaseRevenueContribution so the parsed API
shape matches the runtime validation.
---
Duplicate comments:
In `@packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts`:
- Around line 189-195: The code currently accepts an arbitrary
uncappedAwardValue and only validates it as a generic PriceUsdc; instead
recompute and validate uncappedAwardValue from the formula
rules.maxBaseRevenueShare × totalBaseRevenueContribution (or move that
derivation into buildAwardedReferrerMetricsRevShareLimit()) and validate the
computed value against cappedAwardValue, rather than trusting caller input for
AwardedReferrerMetricsRevShareLimit.uncappedAwardValue; likewise remove the
redundant generic non-negative PriceUsdc check for
AwardedReferrerMetricsRevShareLimit.standardAwardValue and keep only the
context-specific invariant/description that it equals
rules.qualifiedRevenueShare × totalBaseRevenueContribution and is
pool-independent. Ensure references to makePriceUsdcSchema, cappedAwardValue,
standardAwardValue, buildAwardedReferrerMetricsRevShareLimit,
rules.maxBaseRevenueShare, rules.qualifiedRevenueShare, and
totalBaseRevenueContribution are used to locate and update the logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0e0628e2-9060-4a54-a071-462eaecf1002
📒 Files selected for processing (2)
packages/ens-referrals/src/v1/award-models/rev-share-limit/api/zod-schemas.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@Goader Thanks for this 👍 Shared a few additional suggestions. Appreciate your advice.
| minBaseRevenueContribution: PriceUsdc; | ||
|
|
||
| /** | ||
| * The fraction of the referrer's base revenue contribution that constitutes their potential award. |
There was a problem hiding this comment.
| * The fraction of the referrer's base revenue contribution that constitutes their potential award. | |
| * The fraction of the referrer's base revenue contribution that constitutes their max potential award. This is the max that ignores the possibility of the award pool becoming exhausted. |
| @@ -60,14 +60,14 @@ export interface ReferralProgramRulesRevShareLimit extends BaseReferralProgramRu | |||
| /** | |||
| * The minimum base revenue contribution required for a referrer to qualify. | |||
There was a problem hiding this comment.
| * The minimum base revenue contribution required for a referrer to qualify. | |
| * The minimum base revenue contribution required for a referrer to qualify for awards. |
| @@ -60,14 +60,14 @@ export interface ReferralProgramRulesRevShareLimit extends BaseReferralProgramRu | |||
| /** | |||
There was a problem hiding this comment.
Suggest we also rename totalAwardPoolValue to just awardPool.
| @@ -60,14 +60,14 @@ export interface ReferralProgramRulesRevShareLimit extends BaseReferralProgramRu | |||
| /** | |||
There was a problem hiding this comment.
Since we're already making a number of breaking changes in this PR, what do you think about changing BASE_REVENUE_CONTRIBUTION_PER_YEAR so that it stops being a hardcoded constant and instead becomes another variable inside the rules here. For example it might be called something like baseAnnualRevenueContribution.
| @@ -159,12 +159,12 @@ export const buildRankedReferrerMetricsRevShareLimit = ( | |||
| export interface AwardedReferrerMetricsRevShareLimit extends RankedReferrerMetricsRevShareLimit { | |||
| /** | |||
| * The standard (uncapped) USDC award value for this referrer, computed as | |||
There was a problem hiding this comment.
| * The standard (uncapped) USDC award value for this referrer, computed as | |
| * The uncapped USDC award value for this referrer, computed as |
|
|
||
| // 3. Sort referrers to assign ranks: | ||
| // 1. qualifiedAwardValue (awardPoolApproxValue) desc — actual pool claims, race winners first | ||
| // 1. qualifiedAwardValue (cappedAwardValue) desc — actual pool claims, race winners first |
There was a problem hiding this comment.
Suggest we also refine the name given to qualifiedAwardValue and qualifiedAwardValueAmount and any other fields we can refine to more consistently name ideas across all of this logic.
| rules.qualifiedRevenueShare, | ||
| rules.maxBaseRevenueShare, | ||
| ).amount; | ||
| const claimAmount = |
There was a problem hiding this comment.
Maybe nice to use min here?
| @@ -162,7 +162,7 @@ export const buildReferrerLeaderboardRevShareLimit = ( | |||
| BigInt(SECONDS_PER_YEAR); | |||
| const incrementalStandardAwardAmount = scalePrice( | |||
There was a problem hiding this comment.
| const incrementalStandardAwardAmount = scalePrice( | |
| const incrementalUncappedAward = scalePrice( |
| rules.qualifiedRevenueShare, | ||
| rules.maxBaseRevenueShare, | ||
| ).amount; | ||
| const claimAmount = |
There was a problem hiding this comment.
| const claimAmount = | |
| const incrementalCappedAward = |
packages/ens-referrals/README.md
Outdated
| console.log(`Max Base Revenue Share: ${leaderboardPage.rules.maxBaseRevenueShare}`); | ||
| console.log( | ||
| `Tentative award for the best referrer: ${firstReferrer !== null ? firstReferrer.awardPoolApproxValue : noReferrersFallback}`, | ||
| `Tentative award for the best referrer: ${firstReferrer !== null ? firstReferrer.cappedAwardValue : noReferrersFallback}`, |
There was a problem hiding this comment.
| `Tentative award for the best referrer: ${firstReferrer !== null ? firstReferrer.cappedAwardValue : noReferrersFallback}`, | |
| `Tentative award for the top ranked referrer: ${firstReferrer !== null ? firstReferrer.cappedAwardValue : noReferrersFallback}`, |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
packages/ens-referrals/src/v1/award-models/rev-share-limit/leaderboard.test.ts:342
- The scenario heading and inline comments still refer to "standard"/"qualifiedAward" terminology, but the implementation now ranks by
cappedAwardValue(pool-claimed) and exposesuncappedAwardValue. Please update this test name/comments to use the current field names so the ranking criteria is accurately documented.
describe("Ranking", () => {
it("ranks referrers by qualifiedAwardValue desc, then uncappedAwardValue desc", () => {
// Pool = $1000 (unlimited for this test)
// ADDR_A: 1 year → qualifies at t=1000, qualifiedAward = $2.50, standardAward = $2.50
// ADDR_B: 2 years → qualifies at t=2000, qualifiedAward = $5.00, standardAward = $5.00
// ADDR_C: 0.5 years → never qualifies, qualifiedAward = $0, standardAward = $1.25
const rules = buildTestRules();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * - qualifiedRevenueShare = 0.5 | ||
| * - baseAnnualRevenueContribution = $5 USDC | ||
| * - maxBaseRevenueShare = 0.5 | ||
| * - 1 year of duration → $5 base revenue → $2.50 standard award |
There was a problem hiding this comment.
This doc comment still describes the computed value as a "standard award", but the code/API now calls this uncappedAwardValue (and the pool-claimed value cappedAwardValue). Please update the terminology here to match the renamed fields to avoid confusion.
| * - 1 year of duration → $5 base revenue → $2.50 standard award | |
| * - 1 year of duration → $5 base revenue → $2.50 uncappedAwardValue |
|
|
||
| describe("Scenario A — unqualified referrer: no award claimed", () => { | ||
| it("accumulates standard award but awardPoolApproxValue is $0 when not qualified", () => { | ||
| it("accumulates standard award but cappedAwardValue is $0 when not qualified", () => { |
There was a problem hiding this comment.
Test description says "accumulates standard award" but the asserted field is uncappedAwardValue. For consistency with the renamed fields, update the test name/description to refer to the "uncapped" award here.
| it("accumulates standard award but cappedAwardValue is $0 when not qualified", () => { | |
| it("accumulates uncapped award but cappedAwardValue is $0 when not qualified", () => { |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/ens-referrals/src/v1/award-models/rev-share-limit/metrics.ts (1)
188-219: 🧹 Nitpick | 🔵 TrivialConsider validating the
uncappedAwardValueformula.The validator parses
uncappedAwardValueas a genericPriceUsdcbut does not verify it equalsmaxBaseRevenueShare × totalBaseRevenueContribution. A caller could pass an arbitrary value that still satisfiescappedAwardValue <= uncappedAwardValue, weakening the field's documented contract.If the formula should be enforced at validation time, compute the expected value from
rules.maxBaseRevenueShareandmetrics.totalBaseRevenueContributionand assert equality. Alternatively, if this is intentional (e.g., to allow flexibility for future use cases), document that the formula is not enforced here.♻️ Suggested validation addition
makePriceUsdcSchema("AwardedReferrerMetricsRevShareLimit.uncappedAwardValue").parse( metrics.uncappedAwardValue, ); + // Validate uncappedAwardValue matches the documented formula + const expectedUncappedAmount = + (BigInt(Math.floor(rules.maxBaseRevenueShare * 1e18)) * + metrics.totalBaseRevenueContribution.amount) / + BigInt(1e18); + if (metrics.uncappedAwardValue.amount !== expectedUncappedAmount) { + throw new Error( + `AwardedReferrerMetricsRevShareLimit: uncappedAwardValue.amount ${metrics.uncappedAwardValue.amount.toString()} does not match expected ${expectedUncappedAmount.toString()} (maxBaseRevenueShare × totalBaseRevenueContribution).`, + ); + } + makePriceUsdcSchema("AwardedReferrerMetricsRevShareLimit.cappedAwardValue").parse(Note: The exact arithmetic for converting
maxBaseRevenueShare(anumber) to work withbigintamounts depends on your decimal precision conventions. Adjust scaling as appropriate.🤖 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/metrics.ts` around lines 188 - 219, The validator currently parses metrics.uncappedAwardValue with makePriceUsdcSchema but doesn't assert it equals the product maxBaseRevenueShare × totalBaseRevenueContribution; add a check in the AwardedReferrerMetricsRevShareLimit validation that computes the expected uncapped amount from rules.maxBaseRevenueShare and metrics.totalBaseRevenueContribution (apply your project's decimal/scale conversion to convert maxBaseRevenueShare into the same bigint units), then throw an Error if metrics.uncappedAwardValue.amount !== expectedAmount; keep this check adjacent to the existing validations that reference metrics.uncappedAwardValue and metrics.totalBaseRevenueContribution so it is clear and consistently enforced.
🤖 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/metrics.ts`:
- Around line 188-219: The validator currently parses metrics.uncappedAwardValue
with makePriceUsdcSchema but doesn't assert it equals the product
maxBaseRevenueShare × totalBaseRevenueContribution; add a check in the
AwardedReferrerMetricsRevShareLimit validation that computes the expected
uncapped amount from rules.maxBaseRevenueShare and
metrics.totalBaseRevenueContribution (apply your project's decimal/scale
conversion to convert maxBaseRevenueShare into the same bigint units), then
throw an Error if metrics.uncappedAwardValue.amount !== expectedAmount; keep
this check adjacent to the existing validations that reference
metrics.uncappedAwardValue and metrics.totalBaseRevenueContribution so it is
clear and consistently enforced.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f1baee5d-d32b-4c01-91a8-a475c2d0c664
📒 Files selected for processing (18)
.changeset/bright-foxes-dance.mdapps/ensapi/src/lib/ensanalytics/referrer-leaderboard/mocks-v1.tspackages/ens-referrals/README.mdpackages/ens-referrals/src/v1/api/zod-schemas.test.tspackages/ens-referrals/src/v1/award-models/pie-split/api/serialize.tspackages/ens-referrals/src/v1/award-models/pie-split/api/serialized-types.tspackages/ens-referrals/src/v1/award-models/pie-split/api/zod-schemas.tspackages/ens-referrals/src/v1/award-models/pie-split/metrics.tspackages/ens-referrals/src/v1/award-models/pie-split/rules.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/api/serialize.tspackages/ens-referrals/src/v1/award-models/rev-share-limit/api/serialized-types.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/leaderboard-page.test.ts
Refined
rev-share-limitvariable namescloses: #1786
Summary
rev-share-limitvariable names #1786Why
Testing
Pre-Review Checklist (Blocking)