Skip to content

feat(perps): [perps-controller] Unified fee resolver for subscription waiver (cached, no submit-path calls) - #9857

Open
abretonc7s wants to merge 5 commits into
mainfrom
TAT-3618-feat-perps-controller-unified-fee-r
Open

feat(perps): [perps-controller] Unified fee resolver for subscription waiver (cached, no submit-path calls)#9857
abretonc7s wants to merge 5 commits into
mainfrom
TAT-3618-feat-perps-controller-unified-fee-r

Conversation

@abretonc7s

@abretonc7s abretonc7s commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Explanation

Current state. RewardsIntegrationService.calculateUserFeeDiscount() resolves exactly one source for the MetaMask builder fee: the rewards discount (VIP and season, already collapsed into one value by RewardsController). ADR 0064 (rev 2026-07-29) adds subscription as another source — and drops the earlier decide-at-submit + reserve model, so there is no reserve call, no cloid, and nothing may go over the network while an order is being signed.

Solution. RewardsIntegrationService now owns a unified fee resolver. It works in fee basis points, lowest wins, and converts once at the edge to the discount bips providers already consume:

Source Fee bips
default BUILDER_FEE_CONFIG.MaxFeeDecimal * BASIS_POINTS_DIVISOR = 10
rewards (VIP + season) 10 * (1 - rewardsDiscountBips / 10000)
subscription 0, only when the eligibility gate passes

The subscription source is gated on status=activeperpsFeeWaiver entitled ∧ usage=available ∧ not exhausted, evaluated against a cached benefits snapshot. The snapshot arrives through a new optional DI dependency, PerpsPlatformDependencies.subscription.getPerpsBenefits() — the client owns the Profile JWT and performs GET /v1/profiles/{profileId}/benefits, mirroring how the rewards read is already injected. The cache is stale-while-revalidate: served as-is inside SUBSCRIPTION_BENEFITS_CACHE.FreshMs, still served past it while a background refresh runs, and no longer trusted to grant the waiver past MaxStaleMs. getSubscriptionFeeWaiverStatus() is synchronous and only kicks the refresh off, so the order-signing path never awaits a benefits request.

Degradation is one-directional. A missing, hard-stale, or unreachable snapshot fails the gate and the resolver falls back to the next-lowest source — it never errors and never over-grants. exhausted=true needs no client action at all: nothing is reserved client-side, so the next refresh simply stops passing the gate.

PerpsController.calculateFees() reads that same cached gate and surfaces it as FeeCalculationResult.subscription (eligible, reason, remainingNotionalUsd). The preview is read-only: it does not adjust the quoted rates, mutate the cap, or issue a request. Client note: render "fees waived" from subscription.eligible rather than expecting a zeroed metamaskFeeRate — re-deriving the waived rate in the preview would put a second source of truth next to the resolver.

Package effects.

  • New exported types: PerpsSubscriptionBenefits, PerpsSubscriptionUsage, PerpsSubscriptionFeeWaiverStatus, PerpsFeeSource, PerpsFeeResolution; new constant SUBSCRIPTION_BENEFITS_CACHE.
  • New public methods on RewardsIntegrationService: resolveFee(), getSubscriptionFeeWaiverStatus(), refreshSubscriptionBenefits().
  • The cached waiver status reaches the preview on the existing per-call ServiceContext (subscriptionFeeWaiver?), so MarketDataService.calculateFees() keeps its { provider, params, context } signature. It is deliberately not on FeeCalculationParams: that object is passed verbatim to provider.calculateFees() and is the public argument of PerpsController.calculateFees(), so a controller-owned cache value there would widen the provider interface and let callers supply their own waiver status.
  • Non-breaking. subscription is optional and FeeCalculationResult.subscription is omitted entirely when it is not wired, so clients without the subscription waiver (extension) see no change.
  • No provider change was needed. A subscription win resolves to a 10000 bips discount, and the provider's existing Math.floor(MaxFeeTenthsBps * (1 - d/10000)) already lands on builder.f = 0. Added a test pinning that so the ADR requirement cannot silently regress.
  • Not in scope, per the ticket: the subscription-api benefits endpoint and usage ledger, HL fill fan-out, the fill consumer, and any reserve/cloid submit-path call.

Validation.

  • Full @metamask/perps-controller suite: 77 suites, 2708 passed (40 pre-existing skips).
  • 14 new tests covering the precedence matrix, the 7-shape eligibility gate table, the non-blocking signing path (proved by construction — the injected benefits read never settles, yet resolveFee() still resolves), all three cache-freshness windows, hard-stale and unreachable fallback, backend exhaustion, refresh dedupe, the fee-preview surfacing and its absence of side effects, and builder.f = 0.
  • Changed-file ESLint, Prettier/oxfmt, and changed-test gate: pass. Monorepo yarn build: pass, with the new symbols present in the emitted .d.cts/.cjs. changelog:validate: pass.

