Feat/subscription delegation update - #10339
Merged
Merged
Conversation
…oval confirmation
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
…ult readiness before subscription approval
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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 21de504. Configure here.
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
tanguyenvn
reviewed
Sep 29, 2026
tanguyenvn
previously approved these changes
Sep 29, 2026
GuillaumeRx
previously approved these changes
Sep 29, 2026
GuillaumeRx
left a comment
GuillaumeRx
left a comment
Contributor
There was a problem hiding this comment.
LGTM for core-platform
tuna1207
disabled auto-merge
September 29, 2026 09:18
Member
Author
|
@metamaskbot publish-previews |
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
tuna1207
enabled auto-merge
September 29, 2026 10:25
tanguyenvn
previously approved these changes
Sep 29, 2026
GuillaumeRx
requested changes
Sep 29, 2026
|
|
||
| ### Changed | ||
|
|
||
| - **BREAKING:** Grant `SubscriptionDelegationService` access to the additional messenger actions required by `SubscriptionDelegationService:startSubscriptionWithDelegation` ([#10339](https://github.com/MetaMask/core/pull/10339)) |
Contributor
There was a problem hiding this comment.
Sorry, I missed this. It's not breaking on the wallet side
GuillaumeRx
approved these changes
Sep 29, 2026
tanguyenvn
approved these changes
Sep 29, 2026
3 of 4 tasks
pull Bot
pushed a commit
to Reality2byte/core
that referenced
this pull request
Sep 29, 2026
## Explanation
The crypto subscription start flow and `getSubscriptions` were both
failing on successful API responses because our response structs did not
match what the Subscription API actually returns.
### Crypto start response shape
`SubscriptionService:startSubscriptionWithCrypto` validated the `POST
/subscriptions/crypto` response against
`StartCryptoSubscriptionResponseStruct` (`{ subscriptionId, status }`).
The API actually returns the full created `Subscription` object (crypto
subscriptions are created immediately, unlike card checkout which
returns a checkout session URL). Because the response never had a
`subscriptionId` field, `create()` threw on every successful call, so
the crypto start flow always failed after the API had already created
the subscription.
This PR:
- Removes `StartCryptoSubscriptionResponseStruct` and validates the
response against `SubscriptionStruct` instead.
- Redefines `StartCryptoSubscriptionResponse` as an alias of
`Subscription`. This is **BREAKING** for consumers reading
`response.subscriptionId`; they should read `response.id` instead.
`response.status` is unchanged. This affects
`SubscriptionService:startSubscriptionWithCrypto`,
`SubscriptionController:startSubscriptionWithCrypto`, and
`SubscriptionDelegationService:startSubscriptionWithDelegation`.
### Optional fields on `Subscription`
`SubscriptionStruct` required `lastInvoice.updatedAt` and
`paymentMethod.card.displayBrand`, but both are optional in the API. Any
subscription with a `lastInvoice` (i.e. anything that has been billed at
least once) failed validation and `getSubscriptions` threw. Both fields
are now optional in the struct and on the `SubscriptionInvoice` /
`SubscriptionCardPaymentMethod` types.
### `awaiting_funds` status
The API returns an `awaiting_funds` status for crypto subscriptions that
were created but whose first invoice has not been funded yet. This
status was not in `SUBSCRIPTION_STATUSES`, so such subscriptions also
failed validation. This PR adds `SUBSCRIPTION_STATUSES.awaitingFunds`
and teaches `SubscriptionController:submitSubscriptionCryptoApproval` to
treat it like `past_due` / `unpaid`: submitting a new approval for a
subscription in this state updates the existing subscription's payment
method instead of attempting to start a new one.
### Tests
- `SubscriptionService.test.ts`: crypto start tests now use full
subscription fixtures; added cases for a missing `updatedAt`, a missing
`displayBrand`, the `awaiting_funds` status, pass-through of additional
subscription fields, and rejection of a non-subscription response.
- `SubscriptionController.test.ts` and
`SubscriptionDelegationService.test.ts`: updated to the new response
shape and added coverage for the `awaiting_funds` branch in
`submitSubscriptionCryptoApproval`.
## References
- Follow-up to MetaMask#10339 (subscription delegation update), which added the
crypto start flow this fixes.
<!-- Add client PRs adopting the `response.subscriptionId` ->
`response.id` change here -->
## 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**
> Breaking return type on crypto subscription start affects all
consumers; changes subscription status handling and crypto approval
routing for payment recovery.
>
> **Overview**
> Fixes crypto subscription and `getSubscriptions` flows that **threw on
successful API responses** because client validation did not match the
Subscription API.
>
> **Breaking:** `StartCryptoSubscriptionResponse` is now the full
created **`Subscription`** (validated with `SubscriptionStruct`), not `{
subscriptionId, status }`. Callers must use **`response.id`** instead of
`response.subscriptionId` on `startSubscriptionWithCrypto` and
delegation start paths.
>
> **Validation fixes:** `lastInvoice.updatedAt` and card
**`displayBrand`** are optional so billed subscriptions and card payment
methods no longer fail `getSubscriptions`. Adds
**`SUBSCRIPTION_STATUSES.awaitingFunds`** for crypto subs waiting on
first-invoice funding.
>
> **Behavior:** `submitSubscriptionCryptoApproval` treats
**`awaiting_funds`** like `past_due` / `unpaid`—a new approval **updates
the existing subscription’s payment method** instead of starting a new
subscription.
>
> Tests and changelog updated for the new response shape, optional
fields, and the awaiting-funds approval branch.
>
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
82a2b98. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
3 of 4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
Adds
SubscriptionDelegationService:startSubscriptionWithDelegation, providing a single-approval subscription checkout flow that:ApprovalController.This also:
The authorization bundle is sent to confirmations so the complete effective permission set can be displayed. The local
assertTrialEligibilityguard is removed before the backend request.References
Checklist
Note
High Risk
Orchestrates user consent, MM Pay funding, delegation signing, and subscription creation with a breaking messenger contract—errors or ordering bugs could affect payments and recurring charges.
Overview
Adds
SubscriptionDelegationService:startSubscriptionWithDelegation, an end-to-end Money Account Plus checkout that refreshes subscription state, ensures vault readiness viaMoneyAccountUpgradeController:forceUpgradeAccount, shows a singlesubscription_delegationapproval (immutable permission bundle + mUSD funding), then signs/commits the recurring payment delegation (CHOMP verify, AUS persist, active intent) and callsstartSubscriptionWithCryptowithassertTrialEligibility: true. OptionalskipApprovalskips consent/funding when the caller already handled it.Breaking:
SubscriptionDelegationServiceMessengermust delegateApprovalController:addRequest,MoneyAccountUpgradeController:forceUpgradeAccount, and severalSubscriptionController:*actions; default wallet initialization is updated accordingly. New dependencies on approval-controller and money-account-upgrade-controller.SubscriptionController:startSubscriptionWithCryptogains optionalassertTrialEligibilityand throwsTrialEligibilityChangedwhen trial intent no longer matches authoritative state after authorization. Supporting exports include EIP-712typed-datahelpers and approval/bundle types; CHOMP intent registration now uses the typedcash-subscriptionmetadata without alpha workarounds.Reviewed by Cursor Bugbot for commit dafb59d. Bugbot is set up for automated code reviews on this repo. Configure here.