From 0e5e4dc23f9ebe8ae47a2a42e1487fa3e075ad32 Mon Sep 17 00:00:00 2001 From: Tuna Date: Tue, 25 Nov 2025 17:24:33 +0700 Subject: [PATCH 1/6] feat: handle subscription payment method change transaction --- .../src/SubscriptionController.ts | 54 +++++++++++++------ 1 file changed, 39 insertions(+), 15 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionController.ts b/packages/subscription-controller/src/SubscriptionController.ts index 6a654d39fed..10132696331 100644 --- a/packages/subscription-controller/src/SubscriptionController.ts +++ b/packages/subscription-controller/src/SubscriptionController.ts @@ -37,6 +37,7 @@ import type { import { PAYMENT_TYPES, PRODUCT_TYPES, + SUBSCRIPTION_STATUSES, type ISubscriptionService, type PricingResponse, type ProductType, @@ -509,26 +510,49 @@ export class SubscriptionController extends StaticIntervalPollingController()< lastSelectedPaymentMethod[PRODUCT_TYPES.SHIELD]; this.#assertIsPaymentMethodCrypto(lastSelectedPaymentMethodShield); - const isTrialed = trialedProducts?.includes(PRODUCT_TYPES.SHIELD); - const productPrice = this.#getProductPriceByProductAndPlan( PRODUCT_TYPES.SHIELD, lastSelectedPaymentMethodShield.plan, ); + const isTrialed = trialedProducts?.includes(PRODUCT_TYPES.SHIELD); + const currentSubscription = this.state.subscriptions.find((subscription) => + subscription.products.some((p) => p.name === PRODUCT_TYPES.SHIELD), + ); + // is shield subscription is active, this transaction is for changing payment method + const isChangePaymentMethod = + Boolean(currentSubscription) && + currentSubscription?.status !== SUBSCRIPTION_STATUSES.canceled; + + if (isChangePaymentMethod) { + if (!currentSubscription) { + throw new Error('Current subscription not found'); + } + await this.updatePaymentMethod({ + paymentType: PAYMENT_TYPES.byCrypto, + subscriptionId: currentSubscription.id, + chainId, + payerAddress: txMeta.txParams.from as Hex, + tokenSymbol: lastSelectedPaymentMethodShield.paymentTokenSymbol, + rawTransaction: rawTx as Hex, + recurringInterval: productPrice.interval, + billingCycles: productPrice.minBillingCycles, + }); + } else { + const params = { + products: [PRODUCT_TYPES.SHIELD], + isTrialRequested: !isTrialed, + recurringInterval: productPrice.interval, + billingCycles: productPrice.minBillingCycles, + chainId, + payerAddress: txMeta.txParams.from as Hex, + tokenSymbol: lastSelectedPaymentMethodShield.paymentTokenSymbol, + rawTransaction: rawTx as Hex, + isSponsored, + useTestClock: lastSelectedPaymentMethodShield.useTestClock, + }; + await this.startSubscriptionWithCrypto(params); + } - const params = { - products: [PRODUCT_TYPES.SHIELD], - isTrialRequested: !isTrialed, - recurringInterval: productPrice.interval, - billingCycles: productPrice.minBillingCycles, - chainId, - payerAddress: txMeta.txParams.from as Hex, - tokenSymbol: lastSelectedPaymentMethodShield.paymentTokenSymbol, - rawTransaction: rawTx as Hex, - isSponsored, - useTestClock: lastSelectedPaymentMethodShield.useTestClock, - }; - await this.startSubscriptionWithCrypto(params); // update the subscriptions state after subscription created in server await this.getSubscriptions(); } From 83955daf5ed2bf298f39a5040a9c9d37f00cc0d0 Mon Sep 17 00:00:00 2001 From: Tuna Date: Tue, 25 Nov 2025 17:26:43 +0700 Subject: [PATCH 2/6] chore: update changelog --- packages/subscription-controller/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/subscription-controller/CHANGELOG.md b/packages/subscription-controller/CHANGELOG.md index f1fb6d3f3cb..0b4f8c5fc15 100644 --- a/packages/subscription-controller/CHANGELOG.md +++ b/packages/subscription-controller/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Updated `submitShieldSubscriptionCryptoApproval` to handle change payment method transaction if subscription already existed ([#7231](https://github.com/MetaMask/core/pull/7231)) - Bump `@metamask/transaction-controller` from `^62.0.0` to `^62.2.0` ([#7215](https://github.com/MetaMask/core/pull/7215), [#7220](https://github.com/MetaMask/core/pull/7220)) - Move peer dependencies for controller and service packages to direct dependencies ([#7209](https://github.com/MetaMask/core/pull/7209)) - The dependencies moved are: From 671d48f06e98ce7fc29fdd594de877cde5022f26 Mon Sep 17 00:00:00 2001 From: Tuna Date: Tue, 25 Nov 2025 17:33:42 +0700 Subject: [PATCH 3/6] feat: update test case --- .../src/SubscriptionController.test.ts | 48 +++++++++++++++++++ .../src/SubscriptionController.ts | 5 +- 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionController.test.ts b/packages/subscription-controller/src/SubscriptionController.test.ts index eac28dde529..b4763443843 100644 --- a/packages/subscription-controller/src/SubscriptionController.test.ts +++ b/packages/subscription-controller/src/SubscriptionController.test.ts @@ -2095,5 +2095,53 @@ describe('SubscriptionController', () => { }, ); }); + + it('should update payment method when user has active subscription', async () => { + await withController( + { + state: { + pricing: MOCK_PRICE_INFO_RESPONSE, + trialedProducts: [], + subscriptions: [MOCK_SUBSCRIPTION], + lastSelectedPaymentMethod: { + [PRODUCT_TYPES.SHIELD]: { + type: PAYMENT_TYPES.byCrypto, + paymentTokenAddress: '0xtoken', + paymentTokenSymbol: 'USDT', + plan: RECURRING_INTERVALS.month, + }, + }, + }, + }, + async ({ controller, mockService }) => { + mockService.updatePaymentMethodCrypto.mockResolvedValue({}); + mockService.getSubscriptions.mockResolvedValue( + MOCK_GET_SUBSCRIPTIONS_RESPONSE, + ); + + const txMeta = { + ...generateMockTxMeta(), + type: TransactionType.shieldSubscriptionApprove, + chainId: '0x1' as Hex, + rawTx: '0x123', + txParams: { + data: '0x456', + from: '0x1234567890123456789012345678901234567890', + to: '0xtoken', + }, + status: TransactionStatus.submitted, + }; + + await controller.submitShieldSubscriptionCryptoApproval(txMeta); + + expect(mockService.updatePaymentMethodCrypto).toHaveBeenCalledTimes( + 1, + ); + expect( + mockService.startSubscriptionWithCrypto, + ).not.toHaveBeenCalled(); + }, + ); + }); }); }); diff --git a/packages/subscription-controller/src/SubscriptionController.ts b/packages/subscription-controller/src/SubscriptionController.ts index 10132696331..2685a694005 100644 --- a/packages/subscription-controller/src/SubscriptionController.ts +++ b/packages/subscription-controller/src/SubscriptionController.ts @@ -524,12 +524,9 @@ export class SubscriptionController extends StaticIntervalPollingController()< currentSubscription?.status !== SUBSCRIPTION_STATUSES.canceled; if (isChangePaymentMethod) { - if (!currentSubscription) { - throw new Error('Current subscription not found'); - } await this.updatePaymentMethod({ paymentType: PAYMENT_TYPES.byCrypto, - subscriptionId: currentSubscription.id, + subscriptionId: (currentSubscription as Subscription).id, chainId, payerAddress: txMeta.txParams.from as Hex, tokenSymbol: lastSelectedPaymentMethodShield.paymentTokenSymbol, From 63d1693b55c7382bc0dee85bf6eb8b9eed28eb67 Mon Sep 17 00:00:00 2001 From: Tuna Date: Tue, 25 Nov 2025 18:02:08 +0700 Subject: [PATCH 4/6] feat: refetch latest subscription before continue handling transaction --- .../src/SubscriptionController.test.ts | 9 ++++++--- .../src/SubscriptionController.ts | 2 ++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionController.test.ts b/packages/subscription-controller/src/SubscriptionController.test.ts index b4763443843..cfeff4d210d 100644 --- a/packages/subscription-controller/src/SubscriptionController.test.ts +++ b/packages/subscription-controller/src/SubscriptionController.test.ts @@ -1891,9 +1891,12 @@ describe('SubscriptionController', () => { status: SUBSCRIPTION_STATUSES.trialing, }); - mockService.getSubscriptions.mockResolvedValue( - MOCK_GET_SUBSCRIPTIONS_RESPONSE, - ); + mockService.getSubscriptions + .mockResolvedValueOnce({ + subscriptions: [], + trialedProducts: [], + }) + .mockResolvedValue(MOCK_GET_SUBSCRIPTIONS_RESPONSE); // Create a shield subscription approval transaction const txMeta = { diff --git a/packages/subscription-controller/src/SubscriptionController.ts b/packages/subscription-controller/src/SubscriptionController.ts index 2685a694005..4c79c53668d 100644 --- a/packages/subscription-controller/src/SubscriptionController.ts +++ b/packages/subscription-controller/src/SubscriptionController.ts @@ -515,6 +515,8 @@ export class SubscriptionController extends StaticIntervalPollingController()< lastSelectedPaymentMethodShield.plan, ); const isTrialed = trialedProducts?.includes(PRODUCT_TYPES.SHIELD); + // get the latest subscriptions state to check if the user has an active shield subscription + await this.getSubscriptions(); const currentSubscription = this.state.subscriptions.find((subscription) => subscription.products.some((p) => p.name === PRODUCT_TYPES.SHIELD), ); From c879cd7137534a67e1b8055561fc2ef5e70802ba Mon Sep 17 00:00:00 2001 From: Tuna Date: Thu, 27 Nov 2025 09:41:38 +0700 Subject: [PATCH 5/6] feat: update subscription can do crypto approval logic --- .../src/SubscriptionController.test.ts | 47 +++++++++++++++++++ .../src/SubscriptionController.ts | 39 +++++++++++++-- .../subscription-controller/src/constants.ts | 1 + 3 files changed, 83 insertions(+), 4 deletions(-) diff --git a/packages/subscription-controller/src/SubscriptionController.test.ts b/packages/subscription-controller/src/SubscriptionController.test.ts index cfeff4d210d..cbb04c0143b 100644 --- a/packages/subscription-controller/src/SubscriptionController.test.ts +++ b/packages/subscription-controller/src/SubscriptionController.test.ts @@ -2146,5 +2146,52 @@ describe('SubscriptionController', () => { }, ); }); + + it('should throw error when subscription status is not valid for crypto approval', 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 ({ controller, mockService }) => { + mockService.getSubscriptions.mockResolvedValue({ + subscriptions: [ + { ...MOCK_SUBSCRIPTION, status: SUBSCRIPTION_STATUSES.incomplete }, + ], + trialedProducts: [], + }); + + const txMeta = { + ...generateMockTxMeta(), + type: TransactionType.shieldSubscriptionApprove, + chainId: '0x1' as Hex, + rawTx: '0x123', + txParams: { + data: '0x456', + from: '0x1234567890123456789012345678901234567890', + to: '0xtoken', + }, + status: TransactionStatus.submitted, + }; + + await expect( + controller.submitShieldSubscriptionCryptoApproval(txMeta), + ).rejects.toThrow( + SubscriptionControllerErrorMessage.SubscriptionNotValidForCryptoApproval, + ); + }, + ); + }); }); }); diff --git a/packages/subscription-controller/src/SubscriptionController.ts b/packages/subscription-controller/src/SubscriptionController.ts index 4c79c53668d..ffdb2c2cdbc 100644 --- a/packages/subscription-controller/src/SubscriptionController.ts +++ b/packages/subscription-controller/src/SubscriptionController.ts @@ -33,6 +33,7 @@ import type { CachedLastSelectedPaymentMethod, SubmitSponsorshipIntentsMethodParams, RecurringInterval, + SubscriptionStatus, } from './types'; import { PAYMENT_TYPES, @@ -520,10 +521,12 @@ export class SubscriptionController extends StaticIntervalPollingController()< const currentSubscription = this.state.subscriptions.find((subscription) => subscription.products.some((p) => p.name === PRODUCT_TYPES.SHIELD), ); - // is shield subscription is active, this transaction is for changing payment method - const isChangePaymentMethod = - Boolean(currentSubscription) && - currentSubscription?.status !== SUBSCRIPTION_STATUSES.canceled; + + this.#assertValidSubscriptionStateForCryptoApproval({ + product: PRODUCT_TYPES.SHIELD, + }); + // if shield subscription exists, this transaction is for changing payment method + const isChangePaymentMethod = Boolean(currentSubscription); if (isChangePaymentMethod) { await this.updatePaymentMethod({ @@ -822,6 +825,34 @@ export class SubscriptionController extends StaticIntervalPollingController()< return productPrice; } + #assertValidSubscriptionStateForCryptoApproval({ + product, + }: { + product: ProductType; + }) { + const subscription = this.state.subscriptions.find((sub) => + sub.products.some((p) => p.name === product), + ); + + const isValid = + !subscription || + ( + [ + SUBSCRIPTION_STATUSES.pastDue, + SUBSCRIPTION_STATUSES.unpaid, + SUBSCRIPTION_STATUSES.paused, + SUBSCRIPTION_STATUSES.provisional, + SUBSCRIPTION_STATUSES.active, + SUBSCRIPTION_STATUSES.trialing, + ] as SubscriptionStatus[] + ).includes(subscription.status); + if (!isValid) { + throw new Error( + SubscriptionControllerErrorMessage.SubscriptionNotValidForCryptoApproval, + ); + } + } + #assertIsUserNotSubscribed({ products }: { products: ProductType[] }) { const subscription = this.state.subscriptions.find((sub) => sub.products.some((p) => products.includes(p.name)), diff --git a/packages/subscription-controller/src/constants.ts b/packages/subscription-controller/src/constants.ts index d5bdc8ed0f7..a86a1e22dc0 100644 --- a/packages/subscription-controller/src/constants.ts +++ b/packages/subscription-controller/src/constants.ts @@ -45,6 +45,7 @@ export enum SubscriptionControllerErrorMessage { PaymentTokenAddressAndSymbolRequiredForCrypto = `${controllerName} - Payment token address and symbol are required for crypto payment`, PaymentMethodNotCrypto = `${controllerName} - Payment method is not crypto`, ProductPriceNotFound = `${controllerName} - Product price not found`, + SubscriptionNotValidForCryptoApproval = `${controllerName} - Subscription is not valid for crypto approval`, } export const DEFAULT_POLLING_INTERVAL = 5 * 60 * 1_000; // 5 minutes From 93c103b628e35869c4ac5bfb42df219fbf777365 Mon Sep 17 00:00:00 2001 From: Tuna Date: Thu, 27 Nov 2025 11:31:17 +0700 Subject: [PATCH 6/6] fix: lint --- .../src/SubscriptionController.test.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/subscription-controller/src/SubscriptionController.test.ts b/packages/subscription-controller/src/SubscriptionController.test.ts index cbb04c0143b..3b8ea5f9f1c 100644 --- a/packages/subscription-controller/src/SubscriptionController.test.ts +++ b/packages/subscription-controller/src/SubscriptionController.test.ts @@ -2167,7 +2167,10 @@ describe('SubscriptionController', () => { async ({ controller, mockService }) => { mockService.getSubscriptions.mockResolvedValue({ subscriptions: [ - { ...MOCK_SUBSCRIPTION, status: SUBSCRIPTION_STATUSES.incomplete }, + { + ...MOCK_SUBSCRIPTION, + status: SUBSCRIPTION_STATUSES.incomplete, + }, ], trialedProducts: [], });