References

  • https://consensyssoftware.atlassian.net/browse/TAT-3618
  • ADR 0064 (rev 2026-07-29) — milestone 1 (unified fee resolver) and milestone 4 (remaining-notional preview), both Perps-team owned.
  • Follow-up owned by other teams: subscription-api benefits endpoint + usage ledger (Subscription), HL fill fan-out / milestone 2 (Rewards), fill consumer + AddressIndex resolution / milestone 3 (Subscription).
  • Mobile client follow-up: wire PerpsPlatformDependencies.subscription and render the waiver from FeeCalculationResult.subscription. Ships mobile-only for v1.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them — not applicable: the subscription dependency is optional and the new FeeCalculationResult.subscription field is omitted when it is not wired, so existing consumers are unaffected.

Note for the reviewer: the changelog entries cite #9847 as the PR number. Please correct it if this PR lands on a different number.

Screenshots/Recordings


Note

Medium Risk
Changes which discount applies at order placement (fee/revenue) and uses cached subscription state with fail-closed stale handling; mis-wiring or missing invalidation could under-charge or briefly mis-attribute waivers across profiles.

Overview
Adds a unified MetaMask builder fee resolver in RewardsIntegrationService that picks the lowest fee among default, rewards (VIP/season), and an optional subscription source injected via PerpsPlatformDependencies.subscription.getPerpsBenefits(). Subscription contributes 0 bips only when a strict eligibility gate passes on a stale-while-revalidate cache (SUBSCRIPTION_BENEFITS_CACHE); order signing never awaits benefits—reads are synchronous and refresh runs in the background with attempt-based throttling and epoch fencing on invalidateSubscriptionBenefits() (also on PerpsController / messenger) for sign-out and profile switches.

calculateUserFeeDiscount() now returns the resolver’s winning discount (full waiver → 10000 bips / builder.f = 0 on HyperLiquid). calculateFees() attaches a read-only FeeCalculationResult.subscription preview (eligible, reason, remainingNotionalUsd) without changing quoted rates. Clients without subscription wired are unchanged.

Reviewed by Cursor Bugbot for commit 8cc36fc. Bugbot is set up for automated code reviews on this repo. Configure here.

Resolve the MetaMask builder fee across every source and return the
lowest: default (BUILDER_FEE_CONFIG), rewards (VIP + season, already
collapsed by RewardsController), and subscription. The subscription
source contributes 0 bips only when the eligibility gate — status
active, perpsFeeWaiver entitled, usage available, not exhausted —
passes on a cached benefits snapshot.

Benefits arrive through a new optional
PerpsPlatformDependencies.subscription.getPerpsBenefits() dependency and
are cached stale-while-revalidate, mirroring the existing VIP pattern.
getSubscriptionFeeWaiverStatus() reads the gate synchronously and only
kicks off an opportunistic refresh, so no benefits request is ever
awaited on the order-signing path. A missing, hard-stale, or unreachable
snapshot fails the gate and falls back to the next-lowest source rather
than erroring or over-granting, and backend exhaustion needs no client
action because nothing is reserved client-side.

calculateFees() surfaces the same cached gate as
FeeCalculationResult.subscription (eligible, reason,
remainingNotionalUsd) without adjusting the quoted rates, mutating the
cap, or issuing a request.

No provider change is required: a subscription win resolves to a 10000
bips discount, which the existing builder-fee math already maps to
builder.f = 0. A test pins that so it cannot regress.
Throttle the opportunistic benefits refresh on the last read attempt
rather than the last success. A failing read never advanced the snapshot
timestamp, so during a benefits outage every fee preview started a new
request; calculateFees() is a per-input call in a trading UI.

Report a null benefits payload as reason 'no-subscription' instead of
'inactive'. The DI contract documents null as "no subscription to report"
while the published type documents 'inactive' as "status is not active",
so the reason string a client renders was wrong. The gate outcome is
unchanged.

Re-read the cached waiver status after the awaited rewards round trip so
a background refresh landing during that window is picked up.

Add invalidateSubscriptionBenefits() so clients can drop the snapshot on
sign-out or a profile switch; the snapshot carries no profile identity
and would otherwise keep answering for the previous profile until the
next successful refresh.

Cover the PerpsController -> MarketDataService fee-preview wiring with
two controller-level tests, both branches. Deleting the wiring previously
left the whole suite green.
Expose subscription benefits invalidation to clients. The previous pass
added invalidateSubscriptionBenefits() to RewardsIntegrationService, but
that service is private to the controller and is not exported from the
package, so no consumer could reach it while the changelog instructed
them to call it. PerpsController now delegates to it and the method is
registered in MESSENGER_EXPOSED_METHODS, making it callable as the
PerpsController:invalidateSubscriptionBenefits action; the generated
action types are regenerated to match.

