Skip to content

fix(perps-controller): degrade quietly when optional messenger actions are missing - #10665

Merged
abretonc7s merged 7 commits into
mainfrom
fix/perps-optional-messenger-delegation
Oct 2, 2026
Merged

abretonc7s merged 7 commits into
mainfrom
fix/perps-optional-messenger-delegation

Conversation

@abretonc7s

@abretonc7s abretonc7s commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Explanation

RewardsIntegrationService only recognised the "has not been registered" error for a missing optional action. A host whose child messenger doesn't delegate the action gets "has not been delegated to PerpsController" instead. In that case the fee waiver, rewards discount, data-lake report and watchlist sync logged logger.error, and the data-lake report also retried. NetworkController, DataLake and AUS also logged errors when the action was simply unregistered.

An internal isMissingActionHandlerError(error, action) matches the full Messenger message for the action the call site attempted, in either form. Each AUS and NetworkController call checks only its own action. A missing handler deeper in a delegated dependency, such as AUS lacking AuthenticationController:getBearerToken, is still reported, and so are injected-rewards failures. The helper is used only for optional actions:

Action Class When missing
SubscriptionController:getBenefits, :registerAddress optional no source / DI fallback
NetworkController:getState, :getNetworkClientById (rewards discount) optional no discount, debug log
AuthenticationController:getBearerToken (data lake) optional skipped, no retry, trace ended
AuthenticatedUserStorageService:get/putNotificationPreferences optional local watchlist kept; AUS stays source of truth on the next hydration (init, toggleTestnet, switchProvider)
Account lookups; RFF in the Lighter gate and the kill switch already silent unchanged
RFF in the ctor and startEligibilityMonitoring, GeolocationController, KeyringController:*, TransactionController, findNetworkClientIdByChainId required still an error

Toggles made while the AUS hydration read is in flight are now replayed onto the remote watchlist instead of being overwritten (a pre-existing race). Tests use a real child Messenger that delegates only the given actions, plus real nested Messenger calls for handlers that fail on a missing dependency of their own.

References

  • Undelegated-action errors seen in the Web Terminal host. Its stub handlers can go after this lands.
  • Follow-up: a typed MissingActionHandlerError (action and calling namespace) in @metamask/messenger would replace the message match, and separate a handler re-calling the same action from a truly missing one.

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

Note

Medium Risk
Changes fee, reporting, and watchlist sync behavior across messenger integrations; degradation is intentional but affects how partial hosts behave at runtime.

Overview
Hosts that do not register or delegate optional messenger actions (e.g. Web Terminal) no longer get error logs and retries for integrations Perps can live without. A shared isMissingActionHandlerError helper treats both “has not been registered” and “has not been delegated to …” as a missing handler only for the action that was called, so failures inside a delegated handler (missing nested dependency) still surface as real errors.

Optional paths now degrade: subscription benefits/registration fall back like before; rewards discount skips with a debug log when NetworkController reads are absent; data-lake reporting skips without retry and closes the trace when there is no bearer token; AUS watchlist read/write skips keep local watchlist state instead of reverting toggles. Required actions (geolocation, keyring, etc.) are unchanged.

Watchlist: toggles made while an AUS hydration read is in flight are recorded and replayed on top of the remote list when it arrives, fixing a race where late hydration could wipe user favorites.

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

@abretonc7s
abretonc7s marked this pull request as ready for review October 2, 2026 09:36
Scope isMissingActionHandlerError to the action a call site attempted and
anchor it to the full Messenger message, so a missing handler deeper in a
delegated dependency is still reported. Narrow the rewards-discount catch to
the NetworkController reads, treat a missing AUS put like a missing get, and
keep the helper internal.
@abretonc7s
abretonc7s requested review from a team as code owners October 2, 2026 09:36
@abretonc7s
abretonc7s deployed to default-branch October 2, 2026 09:36 — with GitHub Actions Active
@abretonc7s abretonc7s changed the title fix(perps-controller): degrade quietly when optional messenger actions are not delegated fix(perps-controller): degrade quietly when optional messenger actions are missing Oct 2, 2026
…its own action

