From ee22c58fc5d9945a14df40558e1fde648e3f2b8b Mon Sep 17 00:00:00 2001 From: dan437 <80175477+dan437@users.noreply.github.com> Date: Wed, 8 Jul 2026 14:30:57 +0200 Subject: [PATCH 1/2] fix(transaction-pay-controller): subscribe to all asset event sources unconditionally subscribeAssetChanges picked a single asset event source at construction time based on the assetsUnifyState feature flag: AssetsController when enabled, or the legacy Tokens/TokenRates/CurrencyRate controllers otherwise. On a fresh profile the flag hasn't loaded from remote config yet, so the controller subscribes to the legacy source. Once the flag turns on, reads switch to AssetsController, whose events were never subscribed to, so required tokens for in-flight transactions never re-parse and consuming UI can hang on a loading state indefinitely. Subscribe to all four sources unconditionally instead of branching on the flag. The handler is idempotent, so extra events from an inactive source are harmless no-ops. --- .../transaction-pay-controller/CHANGELOG.md | 5 +++++ .../src/utils/transaction.test.ts | 16 ++++---------- .../src/utils/transaction.ts | 21 +++++++++++-------- 3 files changed, 21 insertions(+), 21 deletions(-) diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index ffcac29a48..557201484d 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Subscribe to all asset event sources (`AssetsController`, `TokensController`, `TokenRatesController`, `CurrencyRateController`) unconditionally instead of picking one based on the unify-state feature flag at construction time ([#9427](https://github.com/MetaMask/core/pull/9427)) + - The flag can flip from disabled to enabled after this controller is constructed (e.g. remote feature flags loading after startup), which previously left the controller subscribed to a source that never fired, causing required tokens to never resolve for transactions started before the flag loaded. + ### Changed - Refactor vault deposit utilities into shared `utils/` modules (`chomp`, `ma-vault-deposit`, `relay-post-ma-vault`) to prepare for the Relay Money Account deposit path ([#9303](https://github.com/MetaMask/core/pull/9303)) diff --git a/packages/transaction-pay-controller/src/utils/transaction.test.ts b/packages/transaction-pay-controller/src/utils/transaction.test.ts index edeab6a8b9..97068ab8d5 100644 --- a/packages/transaction-pay-controller/src/utils/transaction.test.ts +++ b/packages/transaction-pay-controller/src/utils/transaction.test.ts @@ -17,7 +17,6 @@ import type { TransactionPayControllerState, TransactionPayRequiredToken, } from '../types'; -import { getAssetsUnifyStateFeature } from './feature-flags'; import { parseRequiredTokens } from './required-tokens'; import { FINALIZED_STATUSES, @@ -31,7 +30,6 @@ import { waitForTransactionConfirmed, } from './transaction'; -jest.mock('./feature-flags'); jest.mock('./required-tokens'); const TRANSACTION_ID_MOCK = '123-456'; @@ -56,9 +54,6 @@ const TRANSCTION_TOKEN_REQUIRED_MOCK = { describe('Transaction Utils', () => { const parseRequiredTokensMock = jest.mocked(parseRequiredTokens); - const getAssetsUnifyStateFeatureMock = jest.mocked( - getAssetsUnifyStateFeature, - ); const { messenger, getTransactionControllerStateMock, @@ -72,8 +67,6 @@ describe('Transaction Utils', () => { getTransactionControllerStateMock.mockReturnValue({ transactions: [] as TransactionMeta[], } as TransactionControllerState); - - getAssetsUnifyStateFeatureMock.mockReturnValue(false); }); describe('getTransaction', () => { @@ -301,10 +294,9 @@ describe('Transaction Utils', () => { expect(updateTransactionDataMock).toHaveBeenCalledTimes(1); }); - it('subscribes to AssetsController state changes when unify-state feature is enabled', () => { + it('subscribes to AssetsController state changes', () => { const updateTransactionDataMock = jest.fn(); - getAssetsUnifyStateFeatureMock.mockReturnValue(true); parseRequiredTokensMock.mockReturnValue([TRANSCTION_TOKEN_REQUIRED_MOCK]); isolatedGetTransactionControllerStateMock.mockReturnValue({ transactions: [TRANSACTION_META_MOCK], @@ -321,10 +313,10 @@ describe('Transaction Utils', () => { expect(updateTransactionDataMock).toHaveBeenCalledTimes(1); }); - it('does not subscribe to per-source events when unify-state feature is enabled', () => { + it('subscribes to all per-source events in addition to AssetsController', () => { const updateTransactionDataMock = jest.fn(); - getAssetsUnifyStateFeatureMock.mockReturnValue(true); + parseRequiredTokensMock.mockReturnValue([TRANSCTION_TOKEN_REQUIRED_MOCK]); isolatedGetTransactionControllerStateMock.mockReturnValue({ transactions: [TRANSACTION_META_MOCK], } as TransactionControllerState); @@ -339,7 +331,7 @@ describe('Transaction Utils', () => { isolatedPublish('TokenRatesController:stateChange', {} as never, []); isolatedPublish('CurrencyRateController:stateChange', {} as never, []); - expect(updateTransactionDataMock).not.toHaveBeenCalled(); + expect(updateTransactionDataMock).toHaveBeenCalledTimes(3); }); it('skips transactions whose tokens are already populated', () => { diff --git a/packages/transaction-pay-controller/src/utils/transaction.ts b/packages/transaction-pay-controller/src/utils/transaction.ts index b8a64c31b7..fa2b8c8f1e 100644 --- a/packages/transaction-pay-controller/src/utils/transaction.ts +++ b/packages/transaction-pay-controller/src/utils/transaction.ts @@ -17,7 +17,6 @@ import type { TransactionPayControllerState, UpdateTransactionDataCallback, } from '../types'; -import { getAssetsUnifyStateFeature } from './feature-flags'; import { rpcRequest } from './provider'; import { parseRequiredTokens } from './required-tokens'; import { getNativeToken } from './token'; @@ -115,6 +114,14 @@ export function subscribeTransactionChanges( * Subscribe to asset state changes and re-parse required tokens for * in-flight transactions whose tokens have not yet resolved. * + * Subscribes to all known asset event sources unconditionally, rather than + * choosing a single source based on the unify-state feature flag. The flag + * value can change between when this controller is constructed and when it + * is read elsewhere (e.g. remote feature flags loading after startup), so + * relying on it here to pick a single source risks missing the events that + * actually fire. The handler is idempotent, so subscribing to extra sources + * that never fire is harmless. + * * @param messenger - Controller messenger. * @param getControllerState - Callback returning the current controller state. * @param updateTransactionData - Callback to update transaction data. @@ -146,14 +153,10 @@ export function subscribeAssetChanges( } }; - if (getAssetsUnifyStateFeature(messenger)) { - messenger.subscribe( - 'AssetsController:stateChange', - buildHandler('AssetsController'), - ); - return; - } - + messenger.subscribe( + 'AssetsController:stateChange', + buildHandler('AssetsController'), + ); messenger.subscribe( 'TokensController:stateChange', buildHandler('TokensController'), From 59277a0e735e38a82ec805c26ca9a25893c20c20 Mon Sep 17 00:00:00 2001 From: dan437 <80175477+dan437@users.noreply.github.com> Date: Wed, 8 Jul 2026 15:37:58 +0200 Subject: [PATCH 2/2] fix(transaction-pay-controller): fix changelog category order Keep a Changelog orders sections as Added, Changed, Deprecated, Removed, Fixed, Security. Fixed was listed above Changed. --- packages/transaction-pay-controller/CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index 557201484d..c214f3a39a 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -7,11 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Fixed - -- Subscribe to all asset event sources (`AssetsController`, `TokensController`, `TokenRatesController`, `CurrencyRateController`) unconditionally instead of picking one based on the unify-state feature flag at construction time ([#9427](https://github.com/MetaMask/core/pull/9427)) - - The flag can flip from disabled to enabled after this controller is constructed (e.g. remote feature flags loading after startup), which previously left the controller subscribed to a source that never fired, causing required tokens to never resolve for transactions started before the flag loaded. - ### Changed - Refactor vault deposit utilities into shared `utils/` modules (`chomp`, `ma-vault-deposit`, `relay-post-ma-vault`) to prepare for the Relay Money Account deposit path ([#9303](https://github.com/MetaMask/core/pull/9303)) @@ -20,6 +15,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/assets-controller` from `^10.0.1` to `^10.1.0` ([#9411](https://github.com/MetaMask/core/pull/9411)) - Bump `@metamask/transaction-controller` from `^68.2.2` to `^68.3.0` ([#9421](https://github.com/MetaMask/core/pull/9421)) +### Fixed + +- Subscribe to all asset event sources (`AssetsController`, `TokensController`, `TokenRatesController`, `CurrencyRateController`) unconditionally instead of picking one based on the unify-state feature flag at construction time ([#9427](https://github.com/MetaMask/core/pull/9427)) + - The flag can flip from disabled to enabled after this controller is constructed (e.g. remote feature flags loading after startup), which previously left the controller subscribed to a source that never fired, causing required tokens to never resolve for transactions started before the flag loaded. + ## [23.17.4] ### Fixed