Fence in-flight benefits reads behind an epoch counter. Invalidation
cleared the snapshot but left a running read free to write its result
back, so a read issued for the previous profile could repopulate the
cache after a sign-out and mark it fresh. The epoch is captured when the
read starts and compared before the write; a superseded read is
discarded. The same check guards the attempt timestamp, so a discarded
read cannot throttle the new identity's first fetch.

Reword the changelog to name the controller method and messenger action
rather than the unreachable service method.
@abretonc7s abretonc7s changed the title chore: prepare farmslot publication pkg-098a5060-msqvuy9t feat(perps): [perps-controller] Unified fee resolver for subscription waiver (cached, no submit-path calls) Aug 13, 2026
@abretonc7s
abretonc7s marked this pull request as ready for review August 13, 2026 02:16
@abretonc7s
abretonc7s requested review from a team as code owners August 13, 2026 02:16
@abretonc7s
abretonc7s deployed to default-branch August 13, 2026 02:16 — with GitHub Actions Active

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5e6f40e. Configure here.

Comment thread packages/perps-controller/src/services/RewardsIntegrationService.ts
Comment thread packages/perps-controller/src/index.ts
- Clear the benefits dedupe handle in invalidateSubscriptionBenefits so the
  next refresh starts a fresh read for the new identity instead of awaiting
  the fenced in-flight one.
- Export PerpsControllerInvalidateSubscriptionBenefitsAction from the package
  root, matching every sibling PerpsController*Action.
@abretonc7s

Copy link
Copy Markdown
Contributor Author

Automated pr-complete run — #9857

Metric Value
Run 48c0eb51
Duration ?
Model claude/opus
Nudges 0
Worker report

PR #9857 — Comments Report

PR: #9857
Branch: TAT-3618-feat-perps-controller-unified-fee-r
Integration: skipped (branch already contains origin/main)

Fetched actionable comments

# Source Author Comment ID Location Summary Triage Planned action
1 inline review cursor[bot] 3771918858 packages/perps-controller/src/services/RewardsIntegrationService.ts:283 "Stale refresh blocks post-invalidate fetch" (Medium). invalidateSubscriptionBenefits bumps the epoch and clears the snapshot but leaves #benefitsRefresh set; the next status read awaits the fenced in-flight promise and returns without starting a fetch for the new identity. REAL Clear #benefitsRefresh in invalidateSubscriptionBenefits() so the next refresh starts a new fetch.
2 inline review cursor[bot] 3771918862 packages/perps-controller/src/index.ts:246 "Missing public action type export" (Low). PerpsControllerInvalidateSubscriptionBenefitsAction is in the method-actions union and registered on the controller but not re-exported from the package root. REAL Add the type to the explicit export type { … } list in src/index.ts.

Triage evidence

Comment 1 — REAL. Reproduction path in the code as of f960dd37c:

  1. A background refresh is in flight, so #benefitsRefresh = P (RewardsIntegrationService.ts:256).
  2. invalidateSubscriptionBenefits() clears #benefitsSnapshot and #lastAttemptAt and bumps #benefitsEpoch, but never touches #benefitsRefresh (:278-286).
  3. getSubscriptionFeeWaiverStatus() now sees age = Infinity and sinceAttempt = Infinity, so it calls refreshSubscriptionBenefits() (:214-219).
  4. refreshSubscriptionBenefits() takes the dedupe branch (:250-252), awaits P, and returns without issuing a request.
  5. P is fenced by the epoch check (:305), so it discards its result and — correctly — does not record #lastAttemptAt (:346).

Net effect: the first status read after an identity change consumes its refresh opportunity on a read that can only be discarded, contradicting the method's own documented contract ("The next status read reports not-hydrated and starts a fresh fetch"). A caller that awaits refreshSubscriptionBenefits() directly right after invalidation gets the same no-op.

Comment 2 — REAL. PerpsControllerInvalidateSubscriptionBenefitsAction is declared at PerpsController-method-action-types.ts:791, joined into the actions union at :1215, and the handler is registered on the messenger at PerpsController.ts:908. Every other PerpsController*Action in that union appears in the explicit export type { … } block in src/index.ts:43-146; this one does not, so consumers cannot import it from @metamask/perps-controller.

General PR-conversation comments: none.
Review decision: REVIEW_REQUIRED (only cursor COMMENTED). CI status checks: all passing/neutral.
Unresolved review threads: PRRT_kwDOCBB0Cc6YyrB0 (3771918858), PRRT_kwDOCBB0Cc6YyrB3 (3771918862).