Check the AUS read and write, and the two NetworkController reads, each
against only the action that call attempted, so a handler failing on a
missing handler for the other action is still reported. End the data-lake
trace when a report is skipped. Document that AUS stays the source of truth
when only its read is delegated.
Replay toggles made while the AUS hydration read is in flight onto the
remote watchlist instead of overwriting them, and document which
re-initializations replace toggles when only the AUS read is delegated.
Report the getNetworkClientById cause in the chain ID error, and cover each
nested boundary with real Messenger calls.
@abretonc7s
abretonc7s added this pull request to the merge queue Oct 2, 2026
Merged via the queue into main with commit baffdf6 Oct 2, 2026
45 checks passed
@abretonc7s
abretonc7s deleted the fix/perps-optional-messenger-delegation branch October 2, 2026 23:49
Naz-Ovh pushed a commit to 0x-fork/metamask-core that referenced this pull request Oct 7, 2026
…ions and failed writes (MetaMask#10670)

## Explanation

This is a follow-up to MetaMask#10665. Two existing races could lose a watchlist
star.

- **Overlapping hydrations:** `#performInitialization` replaced
`#ausQueue` with the AUS hydration instead of chaining onto it. Here is
how a star got lost with full AUS:
  1. The user stars a market while hydration is in flight.
2. `toggleTestnet()` or `switchProvider()` starts a second hydration at
once. Its read doesn't include the star, because the star hasn't been
written yet.
3. That read overwrites local state, and the star's queued write then
saves the list without it.
- **Failed write:** a rejected PUT restored the list from before the
toggle. That discarded markets hydrated since then and later toggles, so
a later star could be lost locally and remotely.

The fix:
- Hydration now runs through `#ausQueue`, so its read waits for earlier
writes. Its edit list is registered when it is queued, so a toggle made
while it waits is replayed onto the remote list too.
- A failed write undoes only its own toggle. A removed market goes back
to its position. The undo is skipped when a later toggle of the same
market is pending, because that toggle's own sync decides the market's
state.

No in-flight hydration can still hold the failed edit. Any hydration
that recorded it was queued before the edit's write, so it has finished
by the time the write fails.

With only the AUS read delegated, a hydration queued after a star still
replaces it, as MetaMask#10665 documents. A test pins this. Trade-off:
re-initialization no longer replaces the AUS queue, so a hydration read
that never settles now also holds later watchlist writes. Stars still
apply locally, and init stays non-blocking.

## References

- Follow-up to MetaMask#10665.
- Follow-ups: bound queued AUS hydration with a deadline that also stops
a late read from hydrating; when the latest toggle of a market fails
after earlier toggles of it, set the market from what AUS holds rather
than undoing only the latest; an earlier successful whole-list write may
already hold a later edit (pre-existing).

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [ ] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes serialized AUS watchlist sync and optimistic rollback in
user-facing favorites; wrong ordering could still lose stars or block
writes if hydration hangs, but scope is isolated to perps watchlist
persistence.
> 
> **Overview**
> Fixes **watchlist favorite races** when syncing with Authenticated
User Storage (AUS), following MetaMask#10665.
> 
> **Overlapping hydrations:** Re-init (`toggleTestnet`,
`switchProvider`) no longer **replaces** `#ausQueue` with a new
hydration. Hydration is **chained** so its read runs after pending
toggle writes, and toggles made while hydration is **queued** are
recorded and replayed on the remote list (not only in-flight reads).
> 
> **Failed AUS writes:** On PUT failure, the controller **reverts only
the failed toggle** (re-insert removed symbols at their prior index)
instead of restoring a pre-toggle snapshot, so hydrated markets and
other toggles are not wiped. A per-`network:symbol` **pending toggle
counter** skips rollback when a later toggle of the same market
superseded the failed one.
> 
> Changelog and state tests (stateful AUS store) document the trade-off:
a hydration read that never settles can block later writes on the queue.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
5431555. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Naz-Ovh pushed a commit to 0x-fork/metamask-core that referenced this pull request Oct 7, 2026
## Explanation

Releases `@metamask/perps-controller` **19.0.0 → 20.0.0**. The monorepo
version goes **1314.0.0 → 1315.0.0**.

No other package is being published. Perps-controller has no in-monorepo
dependents that need a workspace range bump.

The bump is **major**. Public unions and Lighter order validation
change:

- **BREAKING:** Lighter `placeOrder` / `validateOrder` accept supported
native attached TP/SL instead of refusing all attachments. Gate
forwarding on `attachedTpsl`.
([MetaMask#10638](MetaMask#10638))
- **BREAKING:** Grouped Lighter signer calls require grouping/count
`1/2` or `2/2` with two orders, or `3/3` with three orders.
([MetaMask#10638](MetaMask#10638))
- **BREAKING:** `ScaleOrderChild.state` adds `canceled`.
([MetaMask#10638](MetaMask#10638))
- **BREAKING:** `DirectProviderOrderCapabilitiesUnavailableReason` and
`OrderCapabilitiesUnavailableReason` add `order_market_unsupported`.
([MetaMask#10638](MetaMask#10638))
- **BREAKING:** Stray `triggerPrice` on Lighter basic market/limit
orders is refused with `ORDER_TRIGGER_PRICE_NOT_SUPPORTED`.
([MetaMask#10638](MetaMask#10638))
- **BREAKING:** Lighter position TP/SL replacement and removal preserve
independent partial triggers.
([MetaMask#10638](MetaMask#10638))
- **BREAKING:** `OrderFill.pnl` is optional when the venue omits
realized PnL. Treat missing as unknown, not zero.
([MetaMask#10605](MetaMask#10605))

Also ships Lighter Scale/Chase/TWAP probe work, fee quote attribution
(`feeSource`, `metamaskFeeDiscountBips`), HyperLiquid agent/signing
fixes, quieter optional-messenger failures, and watchlist
hydration/write races.

## Changelog

Moved Unreleased entries in `packages/perps-controller/CHANGELOG.md`
under `[20.0.0]`. Removed Uncategorized monorepo release markers
(1311–1314) that do not affect package consumers. Dropped a duplicate
Chase probe summary already covered by more specific entries.

## References

- Source PRs: [MetaMask#10638](MetaMask#10638),
[MetaMask#10618](MetaMask#10618),
[MetaMask#10605](MetaMask#10605),
[MetaMask#10643](MetaMask#10643),
[MetaMask#10650](MetaMask#10650),
[MetaMask#10651](MetaMask#10651),
[MetaMask#10665](MetaMask#10665),
[MetaMask#10670](MetaMask#10670),
[MetaMask#10683](MetaMask#10683)
- No in-monorepo consumer packages to bump. Mobile and Extension
exhaustive union matches need the new members on upgrade. Lighter
attached TP/SL stays gated on `attachedTpsl` and client rollout.

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've communicated my changes to consumers by [updating changelogs
for packages I've
changed](https://github.com/MetaMask/core/tree/main/docs/processes/updating-changelogs.md)
- [x] I've introduced [breaking
changes](https://github.com/MetaMask/core/tree/main/docs/processes/breaking-changes.md)
in this PR and have prepared draft pull requests for clients and
consumer packages to resolve them

Made with [Cursor](https://cursor.com)

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
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.

2 participants