feat: correct subscription response and statuses

packages/subscription-controller/CHANGELOG.md

@@ -15,9 +15,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   - Bind recurring permissions to the token, amount, period, start, and zero native value, and verify reusable delegation hashes with CHOMP.
   - Derive trial timing from authoritative subscription state.
   - Add optional `skipApproval` to `StartSubscriptionWithDelegationRequest` to bypass the `ApprovalController:addRequest` consent and funding step; the caller is then responsible for consent and funding.
+- Add `SUBSCRIPTION_STATUSES.awaitingFunds` (`awaiting_funds`) for crypto subscriptions that are waiting for the payer to fund the first invoice ([#10566](https://github.com/MetaMask/core/pull/10566))
+  - `SubscriptionController:submitSubscriptionCryptoApproval` treats it like `past_due` / `unpaid`: a new approval updates the existing subscription's payment method.
 
 ### Changed
 
+- **BREAKING:** `StartCryptoSubscriptionResponse` (returned by `SubscriptionService:startSubscriptionWithCrypto`, `SubscriptionController:startSubscriptionWithCrypto`, and `SubscriptionDelegationService:startSubscriptionWithDelegation`) is now the created `Subscription` instead of `{ subscriptionId, status }`, matching what `POST /subscriptions/crypto` actually returns ([#10566](https://github.com/MetaMask/core/pull/10566))
+  - Read `response.id` instead of `response.subscriptionId`. `response.status` is unchanged.
+  - Previously the response failed validation on every successful call, so the crypto start flow always threw.
 - **BREAKING:** Widen `SubscriptionDelegationServiceMessenger` so `SubscriptionDelegationService:startSubscriptionWithDelegation` can call the actions it needs ([#10339](https://github.com/MetaMask/core/pull/10339))
   - The messenger must now allow `ApprovalController:addRequest`, `MoneyAccountUpgradeController:forceUpgradeAccount`, `SubscriptionController:getState`, `SubscriptionController:getSubscriptions`, and `SubscriptionController:startSubscriptionWithCrypto`.
   - Clients that construct this messenger must delegate those actions before calling `startSubscriptionWithDelegation`. A messenger typed against the previous `SubscriptionDelegationServiceMessenger` is no longer assignable.
@@ -36,6 +41,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Bump `@metamask/money-account-utils` from `^2.0.1` to `^2.1.0` ([#10505](https://github.com/MetaMask/core/pull/10505))
 - Bump `@tanstack/query-core` from `^5.89.0` to `^5.103.2` ([#10511](https://github.com/MetaMask/core/pull/10511))
 
+### Fixed
+
+- Stop rejecting Subscription API responses whose `lastInvoice` omits `updatedAt` or whose card payment method omits `displayBrand`; both fields are optional in the API and are now optional on `SubscriptionInvoice` and `SubscriptionCardPaymentMethod` ([#10566](https://github.com/MetaMask/core/pull/10566))
+  - Previously a subscription with a `lastInvoice` failed validation and `getSubscriptions` threw.
+
 ## [10.0.1]
 
 ### Changed

packages/subscription-controller/src/SubscriptionController.test.ts

@@ -115,6 +115,19 @@ const MOCK_MONEY_ACCOUNT_SUBSCRIPTION: Subscription = {
   },
 };
 
+const MOCK_CRYPTO_SUBSCRIPTION: Subscription = {
+  ...MOCK_SUBSCRIPTION,
+  id: 'sub_crypto_123',
+  paymentMethod: {
+    type: PAYMENT_TYPES.byCrypto,
+    crypto: {
+      payerAddress: '0x0000000000000000000000000000000000000001',
+      chainId: '0x1',
+      tokenSymbol: 'USDC',
+    },
+  },
+};
+
 const MOCK_PRODUCT_PRICE: ProductPricing = {
   name: PRODUCT_TYPES.SHIELD,
   prices: [
@@ -2688,10 +2701,8 @@ describe('SubscriptionController', () => {
             rawTransaction: '0xdeadbeef',
           };
 
-          const response: StartCryptoSubscriptionResponse = {
-            subscriptionId: 'sub_crypto_123',
-            status: SUBSCRIPTION_STATUSES.active,
-          };
+          const response: StartCryptoSubscriptionResponse =
+            MOCK_CRYPTO_SUBSCRIPTION;
 
           mockService.startSubscriptionWithCrypto.mockResolvedValue(response);
           mockService.getSubscriptions
@@ -2824,10 +2835,9 @@ describe('SubscriptionController', () => {
             delegationHash: '0xabc',
           };
 
-          mockService.startSubscriptionWithCrypto.mockResolvedValue({
-            subscriptionId: 'sub_money_account',
-            status: SUBSCRIPTION_STATUSES.active,
-          });
+          mockService.startSubscriptionWithCrypto.mockResolvedValue(
+            MOCK_MONEY_ACCOUNT_SUBSCRIPTION,
+          );
           mockService.getSubscriptions
             .mockResolvedValueOnce({
               subscriptions: [],
@@ -2971,10 +2981,9 @@ describe('SubscriptionController', () => {
           },
         },
         async ({ rootMessenger, mockService }) => {
-          mockService.startSubscriptionWithCrypto.mockResolvedValue({
-            subscriptionId: 'sub_crypto_123',
-            status: SUBSCRIPTION_STATUSES.active,
-          });
+          mockService.startSubscriptionWithCrypto.mockResolvedValue(
+            MOCK_CRYPTO_SUBSCRIPTION,
+          );
           mockService.getSubscriptions
             .mockResolvedValueOnce(MOCK_EMPTY_GET_SUBSCRIPTIONS_RESPONSE)
             .mockResolvedValue(MOCK_GET_SUBSCRIPTIONS_RESPONSE);
@@ -3051,10 +3060,9 @@ describe('SubscriptionController', () => {
           },
         },
         async ({ rootMessenger, mockService }) => {
-          mockService.startSubscriptionWithCrypto.mockResolvedValue({
-            subscriptionId: 'sub_crypto_123',
-            status: SUBSCRIPTION_STATUSES.active,
-          });
+          mockService.startSubscriptionWithCrypto.mockResolvedValue(
+            MOCK_CRYPTO_SUBSCRIPTION,
+          );
           mockService.getSubscriptions
             .mockResolvedValueOnce({
               customerId: 'cus_1',
@@ -3099,10 +3107,9 @@ describe('SubscriptionController', () => {
           },
         },
         async ({ rootMessenger, mockService }) => {
-          mockService.startSubscriptionWithCrypto.mockResolvedValue({
-            subscriptionId: 'sub_money_account',
-            status: SUBSCRIPTION_STATUSES.active,
-          });
+          mockService.startSubscriptionWithCrypto.mockResolvedValue(
+            MOCK_MONEY_ACCOUNT_SUBSCRIPTION,
+          );
           mockService.getSubscriptions.mockResolvedValue({
             customerId: 'cus_1',
             subscriptions: [],
@@ -3146,10 +3153,9 @@ describe('SubscriptionController', () => {
           },
         },
         async ({ rootMessenger, mockService }) => {
-          mockService.startSubscriptionWithCrypto.mockResolvedValue({
-            subscriptionId: 'sub_crypto_123',
-            status: SUBSCRIPTION_STATUSES.active,
-          });
+          mockService.startSubscriptionWithCrypto.mockResolvedValue(
+            MOCK_CRYPTO_SUBSCRIPTION,
+          );
           mockService.getSubscriptions
             .mockResolvedValueOnce(MOCK_EMPTY_GET_SUBSCRIPTIONS_RESPONSE)
             .mockResolvedValue(MOCK_GET_SUBSCRIPTIONS_RESPONSE);
@@ -3187,10 +3193,9 @@ describe('SubscriptionController', () => {
           },
         },
         async ({ rootMessenger, mockService }) => {
-          mockService.startSubscriptionWithCrypto.mockResolvedValue({
-            subscriptionId: 'sub_crypto_123',
-            status: SUBSCRIPTION_STATUSES.active,
-          });
+          mockService.startSubscriptionWithCrypto.mockResolvedValue(
+            MOCK_CRYPTO_SUBSCRIPTION,
+          );
           mockService.getSubscriptions
             .mockResolvedValueOnce({
               customerId: 'cus_1',
@@ -3233,10 +3238,9 @@ describe('SubscriptionController', () => {
           },
         },
         async ({ rootMessenger, mockService }) => {
-          mockService.startSubscriptionWithCrypto.mockResolvedValue({
-            subscriptionId: 'sub_crypto_123',
-            status: SUBSCRIPTION_STATUSES.active,
-          });
+          mockService.startSubscriptionWithCrypto.mockResolvedValue(
+            MOCK_CRYPTO_SUBSCRIPTION,
+          );
           mockService.getSubscriptions
             .mockResolvedValueOnce({
               customerId: 'cus_1',
@@ -3281,10 +3285,9 @@ describe('SubscriptionController', () => {
           },
         },
         async ({ controller, rootMessenger, mockService }) => {
-          mockService.startSubscriptionWithCrypto.mockResolvedValue({
-            subscriptionId: 'sub_money_account',
-            status: SUBSCRIPTION_STATUSES.active,
-          });
+          mockService.startSubscriptionWithCrypto.mockResolvedValue(
+            MOCK_MONEY_ACCOUNT_SUBSCRIPTION,
+          );
           mockService.getSubscriptions
             .mockResolvedValueOnce({
               customerId: 'cus_1',
@@ -5990,6 +5993,66 @@ describe('SubscriptionController', () => {
       );
     });
 
+    it('should update payment method when the subscription is awaiting funds', async () => {
+      await withController(
+        {
+          state: {
+            pricing: MOCK_PRICE_INFO_RESPONSE,
+            trialedProducts: [],
+            subscriptions: [],
+            lastSelectedPaymentMethod: {
+              [PRODUCT_TYPES.SHIELD]: {
+                type: PAYMENT_TYPES.byCrypto,
+                paymentTokenAddress: '0xtoken',
+                paymentTokenSymbol: 'USDT',
+                plan: RECURRING_INTERVALS.month,
+              },
+            },
+          },
+        },
+        async ({ rootMessenger, mockService }) => {
+          mockService.updatePaymentMethodCrypto.mockResolvedValue(undefined);
+          mockService.getSubscriptions.mockResolvedValue({
+            subscriptions: [
+              {
+                ...MOCK_CRYPTO_SUBSCRIPTION,
+                status: SUBSCRIPTION_STATUSES.awaitingFunds,
+              },
+            ],
+            trialedProducts: [],
+          });
+
+          const txMeta = {
+            ...generateMockTxMeta(),
+            type: TransactionType.shieldSubscriptionApprove,
+            chainId: '0x1' as Hex,
+            rawTx: '0x123',
+            txParams: {
+              data: '0x456',
+              from: '0x1234567890123456789012345678901234567890',
+              to: '0xtoken',
+            },
+            status: TransactionStatus.submitted,
+          };
+
+          await rootMessenger.call(
+            'SubscriptionController:submitSubscriptionCryptoApproval',
+            {
+              productType: PRODUCT_TYPES.SHIELD,
+              txMeta,
+            },
+          );
+
+          expect(mockService.updatePaymentMethodCrypto).toHaveBeenCalledTimes(
+            1,
+          );
+          expect(
+            mockService.startSubscriptionWithCrypto,
+          ).not.toHaveBeenCalled();
+        },
+      );
+    });
+
     it('should throw error when subscription status is not valid for crypto approval', async () => {
       await withController(
         {

packages/subscription-controller/src/SubscriptionController.ts

@@ -1150,6 +1150,7 @@ export class SubscriptionController extends StaticIntervalPollingController()<
       !subscription ||
       (
         [
+          SUBSCRIPTION_STATUSES.awaitingFunds,
           SUBSCRIPTION_STATUSES.pastDue,
           SUBSCRIPTION_STATUSES.unpaid,
           SUBSCRIPTION_STATUSES.paused,

packages/subscription-controller/src/SubscriptionService-method-action-types.ts

@@ -64,7 +64,8 @@ export type SubscriptionServiceStartSubscriptionWithCardAction = {
  * Starts a subscription with a crypto payment method.
  *
  * @param request - The start crypto subscription request.
- * @returns The created subscription response.
+ * @returns The created subscription. Unlike card checkout, the Subscription
+ * API creates the subscription immediately and returns it in full.
  * @throws If `products` is empty.
  * @throws If the request does not use exactly one of `rawTransaction`
  * (ERC-20 approval) or `delegationHash` (delegation).

packages/subscription-controller/src/SubscriptionService-structs.ts

@@ -76,7 +76,7 @@ const SubscriptionCardPaymentMethodStruct = type({
   type: enums([PAYMENT_TYPES.byCard]),
   card: type({
     brand: string(),
-    displayBrand: string(),
+    displayBrand: optional(string()),
     last4: string(),
   }),
 });
@@ -119,7 +119,7 @@ export const SubscriptionStruct = type({
       id: string(),
       status: InvoicePaymentStatusStruct,
       errorCode: optional(CryptoPaymentErrorStruct),
-      updatedAt: string(),
+      updatedAt: optional(string()),
     }),
   ),
 });
@@ -172,11 +172,6 @@ export const StartSubscriptionResponseStruct = type({
   checkoutSessionUrl: string(),
 });
 
-export const StartCryptoSubscriptionResponseStruct = type({
-  subscriptionId: string(),
-  status: SubscriptionStatusStruct,
-});
-
 export const UpdatePaymentMethodCardResponseStruct = type({
   redirectUrl: string(),
 });