Fixes applied

Commit: 8cc36fce9fix(perps): address PR feedback (pushed to TAT-3618-feat-perps-controller-unified-fee-r; non-force, integration was skipped).

Files changed (39 insertions, 0 deletions):

File Change
packages/perps-controller/src/services/RewardsIntegrationService.ts invalidateSubscriptionBenefits() now sets #benefitsRefresh = undefined alongside the epoch bump.
packages/perps-controller/src/index.ts Added PerpsControllerInvalidateSubscriptionBenefitsAction to the explicit export type { … } list.
packages/perps-controller/tests/src/services/RewardsIntegrationService.test.ts New test: "starts a fresh read when the next caller arrives while a fenced read is still in flight".

Comment 1 fix rationale. Clearing the dedupe handle is the minimal correction: the fenced read can only be discarded, so deduping the new identity's caller onto it consumed the refresh opportunity without hydrating the cache. refreshSubscriptionBenefits()'s finally guard already compares this.#benefitsRefresh === refresh, so the discarded read cannot clear whatever handle replaces it. Behaviour now matches the method's own documented contract and the existing changelog wording ("The next status read reports not-hydrated and starts a fresh fetch").

Comment 2 fix rationale. One line, alphabetically placed between PerpsControllerInitAction and PerpsControllerIsCurrentlyReinitializingAction. Verified present in the emitted dist/index.d.cts after a root build.

Downstream compatibility assessment (step 8)

  • Non-breaking, additive only. The index.ts change adds one type export; nothing is removed, renamed, or re-signed. Consumers that do not import the new name are unaffected.
  • Runtime change is internal. #benefitsRefresh is a private field; the only observable difference is that the first refresh after invalidateSubscriptionBenefits() now issues a request rather than awaiting a to-be-discarded one. That is strictly closer to the documented and changelogged behaviour, and it cannot over-grant the waiver — the gate still requires a hydrated, fresh, eligible snapshot.
  • No changelog delta needed. The existing #9857 Unreleased entries already describe PerpsController.invalidateSubscriptionBenefits(), the PerpsController:invalidateSubscriptionBenefits messenger action, and the discard-in-flight semantics. Sibling PerpsController*Action type names are not individually enumerated in changelog entries, so the new export needs no separate line.
  • Mobile/Extension risk: none. Extension does not wire PerpsPlatformDependencies.subscription, so the subscription source never contributes and this code path is inert there. Mobile's pending wiring benefits from the fix (a profile switch now rehydrates on the next read).
  • No package manifests, lockfile, error codes, or state types were touched. No e2e/ changes.

Validation

Check Command Result
Targeted Jest cd packages/perps-controller && NODE_OPTIONS=--experimental-vm-modules yarn jest tests/src/services/RewardsIntegrationService.test.ts --no-coverage PASS — 1 suite, 25/25 tests (includes the new fenced-read test)
ESLint (changed files) yarn eslint packages/perps-controller/src/index.ts .../RewardsIntegrationService.ts .../RewardsIntegrationService.test.ts PASS — no output
Root build (exported types changed) yarn build PASS — exit 0, "Project built successfully"
Emitted-type assertion grep -c PerpsControllerInvalidateSubscriptionBenefitsAction packages/perps-controller/dist/index.d.cts PASS — 1 occurrence
Perps headless smoke mm-harness run …/core/perps/read-markets.recipe.json --adapter core PASS — exit 0, artifacts in artifacts/perps-smoke/
Inherited recipe re-run mm-harness run artifacts/recipe.json --adapter core PASS — exit 0, artifacts in artifacts/recipe-rerun/

Runner-flag note: the checklist's --project-root is not a valid mm-harness option in the provisioned runner (CLI_UNKNOWN_OPTION); --target is the equivalent and was used. The smoke recipe also lives at recipes/core/perps/read-markets.recipe.json, not the checklist's read-markets.core.recipe.json.

Replies and thread resolution

Comment ID Triage Reply ID Thread Resolved
3771918858 REAL (fixed) 3772082211 PRRT_kwDOCBB0Cc6YyrB0 isResolved: true
3771918862 REAL (fixed) 3772082299 PRRT_kwDOCBB0Cc6YyrB3 isResolved: true

Live re-fetch at step 4 confirmed the pre-rendered snapshot: exactly 2 unresolved inline comments, 0 general PR-conversation comments, 0 human reviews, reviewDecision: REVIEW_REQUIRED, CI 40 SUCCESS / 5 SKIPPED / 1 NEUTRAL.

@abretonc7s
abretonc7s enabled auto-merge August 13, 2026 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant