diff --git a/packages/transaction-pay-controller/CHANGELOG.md b/packages/transaction-pay-controller/CHANGELOG.md index ffcac29a48..c214f3a39a 100644 --- a/packages/transaction-pay-controller/CHANGELOG.md +++ b/packages/transaction-pay-controller/CHANGELOG.md @@ -15,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 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'),