diff --git a/package.json b/package.json index b23b1a9d3fc..ee2ef7a43b0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/core-monorepo", - "version": "151.0.0", + "version": "154.0.0", "private": true, "description": "Monorepo for packages shared between MetaMask clients", "repository": { diff --git a/packages/assets-controllers/CHANGELOG.md b/packages/assets-controllers/CHANGELOG.md index 98d0840d6e1..4b4d8116665 100644 --- a/packages/assets-controllers/CHANGELOG.md +++ b/packages/assets-controllers/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Uncategorized + +- **BREAKING:** The NftController messenger must now allow the `NetworkController:getNetworkClientById` action ([#4305](https://github.com/MetaMask/core/pull/4305)) +- NftControllerMessenger now makes use of `selectedNetworkClientId` when responding to changes in NetworkController state to capture the currently selected chain rather than `providerConfig` ([#4305](https://github.com/MetaMask/core/pull/4305)) + - This should be functionally equivalent, but is being noted anyway. +- NftDetectionController now makes use of `selectedNetworkClientId` when responding to changes in NetworkController state to capture the currently selected chain rather than `providerConfig` ([#4307](https://github.com/MetaMask/core/pull/4307)) + - This should be functionally equivalent, but is being noted anyway. + ## [29.0.0] ### Added diff --git a/packages/assets-controllers/README.md b/packages/assets-controllers/README.md index 95dcb8b0451..ea6618a2196 100644 --- a/packages/assets-controllers/README.md +++ b/packages/assets-controllers/README.md @@ -19,12 +19,41 @@ This package features the following controllers: - [**CollectibleDetectionController**](src/CollectibleDetectionController.ts) keeps a periodically updated list of ERC-721 tokens assigned to the currently selected address. - [**CollectiblesController**](src/CollectiblesController.ts) tracks ERC-721 and ERC-1155 tokens assigned to the currently selected address, using OpenSea to retrieve token information. - [**CurrencyRateController**](src/CurrencyRateController.ts) keeps a periodically updated value of the exchange rate from the currently selected "native" currency to another (handling testnet tokens specially). +- [**RatesController**](src/RatesController/RatesController.ts) keeps a periodically updated value for the exchange rates for different cryptocurrencies. The difference between the `RatesController` and `CurrencyRateController` is that the second one is coupled to the `NetworksController` and is EVM specific, whilst the first one can handle different blockchain currencies like BTC and SOL. - [**TokenBalancesController**](src/TokenBalancesController.ts) keeps a periodically updated set of balances for the current set of ERC-20 tokens. - [**TokenDetectionController**](src/TokenDetectionController.ts) keeps a periodically updated list of ERC-20 tokens assigned to the currently selected address. - [**TokenListController**](src/TokenListController.ts) uses the MetaSwap API to keep a periodically updated list of known ERC-20 tokens along with their metadata. - [**TokenRatesController**](src/TokenRatesController.ts) keeps a periodically updated list of exchange rates for known ERC-20 tokens relative to the currently selected native currency. - [**TokensController**](src/TokensController.ts) stores the ERC-20 and ERC-721 tokens, along with their metadata, that are listed in the wallet under the currently selected address on the currently selected chain. +### `RatesController` + +The `RatesController` is responsible for managing the state related to cryptocurrency exchange rates and periodically updating these rates by fetching new data from an external API. + +```ts +// Initialize the RatesController +const ratesController = new RatesController({ + interval: 180000, + includeUsdRate: true, + state: { + fiatCurrency: 'eur', + cryptocurrencies: [Cryptocurrency.Btc], + }, +}); + +// Start the polling process +ratesController.start().then(() => { + console.log('Polling for exchange rates has started.'); +}); + +// Stop the polling process after some time +setTimeout(() => { + ratesController.stop().then(() => { + console.log('Polling for exchange rates has stopped.'); + }); +}, 300000); +``` + ## Contributing This package is part of a monorepo. Instructions for contributing can be found in the [monorepo README](https://github.com/MetaMask/core#readme). diff --git a/packages/assets-controllers/src/CurrencyRateController.ts b/packages/assets-controllers/src/CurrencyRateController.ts index bf3509f3100..62a8069c5a5 100644 --- a/packages/assets-controllers/src/CurrencyRateController.ts +++ b/packages/assets-controllers/src/CurrencyRateController.ts @@ -14,7 +14,7 @@ import type { import { StaticIntervalPollingController } from '@metamask/polling-controller'; import { Mutex } from 'async-mutex'; -import { fetchExchangeRate as defaultFetchExchangeRate } from './crypto-compare'; +import { fetchExchangeRate as defaultFetchExchangeRate } from './crypto-compare-service'; /** * @type CurrencyRateState diff --git a/packages/assets-controllers/src/NftDetectionController.test.ts b/packages/assets-controllers/src/NftDetectionController.test.ts index 9f06ee94215..5f6ca9a8ff5 100644 --- a/packages/assets-controllers/src/NftDetectionController.test.ts +++ b/packages/assets-controllers/src/NftDetectionController.test.ts @@ -1,6 +1,14 @@ import { NFT_API_BASE_URL, ChainId, toHex } from '@metamask/controller-utils'; -import { NetworkClientType } from '@metamask/network-controller'; -import type { NetworkClient } from '@metamask/network-controller'; +import { + NetworkClientType, + defaultState as defaultNetworkState, +} from '@metamask/network-controller'; +import type { + NetworkClient, + NetworkClientConfiguration, + NetworkClientId, + NetworkState, +} from '@metamask/network-controller'; import { getDefaultPreferencesState, type PreferencesState, @@ -11,6 +19,10 @@ import * as sinon from 'sinon'; import { FakeBlockTracker } from '../../../tests/fake-block-tracker'; import { FakeProvider } from '../../../tests/fake-provider'; import { advanceTime } from '../../../tests/helpers'; +import { + buildCustomNetworkClientConfiguration, + buildMockGetNetworkClientById, +} from '../../network-controller/tests/helpers'; import { Source } from './constants'; import { getDefaultNftState, type NftState } from './NftController'; import { @@ -264,7 +276,6 @@ describe('NftDetectionController', () => { }, ], }); - console.log(nock.activeMocks()); }); afterEach(() => { @@ -286,9 +297,11 @@ describe('NftDetectionController', () => { it('should poll and detect NFTs on interval while on mainnet', async () => { await withController( { config: { interval: 10 } }, - async ({ controller, triggerPreferencesStateChange }) => { - const mockNfts = sinon.stub(controller, 'detectNfts'); - triggerPreferencesStateChange({ + async ({ controller, controllerEvents }) => { + const mockNfts = sinon + .stub(controller, 'detectNfts') + .returns(Promise.resolve()); + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), useNftDetection: true, }); @@ -358,6 +371,95 @@ describe('NftDetectionController', () => { }); }); + it('should not rely on the currently selected chain to poll for NFTs when a specific chain is being targeted for polling', async () => { + await withController( + { + mockNetworkClientConfigurationsByNetworkClientId: { + 'AAAA-AAAA-AAAA-AAAA': buildCustomNetworkClientConfiguration({ + chainId: '0x1337', + }), + }, + }, + async ({ controller, controllerEvents }) => { + const spy = jest + .spyOn(controller, 'detectNfts') + .mockImplementation(() => { + return Promise.resolve(); + }); + + controller.startPollingByNetworkClientId('mainnet', { + address: '0x1', + }); + + await advanceTime({ clock, duration: 0 }); + expect(spy.mock.calls).toHaveLength(1); + await advanceTime({ + clock, + duration: DEFAULT_INTERVAL / 2, + }); + expect(spy.mock.calls).toHaveLength(1); + await advanceTime({ + clock, + duration: DEFAULT_INTERVAL / 2, + }); + expect(spy.mock.calls).toHaveLength(2); + await advanceTime({ clock, duration: DEFAULT_INTERVAL }); + expect(spy.mock.calls).toMatchObject([ + [ + { + networkClientId: 'mainnet', + userAddress: '0x1', + }, + ], + [ + { + networkClientId: 'mainnet', + userAddress: '0x1', + }, + ], + [ + { + networkClientId: 'mainnet', + userAddress: '0x1', + }, + ], + ]); + + controllerEvents.networkStateChange({ + ...defaultNetworkState, + selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA', + }); + await advanceTime({ clock, duration: DEFAULT_INTERVAL }); + expect(spy.mock.calls).toMatchObject([ + [ + { + networkClientId: 'mainnet', + userAddress: '0x1', + }, + ], + [ + { + networkClientId: 'mainnet', + userAddress: '0x1', + }, + ], + [ + { + networkClientId: 'mainnet', + userAddress: '0x1', + }, + ], + [ + { + networkClientId: 'mainnet', + userAddress: '0x1', + }, + ], + ]); + }, + ); + }); + it('should detect mainnet correctly', async () => { await withController(({ controller }) => { controller.configure({ chainId: ChainId.mainnet }); @@ -378,13 +480,107 @@ describe('NftDetectionController', () => { }); }); + it('should respond to chain ID changing when using legacy polling', async () => { + const mockAddNft = jest.fn(); + const pollingInterval = 100; + + await withController( + { + config: { + interval: pollingInterval, + }, + options: { + addNft: mockAddNft, + chainId: '0x1', + disabled: false, + selectedAddress: '0x1', + }, + mockNetworkClientConfigurationsByNetworkClientId: { + 'AAAA-AAAA-AAAA-AAAA': buildCustomNetworkClientConfiguration({ + chainId: '0x123', + }), + }, + }, + async ({ controller, controllerEvents }) => { + await controller.start(); + // await clock.tickAsync(pollingInterval); + + expect(mockAddNft).toHaveBeenNthCalledWith( + 1, + '0xCE7ec4B2DfB30eB6c0BB5656D33aAd6BFb4001Fc', + '2577', + { + nftMetadata: { + description: + "Redacted Remilio Babies is a collection of 10,000 neochibi pfpNFT's expanding the Milady Maker paradigm with the introduction of young J.I.T. energy and schizophrenic reactionary aesthetics. We are #REMILIONAIREs.", + image: 'https://imgtest', + imageOriginal: 'https://remilio.org/remilio/632.png', + imageThumbnail: 'https://imgSmall', + name: 'Remilio 632', + rarityRank: 8872, + rarityScore: 343.443, + standard: 'ERC721', + }, + userAddress: '0x1', + source: Source.Detected, + }, + ); + expect(mockAddNft).toHaveBeenNthCalledWith( + 2, + '0x0B0fa4fF58D28A88d63235bd0756EDca69e49e6d', + '2578', + { + nftMetadata: { + description: 'Description 2578', + image: 'https://imgtest', + imageOriginal: 'https://remilio.org/remilio/632.png', + imageThumbnail: 'https://imgSmall', + name: 'ID 2578', + rarityRank: 8872, + rarityScore: 343.443, + standard: 'ERC721', + }, + userAddress: '0x1', + source: Source.Detected, + }, + ); + expect(mockAddNft).toHaveBeenNthCalledWith( + 3, + '0xebE4e5E773AFD2bAc25De0cFafa084CFb3cBf1eD', + '2574', + { + nftMetadata: { + description: 'Description 2574', + image: 'image/2574.png', + imageOriginal: 'imageOriginal/2574.png', + name: 'ID 2574', + standard: 'ERC721', + }, + userAddress: '0x1', + source: Source.Detected, + }, + ); + + controllerEvents.networkStateChange({ + ...defaultNetworkState, + selectedNetworkClientId: 'AAAA-AAAA-AAAA-AAAA', + }); + await clock.tickAsync(pollingInterval); + + // Not 6 times, which is what would happen if detectNfts were called + // again + expect(mockAddNft).toHaveBeenCalledTimes(3); + }, + ); + }); + it('should detect and add NFTs correctly when blockaid result is not included in response', async () => { const mockAddNft = jest.fn(); await withController( { options: { addNft: mockAddNft } }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, controllerEvents }) => { const selectedAddress = '0x1'; - triggerPreferencesStateChange({ + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -422,9 +618,9 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); await withController( { options: { addNft: mockAddNft } }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, controllerEvents }) => { const selectedAddress = '0x123'; - triggerPreferencesStateChange({ + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -471,9 +667,9 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); await withController( { options: { addNft: mockAddNft } }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, controllerEvents }) => { const selectedAddress = '0x12345'; - triggerPreferencesStateChange({ + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -532,9 +728,9 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); await withController( { options: { addNft: mockAddNft } }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, controllerEvents }) => { const selectedAddress = '0x1'; - triggerPreferencesStateChange({ + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -589,9 +785,9 @@ describe('NftDetectionController', () => { }); await withController( { options: { addNft: mockAddNft, getNftState: mockGetNftState } }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, controllerEvents }) => { const selectedAddress = '0x9'; - triggerPreferencesStateChange({ + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -614,9 +810,9 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); await withController( { options: { addNft: mockAddNft } }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, controllerEvents }) => { const selectedAddress = ''; // Emtpy selected address - triggerPreferencesStateChange({ + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, // auto-detect is enabled so it proceeds to check userAddress @@ -659,9 +855,9 @@ describe('NftDetectionController', () => { it('should not detectNfts when disabled is false and useNftDetection is true', async () => { await withController( { config: { interval: 10 }, options: { disabled: false } }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, controllerEvents }) => { const mockNfts = sinon.stub(controller, 'detectNfts'); - triggerPreferencesStateChange({ + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), useNftDetection: true, }); @@ -687,9 +883,9 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); await withController( { options: { addNft: mockAddNft } }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, controllerEvents }) => { const selectedAddress = '0x9'; - triggerPreferencesStateChange({ + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: false, @@ -723,8 +919,8 @@ describe('NftDetectionController', () => { const mockAddNft = jest.fn(); await withController( { options: { addNft: mockAddNft } }, - async ({ controller, triggerPreferencesStateChange }) => { - triggerPreferencesStateChange({ + async ({ controller, controllerEvents }) => { + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -745,55 +941,53 @@ describe('NftDetectionController', () => { it('should rethrow error when Nft APi server fails with error other than fetch failure', async () => { const selectedAddress = '0x4'; - await withController( - async ({ controller, triggerPreferencesStateChange }) => { - // This mock is for the initial detect call after preferences change - nock(NFT_API_BASE_URL) - .get(`/users/${selectedAddress}/tokens`) - .query({ - continuation: '', - limit: '50', - chainIds: '1', - includeTopBid: true, - }) - .reply(200, { - tokens: [], - }); - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - selectedAddress, - useNftDetection: true, - }); - // Wait for detect call triggered by preferences state change to settle - await advanceTime({ - clock, - duration: 1, + await withController(async ({ controller, controllerEvents }) => { + // This mock is for the initial detect call after preferences change + nock(NFT_API_BASE_URL) + .get(`/users/${selectedAddress}/tokens`) + .query({ + continuation: '', + limit: '50', + chainIds: '1', + includeTopBid: true, + }) + .reply(200, { + tokens: [], }); - // This mock is for the call under test - nock(NFT_API_BASE_URL) - .get(`/users/${selectedAddress}/tokens`) - .query({ - continuation: '', - limit: '50', - chainIds: '1', - includeTopBid: true, - }) - .replyWithError(new Error('UNEXPECTED ERROR')); - - await expect(() => controller.detectNfts()).rejects.toThrow( - 'UNEXPECTED ERROR', - ); - }, - ); + controllerEvents.preferencesStateChange({ + ...getDefaultPreferencesState(), + selectedAddress, + useNftDetection: true, + }); + // Wait for detect call triggered by preferences state change to settle + await advanceTime({ + clock, + duration: 1, + }); + // This mock is for the call under test + nock(NFT_API_BASE_URL) + .get(`/users/${selectedAddress}/tokens`) + .query({ + continuation: '', + limit: '50', + chainIds: '1', + includeTopBid: true, + }) + .replyWithError(new Error('UNEXPECTED ERROR')); + + await expect(() => controller.detectNfts()).rejects.toThrow( + 'UNEXPECTED ERROR', + ); + }); }); it('should rethrow error when attempt to add NFT fails', async () => { const mockAddNft = jest.fn(); await withController( { options: { addNft: mockAddNft } }, - async ({ controller, triggerPreferencesStateChange }) => { + async ({ controller, controllerEvents }) => { const selectedAddress = '0x1'; - triggerPreferencesStateChange({ + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), selectedAddress, useNftDetection: true, @@ -814,45 +1008,54 @@ describe('NftDetectionController', () => { }); it('should only re-detect when relevant settings change', async () => { - await withController( - {}, - async ({ controller, triggerPreferencesStateChange }) => { - const detectNfts = sinon.stub(controller, 'detectNfts'); - - // Repeated preference changes should only trigger 1 detection - for (let i = 0; i < 5; i++) { - triggerPreferencesStateChange({ - ...getDefaultPreferencesState(), - useNftDetection: true, - }); - } - await advanceTime({ clock, duration: 1 }); - expect(detectNfts.callCount).toBe(1); + await withController({}, async ({ controller, controllerEvents }) => { + const detectNfts = sinon.stub(controller, 'detectNfts'); - // Irrelevant preference changes shouldn't trigger a detection - triggerPreferencesStateChange({ + // Repeated preference changes should only trigger 1 detection + for (let i = 0; i < 5; i++) { + controllerEvents.preferencesStateChange({ ...getDefaultPreferencesState(), useNftDetection: true, - securityAlertsEnabled: true, }); - await advanceTime({ clock, duration: 1 }); - expect(detectNfts.callCount).toBe(1); - }, - ); + } + await advanceTime({ clock, duration: 1 }); + expect(detectNfts.callCount).toBe(1); + + // Irrelevant preference changes shouldn't trigger a detection + controllerEvents.preferencesStateChange({ + ...getDefaultPreferencesState(), + useNftDetection: true, + securityAlertsEnabled: true, + }); + await advanceTime({ clock, duration: 1 }); + expect(detectNfts.callCount).toBe(1); + }); }); }); +/** + * A collection of mock external controller events. + */ +type ControllerEvents = { + nftsStateChange: (state: NftState) => void; + preferencesStateChange: (state: PreferencesState) => void; + networkStateChange: (state: NetworkState) => void; +}; + type WithControllerCallback = ({ controller, }: { controller: NftDetectionController; - triggerNftStateChange: (state: NftState) => void; - triggerPreferencesStateChange: (state: PreferencesState) => void; + controllerEvents: ControllerEvents; }) => Promise | ReturnValue; type WithControllerOptions = { options?: Partial[0]>; config?: Partial; + mockNetworkClientConfigurationsByNetworkClientId?: Record< + NetworkClientId, + NetworkClientConfiguration + >; }; type WithControllerArgs = @@ -871,33 +1074,35 @@ type WithControllerArgs = async function withController( ...args: WithControllerArgs ): Promise { - const [{ ...rest }, fn] = args.length === 2 ? args : [{}, args[0]]; - const { options, config } = rest; + const [ + { + options = {}, + config = {}, + mockNetworkClientConfigurationsByNetworkClientId = {}, + }, + testFunction, + ] = args.length === 2 ? args : [{}, args[0]]; - const getNetworkClientById = jest.fn().mockImplementation(() => { - return { - configuration: { - chainId: ChainId.mainnet, - }, - provider: jest.fn(), - blockTracker: jest.fn(), - destroy: jest.fn(), - }; - }); + // Explicit cast used here because we know the `on____` functions are always + // set in the constructor. + const controllerEvents = {} as ControllerEvents; + + const getNetworkClientById = buildMockGetNetworkClientById( + mockNetworkClientConfigurationsByNetworkClientId, + ); - const nftStateChangeListeners: ((state: NftState) => void)[] = []; - const preferencesStateChangeListeners: ((state: PreferencesState) => void)[] = - []; const controller = new NftDetectionController( { chainId: ChainId.mainnet, onNftsStateChange: (listener) => { - nftStateChangeListeners.push(listener); + controllerEvents.nftsStateChange = listener; }, onPreferencesStateChange: (listener) => { - preferencesStateChangeListeners.push(listener); + controllerEvents.preferencesStateChange = listener; + }, + onNetworkStateChange: (listener) => { + controllerEvents.networkStateChange = listener; }, - onNetworkStateChange: jest.fn(), getOpenSeaApiKey: jest.fn(), addNft: jest.fn(), getNftApi: jest.fn(), @@ -910,18 +1115,9 @@ async function withController( config, ); try { - return await fn({ + return await testFunction({ controller, - triggerNftStateChange: (state: NftState) => { - for (const listener of nftStateChangeListeners) { - listener(state); - } - }, - triggerPreferencesStateChange: (state: PreferencesState) => { - for (const listener of preferencesStateChangeListeners) { - listener(state); - } - }, + controllerEvents, }); } finally { controller.stop(); diff --git a/packages/assets-controllers/src/RatesController/RatesController.test.ts b/packages/assets-controllers/src/RatesController/RatesController.test.ts new file mode 100644 index 00000000000..aea62d27be0 --- /dev/null +++ b/packages/assets-controllers/src/RatesController/RatesController.test.ts @@ -0,0 +1,381 @@ +import { ControllerMessenger } from '@metamask/base-controller'; +import { useFakeTimers } from 'sinon'; + +import { advanceTime } from '../../../../tests/helpers'; +import type { fetchMultiExchangeRate as defaultFetchExchangeRate } from '../crypto-compare-service'; +import { + Cryptocurrency, + RatesController, + name as ratesControllerName, +} from './RatesController'; +import type { + RatesControllerActions, + RatesControllerEvents, + RatesControllerMessenger, + RatesControllerState, +} from './types'; + +const MOCK_TIMESTAMP = 1709983353; + +/** + * Returns a stubbed date based on a predefined timestamp. + * @returns The stubbed date in milliseconds. + */ +function getStubbedDate(): number { + return new Date(MOCK_TIMESTAMP).getTime(); +} + +/** + * Builds a new ControllerMessenger instance for RatesController. + * @returns A new ControllerMessenger instance. + */ +function buildMessenger(): ControllerMessenger< + RatesControllerActions, + RatesControllerEvents +> { + return new ControllerMessenger< + RatesControllerActions, + RatesControllerEvents + >(); +} + +/** + * Builds a restricted messenger for the RatesController. + * @param messenger - The base messenger instance. + * @returns A restricted messenger for the RatesController. + */ +function buildRatesControllerMessenger( + messenger: ControllerMessenger, +): RatesControllerMessenger { + return messenger.getRestricted({ + name: ratesControllerName, + allowedEvents: [], + allowedActions: [], + }); +} + +/** + * Sets up and returns a new instance of RatesController with the provided configuration. + * @param config - The configuration object for the RatesController. + * @param config.interval - Polling interval. + * @param config.initialState - Initial state of the controller. + * @param config.messenger - ControllerMessenger instance. + * @param config.includeUsdRate - Indicates if the USD rate should be included. + * @param config.fetchMultiExchangeRate - Callback to fetch rates data. + * @returns A new instance of RatesController. + */ +function setupRatesController({ + interval, + initialState, + messenger, + includeUsdRate, + fetchMultiExchangeRate, +}: { + interval?: number; + initialState: Partial; + messenger: ControllerMessenger; + includeUsdRate: boolean; + fetchMultiExchangeRate?: typeof defaultFetchExchangeRate; +}) { + const ratesControllerMessenger = buildRatesControllerMessenger(messenger); + return new RatesController({ + interval, + messenger: ratesControllerMessenger, + state: initialState, + includeUsdRate, + fetchMultiExchangeRate, + }); +} + +describe('RatesController', () => { + let clock: sinon.SinonFakeTimers; + + afterEach(() => { + jest.resetAllMocks(); + }); + + describe('construct', () => { + it('constructs the RatesController with default values', () => { + const ratesController = setupRatesController({ + initialState: {}, + messenger: buildMessenger(), + includeUsdRate: false, + }); + const { fiatCurrency, rates, cryptocurrencies } = ratesController.state; + expect(ratesController).toBeDefined(); + expect(fiatCurrency).toBe('usd'); + expect(Object.keys(rates)).toStrictEqual(['btc']); + expect(cryptocurrencies).toStrictEqual(['btc']); + }); + }); + + describe('start', () => { + beforeEach(() => { + clock = useFakeTimers(); + }); + + afterEach(() => { + clock.restore(); + jest.restoreAllMocks(); + }); + + it('starts the polling process with default values', async () => { + const messenger = buildMessenger(); + const publishActionSpy = jest.spyOn(messenger, 'publish'); + + jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); + const mockRateValue = '57715.42'; + const fetchExchangeRateStub = jest.fn(() => { + return Promise.resolve({ + btc: { + eur: mockRateValue, + }, + }); + }); + const ratesController = setupRatesController({ + interval: 150, + initialState: { + fiatCurrency: 'eur', + }, + messenger, + fetchMultiExchangeRate: fetchExchangeRateStub, + includeUsdRate: false, + }); + + const ratesPreUpdate = ratesController.state.rates; + + expect(ratesPreUpdate).toStrictEqual({ + btc: { + conversionDate: 0, + conversionRate: '0', + }, + }); + + await ratesController.start(); + + expect(publishActionSpy).toHaveBeenNthCalledWith( + 1, + `${ratesControllerName}:pollingStarted`, + ); + + await advanceTime({ clock, duration: 200 }); + + const ratesPosUpdate = ratesController.state.rates; + + // checks for the RatesController:stateChange event + expect(publishActionSpy).toHaveBeenCalledTimes(2); + expect(fetchExchangeRateStub).toHaveBeenCalled(); + expect(ratesPosUpdate).toStrictEqual({ + btc: { + conversionDate: MOCK_TIMESTAMP, + conversionRate: mockRateValue, + }, + }); + + await ratesController.start(); + + // since the polling has already started + // a second call to the start method should + // return immediately and no extra logic is executed + expect(publishActionSpy).not.toHaveBeenNthCalledWith(3); + }); + + it('starts the polling process with custom values', async () => { + jest.spyOn(global.Date, 'now').mockImplementation(() => getStubbedDate()); + const mockBtcUsdRateValue = '62235.48'; + const mockSolUsdRateValue = '148.41'; + const mockStrkUsdRateValue = '1.248'; + const mockBtcEurRateValue = '57715.42'; + const mockSolEurRateValue = '137.68'; + const mockStrkEurRateValue = '1.157'; + const fetchExchangeRateStub = jest.fn(() => { + return Promise.resolve({ + btc: { + usd: mockBtcUsdRateValue, + eur: mockBtcEurRateValue, + }, + sol: { + usd: mockSolUsdRateValue, + eur: mockSolEurRateValue, + }, + strk: { + usd: mockStrkUsdRateValue, + eur: mockStrkEurRateValue, + }, + }); + }); + + const ratesController = setupRatesController({ + interval: 150, + initialState: { + cryptocurrencies: [Cryptocurrency.Btc], + fiatCurrency: 'eur', + }, + messenger: buildMessenger(), + includeUsdRate: true, + fetchMultiExchangeRate: fetchExchangeRateStub, + }); + + await ratesController.start(); + + await advanceTime({ clock, duration: 200 }); + + const { rates } = ratesController.state; + expect(fetchExchangeRateStub).toHaveBeenCalled(); + expect(rates).toStrictEqual({ + btc: { + conversionDate: MOCK_TIMESTAMP, + conversionRate: mockBtcEurRateValue, + usdConversionRate: mockBtcUsdRateValue, + }, + sol: { + conversionDate: MOCK_TIMESTAMP, + conversionRate: mockSolEurRateValue, + usdConversionRate: mockSolUsdRateValue, + }, + strk: { + conversionDate: MOCK_TIMESTAMP, + conversionRate: mockStrkEurRateValue, + usdConversionRate: mockStrkUsdRateValue, + }, + }); + }); + }); + + describe('stop', () => { + beforeEach(() => { + clock = useFakeTimers(); + }); + + afterEach(() => { + clock.restore(); + jest.restoreAllMocks(); + }); + + it('stops the polling process', async () => { + const messenger = buildMessenger(); + const publishActionSpy = jest.spyOn(messenger, 'publish'); + const fetchExchangeRateStub = jest.fn().mockResolvedValue({}); + const ratesController = setupRatesController({ + interval: 150, + initialState: {}, + messenger, + fetchMultiExchangeRate: fetchExchangeRateStub, + includeUsdRate: false, + }); + + await ratesController.start(); + + expect(publishActionSpy).toHaveBeenNthCalledWith( + 1, + `${ratesControllerName}:pollingStarted`, + ); + + await advanceTime({ clock, duration: 200 }); + + expect(fetchExchangeRateStub).toHaveBeenCalledTimes(1); + + await ratesController.stop(); + + // check the 3rd call since the 2nd one is for the + // event stateChange + expect(publishActionSpy).toHaveBeenNthCalledWith( + 3, + `${ratesControllerName}:pollingStopped`, + ); + + await advanceTime({ clock, duration: 200 }); + + expect(fetchExchangeRateStub).toHaveBeenCalledTimes(1); + + await ratesController.stop(); + + // check if the stop method is called again, it returns early + // and no extra logic is executed + expect(publishActionSpy).not.toHaveBeenNthCalledWith( + 4, + `${ratesControllerName}:pollingStopped`, + ); + }); + }); + + describe('getCryptocurrencyList', () => { + it('returns the current cryptocurrency list', () => { + const fetchExchangeRateStub = jest.fn().mockResolvedValue({}); + const mockCryptocurrencyList = [Cryptocurrency.Btc]; + const ratesController = setupRatesController({ + interval: 150, + initialState: { + cryptocurrencies: mockCryptocurrencyList, + }, + messenger: buildMessenger(), + fetchMultiExchangeRate: fetchExchangeRateStub, + includeUsdRate: false, + }); + + const cryptocurrencyList = ratesController.getCryptocurrencyList(); + expect(cryptocurrencyList).toStrictEqual(mockCryptocurrencyList); + }); + }); + + describe('setCryptocurrencyList', () => { + it('updates the cryptocurrency list', async () => { + const fetchExchangeRateStub = jest.fn().mockResolvedValue({}); + const mockCryptocurrencyList = [Cryptocurrency.Btc]; + const ratesController = setupRatesController({ + interval: 150, + initialState: {}, + messenger: buildMessenger(), + fetchMultiExchangeRate: fetchExchangeRateStub, + includeUsdRate: false, + }); + + const cryptocurrencyListPreUpdate = + ratesController.getCryptocurrencyList(); + expect(cryptocurrencyListPreUpdate).toStrictEqual(['btc']); + + await ratesController.setCryptocurrencyList(mockCryptocurrencyList); + const cryptocurrencyListPostUpdate = + ratesController.getCryptocurrencyList(); + expect(cryptocurrencyListPostUpdate).toStrictEqual( + mockCryptocurrencyList, + ); + }); + }); + + describe('setCurrentCurrency', () => { + it('sets the currency to a new value', async () => { + const fetchExchangeRateStub = jest.fn().mockResolvedValue({}); + const ratesController = setupRatesController({ + interval: 150, + initialState: {}, + messenger: buildMessenger(), + fetchMultiExchangeRate: fetchExchangeRateStub, + includeUsdRate: false, + }); + + const currencyPreUpdate = ratesController.state.fiatCurrency; + expect(currencyPreUpdate).toBe('usd'); + + await ratesController.setFiatCurrency('eur'); + + const currencyPostUpdate = ratesController.state.fiatCurrency; + expect(currencyPostUpdate).toBe('eur'); + }); + + it('throws if input is an empty string', async () => { + const fetchExchangeRateStub = jest.fn().mockResolvedValue({}); + const ratesController = setupRatesController({ + interval: 150, + initialState: {}, + messenger: buildMessenger(), + fetchMultiExchangeRate: fetchExchangeRateStub, + includeUsdRate: false, + }); + + await expect(ratesController.setFiatCurrency('')).rejects.toThrow( + 'The currency can not be an empty string', + ); + }); + }); +}); diff --git a/packages/assets-controllers/src/RatesController/RatesController.ts b/packages/assets-controllers/src/RatesController/RatesController.ts new file mode 100644 index 00000000000..f5dcb89b24a --- /dev/null +++ b/packages/assets-controllers/src/RatesController/RatesController.ts @@ -0,0 +1,215 @@ +import { BaseController } from '@metamask/base-controller'; +import { Mutex } from 'async-mutex'; + +import { fetchMultiExchangeRate as defaultFetchExchangeRate } from '../crypto-compare-service'; +import type { + ConversionRates, + RatesControllerState, + RatesControllerOptions, + RatesControllerMessenger, +} from './types'; + +export const name = 'RatesController'; + +export enum Cryptocurrency { + Btc = 'btc', +} + +const DEFAULT_INTERVAL = 180000; + +const metadata = { + fiatCurrency: { persist: true, anonymous: true }, + rates: { persist: true, anonymous: true }, + cryptocurrencies: { persist: true, anonymous: true }, +}; + +const defaultState = { + fiatCurrency: 'usd', + rates: { + [Cryptocurrency.Btc]: { + conversionDate: 0, + conversionRate: '0', + }, + }, + cryptocurrencies: [Cryptocurrency.Btc], +}; + +export class RatesController extends BaseController< + typeof name, + RatesControllerState, + RatesControllerMessenger +> { + readonly #mutex = new Mutex(); + + readonly #fetchMultiExchangeRate; + + readonly #includeUsdRate; + + #intervalLength: number; + + #intervalId: NodeJS.Timeout | undefined; + + /** + * Creates a RatesController instance. + * + * @param options - Constructor options. + * @param options.includeUsdRate - Keep track of the USD rate in addition to the current currency rate. + * @param options.interval - The polling interval, in milliseconds. + * @param options.messenger - A reference to the messaging system. + * @param options.state - Initial state to set on this controller. + * @param options.fetchMultiExchangeRate - Fetches the exchange rate from an external API. This option is primarily meant for use in unit tests. + */ + constructor({ + interval = DEFAULT_INTERVAL, + messenger, + state, + includeUsdRate, + fetchMultiExchangeRate = defaultFetchExchangeRate, + }: RatesControllerOptions) { + super({ + name, + metadata, + messenger, + state: { ...defaultState, ...state }, + }); + this.#includeUsdRate = includeUsdRate; + this.#fetchMultiExchangeRate = fetchMultiExchangeRate; + this.#intervalLength = interval; + } + + /** + * Executes a function `callback` within a mutex lock to ensure that only one instance of `callback` runs at a time across all invocations of `#withLock`. + * This method is useful for synchronizing access to a resource or section of code that should not be executed concurrently. + * + * @template R - The return type of the function `callback`. + * @param callback - A callback to execute once the lock is acquired. This callback can be synchronous or asynchronous. + * @returns A promise that resolves to the result of the function `callback`. The promise is fulfilled once `callback` has completed execution. + * @example + * async function criticalLogic() { + * // Critical logic code goes here. + * } + * + * // Execute criticalLogic within a lock. + * const result = await this.#withLock(criticalLogic); + */ + async #withLock(callback: () => R) { + const releaseLock = await this.#mutex.acquire(); + try { + return callback(); + } finally { + releaseLock(); + } + } + + /** + * Executes the polling operation to update rates. + */ + async #executePoll(): Promise { + await this.#updateRates(); + } + + /** + * Updates the rates by fetching new data. + */ + async #updateRates(): Promise { + await this.#withLock(async () => { + const { fiatCurrency, cryptocurrencies } = this.state; + const response: Record< + Cryptocurrency, + Record + > = await this.#fetchMultiExchangeRate( + fiatCurrency, + cryptocurrencies, + this.#includeUsdRate, + ); + + const updatedRates: ConversionRates = {}; + for (const [cryptocurrency, values] of Object.entries(response)) { + updatedRates[cryptocurrency] = { + conversionDate: Date.now(), + conversionRate: values[fiatCurrency], + ...(this.#includeUsdRate && { usdConversionRate: values.usd }), + }; + } + + this.update(() => { + return { + ...this.state, + rates: updatedRates, + }; + }); + }); + } + + /** + * Starts the polling process. + */ + async start(): Promise { + if (this.#intervalId) { + return; + } + + this.messagingSystem.publish(`${name}:pollingStarted`); + + this.#intervalId = setInterval(() => { + this.#executePoll().catch(console.error); + }, this.#intervalLength); + } + + /** + * Stops the polling process. + */ + async stop(): Promise { + if (!this.#intervalId) { + return; + } + + clearInterval(this.#intervalId); + this.#intervalId = undefined; + this.messagingSystem.publish(`${name}:pollingStopped`); + } + + /** + * Returns the current list of cryptocurrency. + * @returns The cryptocurrency list. + */ + getCryptocurrencyList(): Cryptocurrency[] { + const { cryptocurrencies } = this.state; + return cryptocurrencies; + } + + /** + * Sets the list of supported cryptocurrencies. + * @param list - The list of supported cryptocurrencies. + */ + async setCryptocurrencyList(list: Cryptocurrency[]): Promise { + await this.#withLock(() => { + this.update(() => { + return { + ...this.state, + fromCurrencies: list, + }; + }); + }); + } + + /** + * Sets the internal fiat currency and update rates accordingly. + * @param fiatCurrency - The fiat currency. + */ + async setFiatCurrency(fiatCurrency: string): Promise { + if (fiatCurrency === '') { + throw new Error('The currency can not be an empty string'); + } + + await this.#withLock(() => { + this.update(() => { + return { + ...defaultState, + fiatCurrency, + }; + }); + }); + await this.#updateRates(); + } +} diff --git a/packages/assets-controllers/src/RatesController/index.ts b/packages/assets-controllers/src/RatesController/index.ts new file mode 100644 index 00000000000..bf1a078eb2e --- /dev/null +++ b/packages/assets-controllers/src/RatesController/index.ts @@ -0,0 +1,11 @@ +export { RatesController, Cryptocurrency } from './RatesController'; +export type { + RatesControllerState, + RatesControllerEvents, + RatesControllerActions, + RatesControllerMessenger, + RatesControllerGetStateAction, + RatesControllerStateChangeEvent, + RatesControllerPollingStartedEvent, + RatesControllerPollingStoppedEvent, +} from './types'; diff --git a/packages/assets-controllers/src/RatesController/types.ts b/packages/assets-controllers/src/RatesController/types.ts new file mode 100644 index 00000000000..7a422aaef2a --- /dev/null +++ b/packages/assets-controllers/src/RatesController/types.ts @@ -0,0 +1,133 @@ +import type { + RestrictedControllerMessenger, + ControllerGetStateAction, + ControllerStateChangeEvent, +} from '@metamask/base-controller'; + +import type { fetchMultiExchangeRate as defaultFetchExchangeRate } from '../crypto-compare-service'; +import type { + name as ratesControllerName, + Cryptocurrency, +} from './RatesController'; + +/** + * Represents the conversion rates from one currency to others, including the conversion date. + * The `conversionRate` field is a string that maps a cryptocurrency code (e.g., "BTC") to its + * conversion rate. For this field we use string as the data type to avoid potential rounding + * errors and precision loss. + * The `usdConversionRate` provides the conversion rate to USD as a string, or `null` if the + * conversion rate to USD is not available. We also use string for the same reason as stated before. + * The `conversionDate` is a Unix timestamp (number) indicating when the conversion rate was last updated. + */ +export type Rate = { + conversionRate: string; + conversionDate: number; + usdConversionRate?: string; +}; + +/** + * Represents the conversion rates for multiple cryptocurrencies. + * Each key is a string representing the cryptocurrency symbol (e.g., "BTC", "SOL"), + * and its value is a `Rate` object containing conversion rates from that cryptocurrency + * to a fiat currencies and an optional USD rate. + */ +export type ConversionRates = Record; + +/** + * Represents the state structure for the RatesController. + */ +export type RatesControllerState = { + /** + * The fiat currency in which conversion rates are expressed + * (i.e., the "to" currency). + */ + fiatCurrency: string; + /** + * The conversion rates for multiple cryptocurrencies. + */ + rates: ConversionRates; + /** + * A list of supported cryptocurrency symbols. + * (i.e., the "from" currencies). + */ + cryptocurrencies: Cryptocurrency[]; +}; + +/** + * Type definition for RatesController state change events. + */ +export type RatesControllerStateChangeEvent = ControllerStateChangeEvent< + typeof ratesControllerName, + RatesControllerState +>; + +/** + * Type definition for the RatesController polling started event. + */ +export type RatesControllerPollingStartedEvent = { + type: `${typeof ratesControllerName}:pollingStarted`; + payload: []; +}; + +/** + * Type definition for the RatesController polling stopped event. + */ +export type RatesControllerPollingStoppedEvent = { + type: `${typeof ratesControllerName}:pollingStopped`; + payload: []; +}; + +/** + * Defines the events that the RatesController can emit. + */ +export type RatesControllerEvents = + | RatesControllerStateChangeEvent + | RatesControllerPollingStartedEvent + | RatesControllerPollingStoppedEvent; + +export type RatesControllerGetStateAction = ControllerGetStateAction< + typeof ratesControllerName, + RatesControllerState +>; + +/** + * Defines the actions that can be performed to get the state of the RatesController. + */ +export type RatesControllerActions = RatesControllerGetStateAction; + +/** + * Defines the actions that the RatesController can perform. + */ +export type RatesControllerMessenger = RestrictedControllerMessenger< + typeof ratesControllerName, + RatesControllerActions, + RatesControllerEvents, + never, + never +>; + +/** + * The options required to initialize a RatesController. + */ +export type RatesControllerOptions = { + /** + * Whether to include USD rates in the conversion rates. + */ + includeUsdRate: boolean; + /** + * The polling interval in milliseconds. + */ + interval?: number; + /** + * The messenger instance for communication. + */ + messenger: RatesControllerMessenger; + /** + * The initial state of the controller. + */ + state?: Partial; + /** + * The function to fetch exchange rates. + */ + fetchMultiExchangeRate?: typeof defaultFetchExchangeRate; +}; diff --git a/packages/assets-controllers/src/TokenDetectionController.test.ts b/packages/assets-controllers/src/TokenDetectionController.test.ts index 24e8fdfba05..d856986b97f 100644 --- a/packages/assets-controllers/src/TokenDetectionController.test.ts +++ b/packages/assets-controllers/src/TokenDetectionController.test.ts @@ -93,7 +93,7 @@ const sampleTokenA = { symbol: tokenAFromList.symbol, decimals: tokenAFromList.decimals, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', isERC721: false, aggregators: formattedSampleAggregators, name: 'Chainlink', @@ -103,7 +103,7 @@ const sampleTokenB = { symbol: tokenBFromList.symbol, decimals: tokenBFromList.decimals, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x1f573d6fb3f13d689ff844b4ce37794d79a7ff1c.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x1f573d6fb3f13d689ff844b4ce37794d79a7ff1c.png', isERC721: false, aggregators: formattedSampleAggregators, name: 'Bancor', diff --git a/packages/assets-controllers/src/TokenListController.test.ts b/packages/assets-controllers/src/TokenListController.test.ts index bcb0f344e6c..66a4953a1be 100644 --- a/packages/assets-controllers/src/TokenListController.test.ts +++ b/packages/assets-controllers/src/TokenListController.test.ts @@ -40,7 +40,7 @@ const sampleMainnetTokenList = [ occurrences: 11, name: 'Synthetix', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f.png', aggregators: [ 'Aave', 'Bancor', @@ -63,7 +63,7 @@ const sampleMainnetTokenList = [ occurrences: 11, name: 'Chainlink', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', aggregators: [ 'Aave', 'Bancor', @@ -85,7 +85,7 @@ const sampleMainnetTokenList = [ occurrences: 11, name: 'Bancor', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x1f573d6fb3f13d689ff844b4ce37794d79a7ff1c.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x1f573d6fb3f13d689ff844b4ce37794d79a7ff1c.png', aggregators: [ 'Bancor', 'CMC', @@ -123,7 +123,7 @@ const sampleBinanceTokenList = [ 'Paraswap', ], iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/56/0x7083609fce4d1d8dc0c979aab8c869ea2c873402.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/56/0x7083609fce4d1d8dc0c979aab8c869ea2c873402.png', }, { address: '0x1af3f329e8be154074d8769d1ffa4ee058b1dbc3', @@ -140,7 +140,7 @@ const sampleBinanceTokenList = [ 'Paraswap', ], iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/56/0x1af3f329e8be154074d8769d1ffa4ee058b1dbc3.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/56/0x1af3f329e8be154074d8769d1ffa4ee058b1dbc3.png', }, ]; @@ -161,7 +161,7 @@ const sampleSingleChainState = { occurrences: 11, name: 'Synthetix', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0xc011a73ee8576fb46f5e1c5751ca3b9fe0af2a6f.png', aggregators: [ 'Aave', 'Bancor', @@ -184,7 +184,7 @@ const sampleSingleChainState = { occurrences: 11, name: 'Chainlink', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', aggregators: [ 'Aave', 'Bancor', @@ -206,7 +206,7 @@ const sampleSingleChainState = { occurrences: 11, name: 'Bancor', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x1f573d6fb3f13d689ff844b4ce37794d79a7ff1c.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x1f573d6fb3f13d689ff844b4ce37794d79a7ff1c.png', aggregators: [ 'Bancor', 'CMC', @@ -235,7 +235,7 @@ const sampleSepoliaTokenList = [ decimals: 8, name: 'Wrapped BTC', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/11155111/0x2260fac5e5542a773aa44fbcfedf7c193bc2c599.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/11155111/0x2260fac5e5542a773aa44fbcfedf7c193bc2c599.png', type: 'erc20', aggregators: [ 'Metamask', @@ -266,7 +266,7 @@ const sampleSepoliaTokenList = [ decimals: 18, name: 'UMA', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/11155111/0x04fa0d235c4abf4bcf4787af4cf447de572ef828.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/11155111/0x04fa0d235c4abf4bcf4787af4cf447de572ef828.png', type: 'erc20', aggregators: [ 'Metamask', @@ -292,7 +292,7 @@ const sampleSepoliaTokenList = [ decimals: 18, name: 'Gnosis Token', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/11155111/0x6810e776880c02933d47db1b9fc05908e5386b96.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/11155111/0x6810e776880c02933d47db1b9fc05908e5386b96.png', type: 'erc20', aggregators: [ 'Metamask', @@ -337,7 +337,7 @@ const sampleTwoChainState = { 'Paraswap', ], iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/56/0x7083609fce4d1d8dc0c979aab8c869ea2c873402.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/56/0x7083609fce4d1d8dc0c979aab8c869ea2c873402.png', }, '0x1af3f329e8be154074d8769d1ffa4ee058b1dbc3': { address: '0x1af3f329e8be154074d8769d1ffa4ee058b1dbc3', @@ -354,7 +354,7 @@ const sampleTwoChainState = { 'Paraswap', ], iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/56/0x1af3f329e8be154074d8769d1ffa4ee058b1dbc3.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/56/0x1af3f329e8be154074d8769d1ffa4ee058b1dbc3.png', }, }, tokensChainsCache: { @@ -378,7 +378,7 @@ const existingState = { occurrences: 11, name: 'Chainlink', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', aggregators: [ 'Aave', 'Bancor', @@ -412,7 +412,7 @@ const outdatedExistingState = { occurrences: 11, name: 'Chainlink', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', aggregators: [ 'Aave', 'Bancor', @@ -446,7 +446,7 @@ const expiredCacheExistingState: TokenListState = { occurrences: 9, name: 'Chainlink', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', aggregators: [ 'Aave', 'Bancor', @@ -473,7 +473,7 @@ const expiredCacheExistingState: TokenListState = { occurrences: 11, name: 'Chainlink', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', aggregators: [ 'Aave', 'Bancor', @@ -561,7 +561,7 @@ describe('TokenListController', () => { occurrences: 11, name: 'Chainlink', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x514910771af9ca656af840dff83e8264ecf986ca.png', aggregators: [ 'Aave', 'Bancor', diff --git a/packages/assets-controllers/src/TokenRatesController.ts b/packages/assets-controllers/src/TokenRatesController.ts index af8273d5765..a564e8b5a07 100644 --- a/packages/assets-controllers/src/TokenRatesController.ts +++ b/packages/assets-controllers/src/TokenRatesController.ts @@ -16,7 +16,7 @@ import { createDeferredPromise, type Hex } from '@metamask/utils'; import { isEqual } from 'lodash'; import { reduceInBatchesSerially, TOKEN_PRICES_BATCH_SIZE } from './assetsUtil'; -import { fetchExchangeRate as fetchNativeCurrencyExchangeRate } from './crypto-compare'; +import { fetchExchangeRate as fetchNativeCurrencyExchangeRate } from './crypto-compare-service'; import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service'; import type { TokensState } from './TokensController'; diff --git a/packages/assets-controllers/src/TokensController.test.ts b/packages/assets-controllers/src/TokensController.test.ts index da374aa189f..485a1df884f 100644 --- a/packages/assets-controllers/src/TokensController.test.ts +++ b/packages/assets-controllers/src/TokensController.test.ts @@ -95,8 +95,7 @@ describe('TokensController', () => { expect(controller.state.tokens[0]).toStrictEqual({ address: '0x01', decimals: 2, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x01.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x01.png', symbol: 'bar', isERC721: false, aggregators: [], @@ -111,8 +110,7 @@ describe('TokensController', () => { expect(controller.state.tokens[0]).toStrictEqual({ address: '0x01', decimals: 2, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x01.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x01.png', symbol: 'baz', isERC721: false, aggregators: [], @@ -293,8 +291,7 @@ describe('TokensController', () => { expect(controller.state.tokens[0]).toStrictEqual({ address: '0x01', decimals: 2, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x01.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x01.png', symbol: 'bar', isERC721: false, aggregators: [], @@ -321,7 +318,7 @@ describe('TokensController', () => { address: '0x01', decimals: 2, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/11155111/0x01.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/11155111/0x01.png', symbol: 'bar', isERC721: false, aggregators: [], @@ -354,8 +351,7 @@ describe('TokensController', () => { expect(controller.state.tokens[0]).toStrictEqual({ address: '0x01', decimals: 2, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/5/0x01.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/5/0x01.png', symbol: 'bar', isERC721: false, aggregators: [], @@ -365,8 +361,7 @@ describe('TokensController', () => { { address: '0x01', decimals: 2, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/5/0x01.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/5/0x01.png', symbol: 'bar', isERC721: false, aggregators: [], @@ -444,8 +439,7 @@ describe('TokensController', () => { expect(controller.state.tokens[0]).toStrictEqual({ address: '0x02', decimals: 2, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x02.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x02.png', symbol: 'baz', isERC721: false, aggregators: [], @@ -481,7 +475,7 @@ describe('TokensController', () => { address: '0x02', decimals: 2, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/11155111/0x02.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/11155111/0x02.png', symbol: 'baz', isERC721: false, aggregators: [], @@ -687,8 +681,7 @@ describe('TokensController', () => { { address: '0x01', decimals: 4, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x01.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x01.png', isERC721: false, symbol: 'A', aggregators: [], @@ -697,8 +690,7 @@ describe('TokensController', () => { { address: '0x02', decimals: 5, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x02.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x02.png', isERC721: false, symbol: 'B', aggregators: [], @@ -836,7 +828,7 @@ describe('TokensController', () => { symbol: 'REST', isERC721: true, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0xda5584cc586d07c7141aa427224a4bd58e64af7d.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0xda5584cc586d07c7141aa427224a4bd58e64af7d.png', decimals: 4, aggregators: [], name: undefined, @@ -886,7 +878,7 @@ describe('TokensController', () => { symbol: 'LEST', isERC721: false, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0xda5584cc586d07c7141aa427224a4bd58e64af7d.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0xda5584cc586d07c7141aa427224a4bd58e64af7d.png', decimals: 5, aggregators: [], name: undefined, @@ -966,8 +958,7 @@ describe('TokensController', () => { }; const dummyAddedToken: Token = { ...dummyDetectedToken, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x01.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x01.png', }; await controller.addDetectedTokens([dummyDetectedToken]); @@ -1021,7 +1012,7 @@ describe('TokensController', () => { aggregators: [], name: undefined, isERC721: false, - image: `https://static.metafi.codefi.network/api/v1/tokenIcons/11155111/0x${i}.png`, + image: `https://static.cx.metamask.io/api/v1/tokenIcons/11155111/0x${i}.png`, })); const [ @@ -1994,7 +1985,7 @@ describe('TokensController', () => { address: '0x01', decimals: 4, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x01.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x01.png', isERC721: false, symbol: 'A', aggregators: [], @@ -2004,7 +1995,7 @@ describe('TokensController', () => { address: '0x02', decimals: 5, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x02.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x02.png', isERC721: false, symbol: 'B', aggregators: [], @@ -2021,7 +2012,7 @@ describe('TokensController', () => { address: '0x03', decimals: 6, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x03.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x03.png', isERC721: false, symbol: 'C', aggregators: [], @@ -2072,7 +2063,7 @@ describe('TokensController', () => { address: '0x01', decimals: 4, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/11155111/0x01.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/11155111/0x01.png', isERC721: false, symbol: 'A', aggregators: [], @@ -2082,7 +2073,7 @@ describe('TokensController', () => { address: '0x02', decimals: 5, image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/11155111/0x02.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/11155111/0x02.png', isERC721: false, symbol: 'B', aggregators: [], @@ -2093,8 +2084,7 @@ describe('TokensController', () => { { address: '0x03', decimals: 4, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/5/0x03.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/5/0x03.png', isERC721: false, symbol: 'C', aggregators: [], @@ -2103,8 +2093,7 @@ describe('TokensController', () => { { address: '0x04', decimals: 5, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/5/0x04.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/5/0x04.png', isERC721: false, symbol: 'D', aggregators: [], @@ -2239,8 +2228,7 @@ describe('TokensController', () => { expect(controller.state.tokens[0]).toStrictEqual({ address: '0x01', decimals: 2, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x01.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x01.png', symbol: 'bar', isERC721: false, aggregators: [], @@ -2259,7 +2247,7 @@ describe('TokensController', () => { occurrences: 1, name: 'BarName', iconUrl: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x01.png', + 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x01.png', aggregators: ['Aave'], }, }, @@ -2270,8 +2258,7 @@ describe('TokensController', () => { expect(controller.state.tokens[0]).toStrictEqual({ address: '0x01', decimals: 2, - image: - 'https://static.metafi.codefi.network/api/v1/tokenIcons/1/0x01.png', + image: 'https://static.cx.metamask.io/api/v1/tokenIcons/1/0x01.png', symbol: 'bar', isERC721: false, aggregators: [], diff --git a/packages/assets-controllers/src/assetsUtil.test.ts b/packages/assets-controllers/src/assetsUtil.test.ts index 6c3d106e51a..6fb13a06bb0 100644 --- a/packages/assets-controllers/src/assetsUtil.test.ts +++ b/packages/assets-controllers/src/assetsUtil.test.ts @@ -152,7 +152,7 @@ describe('assetsUtil', () => { chainId: ChainId.mainnet, tokenAddress: linkTokenAddress, }); - const expectedValue = `https://static.metafi.codefi.network/api/v1/tokenIcons/${convertHexToDecimal( + const expectedValue = `https://static.cx.metamask.io/api/v1/tokenIcons/${convertHexToDecimal( ChainId.mainnet, )}/${linkTokenAddress}.png`; expect(formattedIconUrl).toStrictEqual(expectedValue); diff --git a/packages/assets-controllers/src/assetsUtil.ts b/packages/assets-controllers/src/assetsUtil.ts index 031388b299a..195d66d7b4d 100644 --- a/packages/assets-controllers/src/assetsUtil.ts +++ b/packages/assets-controllers/src/assetsUtil.ts @@ -107,7 +107,7 @@ export const formatIconUrlWithProxy = ({ tokenAddress: string; }) => { const chainIdDecimal = convertHexToDecimal(chainId).toString(); - return `https://static.metafi.codefi.network/api/v1/tokenIcons/${chainIdDecimal}/${tokenAddress.toLowerCase()}.png`; + return `https://static.cx.metamask.io/api/v1/tokenIcons/${chainIdDecimal}/${tokenAddress.toLowerCase()}.png`; }; /** diff --git a/packages/assets-controllers/src/crypto-compare.test.ts b/packages/assets-controllers/src/crypto-compare-service/crypto-compare.test.ts similarity index 76% rename from packages/assets-controllers/src/crypto-compare.test.ts rename to packages/assets-controllers/src/crypto-compare-service/crypto-compare.test.ts index dc1958c895a..b7bdfb26645 100644 --- a/packages/assets-controllers/src/crypto-compare.test.ts +++ b/packages/assets-controllers/src/crypto-compare-service/crypto-compare.test.ts @@ -1,6 +1,6 @@ import nock from 'nock'; -import { fetchExchangeRate } from './crypto-compare'; +import { fetchExchangeRate, fetchMultiExchangeRate } from './crypto-compare'; const cryptoCompareHost = 'https://min-api.cryptocompare.com'; @@ -149,4 +149,51 @@ describe('CryptoCompare', () => { const { conversionRate } = await fetchExchangeRate('USD', 'MNT'); expect(conversionRate).toBe(123); }); + + describe('fetchMultiExchangeRate', () => { + it('should return CAD and USD conversion rate for BTC, ETH, and SOL', async () => { + nock(cryptoCompareHost) + .get('/data/pricemulti?fsyms=BTC,ETH,SOL&tsyms=CAD,USD') + .reply(200, { + BTC: { CAD: 2000.42, USD: 1000.42 }, + ETH: { CAD: 3000.42, USD: 2000.42 }, + SOL: { CAD: 4000.42, USD: 3000.42 }, + }); + + const response = await fetchMultiExchangeRate( + 'CAD', + ['BTC', 'ETH', 'SOL'], + true, + ); + + expect(response).toStrictEqual({ + btc: { cad: 2000.42, usd: 1000.42 }, + eth: { cad: 3000.42, usd: 2000.42 }, + sol: { cad: 4000.42, usd: 3000.42 }, + }); + }); + + it('should not return USD value if not requested', async () => { + nock(cryptoCompareHost) + .get('/data/pricemulti?fsyms=BTC,ETH,SOL&tsyms=EUR') + .reply(200, { + BTC: { EUR: 1000 }, + ETH: { EUR: 2000 }, + SOL: { EUR: 3000 }, + }); + + // @ts-expect-error Testing the case where the USD rate is not included + const response = await fetchMultiExchangeRate('EUR', [ + 'BTC', + 'ETH', + 'SOL', + ]); + + expect(response).toStrictEqual({ + btc: { eur: 1000 }, + eth: { eur: 2000 }, + sol: { eur: 3000 }, + }); + }); + }); }); diff --git a/packages/assets-controllers/src/crypto-compare-service/crypto-compare.ts b/packages/assets-controllers/src/crypto-compare-service/crypto-compare.ts new file mode 100644 index 00000000000..6827e4e82f4 --- /dev/null +++ b/packages/assets-controllers/src/crypto-compare-service/crypto-compare.ts @@ -0,0 +1,154 @@ +import { handleFetch } from '@metamask/controller-utils'; + +/** + * A map from native currency symbol to CryptoCompare identifier. + * This is only needed when the values don't match. + */ +const nativeSymbolOverrides = new Map([['MNT', 'MANTLE']]); + +const CRYPTO_COMPARE_DOMAIN = 'https://min-api.cryptocompare.com'; + +/** + * Get the CryptoCompare API URL for getting the conversion rate from the given native currency to + * the given currency. Optionally, the conversion rate from the native currency to USD can also be + * included in the response. + * + * @param currentCurrency - The currency to get a conversion rate for. + * @param nativeCurrency - The native currency to convert from. + * @param includeUSDRate - Whether or not the native currency to USD conversion rate should be + * included in the response as well. + * @returns The API URL for getting the conversion rate. + */ +function getPricingURL( + currentCurrency: string, + nativeCurrency: string, + includeUSDRate?: boolean, +) { + nativeCurrency = nativeCurrency.toUpperCase(); + const fsym = nativeSymbolOverrides.get(nativeCurrency) ?? nativeCurrency; + return ( + `${CRYPTO_COMPARE_DOMAIN}/data/price?fsym=` + + `${fsym}&tsyms=${currentCurrency.toUpperCase()}` + + `${includeUSDRate && currentCurrency.toUpperCase() !== 'USD' ? ',USD' : ''}` + ); +} + +/** + * Get the CryptoCompare API URL for getting the conversion rate from a given array of native currencies + * to the given currency. Optionally, the conversion rate from the native currency to USD can also be + * included in the response. + * + * @param fsyms - The native currencies to get conversion rates for. + * @param tsyms - The currency to convert to. + * @param includeUSDRate - Whether or not the native currency to USD conversion rate should be included. + * @returns The API URL for getting the conversion rates. + */ +function getMultiPricingURL( + fsyms: string, + tsyms: string, + includeUSDRate = false, +) { + const updatedTsyms = + includeUSDRate && !tsyms.includes('USD') ? `${tsyms},USD` : tsyms; + + const params = new URLSearchParams(); + params.append('fsyms', fsyms); + params.append('tsyms', updatedTsyms); + + const url = new URL(`${CRYPTO_COMPARE_DOMAIN}/data/pricemulti`); + url.search = params.toString(); + + return url.toString(); +} + +/** + * Handles an error response from the CryptoCompare API. + * Expected error response format + * { Response: "Error", Message: "...", HasWarning: false } + * + * @param json - The JSON response from the CryptoCompare API. + * @param json.Response - The response status. + * @param json.Message - The error message. + */ +function handleErrorResponse(json: { Response?: string; Message?: string }) { + if (json.Response === 'Error') { + throw new Error(json.Message); + } +} + +/** + * Fetches the exchange rate for a given currency. + * + * @param currency - ISO 4217 currency code. + * @param nativeCurrency - Symbol for base asset. + * @param includeUSDRate - Whether to add the USD rate to the fetch. + * @returns Promise resolving to exchange rate for given currency. + */ +export async function fetchExchangeRate( + currency: string, + nativeCurrency: string, + includeUSDRate?: boolean, +): Promise<{ + conversionRate: number; + usdConversionRate: number; +}> { + const json = await handleFetch( + getPricingURL(currency, nativeCurrency, includeUSDRate), + ); + + handleErrorResponse(json); + const conversionRate = Number(json[currency.toUpperCase()]); + + const usdConversionRate = Number(json.USD); + if (!Number.isFinite(conversionRate)) { + throw new Error( + `Invalid response for ${currency.toUpperCase()}: ${ + json[currency.toUpperCase()] + }`, + ); + } + + if (includeUSDRate && !Number.isFinite(usdConversionRate)) { + throw new Error(`Invalid response for usdConversionRate: ${json.USD}`); + } + + return { + conversionRate, + usdConversionRate, + }; +} + +/** + * Fetches the exchange rates for multiple currencies. + * + * @param fiatCurrency - The currency of the rates (ISO 4217). + * @param cryptocurrencies - The cryptocurrencies to get conversion rates for. Min length: 1. Max length: 300. + * @param includeUSDRate - Whether to add the USD rate to the fetch. + * @returns Promise resolving to exchange rates for given currencies. + */ +export async function fetchMultiExchangeRate( + fiatCurrency: string, + cryptocurrencies: string[], + includeUSDRate: boolean, +): Promise>> { + const url = getMultiPricingURL( + Object.values(cryptocurrencies).join(','), + fiatCurrency, + includeUSDRate, + ); + const response = await handleFetch(url); + handleErrorResponse(response); + + const rates: Record> = {}; + for (const [cryptocurrency, values] of Object.entries(response) as [ + string, + Record, + ][]) { + rates[cryptocurrency.toLowerCase()] = { + [fiatCurrency.toLowerCase()]: values[fiatCurrency.toUpperCase()], + ...(includeUSDRate && { usd: values.USD }), + }; + } + + return rates; +} diff --git a/packages/assets-controllers/src/crypto-compare-service/index.ts b/packages/assets-controllers/src/crypto-compare-service/index.ts new file mode 100644 index 00000000000..33646741118 --- /dev/null +++ b/packages/assets-controllers/src/crypto-compare-service/index.ts @@ -0,0 +1 @@ +export { fetchExchangeRate, fetchMultiExchangeRate } from './crypto-compare'; diff --git a/packages/assets-controllers/src/crypto-compare.ts b/packages/assets-controllers/src/crypto-compare.ts deleted file mode 100644 index c6149dfa1ef..00000000000 --- a/packages/assets-controllers/src/crypto-compare.ts +++ /dev/null @@ -1,85 +0,0 @@ -import { handleFetch } from '@metamask/controller-utils'; - -/** - * A map from native currency symbol to CryptoCompare identifier. - * This is only needed when the values don't match. - */ -const nativeSymbolOverrides = new Map([['MNT', 'MANTLE']]); - -/** - * Get the CryptoCompare API URL for getting the conversion rate from the given native currency to - * the given currency. Optionally, the conversion rate from the native currency to USD can also be - * included in the response. - * - * @param currentCurrency - The currency to get a conversion rate for. - * @param nativeCurrency - The native currency to convert from. - * @param includeUSDRate - Whether or not the native currency to USD conversion rate should be - * included in the response as well. - * @returns The API URL for getting the conversion rate. - */ -function getPricingURL( - currentCurrency: string, - nativeCurrency: string, - includeUSDRate?: boolean, -) { - nativeCurrency = nativeCurrency.toUpperCase(); - const fsym = nativeSymbolOverrides.get(nativeCurrency) ?? nativeCurrency; - return ( - `https://min-api.cryptocompare.com/data/price?fsym=` + - `${fsym}&tsyms=${currentCurrency.toUpperCase()}` + - `${includeUSDRate && currentCurrency.toUpperCase() !== 'USD' ? ',USD' : ''}` - ); -} - -/** - * Fetches the exchange rate for a given currency. - * - * @param currency - ISO 4217 currency code. - * @param nativeCurrency - Symbol for base asset. - * @param includeUSDRate - Whether to add the USD rate to the fetch. - * @returns Promise resolving to exchange rate for given currency. - */ -export async function fetchExchangeRate( - currency: string, - nativeCurrency: string, - includeUSDRate?: boolean, -): Promise<{ - conversionRate: number; - usdConversionRate: number; -}> { - const json = await handleFetch( - getPricingURL(currency, nativeCurrency, includeUSDRate), - ); - - /* - Example expected error response (if pair is not found) - { - Response: "Error", - Message: "cccagg_or_exchange market does not exist for this coin pair (ETH-)", - HasWarning: false, - } - */ - if (json.Response === 'Error') { - throw new Error(json.Message); - } - - const conversionRate = Number(json[currency.toUpperCase()]); - - const usdConversionRate = Number(json.USD); - if (!Number.isFinite(conversionRate)) { - throw new Error( - `Invalid response for ${currency.toUpperCase()}: ${ - json[currency.toUpperCase()] - }`, - ); - } - - if (includeUSDRate && !Number.isFinite(usdConversionRate)) { - throw new Error(`Invalid response for usdConversionRate: ${json.USD}`); - } - - return { - conversionRate, - usdConversionRate, - }; -} diff --git a/packages/assets-controllers/src/index.ts b/packages/assets-controllers/src/index.ts index 306280f429a..b276b2ccd30 100644 --- a/packages/assets-controllers/src/index.ts +++ b/packages/assets-controllers/src/index.ts @@ -58,3 +58,14 @@ export { CodefiTokenPricesServiceV2, SUPPORTED_CHAIN_IDS, } from './token-prices-service'; +export { RatesController, Cryptocurrency } from './RatesController'; +export type { + RatesControllerState, + RatesControllerEvents, + RatesControllerActions, + RatesControllerMessenger, + RatesControllerGetStateAction, + RatesControllerStateChangeEvent, + RatesControllerPollingStartedEvent, + RatesControllerPollingStoppedEvent, +} from './RatesController'; diff --git a/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts index 808a65c0d92..510b837922c 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.test.ts @@ -14,7 +14,7 @@ const defaultMaxRetryDelay = 30_000; describe('CodefiTokenPricesServiceV2', () => { describe('fetchTokenPrices', () => { it('uses the /spot-prices endpoint of the Codefi Price API to gather prices for the given tokens', async () => { - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -59,7 +59,7 @@ describe('CodefiTokenPricesServiceV2', () => { }); it('should not include token price object for token address when token price in not included the response data', async () => { - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -94,7 +94,7 @@ describe('CodefiTokenPricesServiceV2', () => { }); it('should not include token price object for token address when price is undefined for token response data', async () => { - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -131,7 +131,7 @@ describe('CodefiTokenPricesServiceV2', () => { }); it('throws if the request fails consistently', async () => { - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -151,7 +151,7 @@ describe('CodefiTokenPricesServiceV2', () => { it('throws if the initial request and all retries fail', async () => { const retries = 3; - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -172,7 +172,7 @@ describe('CodefiTokenPricesServiceV2', () => { it('succeeds if the last retry succeeds', async () => { const retries = 3; // Initial interceptor for failing requests - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -181,7 +181,7 @@ describe('CodefiTokenPricesServiceV2', () => { .times(retries) .replyWithError('Failed to fetch'); // Interceptor for successful request - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -239,7 +239,7 @@ describe('CodefiTokenPricesServiceV2', () => { it('does not call onDegraded when requests succeeds faster than threshold', async () => { const degradedThreshold = 1000; - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -278,7 +278,7 @@ describe('CodefiTokenPricesServiceV2', () => { const degradedThreshold = defaultMaxRetryDelay + 1000; const retries = 1; // Initial interceptor for failing request - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -286,7 +286,7 @@ describe('CodefiTokenPricesServiceV2', () => { }) .replyWithError('Failed to fetch'); // Second interceptor for successful response - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -327,7 +327,7 @@ describe('CodefiTokenPricesServiceV2', () => { it('calls onDegraded when request fails', async () => { const retries = 0; - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -359,7 +359,7 @@ describe('CodefiTokenPricesServiceV2', () => { it('calls onDegraded when request is slower than threshold', async () => { const degradedThreshold = 1000; const retries = 0; - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -402,7 +402,7 @@ describe('CodefiTokenPricesServiceV2', () => { const degradedThreshold = 1000; const retries = 1; // Initial interceptor for failing request - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -410,7 +410,7 @@ describe('CodefiTokenPricesServiceV2', () => { }) .replyWithError('Failed to fetch'); // Second interceptor for successful response - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -466,7 +466,7 @@ describe('CodefiTokenPricesServiceV2', () => { // Max consencutive failures is set to match number of calls in three update attempts (including retries) const maximumConsecutiveFailures = (1 + retries) * 3; // Initial interceptor for failing requests - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -475,9 +475,7 @@ describe('CodefiTokenPricesServiceV2', () => { .times(maximumConsecutiveFailures) .replyWithError('Failed to fetch'); // This interceptor should not be used - const successfullCallScope = nock( - 'https://price-api.metafi.codefi.network', - ) + const successfullCallScope = nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -537,7 +535,7 @@ describe('CodefiTokenPricesServiceV2', () => { // Max consencutive failures is set to match number of calls in three update attempts (including retries) const maximumConsecutiveFailures = (1 + retries) * 3; // Initial interceptor for failing requests - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -546,7 +544,7 @@ describe('CodefiTokenPricesServiceV2', () => { .times(maximumConsecutiveFailures) .replyWithError('Failed to fetch'); // This interceptor should not be used - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -601,7 +599,7 @@ describe('CodefiTokenPricesServiceV2', () => { // Max consencutive failures is set to match number of calls in three update attempts (including retries) const maximumConsecutiveFailures = (1 + retries) * 3; // Initial interceptor for failing requests - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -665,7 +663,7 @@ describe('CodefiTokenPricesServiceV2', () => { // break doesn't end during a retry attempt const circuitBreakDuration = defaultMaxRetryDelay * 10; // Initial interceptor for failing requests - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -675,9 +673,7 @@ describe('CodefiTokenPricesServiceV2', () => { .times(maximumConsecutiveFailures + 1) .replyWithError('Failed to fetch'); // This interceptor should not be used - const successfullCallScope = nock( - 'https://price-api.metafi.codefi.network', - ) + const successfullCallScope = nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -763,7 +759,7 @@ describe('CodefiTokenPricesServiceV2', () => { // break doesn't end during a retry attempt const circuitBreakDuration = defaultMaxRetryDelay * 10; // Initial interceptor for failing requests - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', @@ -772,7 +768,7 @@ describe('CodefiTokenPricesServiceV2', () => { .times(maximumConsecutiveFailures) .replyWithError('Failed to fetch'); // Later interceptor for successfull request after recovery - nock('https://price-api.metafi.codefi.network') + nock('https://price.api.cx.metamask.io') .get('/v2/chains/1/spot-prices') .query({ tokenAddresses: '0xAAA,0xBBB,0xCCC', diff --git a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts index b9418a0768b..39b403615bf 100644 --- a/packages/assets-controllers/src/token-prices-service/codefi-v2.ts +++ b/packages/assets-controllers/src/token-prices-service/codefi-v2.ts @@ -248,7 +248,7 @@ type SupportedChainId = (typeof SUPPORTED_CHAIN_IDS)[number]; /** * All requests to V2 of the Price API start with this. */ -const BASE_URL = 'https://price-api.metafi.codefi.network/v2'; +const BASE_URL = 'https://price.api.cx.metamask.io/v2'; const DEFAULT_TOKEN_PRICE_RETRIES = 3; // Each update attempt will result (1 + retries) calls if the server is down diff --git a/packages/assets-controllers/src/token-service.ts b/packages/assets-controllers/src/token-service.ts index a73e12723a2..becedb82004 100644 --- a/packages/assets-controllers/src/token-service.ts +++ b/packages/assets-controllers/src/token-service.ts @@ -7,7 +7,7 @@ import type { Hex } from '@metamask/utils'; import { isTokenListSupportedForNetwork } from './assetsUtil'; -export const TOKEN_END_POINT_API = 'https://token-api.metaswap.codefi.network'; +export const TOKEN_END_POINT_API = 'https://token.api.cx.metamask.io'; export const TOKEN_METADATA_NO_SUPPORT_ERROR = 'TokenService Error: Network does not support fetchTokenMetadata'; diff --git a/packages/controller-utils/src/constants.ts b/packages/controller-utils/src/constants.ts index 8037d610aff..915b6fad592 100644 --- a/packages/controller-utils/src/constants.ts +++ b/packages/controller-utils/src/constants.ts @@ -106,7 +106,7 @@ export const BUILT_IN_NETWORKS = { // APIs export const OPENSEA_PROXY_URL = - 'https://proxy.metafi.codefi.network/opensea/v1/api/v2'; + 'https://proxy.api.cx.metamask.io/opensea/v1/api/v2'; export const NFT_API_BASE_URL = 'https://nft.api.cx.metamask.io'; diff --git a/packages/name-controller/src/providers/token.test.ts b/packages/name-controller/src/providers/token.test.ts index e30b11825e7..58bf6c172d2 100644 --- a/packages/name-controller/src/providers/token.test.ts +++ b/packages/name-controller/src/providers/token.test.ts @@ -51,7 +51,7 @@ describe('TokenNameProvider', () => { expect(handleFetchMock).toHaveBeenCalledTimes(1); expect(handleFetchMock).toHaveBeenCalledWith( - `https://token-api.metaswap.codefi.network/token/${CHAIN_ID_MOCK}?address=${VALUE_MOCK}`, + `https://token.api.cx.metamask.io/token/${CHAIN_ID_MOCK}?address=${VALUE_MOCK}`, ); }); diff --git a/packages/name-controller/src/providers/token.ts b/packages/name-controller/src/providers/token.ts index 486b71cc44a..c9a47b355b7 100644 --- a/packages/name-controller/src/providers/token.ts +++ b/packages/name-controller/src/providers/token.ts @@ -43,7 +43,7 @@ export class TokenNameProvider implements NameProvider { } const { value, variation: chainId } = request; - const url = `https://token-api.metaswap.codefi.network/token/${chainId}?address=${value}`; + const url = `https://token.api.cx.metamask.io/token/${chainId}?address=${value}`; log('Sending request', url); diff --git a/packages/permission-controller/ARCHITECTURE.md b/packages/permission-controller/ARCHITECTURE.md index edc87f757c4..8c715234bac 100644 --- a/packages/permission-controller/ARCHITECTURE.md +++ b/packages/permission-controller/ARCHITECTURE.md @@ -3,7 +3,7 @@ The `PermissionController` is the heart of an object capability-inspired permission system. It is the successor of the original MetaMask permission system, [`rpc-cap`](https://github.com/MetaMask/rpc-cap). -## Conceptual Overview +## Conceptual overview The permission system itself belongs to a **host**, and it mediates the access to resources – called **targets** – of distinct **subjects**. A target can belong to the host itself, or another subject. @@ -14,7 +14,7 @@ Permissions are associated with a subject and target, and they are part of the s Permissions can have **caveats**, which are host-defined attenuations of the authority a permission grants over a particular target. -## Implementation Overview +## Implementation overview At any given moment, the `PermissionController` state tree describes the complete state of the permissions of all subjects known to the host (i.e., the MetaMask instance). The `PermissionController` also provides methods for adding, updating, and removing permissions, and enforcing the rules described by its state tree. @@ -30,13 +30,13 @@ Permission system concepts correspond to components of the MetaMask stack as fol | Caveats | Caveat objects | | Permission system | The `PermissionController` and its `json-rpc-engine` middleware | -### Permission / Target Types +### Permission / target Types In practice, targets can be different things, necessitating distinct implementations in order to enforce the logic of the permission system. This being the case, the `PermissionController` defines different **permission / target types**, intended for different kinds of permission targets. At present, there are two permission / target types. -#### JSON-RPC Methods +#### JSON-RPC methods Restricting access to JSON-RPC methods was the motivating and only supported use case for the original permission system, and its successor also implements this feature. The `PermissionController` provides patterns for creating restricted JSON-RPC method implementations and caveats, and a `json-rpc-engine` middleware function factory. @@ -66,6 +66,128 @@ Every permission has a `caveats` field, which is either an array of caveats or ` Every caveat has a string `type`, and every type has an associated function that is used to apply the caveat to a restricted method request. When the `PermissionController` is constructed, the consumer specifies the available caveat types and their implementations. +#### Caveat structure + +The complete authority represented by a permissions is represented by that permission and its caveat. +Accurately and legibly representing this information to the user is one of the most important responsibilities of MetaMask itself. +Therefore, as with any data structure we will use to represent information to the user, the simpler a caveat value type is, the better. + +For the same reason, it is also critical for permission authors to carefully consider the _semantics_ of caveat values. +In particular, the existence of an authority **must** be represented by the **presence** of a value. + +For example, let's say there is a caveat `foo` that restricts the parameters that a method can be called with. +In theory, such a caveat could be implemented such that a value of `[1, 2]` means that the +method will only accept `1` and `2` as parameters, while an empty array `[]` means that +_all_ parameters are permitted. +**This is strictly forbidden.** +Instead, such a hypothetical caveat could use `['*']` to represent that all parameters are permitted. + +We, the maintainers of the permission controller, impose this requirement for two reasons: + +1. We find it more intuitive to reason about caveats structured in this manner. +2. It leaves the door open for establishing a caveat DSL, and subsequently standardized caveat value merger functions in support of [incremental permission requests](#requestpermissionsincremental). + +#### Caveat merging + + + +Consumers may supply a caveat value merger function when specifying a caveat. +This is required to support [incremental permission requests](#requestpermissionsincremental). +Caveat values must be merged in the fashion of a right-biased union. +This operation is _like_ a union in set theory, except the right-hand operand overwrites +the left-hand operand in case of collisions. + +Formally, let: + +- `A` be the value of the existing / left-hand caveat +- `B` be the value of the requested / right-hand caveat +- `C` be the value of the resulting caveat +- `⊕` be the right-biased union operator + +Then the following must be true: + +- `C = A ⊕ B` +- `C ⊇ B` +- `A` and `C` may have all, some, or no values in common. +- If `A = ∅`, then `C = B` + +In addition to merging the values, the caveat value merger implementation must supply +the difference between `C` and `A`, expressed in the relevant caveat value type. +This is necessary so that other parts of the application, especially the UI, can +understand how authority has changed. + +Caveat value mergers should assume that the left- and right-hand values are always defined. +In practice, when the permission controller attempts to merge two permissions, it's possible +that the left-hand side does not exist. +In this case, the value of the right-hand side will also be the value of the diff, `Δ`. +Therefore, caveat value mergers **must** express their diffs in the relevant caveat value type. + +If `Δ` the difference between `C` and `A`, then: + +- `Δ = C - A` + - `Δ ∩ A = ∅` + - `Δ ⊆ C` + - `A ⊕ Δ = C` +- `Δ ⊆ B` +- If `A = ∅`, then `Δ = C = B` + +To exemplify the above in JavaScript: + +```js +// A is empty. +A = undefined; +B = { foo: 'bar' }; +C = { foo: 'bar' }; +Delta = { foo: 'bar' }; + +// A and B are the same. +A = { foo: 'bar' }; +B = { foo: 'bar' }; +C = { foo: 'bar' }; +Delta = undefined; + +// A and B have no values in common. +A = { foo: 'bar' }; +B = { life: 42 }; +C = { foo: 'bar', life: 42 }; +Delta = { life: 42 }; + +// B overwrites A completely. +A = { foo: 'bar' }; +B = { foo: 'baz' }; +C = { foo: 'baz' }; +Delta = { foo: 'baz' }; + +// B partially overwrites A. +A = { foo: 'bar', life: 42 }; +B = { foo: 'baz' }; +C = { foo: 'baz', life: 42 }; +Delta = { foo: 'baz' }; +``` + +### Requesting permissions + +The `PermissionController` provides two methods for requesting permissions: + +#### `requestPermissions()` + +This method accepts an object specifying the requested permissions and any caveats for a particular subject. +The method optionally allows existing permissions not named in the request to be preserved. +Any existing permissions named in the request will be overwritten with the value approved by the user. + +#### `requestPermissionsIncremental()` + +This method also accepts an object of requested permissions, but will preserve the subject's existing authority to the greatest extent possible. +In practice, this means that it will merge the requested permissions and caveats with the existing permissions and subjects. +This merger is performed by way of a right-biased union, where the requested permissions are the right-hand side. + +If a caveat of the same type is encountered on both the left- and right-hand sides, the +new caveat value is determined by calling that caveat type's merger function. +This function must also perform a right-biased union, see [caveat merging](#caveat-merging) for more details. +If no merger exists for a caveat that must be merged, the request will fail. + + + ## Examples In addition to the below examples, the [`PermissionController` unit tests](./PermissionController.test.ts) show how to set up the controller. @@ -99,6 +221,11 @@ const caveatSpecifications = { caveat.value.includes(resultValue), ); }, + // This function is called if two caveats of this type have to be merged + // due to an incremental permissions request. The values must be merged + // in the fashion of a right-biased union. + merger: (leftValue, rightValue) => + Array.from(new Set([...leftValue, ...rightValue])), }, }; @@ -144,7 +271,7 @@ const permissionController = new PermissionController({ }); ``` -### Adding the Permission Middleware +### Adding the permission middleware ```typescript // This should take place where a middleware stack is created for a particular @@ -160,7 +287,7 @@ engine.push(permissionController.createPermissionMiddleware({ origin })); engine.push(/* your other various middleware*/); ``` -### Calling a Restricted Method Internally +### Calling a restricted method internally ```typescript // Sometimes, we need to call a restricted method internally, as a particular subject. @@ -173,7 +300,7 @@ permissionController.executeRestrictedMethod(origin, 'wallet_getSecret', { }); ``` -### Getting Endowments +### Getting endowments ```typescript // Getting endowments internally is the only option, since the host has to apply @@ -187,11 +314,11 @@ const endowments = await permissionController.getEndowments( applyEndowments(origin, endowments); ``` -### Requesting and Getting Permissions +### Requesting and getting permissions ```typescript // This requests the `wallet_getSecretArray` permission. -const approvedPermissions = await ethereum.request({ +const addedPermissions = await ethereum.request({ method: 'wallet_requestPermissions', params: [{ wallet_getSecretArray: {}, @@ -214,7 +341,45 @@ console.log(existingPermissions) // ] ``` -### Restricted Method Caveat Decorators +### Requesting permissions incrementally + +```typescript +// Given an artifically truncated permission state of: +// { +// 'metamask.io': { +// wallet_getSecretArray: { +// caveats: [ +// { type: 'foo', value: ['a'] }, +// ], +// }, +// }, +// } + +// We request: +await permissionController.requestPermissionsIncremental({ + wallet_getSecretArray: { + caveats: [ + { type: 'foo', value: ['b'] }, + { type: 'bar', value: 42 }, + ], + }, +}); + +// Assuming that the caveat value merger implementation for 'foo' naively merges the +// values of the left- and right-hand sides, we end up with: +// { +// 'metamask.io': { +// wallet_getSecretArray: { +// caveats: [ +// { type: 'foo', value: ['a', 'b'] }, +// { type: 'bar', value: 42 }, +// ], +// }, +// }, +// } +``` + +### Restricted method caveat decorators Here follows some more example caveat decorator implementations. diff --git a/packages/permission-controller/src/Caveat.ts b/packages/permission-controller/src/Caveat.ts index b1cb40d97a2..7bfb57b199e 100644 --- a/packages/permission-controller/src/Caveat.ts +++ b/packages/permission-controller/src/Caveat.ts @@ -94,6 +94,7 @@ type ExtractCaveatValueFromDecorator< /** * A function for validating caveats of a particular type. * + * @see `validator` in {@link CaveatSpecificationBase} for more details. * @template ParentCaveat - The caveat type associated with this validator. * @param caveat - The caveat object to validate. * @param origin - The origin associated with the parent permission. @@ -105,6 +106,29 @@ export type CaveatValidator = ( target?: string, ) => void; +/** + * A map of caveat type strings to {@link CaveatDiff} values. + */ +export type CaveatDiffMap = { + [CaveatType in ParentCaveat['type']]: ParentCaveat['value']; +}; + +/** + * A function that merges two caveat values of the same type. The values must be + * merged in the fashion of a right-biased union. + * + * @see `ARCHITECTURE.md` for more details. + * @template Value - The type of the values to merge. + * @param leftValue - The left-hand value. + * @param rightValue - The right-hand value. + * @returns `[newValue, diff]`, i.e. the merged value and the diff between the left value + * and the new value. The diff must be expressed in the same type as the value itself. + */ +export type CaveatValueMerger = ( + leftValue: Value, + rightValue: Value, +) => [Value, Value] | []; + export type CaveatSpecificationBase = { /** * The string type of the caveat. @@ -125,6 +149,17 @@ export type CaveatSpecificationBase = { // TODO: Replace `any` with type // eslint-disable-next-line @typescript-eslint/no-explicit-any validator?: CaveatValidator; + + /** + * The merger function used to merge a pair of values of the associated caveat type + * during incremental permission requests. The values must be merged in the fashion + * of a right-biased union. + * + * @see `ARCHITECTURE.md` for more details. + */ + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + merger?: CaveatValueMerger; }; export type RestrictedMethodCaveatSpecificationConstraint = diff --git a/packages/permission-controller/src/PermissionController.test.ts b/packages/permission-controller/src/PermissionController.test.ts index 18979e0b3dd..c7bea8a0681 100644 --- a/packages/permission-controller/src/PermissionController.test.ts +++ b/packages/permission-controller/src/PermissionController.test.ts @@ -66,10 +66,18 @@ type FilterObjectCaveat = Caveat< type NoopCaveat = Caveat; -const onPermittedMock = jest.fn(() => Promise.resolve('foo')); -const onFailureMock = jest.fn(() => Promise.resolve()); -const onPermitted = () => onPermittedMock(); -const onFailure = () => onFailureMock(); +// A caveat value merger for any caveat whose value is an array of JSON primitives. +const primitiveArrayMerger = ( + a: T[], + b: T[], +) => { + const diff = b.filter((element) => !a.includes(element)); + + if (diff.length > 0) { + return [[...(a ?? []), ...diff], diff] as [T[], T[]]; + } + return [] as []; +}; /** * Gets caveat specifications for: @@ -110,6 +118,7 @@ function getDefaultCaveatSpecifications() { ); } }, + merger: primitiveArrayMerger, }, [CaveatTypes.reverseArrayResponse]: { type: CaveatTypes.reverseArrayResponse, @@ -147,6 +156,7 @@ function getDefaultCaveatSpecifications() { }); return result; }, + merger: primitiveArrayMerger, }, [CaveatTypes.noopCaveat]: { type: CaveatTypes.noopCaveat, @@ -166,6 +176,8 @@ function getDefaultCaveatSpecifications() { throw new Error('NoopCaveat value must be null'); } }, + merger: (a: null | undefined, _b: null) => + a === undefined ? ([null, null] as [null, null]) : ([] as []), }, [CaveatTypes.endowmentCaveat]: { type: CaveatTypes.endowmentCaveat, @@ -203,6 +215,7 @@ const PermissionKeys = { wallet_noopWithValidator: 'wallet_noopWithValidator', wallet_noopWithRequiredCaveat: 'wallet_noopWithRequiredCaveat', wallet_noopWithFactory: 'wallet_noopWithFactory', + wallet_noopWithManyCaveats: 'wallet_noopWithManyCaveats', snap_foo: 'snap_foo', endowmentAnySubject: 'endowmentAnySubject', endowmentSnapsOnly: 'endowmentSnapsOnly', @@ -235,211 +248,294 @@ const PermissionNames = { PermissionKeys.wallet_noopWithPermittedSideEffects, wallet_noopWithRequiredCaveat: PermissionKeys.wallet_noopWithRequiredCaveat, wallet_noopWithFactory: PermissionKeys.wallet_noopWithFactory, + wallet_noopWithManyCaveats: PermissionKeys.wallet_noopWithManyCaveats, snap_foo: PermissionKeys.snap_foo, endowmentAnySubject: PermissionKeys.endowmentAnySubject, endowmentSnapsOnly: PermissionKeys.endowmentSnapsOnly, } as const; +// Default side-effect implementations. +const onPermittedSideEffect = () => Promise.resolve('foo'); +const onFailureSideEffect = () => Promise.resolve(); + +/** + * Gets the mocks for the side effect handlers of the permissions that have side effects. + * Mocking these handlers is complicated by the fact that their use by the permission + * controller precludes using Jest mock functions directly. We must create the mocks + * separately, then wrap them inside a plain function in the actual permission + * specification. This otherwise circuitous nonsense still allows us to access the + * underlying mocks in tests. + * + * @returns The side effect mocks. + */ +function getSideEffectHandlerMocks() { + return { + [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects]: { + onPermitted: jest.fn(onPermittedSideEffect), + onFailure: jest.fn(onFailureSideEffect), + }, + [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2]: { + onPermitted: jest.fn(onPermittedSideEffect), + onFailure: jest.fn(onFailureSideEffect), + }, + [PermissionKeys.wallet_noopWithPermittedSideEffects]: { + onPermitted: jest.fn(onPermittedSideEffect), + }, + } as const; +} + /** * Gets permission specifications for our test permissions. * Used as a default in {@link getPermissionControllerOptions}. * * @returns The permission specifications. */ -function getDefaultPermissionSpecifications() { - return { - [PermissionKeys.wallet_getSecretArray]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_getSecretArray, - allowedCaveats: [ - CaveatTypes.filterArrayResponse, - CaveatTypes.reverseArrayResponse, - ], - methodImplementation: (_args: RestrictedMethodOptions) => { - return ['a', 'b', 'c']; - }, - }, - [PermissionKeys.wallet_getSecretObject]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_getSecretObject, - allowedCaveats: [ - CaveatTypes.filterObjectResponse, - CaveatTypes.noopCaveat, - ], - methodImplementation: ( - _args: RestrictedMethodOptions>, - ) => { - return { a: 'x', b: 'y', c: 'z' }; - }, - validator: (permission: PermissionConstraint) => { - // A dummy validator for a caveat type that should be impossible to add - assert.ok( - !permission.caveats?.some( - (caveat) => caveat.type === CaveatTypes.filterArrayResponse, - ), - 'getSecretObject permission validation failed', - ); +function getDefaultPermissionSpecificationsAndMocks() { + const sideEffectMocks = getSideEffectHandlerMocks(); + + return [ + { + [PermissionKeys.wallet_getSecretArray]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_getSecretArray, + allowedCaveats: [ + CaveatTypes.filterArrayResponse, + CaveatTypes.reverseArrayResponse, + ], + methodImplementation: (_args: RestrictedMethodOptions) => { + return ['a', 'b', 'c']; + }, }, - }, - [PermissionKeys.wallet_doubleNumber]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_doubleNumber, - allowedCaveats: null, - methodImplementation: ({ params }: RestrictedMethodOptions<[number]>) => { - if (!Array.isArray(params)) { - throw new Error( - `Invalid ${PermissionKeys.wallet_doubleNumber} request`, + [PermissionKeys.wallet_getSecretObject]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_getSecretObject, + allowedCaveats: [ + CaveatTypes.filterObjectResponse, + CaveatTypes.noopCaveat, + ], + methodImplementation: ( + _args: RestrictedMethodOptions>, + ) => { + return { a: 'x', b: 'y', c: 'z' }; + }, + validator: (permission: PermissionConstraint) => { + // A dummy validator for a caveat type that should be impossible to add + assert.ok( + !permission.caveats?.some( + (caveat) => caveat.type === CaveatTypes.filterArrayResponse, + ), + 'getSecretObject permission validation failed', ); - } - return params[0] * 2; + }, }, - }, - [PermissionKeys.wallet_noop]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_noop, - allowedCaveats: null, - methodImplementation: (_args: RestrictedMethodOptions) => { - return null; + [PermissionKeys.wallet_doubleNumber]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_doubleNumber, + allowedCaveats: null, + methodImplementation: ({ + params, + }: RestrictedMethodOptions<[number]>) => { + if (!Array.isArray(params)) { + throw new Error( + `Invalid ${PermissionKeys.wallet_doubleNumber} request`, + ); + } + return params[0] * 2; + }, }, - }, - [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects, - allowedCaveats: null, - methodImplementation: (_args: RestrictedMethodOptions) => { - return null; + [PermissionKeys.wallet_noop]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_noop, + allowedCaveats: null, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, }, - sideEffect: { - onPermitted, - onFailure, + [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects]: { + permissionType: PermissionType.RestrictedMethod, + targetName: + PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects, + allowedCaveats: null, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, + sideEffect: { + onPermitted: () => + sideEffectMocks[ + PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects + ].onPermitted(), + onFailure: () => + sideEffectMocks[ + PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects + ].onFailure(), + }, }, - }, - [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2, - allowedCaveats: null, - methodImplementation: (_args: RestrictedMethodOptions) => { - return null; + [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2]: { + permissionType: PermissionType.RestrictedMethod, + targetName: + PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2, + allowedCaveats: null, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, + sideEffect: { + onPermitted: () => + sideEffectMocks[ + PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2 + ].onPermitted(), + onFailure: () => + sideEffectMocks[ + PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2 + ].onFailure(), + }, }, - sideEffect: { - onPermitted, - onFailure, + [PermissionKeys.wallet_noopWithPermittedSideEffects]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_noopWithPermittedSideEffects, + allowedCaveats: null, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, + sideEffect: { + onPermitted: () => + sideEffectMocks[ + PermissionKeys.wallet_noopWithPermittedSideEffects + ].onPermitted(), + }, }, - }, - [PermissionKeys.wallet_noopWithPermittedSideEffects]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_noopWithPermittedSideEffects, - allowedCaveats: null, - methodImplementation: (_args: RestrictedMethodOptions) => { - return null; + // This one exists to check some permission validator logic + [PermissionKeys.wallet_noopWithValidator]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_noopWithValidator, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, + allowedCaveats: [ + CaveatTypes.noopCaveat, + CaveatTypes.filterArrayResponse, + ], + validator: (permission: PermissionConstraint) => { + if ( + permission.caveats?.some( + ({ type }) => type !== CaveatTypes.noopCaveat, + ) + ) { + throw new Error('noop permission validation failed'); + } + }, }, - sideEffect: { - onPermitted, + [PermissionKeys.wallet_noopWithRequiredCaveat]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_noopWithRequiredCaveat, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, + allowedCaveats: [CaveatTypes.noopCaveat], + factory: ( + options: PermissionOptions, + _requestData?: Record, + ) => { + return constructPermission({ + ...options, + caveats: [ + { + type: CaveatTypes.noopCaveat, + value: null, + }, + ], + }); + }, + validator: (permission: PermissionConstraint) => { + if ( + permission.caveats?.length !== 1 || + !permission.caveats?.some( + ({ type }) => type === CaveatTypes.noopCaveat, + ) + ) { + throw new Error( + 'noopWithRequiredCaveat permission validation failed', + ); + } + }, }, - }, - // This one exists to check some permission validator logic - [PermissionKeys.wallet_noopWithValidator]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_noopWithValidator, - methodImplementation: (_args: RestrictedMethodOptions) => { - return null; + // This one exists just to check that permission factories can use the + // requestData of approved permission requests + [PermissionKeys.wallet_noopWithFactory]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_noopWithFactory, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, + allowedCaveats: [CaveatTypes.filterArrayResponse], + factory: ( + options: PermissionOptions, + requestData?: Record, + ) => { + if (!requestData) { + throw new Error('requestData is required'); + } + + return constructPermission({ + ...options, + caveats: [ + { + type: CaveatTypes.filterArrayResponse, + value: requestData.caveatValue as string[], + }, + ], + }); + }, }, - allowedCaveats: [CaveatTypes.noopCaveat, CaveatTypes.filterArrayResponse], - validator: (permission: PermissionConstraint) => { - if ( - permission.caveats?.some( - ({ type }) => type !== CaveatTypes.noopCaveat, - ) - ) { - throw new Error('noop permission validation failed'); - } + // The implementation of this is fundamentally broken due to its allowed + // caveats, but that's okay because we never need to actually execute it. + // Originally created for the purpose of testing caveat merging. + [PermissionKeys.wallet_noopWithManyCaveats]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.wallet_noopWithManyCaveats, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, + allowedCaveats: [ + CaveatTypes.filterArrayResponse, + CaveatTypes.filterObjectResponse, + CaveatTypes.noopCaveat, + ], }, - }, - [PermissionKeys.wallet_noopWithRequiredCaveat]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_noopWithRequiredCaveat, - methodImplementation: (_args: RestrictedMethodOptions) => { - return null; + [PermissionKeys.snap_foo]: { + permissionType: PermissionType.RestrictedMethod, + targetName: PermissionKeys.snap_foo, + allowedCaveats: null, + methodImplementation: (_args: RestrictedMethodOptions) => { + return null; + }, + subjectTypes: [SubjectType.Snap], }, - allowedCaveats: [CaveatTypes.noopCaveat], - factory: ( - options: PermissionOptions, - _requestData?: Record, - ) => { - return constructPermission({ - ...options, - caveats: [ - { - type: CaveatTypes.noopCaveat, - value: null, - }, - ], - }); + [PermissionKeys.endowmentAnySubject]: { + permissionType: PermissionType.Endowment, + targetName: PermissionKeys.endowmentAnySubject, + endowmentGetter: (_options: EndowmentGetterParams) => ['endowment1'], + allowedCaveats: null, }, - validator: (permission: PermissionConstraint) => { - if ( - permission.caveats?.length !== 1 || - !permission.caveats?.some( - ({ type }) => type === CaveatTypes.noopCaveat, - ) - ) { - throw new Error( - 'noopWithRequiredCaveat permission validation failed', - ); - } + [PermissionKeys.endowmentSnapsOnly]: { + permissionType: PermissionType.Endowment, + targetName: PermissionKeys.endowmentSnapsOnly, + endowmentGetter: (_options: EndowmentGetterParams) => ['endowment2'], + allowedCaveats: [CaveatTypes.endowmentCaveat], + subjectTypes: [SubjectType.Snap], }, }, - // This one exists just to check that permission factories can use the - // requestData of approved permission requests - [PermissionKeys.wallet_noopWithFactory]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.wallet_noopWithFactory, - methodImplementation: (_args: RestrictedMethodOptions) => { - return null; - }, - allowedCaveats: [CaveatTypes.filterArrayResponse], - factory: ( - options: PermissionOptions, - requestData?: Record, - ) => { - if (!requestData) { - throw new Error('requestData is required'); - } + sideEffectMocks, + ] as const; +} - return constructPermission({ - ...options, - caveats: [ - { - type: CaveatTypes.filterArrayResponse, - value: requestData.caveatValue as string[], - }, - ], - }); - }, - }, - [PermissionKeys.snap_foo]: { - permissionType: PermissionType.RestrictedMethod, - targetName: PermissionKeys.snap_foo, - allowedCaveats: null, - methodImplementation: (_args: RestrictedMethodOptions) => { - return null; - }, - subjectTypes: [SubjectType.Snap], - }, - [PermissionKeys.endowmentAnySubject]: { - permissionType: PermissionType.Endowment, - targetName: PermissionKeys.endowmentAnySubject, - endowmentGetter: (_options: EndowmentGetterParams) => ['endowment1'], - allowedCaveats: null, - }, - [PermissionKeys.endowmentSnapsOnly]: { - permissionType: PermissionType.Endowment, - targetName: PermissionKeys.endowmentSnapsOnly, - endowmentGetter: (_options: EndowmentGetterParams) => ['endowment2'], - allowedCaveats: [CaveatTypes.endowmentCaveat], - subjectTypes: [SubjectType.Snap], - }, - } as const; +/** + * Gets permission specifications for our test permissions. + * Used as a default in {@link getPermissionControllerOptions}. + * + * @returns The permission specifications. + */ +function getDefaultPermissionSpecifications() { + return getDefaultPermissionSpecificationsAndMocks()[0]; } type DefaultPermissionSpecifications = ExtractSpecifications< @@ -629,6 +725,7 @@ describe('PermissionController', () => { beforeEach(() => { jest.clearAllMocks(); }); + describe('constructor', () => { it('initializes a new PermissionController', () => { const controller = getDefaultPermissionController(); @@ -2891,567 +2988,797 @@ describe('PermissionController', () => { }); }); - describe('requestPermissions', () => { - it('requests a permission', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; + // See requestPermissionsIncremental for further tests + describe('grantPermissionsIncremental', () => { + it('incrementally grants a permission', () => { + const controller = getDefaultPermissionControllerWithState(); const origin = 'metamask.io'; - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; - }); - - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, - { - [PermissionNames.wallet_getSecretArray]: {}, + expect(controller.state).toStrictEqual({ + subjects: { + [origin]: { + origin, + permissions: { + wallet_getSecretArray: getPermissionMatcher({ + parentCapability: 'wallet_getSecretArray', + }), + }, }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretArray, - caveats: null, - invoker: origin, - }), }, - { id: expect.any(String), origin }, - ]); + }); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + controller.grantPermissionsIncremental({ + subject: { origin }, + approvedPermissions: { + wallet_getSecretObject: {}, + }, + }); + + expect(controller.state).toStrictEqual({ + subjects: { + [origin]: { + origin, + permissions: expect.objectContaining({ + wallet_getSecretArray: getPermissionMatcher({ + parentCapability: 'wallet_getSecretArray', + }), + wallet_getSecretObject: getPermissionMatcher({ + parentCapability: 'wallet_getSecretObject', + }), + }), }, - type: MethodNames.requestPermissions, }, - true, - ); + }); }); - it('allows caller passing additional metadata', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; + it('incrementally grants a caveat to an existing permission', () => { + const controller = getDefaultPermissionController(); const origin = 'metamask.io'; + const caveat1 = { type: CaveatTypes.filterArrayResponse, value: ['foo'] }; + const caveat2 = { + type: CaveatTypes.filterObjectResponse, + value: ['bar'], + }; - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; - }); + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + wallet_noopWithManyCaveats: { + caveats: [{ ...caveat1 }], + }, + }, + }); - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, - { - [PermissionNames.wallet_getSecretArray]: {}, + controller.grantPermissionsIncremental({ + subject: { origin }, + approvedPermissions: { + wallet_noopWithManyCaveats: { + caveats: [{ ...caveat2 }], }, - { metadata: { foo: 'bar' } }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretArray, - caveats: null, - invoker: origin, - }), }, - { id: expect.any(String), origin }, - ]); + }); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { foo: 'bar', id: expect.any(String), origin }, - permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + expect(controller.state).toStrictEqual({ + subjects: { + [origin]: { + origin, + permissions: expect.objectContaining({ + wallet_noopWithManyCaveats: getPermissionMatcher({ + parentCapability: 'wallet_noopWithManyCaveats', + caveats: [{ ...caveat1 }, { ...caveat2 }], + }), + }), }, - type: MethodNames.requestPermissions, }, - true, - ); + }); }); - it('requests a permission that requires permitted side-effects', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; + it('incrementally updates a caveat of an existing permission', () => { + const controller = getDefaultPermissionController(); const origin = 'metamask.io'; + const getCaveat = (...values: string[]) => ({ + type: CaveatTypes.filterArrayResponse, + value: values, + }); - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; - }); - - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, - { - [PermissionNames.wallet_noopWithPermittedSideEffects]: {}, + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + wallet_noopWithManyCaveats: { + caveats: [getCaveat('foo')], }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_noopWithPermittedSideEffects]: - getPermissionMatcher({ - parentCapability: - PermissionNames.wallet_noopWithPermittedSideEffects, - caveats: null, - invoker: origin, - }), }, - { - data: { - [PermissionNames.wallet_noopWithPermittedSideEffects]: 'foo', + }); + + controller.grantPermissionsIncremental({ + subject: { origin }, + approvedPermissions: { + wallet_noopWithManyCaveats: { + caveats: [getCaveat('foo', 'bar')], }, - id: expect.any(String), - origin, }, - ]); - expect(onPermittedMock).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { - [PermissionNames.wallet_noopWithPermittedSideEffects]: {}, - }, + }); + + expect(controller.state).toStrictEqual({ + subjects: { + [origin]: { + origin, + permissions: expect.objectContaining({ + wallet_noopWithManyCaveats: getPermissionMatcher({ + parentCapability: 'wallet_noopWithManyCaveats', + caveats: [getCaveat('foo', 'bar')], + }), + }), }, - type: MethodNames.requestPermissions, }, - true, - ); + }); }); + }); - it('requests a permission that requires permitted and failure side-effects', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + describe('requesting permissions', () => { + const getAnyPermissionsDiffMatcher = () => + expect.objectContaining({ + currentPermissions: expect.any(Object), + permissionDiffMap: expect.any(Object), + }); - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; - }); + describe.each([ + 'requestPermissions', + 'requestPermissionsIncremental', + ] as const)('%s', (requestFunctionName) => { + const getRequestDataDiffProperty = () => + requestFunctionName === 'requestPermissionsIncremental' + ? { diff: getAnyPermissionsDiffMatcher() } + : {}; + + it('requests a permission', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, + const controller = getDefaultPermissionController(options); + expect( + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_getSecretArray]: {}, + }, + ), + ).toMatchObject([ { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: {}, - }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: - getPermissionMatcher({ - parentCapability: - PermissionNames.wallet_noopWithPermittedAndFailureSideEffects, + [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_getSecretArray, caveats: null, invoker: origin, }), - }, - { - data: { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: - 'foo', }, - id: expect.any(String), - origin, - }, - ]); + { id: expect.any(String), origin }, + ]); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(onPermittedMock).toHaveBeenCalledTimes(1); - expect(onFailureMock).not.toHaveBeenCalled(); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: - {}, + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + ...getRequestDataDiffProperty(), }, + type: MethodNames.requestPermissions, }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); + true, + ); + }); - it('can handle multiple side effects', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('allows caller passing additional metadata', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; - }); + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, + const controller = getDefaultPermissionController(options); + expect( + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_getSecretArray]: {}, + }, + { metadata: { foo: 'bar' } }, + ), + ).toMatchObject([ { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: {}, - [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2]: {}, - }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: - getPermissionMatcher({ - parentCapability: - PermissionNames.wallet_noopWithPermittedAndFailureSideEffects, + [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_getSecretArray, caveats: null, invoker: origin, }), - }, - { - data: { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: - 'foo', - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2]: - 'foo', }, - id: expect.any(String), - origin, - }, - ]); + { id: expect.any(String), origin }, + ]); - expect(onPermittedMock).toHaveBeenCalledTimes(2); - expect(onFailureMock).not.toHaveBeenCalled(); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: - {}, - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2]: - {}, + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { foo: 'bar', id: expect.any(String), origin }, + permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + ...getRequestDataDiffProperty(), }, + type: MethodNames.requestPermissions, }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); + true, + ); + }); - it('can handle permitted multiple side-effect failure', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('requests a permission that requires permitted side-effects', async () => { + const [permissionSpecifications, sideEffectMocks] = + getDefaultPermissionSpecificationsAndMocks(); + const options = getPermissionControllerOptions({ + permissionSpecifications, + }); + const { messenger } = options; + const origin = 'metamask.io'; - onPermittedMock.mockImplementation(async () => - Promise.reject(new Error('error')), - ); + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; - }); + const controller = getDefaultPermissionController(options); - const controller = getDefaultPermissionController(options); - await expect(async () => - controller.requestPermissions( - { origin }, + expect( + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_noopWithPermittedSideEffects]: {}, + }, + ), + ).toMatchObject([ { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: {}, - [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2]: {}, + [PermissionNames.wallet_noopWithPermittedSideEffects]: + getPermissionMatcher({ + parentCapability: + PermissionNames.wallet_noopWithPermittedSideEffects, + caveats: null, + invoker: origin, + }), }, - ), - ).rejects.toThrow( - 'Multiple errors occurred during side-effects execution', - ); - - expect(onPermittedMock).toHaveBeenCalledTimes(2); - expect(onFailureMock).toHaveBeenCalledTimes(2); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: - {}, - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2]: - {}, + { + data: { + [PermissionNames.wallet_noopWithPermittedSideEffects]: 'foo', }, + id: expect.any(String), + origin, }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); - - it('can handle permitted side-effect rejection', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + ]); - onPermittedMock.mockImplementation(async () => - Promise.reject(new Error('error')), - ); - - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; - }); + expect( + sideEffectMocks[PermissionNames.wallet_noopWithPermittedSideEffects] + .onPermitted, + ).toHaveBeenCalledTimes(1); - const controller = getDefaultPermissionController(options); - await expect(async () => - controller.requestPermissions( - { origin }, + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', { - [PermissionNames.wallet_noopWithPermittedSideEffects]: {}, - }, - ), - ).rejects.toThrow('error'); - - expect(onPermittedMock).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { - [PermissionNames.wallet_noopWithPermittedSideEffects]: {}, + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_noopWithPermittedSideEffects]: {}, + }, + ...getRequestDataDiffProperty(), }, + type: MethodNames.requestPermissions, }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); - - it('can handle failure side-effect rejection', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - - onPermittedMock.mockImplementation(async () => - Promise.reject(new Error('error')), - ); - - onFailureMock.mockImplementation(async () => - Promise.reject(new Error('error')), - ); + true, + ); + }); - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; + it('requests a permission that requires permitted and failure side-effects', async () => { + const [permissionSpecifications, sideEffectMocks] = + getDefaultPermissionSpecificationsAndMocks(); + const options = getPermissionControllerOptions({ + permissionSpecifications, }); + const { messenger } = options; + const origin = 'metamask.io'; - const controller = getDefaultPermissionController(options); - await expect(async () => - controller.requestPermissions( - { origin }, - { - [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: {}, - }, - ), - ).rejects.toThrow('Unexpected error in side-effects'); + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); - expect(onPermittedMock).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { + const controller = getDefaultPermissionController(options); + expect( + await controller[requestFunctionName]( + { origin }, + { [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: {}, }, + ), + ).toMatchObject([ + { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + getPermissionMatcher({ + parentCapability: + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects, + caveats: null, + invoker: origin, + }), }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); + { + data: { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + 'foo', + }, + id: expect.any(String), + origin, + }, + ]); - it('requests a permission that requires requestData in its factory', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onPermitted, + ).toHaveBeenCalledTimes(1); - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - caveatValue: ['foo'], // this will be added to the permission - }; - }); + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onFailure, + ).not.toHaveBeenCalled(); - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', { - [PermissionNames.wallet_noopWithFactory]: {}, - }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_noopWithFactory]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_noopWithFactory, - caveats: [ - { type: CaveatTypes.filterArrayResponse, value: ['foo'] }, - ], - invoker: origin, - }), - }, - { id: expect.any(String), origin }, - ]); + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + {}, + }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, + }, + true, + ); + }); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { - [PermissionNames.wallet_noopWithFactory]: {}, + it('can handle multiple side-effects', async () => { + const [permissionSpecifications, sideEffectMocks] = + getDefaultPermissionSpecificationsAndMocks(); + const options = getPermissionControllerOptions({ + permissionSpecifications, + }); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); + + const controller = getDefaultPermissionController(options); + expect( + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + {}, + [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2]: + {}, }, + ), + ).toMatchObject([ + { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + getPermissionMatcher({ + parentCapability: + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects, + caveats: null, + invoker: origin, + }), }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); + { + data: { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + 'foo', + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2]: + 'foo', + }, + id: expect.any(String), + origin, + }, + ]); - it('requests multiple permissions', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onPermitted, + ).toHaveBeenCalledTimes(1); + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2 + ].onPermitted, + ).toHaveBeenCalledTimes(1); - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onFailure, + ).not.toHaveBeenCalled(); + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2 + ].onFailure, + ).not.toHaveBeenCalled(); + + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + {}, + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2]: + {}, + }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, + }, + true, + ); + }); + + it('can handle multiple permitted side-effect failures', async () => { + const [permissionSpecifications, sideEffectMocks] = + getDefaultPermissionSpecificationsAndMocks(); + const options = getPermissionControllerOptions({ + permissionSpecifications, }); + const { messenger } = options; + const origin = 'metamask.io'; - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onPermitted.mockImplementation(() => + Promise.reject(new Error('error')), + ); + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2 + ].onPermitted.mockImplementation(() => + Promise.reject(new Error('error')), + ); + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); + + const controller = getDefaultPermissionController(options); + await expect(async () => + controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + {}, + [PermissionKeys.wallet_noopWithPermittedAndFailureSideEffects2]: + {}, + }, + ), + ).rejects.toThrow( + 'Multiple errors occurred during side-effects execution', + ); + + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onPermitted, + ).toHaveBeenCalledTimes(1); + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2 + ].onPermitted, + ).toHaveBeenCalledTimes(1); + + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onFailure, + ).toHaveBeenCalledTimes(1); + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2 + ].onFailure, + ).toHaveBeenCalledTimes(1); + + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', { - [PermissionNames.wallet_getSecretArray]: {}, - [PermissionNames.wallet_getSecretObject]: { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + {}, + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects2]: + {}, + }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, + }, + true, + ); + }); + + it('can handle permitted side-effect rejection (no failure handler)', async () => { + const [permissionSpecifications, sideEffectMocks] = + getDefaultPermissionSpecificationsAndMocks(); + const options = getPermissionControllerOptions({ + permissionSpecifications, + }); + const { messenger } = options; + const origin = 'metamask.io'; + + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedSideEffects + ].onPermitted.mockImplementation(() => + Promise.reject(new Error('error')), + ); + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); + + const controller = getDefaultPermissionController(options); + await expect(async () => + controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_noopWithPermittedSideEffects]: {}, + }, + ), + ).rejects.toThrow('error'); + + expect( + sideEffectMocks[PermissionNames.wallet_noopWithPermittedSideEffects] + .onPermitted, + ).toHaveBeenCalledTimes(1); + + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_noopWithPermittedSideEffects]: {}, + }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, + }, + true, + ); + }); + + it('can handle failure side-effect rejection', async () => { + const [permissionSpecifications, sideEffectMocks] = + getDefaultPermissionSpecificationsAndMocks(); + const options = getPermissionControllerOptions({ + permissionSpecifications, + }); + const { messenger } = options; + const origin = 'metamask.io'; + + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onPermitted.mockImplementation(() => + Promise.reject(new Error('error')), + ); + + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onFailure.mockImplementation(() => + Promise.reject(new Error('error')), + ); + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); + + const controller = getDefaultPermissionController(options); + await expect(async () => + controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + {}, + }, + ), + ).rejects.toThrow('Unexpected error in side-effects'); + + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onPermitted, + ).toHaveBeenCalledTimes(1); + + expect( + sideEffectMocks[ + PermissionNames.wallet_noopWithPermittedAndFailureSideEffects + ].onFailure, + ).toHaveBeenCalledTimes(1); + + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_noopWithPermittedAndFailureSideEffects]: + {}, + }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, + }, + true, + ); + }); + + it('requests a permission that requires requestData in its factory', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + caveatValue: ['foo'], // this will be added to the permission + }; + }); + + const controller = getDefaultPermissionController(options); + expect( + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_noopWithFactory]: {}, + }, + ), + ).toMatchObject([ + { + [PermissionNames.wallet_noopWithFactory]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_noopWithFactory, caveats: [ - { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, + { type: CaveatTypes.filterArrayResponse, value: ['foo'] }, ], + invoker: origin, + }), + }, + { id: expect.any(String), origin }, + ]); + + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_noopWithFactory]: {}, + }, + ...getRequestDataDiffProperty(), }, + type: MethodNames.requestPermissions, }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretArray, - caveats: null, - invoker: origin, - }), - [PermissionNames.wallet_getSecretObject]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretObject, - caveats: [ - { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, - ], - invoker: origin, - }), - }, - { id: expect.any(String), origin }, - ]); + true, + ); + }); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { + it('requests multiple permissions', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); + + const controller = getDefaultPermissionController(options); + expect( + await controller[requestFunctionName]( + { origin }, + { [PermissionNames.wallet_getSecretArray]: {}, [PermissionNames.wallet_getSecretObject]: { caveats: [ @@ -3459,77 +3786,73 @@ describe('PermissionController', () => { ], }, }, - }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); - - it('requests multiple permissions (approved permissions are a strict superset)', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - // endowmentAnySubject is added to the request - permissions: { - ...requestData.permissions, - [PermissionNames.endowmentAnySubject]: {}, - }, - }; - }); - - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, + ), + ).toMatchObject([ { - [PermissionNames.wallet_getSecretArray]: {}, - [PermissionNames.wallet_getSecretObject]: { + [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_getSecretArray, + caveats: null, + invoker: origin, + }), + [PermissionNames.wallet_getSecretObject]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_getSecretObject, caveats: [ { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, ], + invoker: origin, + }), + }, + { id: expect.any(String), origin }, + ]); + + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_getSecretArray]: {}, + [PermissionNames.wallet_getSecretObject]: { + caveats: [ + { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, + ], + }, + }, + ...getRequestDataDiffProperty(), }, + type: MethodNames.requestPermissions, }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretArray, - caveats: null, - invoker: origin, - }), - [PermissionNames.wallet_getSecretObject]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretObject, - caveats: [ - { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, - ], - invoker: origin, - }), - [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ - parentCapability: PermissionNames.endowmentAnySubject, - caveats: null, - invoker: origin, - }), - }, - { id: expect.any(String), origin }, - ]); + true, + ); + }); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { + it('requests multiple permissions (approved permissions are a strict superset)', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + // endowmentAnySubject is added to the request + permissions: { + ...requestData.permissions, + [PermissionNames.endowmentAnySubject]: {}, + }, + }; + }); + + const controller = getDefaultPermissionController(options); + expect( + await controller[requestFunctionName]( + { origin }, + { [PermissionNames.wallet_getSecretArray]: {}, [PermissionNames.wallet_getSecretObject]: { caveats: [ @@ -3537,72 +3860,77 @@ describe('PermissionController', () => { ], }, }, - }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); - - it('requests multiple permissions (approved permissions are a strict subset)', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - const approvedPermissions = { ...requestData.permissions }; - delete approvedPermissions[PermissionNames.wallet_getSecretArray]; - - return { - metadata: { ...requestData.metadata }, - permissions: approvedPermissions, - }; - }); - - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, + ), + ).toMatchObject([ { - [PermissionNames.wallet_getSecretArray]: {}, - [PermissionNames.wallet_getSecretObject]: { + [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_getSecretArray, + caveats: null, + invoker: origin, + }), + [PermissionNames.wallet_getSecretObject]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_getSecretObject, caveats: [ { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, ], + invoker: origin, + }), + [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ + parentCapability: PermissionNames.endowmentAnySubject, + caveats: null, + invoker: origin, + }), + }, + { id: expect.any(String), origin }, + ]); + + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_getSecretArray]: {}, + [PermissionNames.wallet_getSecretObject]: { + caveats: [ + { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, + ], + }, + }, + ...getRequestDataDiffProperty(), }, - [PermissionNames.endowmentAnySubject]: {}, + type: MethodNames.requestPermissions, }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_getSecretObject]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretObject, - caveats: [ - { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, - ], - invoker: origin, - }), - [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ - parentCapability: PermissionNames.endowmentAnySubject, - caveats: null, - invoker: origin, - }), - }, - { id: expect.any(String), origin }, - ]); + true, + ); + }); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { + it('requests multiple permissions (approved permissions are a strict subset)', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + const approvedPermissions = { ...requestData.permissions }; + delete approvedPermissions[PermissionNames.wallet_getSecretArray]; + + return { + metadata: { ...requestData.metadata }, + permissions: approvedPermissions, + }; + }); + + const controller = getDefaultPermissionController(options); + expect( + await controller[requestFunctionName]( + { origin }, + { [PermissionNames.wallet_getSecretArray]: {}, [PermissionNames.wallet_getSecretObject]: { caveats: [ @@ -3611,81 +3939,77 @@ describe('PermissionController', () => { }, [PermissionNames.endowmentAnySubject]: {}, }, - }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); - - it('requests multiple permissions (an approved permission is modified)', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - const approvedPermissions = { ...requestData.permissions }; - approvedPermissions[PermissionNames.wallet_getSecretObject] = { - caveats: [ - { type: CaveatTypes.filterObjectResponse, value: ['kaplar'] }, - ], - }; - - return { - metadata: { ...requestData.metadata }, - permissions: approvedPermissions, - }; - }); - - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, + ), + ).toMatchObject([ { - [PermissionNames.wallet_getSecretArray]: {}, - [PermissionNames.wallet_getSecretObject]: { + [PermissionNames.wallet_getSecretObject]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_getSecretObject, caveats: [ { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, ], + invoker: origin, + }), + [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ + parentCapability: PermissionNames.endowmentAnySubject, + caveats: null, + invoker: origin, + }), + }, + { id: expect.any(String), origin }, + ]); + + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_getSecretArray]: {}, + [PermissionNames.wallet_getSecretObject]: { + caveats: [ + { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, + ], + }, + [PermissionNames.endowmentAnySubject]: {}, + }, + ...getRequestDataDiffProperty(), }, - [PermissionNames.endowmentAnySubject]: {}, + type: MethodNames.requestPermissions, }, - ), - ).toMatchObject([ - { - [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretArray, - caveats: null, - invoker: origin, - }), - [PermissionNames.wallet_getSecretObject]: getPermissionMatcher({ - parentCapability: PermissionNames.wallet_getSecretObject, - caveats: [ - { type: CaveatTypes.filterObjectResponse, value: ['kaplar'] }, - ], - invoker: origin, - }), - [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ - parentCapability: PermissionNames.endowmentAnySubject, - caveats: null, - invoker: origin, - }), - }, - { id: expect.any(String), origin }, - ]); + true, + ); + }); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { + it('requests multiple permissions (an approved permission is modified)', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + const approvedPermissions = { ...requestData.permissions }; + approvedPermissions[PermissionNames.wallet_getSecretObject] = { + caveats: [ + { type: CaveatTypes.filterObjectResponse, value: ['kaplar'] }, + ], + }; + + return { + metadata: { ...requestData.metadata }, + permissions: approvedPermissions, + }; + }); + + const controller = getDefaultPermissionController(options); + expect( + await controller[requestFunctionName]( + { origin }, + { [PermissionNames.wallet_getSecretArray]: {}, [PermissionNames.wallet_getSecretObject]: { caveats: [ @@ -3694,386 +4018,517 @@ describe('PermissionController', () => { }, [PermissionNames.endowmentAnySubject]: {}, }, + ), + ).toMatchObject([ + { + [PermissionNames.wallet_getSecretArray]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_getSecretArray, + caveats: null, + invoker: origin, + }), + [PermissionNames.wallet_getSecretObject]: getPermissionMatcher({ + parentCapability: PermissionNames.wallet_getSecretObject, + caveats: [ + { type: CaveatTypes.filterObjectResponse, value: ['kaplar'] }, + ], + invoker: origin, + }), + [PermissionNames.endowmentAnySubject]: getPermissionMatcher({ + parentCapability: PermissionNames.endowmentAnySubject, + caveats: null, + invoker: origin, + }), }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); + { id: expect.any(String), origin }, + ]); - it('throws if requested permissions object is not a plain object', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - const controller = getDefaultPermissionController(options); + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_getSecretArray]: {}, + [PermissionNames.wallet_getSecretObject]: { + caveats: [ + { type: CaveatTypes.filterObjectResponse, value: ['baz'] }, + ], + }, + [PermissionNames.endowmentAnySubject]: {}, + }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, + }, + true, + ); + }); + + it('throws if requested permissions object is not a plain object', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + const controller = getDefaultPermissionController(options); + + const callActionSpy = jest.spyOn(messenger, 'call'); + + for (const invalidInput of [ + // not plain objects + null, + 'foo', + [{ [PermissionNames.wallet_getSecretArray]: {} }], + ]) { + await expect( + async () => + await controller[requestFunctionName]( + { origin }, + // @ts-expect-error Intentional destructive testing + invalidInput, + ), + ).rejects.toThrow( + errors.invalidParams({ + message: `Requested permissions for origin "${origin}" is not a plain object.`, + data: { origin, requestedPermissions: invalidInput }, + }), + ); + } + + expect(callActionSpy).not.toHaveBeenCalled(); + }); + + it('throws if requested permissions object has no permissions', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest.spyOn(messenger, 'call'); + + const controller = getDefaultPermissionController(options); + await expect( + async () => + // No permissions in object + await controller[requestFunctionName]({ origin }, {}), + ).rejects.toThrow( + errors.invalidParams({ + message: `Permissions request for origin "${origin}" contains no permissions.`, + data: { origin, requestedPermissions: {} }, + }), + ); + + expect(callActionSpy).not.toHaveBeenCalled(); + }); - const callActionSpy = jest.spyOn(messenger, 'call'); + it('throws if requested permissions contain a (key : value.parentCapability) mismatch', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; - for (const invalidInput of [ - // not plain objects - null, - 'foo', - [{ [PermissionNames.wallet_getSecretArray]: {} }], - ]) { + const callActionSpy = jest.spyOn(messenger, 'call'); + + const controller = getDefaultPermissionController(options); await expect( async () => - await controller.requestPermissions( + await controller[requestFunctionName]( { origin }, - // @ts-expect-error Intentional destructive testing - invalidInput, + { + [PermissionNames.wallet_getSecretArray]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + // parentCapability value does not match key + [PermissionNames.wallet_getSecretObject]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + }, ), ).rejects.toThrow( errors.invalidParams({ - message: `Requested permissions for origin "${origin}" is not a plain object.`, - data: { origin, requestedPermissions: invalidInput }, + message: `Permissions request for origin "${origin}" contains invalid requested permission(s).`, + data: { + origin, + requestedPermissions: { + [PermissionNames.wallet_getSecretArray]: { + [PermissionNames.wallet_getSecretArray]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + [PermissionNames.wallet_getSecretObject]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + }, + }, + }, }), ); - } - - expect(callActionSpy).not.toHaveBeenCalled(); - }); - - it('throws if requested permissions object has no permissions', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - - const callActionSpy = jest.spyOn(messenger, 'call'); - - const controller = getDefaultPermissionController(options); - await expect( - async () => - // No permissions in object - await controller.requestPermissions({ origin }, {}), - ).rejects.toThrow( - errors.invalidParams({ - message: `Permissions request for origin "${origin}" contains no permissions.`, - data: { origin, requestedPermissions: {} }, - }), - ); - expect(callActionSpy).not.toHaveBeenCalled(); - }); + expect(callActionSpy).not.toHaveBeenCalled(); + }); - it('throws if requested permissions contain a (key : value.parentCapability) mismatch', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('throws if requesting a permission for an unknown target', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; - const callActionSpy = jest.spyOn(messenger, 'call'); + const callActionSpy = jest.spyOn(messenger, 'call'); - const controller = getDefaultPermissionController(options); - await expect( - async () => - await controller.requestPermissions( - { origin }, - { - [PermissionNames.wallet_getSecretArray]: { - parentCapability: PermissionNames.wallet_getSecretArray, - }, - // parentCapability value does not match key - [PermissionNames.wallet_getSecretObject]: { - parentCapability: PermissionNames.wallet_getSecretArray, + const controller = getDefaultPermissionController(options); + await expect( + async () => + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_getSecretArray]: {}, + wallet_getSecretKabob: {}, }, - }, - ), - ).rejects.toThrow( - errors.invalidParams({ - message: `Permissions request for origin "${origin}" contains invalid requested permission(s).`, - data: { + ), + ).rejects.toThrow( + errors.methodNotFound('wallet_getSecretKabob', { origin, requestedPermissions: { [PermissionNames.wallet_getSecretArray]: { - [PermissionNames.wallet_getSecretArray]: { - parentCapability: PermissionNames.wallet_getSecretArray, - }, - [PermissionNames.wallet_getSecretObject]: { - parentCapability: PermissionNames.wallet_getSecretArray, - }, + [PermissionNames.wallet_getSecretArray]: {}, + wallet_getSecretKabob: {}, }, }, - }, - }), - ); + }), + ); - expect(callActionSpy).not.toHaveBeenCalled(); - }); + expect(callActionSpy).not.toHaveBeenCalled(); + }); - it('throws if requesting a permission for an unknown target', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('throws if permission subjectTypes does not include type of subject (restricted method)', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; - const callActionSpy = jest.spyOn(messenger, 'call'); + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(() => { + return { + origin, + name: origin, + subjectType: SubjectType.Website, + iconUrl: null, + extensionId: null, + }; + }); - const controller = getDefaultPermissionController(options); - await expect( - async () => - await controller.requestPermissions( + const controller = getDefaultPermissionController(options); + await expect( + controller[requestFunctionName]( { origin }, { - [PermissionNames.wallet_getSecretArray]: {}, - wallet_getSecretKabob: {}, + [PermissionNames.snap_foo]: {}, }, ), - ).rejects.toThrow( - errors.methodNotFound('wallet_getSecretKabob', { - origin, - requestedPermissions: { - [PermissionNames.wallet_getSecretArray]: { - [PermissionNames.wallet_getSecretArray]: {}, - wallet_getSecretKabob: {}, - }, - }, - }), - ); + ).rejects.toThrow( + 'The method "snap_foo" does not exist / is not available.', + ); - expect(callActionSpy).not.toHaveBeenCalled(); - }); + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'SubjectMetadataController:getSubjectMetadata', + origin, + ); + }); - it('throws if subjectTypes do not match (restricted method)', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('throws if permission subjectTypes does not include type of subject (endowment)', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(() => { - return { - origin, - name: origin, - subjectType: SubjectType.Website, - iconUrl: null, - extensionId: null, - }; - }); + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(() => { + return { + origin, + name: origin, + subjectType: SubjectType.Website, + iconUrl: null, + extensionId: null, + }; + }); - const controller = getDefaultPermissionController(options); - await expect( - controller.requestPermissions( - { origin }, - { - [PermissionNames.snap_foo]: {}, - }, - ), - ).rejects.toThrow( - 'The method "snap_foo" does not exist / is not available.', - ); + const controller = getDefaultPermissionController(options); + await expect( + controller[requestFunctionName]( + { origin }, + { + [PermissionNames.endowmentSnapsOnly]: {}, + }, + ), + ).rejects.toThrow( + 'Subject "metamask.io" has no permission for "endowmentSnapsOnly".', + ); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'SubjectMetadataController:getSubjectMetadata', - origin, - ); - }); + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'SubjectMetadataController:getSubjectMetadata', + origin, + ); + }); - it('throws if subjectTypes do not match (endowment)', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('does not throw if permission subjectTypes includes type of subject', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = '@metamask/test-snap-bip44'; + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementation((...args) => { + const [action, { requestData }] = args as AddPermissionRequestArgs; + if (action === 'ApprovalController:addRequest') { + return Promise.resolve({ + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }); + } else if ( + action === 'SubjectMetadataController:getSubjectMetadata' + ) { + return { + origin, + name: origin, + subjectType: SubjectType.Snap, + iconUrl: null, + extensionId: null, + }; + } + throw new Error(`Unexpected action: "${action}"`); + }); - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(() => { - return { - origin, - name: origin, - subjectType: SubjectType.Website, - iconUrl: null, - extensionId: null, - }; - }); + const controller = getDefaultPermissionController(options); - const controller = getDefaultPermissionController(options); - await expect( - controller.requestPermissions( - { origin }, + expect( + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.snap_foo]: {}, + }, + ), + ).toMatchObject([ { - [PermissionNames.endowmentSnapsOnly]: {}, + [PermissionNames.snap_foo]: getPermissionMatcher({ + parentCapability: PermissionNames.snap_foo, + caveats: null, + invoker: origin, + }), }, - ), - ).rejects.toThrow( - 'Subject "metamask.io" has no permission for "endowmentSnapsOnly".', - ); - - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'SubjectMetadataController:getSubjectMetadata', - origin, - ); - }); - - it('does not throw if subjectTypes match', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = '@metamask/test-snap-bip44'; - - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(() => { - return { - origin, - name: origin, - subjectType: SubjectType.Snap, - iconUrl: null, - extensionId: null, - }; - }) - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { ...requestData.permissions }, - }; - }) - .mockImplementation(() => { - return { - origin, - name: origin, - subjectType: SubjectType.Snap, - iconUrl: null, - extensionId: null, - }; - }); - - const controller = getDefaultPermissionController(options); - expect( - await controller.requestPermissions( - { origin }, { - [PermissionNames.snap_foo]: {}, + id: expect.any(String), + origin, }, - ), - ).toMatchObject([ - { - [PermissionNames.snap_foo]: getPermissionMatcher({ - parentCapability: PermissionNames.snap_foo, - caveats: null, - invoker: origin, - }), - }, - { - id: expect.any(String), - origin, - }, - ]); + ]); - expect(callActionSpy).toHaveBeenCalledTimes(4); - expect(callActionSpy).toHaveBeenNthCalledWith( - 1, - 'SubjectMetadataController:getSubjectMetadata', - origin, - ); - expect(callActionSpy).toHaveBeenNthCalledWith( - 2, - 'ApprovalController:addRequest', - { - id: expect.any(String), + expect(callActionSpy).toHaveBeenCalledWith( + 'SubjectMetadataController:getSubjectMetadata', origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { [PermissionNames.snap_foo]: {} }, - }, - type: MethodNames.requestPermissions, - }, - true, - ); - expect(callActionSpy).toHaveBeenNthCalledWith( - 3, - 'SubjectMetadataController:getSubjectMetadata', - origin, - ); - expect(callActionSpy).toHaveBeenNthCalledWith( - 4, - 'SubjectMetadataController:getSubjectMetadata', - origin, - ); - }); + ); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { [PermissionNames.snap_foo]: {} }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, + }, + true, + ); + }); - it('throws if the "caveats" property of a requested permission is invalid', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('throws if the "caveats" property of a requested permission is invalid', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; - const callActionSpy = jest.spyOn(messenger, 'call'); + const callActionSpy = jest.spyOn(messenger, 'call'); - const controller = getDefaultPermissionController(options); - for (const invalidCaveatsValue of [ - [], // empty array - undefined, - 'foo', - 2, - Symbol('bar'), - ]) { + const controller = getDefaultPermissionController(options); + for (const invalidCaveatsValue of [ + [], // empty array + undefined, + 'foo', + 2, + Symbol('bar'), + ]) { + await expect( + async () => + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_getSecretArray]: { + // @ts-expect-error Intentional destructive testing + caveats: invalidCaveatsValue, + }, + }, + ), + ).rejects.toThrow( + new errors.InvalidCaveatsPropertyError( + origin, + PermissionNames.wallet_getSecretArray, + invalidCaveatsValue, + ), + ); + + expect(callActionSpy).not.toHaveBeenCalled(); + } + }); + + it('throws if a requested permission has duplicate caveats', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest.spyOn(messenger, 'call'); + + const controller = getDefaultPermissionController(options); await expect( async () => - await controller.requestPermissions( + await controller[requestFunctionName]( { origin }, { [PermissionNames.wallet_getSecretArray]: { - // @ts-expect-error Intentional destructive testing - caveats: invalidCaveatsValue, + caveats: [ + { type: CaveatTypes.filterArrayResponse, value: ['foo'] }, + { type: CaveatTypes.filterArrayResponse, value: ['foo'] }, + ], }, }, ), ).rejects.toThrow( - new errors.InvalidCaveatsPropertyError( + new errors.DuplicateCaveatError( + CaveatTypes.filterArrayResponse, origin, PermissionNames.wallet_getSecretArray, - invalidCaveatsValue, ), ); expect(callActionSpy).not.toHaveBeenCalled(); - } - }); + }); - it('throws if a requested permission has duplicate caveats', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('throws if the approved request object is invalid', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + const controller = getDefaultPermissionController(options); + const callActionSpy = jest.spyOn(messenger, 'call'); + + for (const invalidRequestObject of ['foo', null, { metadata: 'foo' }]) { + callActionSpy.mockClear(); + callActionSpy.mockImplementationOnce( + async () => invalidRequestObject, + ); - const callActionSpy = jest.spyOn(messenger, 'call'); + await expect( + async () => + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_getSecretArray]: {}, + }, + ), + ).rejects.toThrow( + errors.internalError( + `Approved permissions request for subject "${origin}" is invalid.`, + { data: { approvedRequest: invalidRequestObject } }, + ), + ); - const controller = getDefaultPermissionController(options); - await expect( - async () => - await controller.requestPermissions( - { origin }, + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', { - [PermissionNames.wallet_getSecretArray]: { - caveats: [ - { type: CaveatTypes.filterArrayResponse, value: ['foo'] }, - { type: CaveatTypes.filterArrayResponse, value: ['foo'] }, - ], + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + ...getRequestDataDiffProperty(), }, + type: MethodNames.requestPermissions, }, + true, + ); + } + }); + + it('throws if the approved request ID changed', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + // different id + metadata: { ...requestData.metadata, id: 'foo' }, + permissions: { + [PermissionNames.wallet_getSecretArray]: {}, + }, + }; + }); + + const controller = getDefaultPermissionController(options); + await expect( + async () => + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_getSecretArray]: {}, + }, + ), + ).rejects.toThrow( + errors.internalError( + `Approved permissions request for subject "${origin}" mutated its id.`, + { originalId: expect.any(String), mutatedId: 'foo' }, ), - ).rejects.toThrow( - new errors.DuplicateCaveatError( - CaveatTypes.filterArrayResponse, - origin, - PermissionNames.wallet_getSecretArray, - ), - ); + ); - expect(callActionSpy).not.toHaveBeenCalled(); - }); + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, + }, + true, + ); + }); - it('throws if the approved request object is invalid', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - const controller = getDefaultPermissionController(options); - const callActionSpy = jest.spyOn(messenger, 'call'); + it('throws if the approved request origin changed', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; - for (const invalidRequestObject of ['foo', null, { metadata: 'foo' }]) { - callActionSpy.mockClear(); - callActionSpy.mockImplementationOnce(async () => invalidRequestObject); + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + // different origin + metadata: { ...requestData.metadata, origin: 'foo.com' }, + permissions: { + [PermissionNames.wallet_getSecretArray]: {}, + }, + }; + }); + const controller = getDefaultPermissionController(options); await expect( async () => - await controller.requestPermissions( + await controller[requestFunctionName]( { origin }, { [PermissionNames.wallet_getSecretArray]: {}, @@ -4081,8 +4536,8 @@ describe('PermissionController', () => { ), ).rejects.toThrow( errors.internalError( - `Approved permissions request for subject "${origin}" is invalid.`, - { data: { approvedRequest: invalidRequestObject } }, + `Approved permissions request for subject "${origin}" mutated its origin.`, + { originalOrigin: origin, mutatedOrigin: 'foo' }, ), ); @@ -4095,203 +4550,238 @@ describe('PermissionController', () => { requestData: { metadata: { id: expect.any(String), origin }, permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + ...getRequestDataDiffProperty(), }, type: MethodNames.requestPermissions, }, true, ); - } - }); + }); - it('throws if the approved request ID changed', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('throws if no permissions were approved', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - // different id - metadata: { ...requestData.metadata, id: 'foo' }, - permissions: { - [PermissionNames.wallet_getSecretArray]: {}, - }, - }; - }); + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: {}, // no permissions + }; + }); - const controller = getDefaultPermissionController(options); - await expect( - async () => - await controller.requestPermissions( - { origin }, + const controller = getDefaultPermissionController(options); + await expect( + async () => + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_getSecretArray]: {}, + }, + ), + ).rejects.toThrow( + errors.internalError( + `Invalid approved permissions request: Permissions request for origin "${origin}" contains no permissions.`, { [PermissionNames.wallet_getSecretArray]: {}, }, ), - ).rejects.toThrow( - errors.internalError( - `Approved permissions request for subject "${origin}" mutated its id.`, - { originalId: expect.any(String), mutatedId: 'foo' }, - ), - ); + ); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); + true, + ); + }); - it('throws if the approved request origin changed', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + it('throws if approved permissions object is not a plain object', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + const id = 'arbitraryId'; + const controller = getDefaultPermissionController(options); - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; + const callActionSpy = jest.spyOn(messenger, 'call'); + + // The metadata is valid, but the permissions are invalid + const getInvalidRequestObject = (invalidPermissions: unknown) => { return { - // different origin - metadata: { ...requestData.metadata, origin: 'foo.com' }, - permissions: { - [PermissionNames.wallet_getSecretArray]: {}, - }, + metadata: { origin, id }, + permissions: invalidPermissions, }; - }); + }; - const controller = getDefaultPermissionController(options); - await expect( - async () => - await controller.requestPermissions( - { origin }, + for (const invalidRequestObject of [ + null, + 'foo', + [{ [PermissionNames.wallet_getSecretArray]: {} }], + ].map((invalidPermissions) => + getInvalidRequestObject(invalidPermissions), + )) { + callActionSpy.mockClear(); + callActionSpy.mockImplementationOnce( + async () => invalidRequestObject, + ); + + await expect( + async () => + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_getSecretArray]: {}, + }, + { id, preserveExistingPermissions: true }, + ), + ).rejects.toThrow( + errors.internalError( + `Invalid approved permissions request: Requested permissions for origin "${origin}" is not a plain object.`, + { data: { approvedRequest: invalidRequestObject } }, + ), + ); + + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', { - [PermissionNames.wallet_getSecretArray]: {}, + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + ...getRequestDataDiffProperty(), + }, + type: MethodNames.requestPermissions, }, - ), - ).rejects.toThrow( - errors.internalError( - `Approved permissions request for subject "${origin}" mutated its origin.`, - { originalOrigin: origin, mutatedOrigin: 'foo' }, - ), - ); + true, + ); + } + }); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { [PermissionNames.wallet_getSecretArray]: {} }, - }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); + it('throws if approved permissions contain a (key : value.parentCapability) mismatch', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + const controller = getDefaultPermissionController(options); - it('throws if no permissions were approved', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { + [PermissionNames.wallet_getSecretArray]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + // parentCapability value does not match key + [PermissionNames.wallet_getSecretObject]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + }, + }; + }); - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: {}, // no permissions - }; - }); + await expect( + async () => + await controller[requestFunctionName]( + { origin }, + { + [PermissionNames.wallet_getSecretArray]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + }, + ), + ).rejects.toThrow( + errors.invalidParams({ + message: `Invalid approved permissions request: Permissions request for origin "${origin}" contains invalid requested permission(s).`, + data: { + origin, + requestedPermissions: { + [PermissionNames.wallet_getSecretArray]: { + [PermissionNames.wallet_getSecretArray]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + [PermissionNames.wallet_getSecretObject]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + }, + }, + }, + }), + ); - const controller = getDefaultPermissionController(options); - await expect( - async () => - await controller.requestPermissions( - { origin }, - { - [PermissionNames.wallet_getSecretArray]: {}, + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_getSecretArray]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + }, + ...getRequestDataDiffProperty(), }, - ), - ).rejects.toThrow( - errors.internalError( - `Invalid approved permissions request: Permissions request for origin "${origin}" contains no permissions.`, - { - [PermissionNames.wallet_getSecretArray]: {}, - }, - ), - ); - - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + type: MethodNames.requestPermissions, }, - type: MethodNames.requestPermissions, - }, - true, - ); - }); - - it('throws if approved permissions object is not a plain object', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - const id = 'arbitraryId'; - const controller = getDefaultPermissionController(options); - - const callActionSpy = jest.spyOn(messenger, 'call'); + true, + ); + }); - // The metadata is valid, but the permissions are invalid - const getInvalidRequestObject = (invalidPermissions: unknown) => { - return { - metadata: { origin, id }, - permissions: invalidPermissions, - }; - }; + it('correctly throws errors that do not inherit from JsonRpcError', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + const controller = getDefaultPermissionController(options); - for (const invalidRequestObject of [ - null, - 'foo', - [{ [PermissionNames.wallet_getSecretArray]: {} }], - ].map((invalidPermissions) => - getInvalidRequestObject(invalidPermissions), - )) { - callActionSpy.mockClear(); - callActionSpy.mockImplementationOnce(async () => invalidRequestObject); + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { + [PermissionNames.wallet_getSecretArray]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + [PermissionNames.wallet_getSecretObject]: { + parentCapability: PermissionNames.wallet_getSecretObject, + caveats: 'foo', // invalid + }, + }, + }; + }); await expect( async () => - await controller.requestPermissions( + await controller[requestFunctionName]( { origin }, { - [PermissionNames.wallet_getSecretArray]: {}, + [PermissionNames.wallet_getSecretArray]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, }, - { id, preserveExistingPermissions: true }, ), ).rejects.toThrow( errors.internalError( - `Invalid approved permissions request: Requested permissions for origin "${origin}" is not a plain object.`, - { data: { approvedRequest: invalidRequestObject } }, + `Invalid approved permissions request: The "caveats" property of permission for "${PermissionNames.wallet_getSecretObject}" of subject "${origin}" is invalid. It must be a non-empty array if specified.`, ), ); @@ -4303,146 +4793,350 @@ describe('PermissionController', () => { origin, requestData: { metadata: { id: expect.any(String), origin }, - permissions: { [PermissionNames.wallet_getSecretArray]: {} }, + permissions: { + [PermissionNames.wallet_getSecretArray]: { + parentCapability: PermissionNames.wallet_getSecretArray, + }, + }, + ...getRequestDataDiffProperty(), }, type: MethodNames.requestPermissions, }, true, ); - } + }); }); - it('throws if approved permissions contain a (key : value.parentCapability) mismatch', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - const controller = getDefaultPermissionController(options); - - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { - [PermissionNames.wallet_getSecretArray]: { - parentCapability: PermissionNames.wallet_getSecretArray, - }, - // parentCapability value does not match key - [PermissionNames.wallet_getSecretObject]: { - parentCapability: PermissionNames.wallet_getSecretArray, + // Permissions and their caveats are merged through a right-biased union. + // The existing permissions are the left-hand side and denoted as `A`. + // The requested permissions are the right-hand side and denoted as `B`. + describe('requestPermissionsIncremental: merging permissions', () => { + const caveatType1 = CaveatTypes.filterArrayResponse; + const caveatType2 = CaveatTypes.filterObjectResponse; + const caveatType3 = CaveatTypes.noopCaveat; + + const makeCaveat = (type: string, value: Json) => ({ type, value }); + const makeCaveat1 = (...value: string[]) => + makeCaveat(caveatType1, value); + const makeCaveat2 = (...value: string[]) => + makeCaveat(caveatType2, value); + const makeCaveat3 = () => makeCaveat(caveatType3, null); + + it.each([ + ['neither A nor B have caveats', null, null], + ['only A has caveats', [makeCaveat1('a', 'b'), makeCaveat3()], null], + [ + 'A and B have the same caveat', + [makeCaveat1('a', 'b')], + [makeCaveat1('a', 'b')], + ], + [ + 'A and B have the same caveats', + [makeCaveat1('a', 'b'), makeCaveat3()], + [makeCaveat1('a', 'b'), makeCaveat3()], + ], + ])( + 'no-ops if request results in no change: %s', + async (_case, leftCaveats, rightCaveats) => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + const controller = getDefaultPermissionController(options); + + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + [PermissionNames.wallet_noopWithManyCaveats]: { + // @ts-expect-error We know that the caveat type is correct. + caveats: leftCaveats, }, }, - }; - }); + }); - await expect( - async () => - await controller.requestPermissions( - { origin }, - { - [PermissionNames.wallet_getSecretArray]: { - parentCapability: PermissionNames.wallet_getSecretArray, + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); + + expect( + await controller.requestPermissionsIncremental( + { origin }, + { + [PermissionNames.wallet_noopWithManyCaveats]: { + // @ts-expect-error We know that the caveat type is correct. + caveats: rightCaveats, + }, + }, + ), + ).toStrictEqual([]); + + expect(callActionSpy).not.toHaveBeenCalled(); + }, + ); + + it.each([ + [ + 'only B has caveats', + null, + [makeCaveat1('a', 'b'), makeCaveat3()], + [makeCaveat1('a', 'b'), makeCaveat3()], + { [caveatType1]: ['a', 'b'], [caveatType3]: null }, + ], + [ + 'A and B have disjoint caveats', + [makeCaveat1('a', 'b')], + [makeCaveat2('y', 'z'), makeCaveat3()], + [makeCaveat1('a', 'b'), makeCaveat2('y', 'z'), makeCaveat3()], + { [caveatType2]: ['y', 'z'], [caveatType3]: null }, + ], + [ + 'A and B have one of the same caveat', + [makeCaveat1('a', 'b')], + [makeCaveat1('c')], + [makeCaveat1('a', 'b', 'c')], + { [caveatType1]: ['c'] }, + ], + [ + 'A and B have one of the same caveat, and others', + [makeCaveat1('a', 'b'), makeCaveat2('x')], + [makeCaveat1('c'), makeCaveat3()], + [makeCaveat1('a', 'b', 'c'), makeCaveat2('x'), makeCaveat3()], + { [caveatType1]: ['c'], [caveatType3]: null }, + ], + [ + 'A and B have two of the same caveat', + [makeCaveat1('a', 'b'), makeCaveat2('x')], + [makeCaveat2('y', 'z'), makeCaveat1('c')], + [makeCaveat1('a', 'b', 'c'), makeCaveat2('x', 'y', 'z')], + { [caveatType1]: ['c'], [caveatType2]: ['y', 'z'] }, + ], + [ + 'A and B have two of the same caveat, and A has one other', + [makeCaveat1('a', 'b'), makeCaveat2('x'), makeCaveat3()], + [makeCaveat2('y', 'z'), makeCaveat1('c')], + [ + makeCaveat1('a', 'b', 'c'), + makeCaveat2('x', 'y', 'z'), + makeCaveat3(), + ], + { [caveatType1]: ['c'], [caveatType2]: ['y', 'z'] }, + ], + [ + 'A and B have two of the same caveat, and B has one other', + [makeCaveat1('a', 'b'), makeCaveat2('x')], + [makeCaveat2('y', 'z'), makeCaveat1('c'), makeCaveat3()], + [ + makeCaveat1('a', 'b', 'c'), + makeCaveat2('x', 'y', 'z'), + makeCaveat3(), + ], + { + [caveatType1]: ['c'], + [caveatType2]: ['y', 'z'], + [caveatType3]: null, + }, + ], + ])( + 'requested permission merges with existing permission: %s', + async ( + _case, + leftCaveats, + rightCaveats, + expectedCaveats, + caveatsDiff, + ) => { + const getPermissionDiffMatcher = ( + previousCaveats: CaveatConstraint[] | null, + diff: Record, + ) => + expect.objectContaining({ + currentPermissions: expect.objectContaining({ + [PermissionNames.wallet_noopWithManyCaveats]: + expect.objectContaining({ + caveats: previousCaveats, + }), + }), + permissionDiffMap: { + [PermissionNames.wallet_noopWithManyCaveats]: diff, + }, + }); + + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; + const controller = getDefaultPermissionController(options); + + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + [PermissionNames.wallet_noopWithManyCaveats]: { + // @ts-expect-error The caveat type is in fact valid. + caveats: leftCaveats, }, }, - ), - ).rejects.toThrow( - errors.invalidParams({ - message: `Invalid approved permissions request: Permissions request for origin "${origin}" contains invalid requested permission(s).`, - data: { - origin, - requestedPermissions: { - [PermissionNames.wallet_getSecretArray]: { - [PermissionNames.wallet_getSecretArray]: { - parentCapability: PermissionNames.wallet_getSecretArray, - }, - [PermissionNames.wallet_getSecretObject]: { - parentCapability: PermissionNames.wallet_getSecretArray, + }); + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); + + expect( + await controller.requestPermissionsIncremental( + { origin }, + { + [PermissionNames.wallet_noopWithManyCaveats]: { + // @ts-expect-error The caveat type is in fact valid. + caveats: rightCaveats, }, }, + ), + ).toMatchObject([ + { + [PermissionNames.wallet_noopWithManyCaveats]: + getPermissionMatcher({ + parentCapability: PermissionNames.wallet_noopWithManyCaveats, + caveats: expectedCaveats, + }), }, - }, - }), - ); + { id: expect.any(String), origin }, + ]); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { - [PermissionNames.wallet_getSecretArray]: { - parentCapability: PermissionNames.wallet_getSecretArray, + expect(callActionSpy).toHaveBeenCalledTimes(1); + expect(callActionSpy).toHaveBeenCalledWith( + 'ApprovalController:addRequest', + { + id: expect.any(String), + origin, + requestData: { + metadata: { id: expect.any(String), origin }, + permissions: { + [PermissionNames.wallet_noopWithManyCaveats]: + getPermissionMatcher({ + parentCapability: + PermissionNames.wallet_noopWithManyCaveats, + caveats: expectedCaveats, + }), + }, + diff: getPermissionDiffMatcher(leftCaveats, caveatsDiff), }, + type: MethodNames.requestPermissions, }, - }, - type: MethodNames.requestPermissions, + true, + ); }, - true, ); - }); - it('correctly throws errors that do not inherit from JsonRpcError', async () => { - const options = getPermissionControllerOptions(); - const { messenger } = options; - const origin = 'metamask.io'; - const controller = getDefaultPermissionController(options); + it('throws if attempting to merge caveats without a merger function', async () => { + const options = getPermissionControllerOptions(); + const { messenger } = options; + const origin = 'metamask.io'; - const callActionSpy = jest - .spyOn(messenger, 'call') - .mockImplementationOnce(async (...args) => { - const [, { requestData }] = args as AddPermissionRequestArgs; - return { - metadata: { ...requestData.metadata }, - permissions: { - [PermissionNames.wallet_getSecretArray]: { - parentCapability: PermissionNames.wallet_getSecretArray, - }, - [PermissionNames.wallet_getSecretObject]: { - parentCapability: PermissionNames.wallet_getSecretObject, - caveats: 'foo', // invalid - }, + const controller = getDefaultPermissionController(options); + + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + [PermissionNames.wallet_getSecretArray]: { + caveats: [makeCaveat(CaveatTypes.reverseArrayResponse, null)], }, - }; + }, }); - await expect( - async () => - await controller.requestPermissions( + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); + + await expect( + controller.requestPermissionsIncremental( { origin }, { [PermissionNames.wallet_getSecretArray]: { - parentCapability: PermissionNames.wallet_getSecretArray, + caveats: [makeCaveat(CaveatTypes.reverseArrayResponse, null)], }, }, ), - ).rejects.toThrow( - errors.internalError( - `Invalid approved permissions request: The "caveats" property of permission for "${PermissionNames.wallet_getSecretObject}" of subject "${origin}" is invalid. It must be a non-empty array if specified.`, - ), - ); + ).rejects.toThrow( + new errors.CaveatMergerDoesNotExistError( + CaveatTypes.reverseArrayResponse, + ), + ); - expect(callActionSpy).toHaveBeenCalledTimes(1); - expect(callActionSpy).toHaveBeenCalledWith( - 'ApprovalController:addRequest', - { - id: expect.any(String), - origin, - requestData: { - metadata: { id: expect.any(String), origin }, - permissions: { + expect(callActionSpy).not.toHaveBeenCalled(); + }); + + it('throws if merged caveats produce an invalid permission', async () => { + const caveatSpecifications = getDefaultCaveatSpecifications(); + // @ts-expect-error Intentional destructive testing + caveatSpecifications[CaveatTypes.filterArrayResponse].merger = () => [ + 'foo', + 'foo', + ]; + + const options = getPermissionControllerOptions({ + caveatSpecifications, + }); + const { messenger } = options; + const origin = 'metamask.io'; + + const controller = getDefaultPermissionController(options); + + controller.grantPermissions({ + subject: { origin }, + approvedPermissions: { + [PermissionNames.wallet_getSecretArray]: { + caveats: [makeCaveat(CaveatTypes.filterArrayResponse, ['a'])], + }, + }, + }); + + const callActionSpy = jest + .spyOn(messenger, 'call') + .mockImplementationOnce(async (...args) => { + const [, { requestData }] = args as AddPermissionRequestArgs; + return { + metadata: { ...requestData.metadata }, + permissions: { ...requestData.permissions }, + }; + }); + + await expect( + controller.requestPermissionsIncremental( + { origin }, + { [PermissionNames.wallet_getSecretArray]: { - parentCapability: PermissionNames.wallet_getSecretArray, + caveats: [makeCaveat(CaveatTypes.filterArrayResponse, ['b'])], }, }, - }, - type: MethodNames.requestPermissions, - }, - true, - ); + ), + ).rejects.toThrow( + new errors.InvalidMergedPermissionsError( + origin, + new Error( + `${CaveatTypes.filterArrayResponse} values must be arrays`, + ), + {}, + ), + ); + + expect(callActionSpy).not.toHaveBeenCalled(); + }); }); }); @@ -5205,7 +5899,7 @@ describe('PermissionController', () => { ); }); - it('action: PermissionsController:grantPermissions', async () => { + it('action: PermissionController:grantPermissions', async () => { const messenger = getUnrestrictedMessenger(); const options = getPermissionControllerOptions({ messenger: getPermissionControllerMessenger(messenger), @@ -5226,7 +5920,31 @@ describe('PermissionController', () => { ); }); - it('action: PermissionsController:requestPermissions', async () => { + it('action: PermissionController:grantPermissionsIncremental', async () => { + const messenger = getUnrestrictedMessenger(); + const options = getPermissionControllerOptions({ + messenger: getPermissionControllerMessenger(messenger), + }); + const controller = new PermissionController< + DefaultPermissionSpecifications, + DefaultCaveatSpecifications + >(options); + + const result = messenger.call( + 'PermissionController:grantPermissionsIncremental', + { + subject: { origin: 'foo' }, + approvedPermissions: { wallet_getSecretArray: {} }, + }, + ); + + expect(result).toHaveProperty('wallet_getSecretArray'); + expect(controller.hasPermission('foo', 'wallet_getSecretArray')).toBe( + true, + ); + }); + + it('action: PermissionController:requestPermissions', async () => { const messenger = getUnrestrictedMessenger(); const options = getPermissionControllerOptions({ messenger: getPermissionControllerMessenger(messenger), @@ -5236,8 +5954,8 @@ describe('PermissionController', () => { DefaultCaveatSpecifications >(options); - // TODO(ritave): requestPermissions calls unregistered action ApprovalController:addRequest that - // can't be easily mocked, thus we mock the whole implementation + // requestPermissions calls unregistered action ApprovalController:addRequest that + // can't be easily mocked, thus we mock the whole implementation const requestPermissionsSpy = jest .spyOn(controller, 'requestPermissions') .mockImplementation(); @@ -5253,6 +5971,33 @@ describe('PermissionController', () => { expect(requestPermissionsSpy).toHaveBeenCalledTimes(1); }); + it('action: PermissionController:requestPermissionsIncremental', async () => { + const messenger = getUnrestrictedMessenger(); + const options = getPermissionControllerOptions({ + messenger: getPermissionControllerMessenger(messenger), + }); + const controller = new PermissionController< + DefaultPermissionSpecifications, + DefaultCaveatSpecifications + >(options); + + // requestPermissionsIncremental calls unregistered action ApprovalController:addRequest + // that can't be easily mocked, thus we mock the whole implementation. + const requestPermissionsIncrementalSpy = jest + .spyOn(controller, 'requestPermissionsIncremental') + .mockImplementation(); + + await messenger.call( + 'PermissionController:requestPermissionsIncremental', + { origin: 'foo' }, + { + wallet_getSecretArray: {}, + }, + ); + + expect(requestPermissionsIncrementalSpy).toHaveBeenCalledTimes(1); + }); + it('action: PermissionController:updateCaveat', async () => { const messenger = getUnrestrictedMessenger(); const state = { diff --git a/packages/permission-controller/src/PermissionController.ts b/packages/permission-controller/src/PermissionController.ts index c6b27b4796c..f23d9da6e63 100644 --- a/packages/permission-controller/src/PermissionController.ts +++ b/packages/permission-controller/src/PermissionController.ts @@ -24,13 +24,15 @@ import { JsonRpcError } from '@metamask/rpc-errors'; import { hasProperty } from '@metamask/utils'; import type { Json, Mutable } from '@metamask/utils'; import deepFreeze from 'deep-freeze-strict'; -import { castDraft, type Draft } from 'immer'; +import { castDraft, produce as immerProduce, type Draft } from 'immer'; import { nanoid } from 'nanoid'; import type { CaveatConstraint, + CaveatDiffMap, CaveatSpecificationConstraint, CaveatSpecificationMap, + CaveatValueMerger, ExtractCaveat, ExtractCaveats, ExtractCaveatValue, @@ -43,6 +45,8 @@ import { CaveatAlreadyExistsError, CaveatDoesNotExistError, CaveatInvalidJsonError, + CaveatMergerDoesNotExistError, + CaveatMergeTypeMismatchError, CaveatMissingValueError, CaveatSpecificationMismatchError, DuplicateCaveatError, @@ -54,6 +58,7 @@ import { InvalidCaveatFieldsError, InvalidCaveatsPropertyError, InvalidCaveatTypeError, + InvalidMergedPermissionsError, invalidParams, InvalidSubjectIdentifierError, methodNotFound, @@ -88,7 +93,7 @@ import { } from './Permission'; import { getPermissionMiddlewareFactory } from './permission-middleware'; import type { GetSubjectMetadata } from './SubjectMetadataController'; -import { MethodNames } from './utils'; +import { collectUniqueAndPairedCaveats, MethodNames } from './utils'; /** * Metadata associated with {@link PermissionController} subjects. @@ -105,18 +110,41 @@ export type PermissionsRequestMetadata = PermissionSubjectMetadata & { [key: string]: Json; }; +/** + * A diff produced by an incremental permissions request. + */ +export type PermissionDiffMap< + TargetName extends string, + AllowedCaveats extends CaveatConstraint, +> = Record>; + /** * Used for prompting the user about a proposed new permission. - * Includes information about the grantee subject, requested permissions, and - * any additional information added by the consumer. + * Includes information about the grantee subject, requested permissions, the + * diff relative to the previously granted permissions (if relevant), and any + * additional information added by the consumer. * - * All properties except `permissions` are passed to any factories found for - * the requested permissions. + * All properties except `diff` and `permissions` are passed to any factories + * for the requested permissions. */ export type PermissionsRequest = { metadata: PermissionsRequestMetadata; permissions: RequestedPermissions; [key: string]: Json; +} & { + diff?: { + currentPermissions: SubjectPermissions; + permissionDiffMap: PermissionDiffMap; + }; +}; + +/** + * Metadata associated with an approved permission request. + */ +type ApprovedPermissionsMetadata = { + data?: Record; + id: string; + origin: OriginString; }; export type SideEffects = { @@ -246,6 +274,14 @@ export type GrantPermissions = { handler: GenericPermissionController['grantPermissions']; }; +/** + * Directly grants given permissions for a specificed origin without requesting user approval + */ +export type GrantPermissionsIncremental = { + type: `${typeof controllerName}:grantPermissionsIncremental`; + handler: GenericPermissionController['grantPermissionsIncremental']; +}; + /** * Requests given permissions for a specified origin */ @@ -254,6 +290,14 @@ export type RequestPermissions = { handler: GenericPermissionController['requestPermissions']; }; +/** + * Requests given permissions for a specified origin + */ +export type RequestPermissionsIncremental = { + type: `${typeof controllerName}:requestPermissionsIncremental`; + handler: GenericPermissionController['requestPermissionsIncremental']; +}; + /** * Removes the specified permissions for each origin. */ @@ -315,7 +359,9 @@ export type PermissionControllerActions = | HasPermission | HasPermissions | GrantPermissions + | GrantPermissionsIncremental | RequestPermissions + | RequestPermissionsIncremental | RevokeAllPermissions | RevokePermissionForAllSubjects | RevokePermissions @@ -415,6 +461,11 @@ type CaveatMutatorResult = >; }>; +type MergeCaveatResult = + T extends undefined + ? [CaveatConstraint, CaveatConstraint['value']] + : [CaveatConstraint, CaveatConstraint['value']] | []; + /** * Extracts the permission(s) specified by the given permission and caveat * specifications. @@ -661,6 +712,24 @@ export class PermissionController< return this._caveatSpecifications[caveatType]; } + /** + * Gets the merger function for the specified caveat. Throws if no + * merger exists. + * + * @param caveatType - The type of the caveat whose merger to get. + * @returns The caveat value merger function for the specified caveat type. + */ + #expectGetCaveatMerger< + CaveatType extends ControllerCaveatSpecification['type'], + >(caveatType: CaveatType): CaveatValueMerger { + const { merger } = this.getCaveatSpecification(caveatType); + + if (merger === undefined) { + throw new CaveatMergerDoesNotExistError(caveatType); + } + return merger; + } + /** * Constructor helper for validating permission specifications. * @@ -768,12 +837,23 @@ export class PermissionController< this.grantPermissions.bind(this), ); + this.messagingSystem.registerActionHandler( + `${controllerName}:grantPermissionsIncremental` as const, + this.grantPermissionsIncremental.bind(this), + ); + this.messagingSystem.registerActionHandler( `${controllerName}:requestPermissions` as const, (subject: PermissionSubjectMetadata, permissions: RequestedPermissions) => this.requestPermissions(subject, permissions), ); + this.messagingSystem.registerActionHandler( + `${controllerName}:requestPermissionsIncremental` as const, + (subject: PermissionSubjectMetadata, permissions: RequestedPermissions) => + this.requestPermissionsIncremental(subject, permissions), + ); + this.messagingSystem.registerActionHandler( `${controllerName}:revokeAllPermissions` as const, (origin: OriginString) => this.revokeAllPermissions(origin), @@ -1523,10 +1603,11 @@ export class PermissionController< /** * Grants _approved_ permissions to the specified subject. Every permission and - * caveat is stringently validated – including by calling every specification - * validator – and an error is thrown if any validation fails. + * caveat is stringently validated—including by calling their specification + * validators—and an error is thrown if validation fails. * - * ATTN: This method does **not** prompt the user for approval. + * ATTN: This method does **not** prompt the user for approval. User consent must + * first be obtained through some other means. * * @see {@link PermissionController.requestPermissions} For initiating a * permissions request requiring user approval. @@ -1538,7 +1619,7 @@ export class PermissionController< * @param options.preserveExistingPermissions - Whether to preserve the * subject's existing permissions. * @param options.subject - The subject to grant permissions to. - * @returns The granted permissions. + * @returns The subject's new permission state. It may or may not have changed. */ grantPermissions({ approvedPermissions, @@ -1550,10 +1631,84 @@ export class PermissionController< subject: PermissionSubjectMetadata; preserveExistingPermissions?: boolean; requestData?: Record; - }): SubjectPermissions< - ExtractPermission< - ControllerPermissionSpecification, - ControllerCaveatSpecification + }): Partial< + SubjectPermissions< + ExtractPermission< + ControllerPermissionSpecification, + ControllerCaveatSpecification + > + > + > { + return this.#applyGrantedPermissions({ + approvedPermissions, + subject, + mergePermissions: false, + preserveExistingPermissions, + requestData, + }); + } + + /** + * Incrementally grants _approved_ permissions to the specified subject. Every + * permission and caveat is stringently validated—including by calling their + * specification validators—and an error is thrown if validation fails. + * + * ATTN: This method does **not** prompt the user for approval. User consent must + * first be obtained through some other means. + * + * @see {@link PermissionController.requestPermissionsIncremental} For initiating + * an incremental permissions request requiring user approval. + * @param options - Options bag. + * @param options.approvedPermissions - The requested permissions approved by + * the user. + * @param options.requestData - Permission request data. Passed to permission + * factory functions. + * @param options.subject - The subject to grant permissions to. + * @returns The subject's new permission state. It may or may not have changed. + */ + grantPermissionsIncremental({ + approvedPermissions, + requestData, + subject, + }: { + approvedPermissions: RequestedPermissions; + subject: PermissionSubjectMetadata; + requestData?: Record; + }): Partial< + SubjectPermissions< + ExtractPermission< + ControllerPermissionSpecification, + ControllerCaveatSpecification + > + > + > { + return this.#applyGrantedPermissions({ + approvedPermissions, + subject, + mergePermissions: true, + preserveExistingPermissions: true, + requestData, + }); + } + + #applyGrantedPermissions({ + approvedPermissions, + subject, + mergePermissions, + preserveExistingPermissions, + requestData, + }: { + approvedPermissions: RequestedPermissions; + subject: PermissionSubjectMetadata; + mergePermissions: boolean; + preserveExistingPermissions: boolean; + requestData?: Record; + }): Partial< + SubjectPermissions< + ExtractPermission< + ControllerPermissionSpecification, + ControllerCaveatSpecification + > > > { const { origin } = subject; @@ -1618,24 +1773,30 @@ export class PermissionController< ControllerPermissionSpecification, ControllerCaveatSpecification >; + let performCaveatValidation = true; + if (specification.factory) { permission = specification.factory(permissionOptions, requestData); - - // Full caveat and permission validation is performed here since the - // factory function can arbitrarily modify the entire permission object, - // including its caveats. - this.validatePermission(specification, permission, origin); } else { permission = constructPermission(permissionOptions); // We do not need to validate caveats in this case, because the plain // permission constructor function does not modify the caveats, which // were already validated by `constructCaveats` above. - this.validatePermission(specification, permission, origin, { - invokePermissionValidator: true, - performCaveatValidation: false, - }); + performCaveatValidation = false; + } + + if (mergePermissions) { + permission = this.#mergePermission( + permissions[targetName], + permission, + )[0]; } + + this.validatePermission(specification, permission, origin, { + invokePermissionValidator: true, + performCaveatValidation, + }); permissions[targetName] = permission; } @@ -1834,9 +1995,11 @@ export class PermissionController< } /** - * Initiates a permission request that requires user approval. This should - * always be used to grant additional permissions to a subject, unless user - * approval has been obtained through some other means. + * Initiates a permission request that requires user approval. + * + * Either this or {@link PermissionController.requestPermissionsIncremental} + * should always be used to grant additional permissions to a subject, + * unless user approval has been obtained through some other means. * * Permissions are validated at every step of the approval process, and this * method will reject if validation fails. @@ -1866,13 +2029,15 @@ export class PermissionController< } = {}, ): Promise< [ - SubjectPermissions< - ExtractPermission< - ControllerPermissionSpecification, - ControllerCaveatSpecification + Partial< + SubjectPermissions< + ExtractPermission< + ControllerPermissionSpecification, + ControllerCaveatSpecification + > > >, - { data?: Record; id: string; origin: OriginString }, + ApprovedPermissionsMetadata, ] > { const { origin } = subject; @@ -1885,47 +2050,128 @@ export class PermissionController< origin, }; - const permissionsRequest = { + const permissionsRequest: PermissionsRequest = { metadata, permissions: requestedPermissions, }; const approvedRequest = await this.requestUserApproval(permissionsRequest); - const { permissions: approvedPermissions, ...requestData } = - approvedRequest; + return await this.#handleApprovedPermissions({ + subject, + metadata, + preserveExistingPermissions, + approvedRequest, + }); + } - const sideEffects = this.getSideEffects(approvedPermissions); + /** + * Initiates an incremental permission request that prompts for user approval. + * Incremental permission requests allow the caller to replace existing and/or + * add brand new permissions and caveats for the specified subject. + * + * Incremental permission request are merged with the subject's existing permissions + * through a right-biased union, where the incremental permission are the right-hand + * side of the merger. If both sides of the merger specify the same caveats for a + * given permission, the caveats are merged using their specification's caveat value + * merger property. + * + * Either this or {@link PermissionController.requestPermissions} should + * always be used to grant additional permissions to a subject, unless user + * approval has been obtained through some other means. + * + * Permissions are validated at every step of the approval process, and this + * method will reject if validation fails. + * + * @see {@link ApprovalController} For the user approval logic. + * @see {@link PermissionController.acceptPermissionsRequest} For the method + * that _accepts_ the request and resolves the user approval promise. + * @see {@link PermissionController.rejectPermissionsRequest} For the method + * that _rejects_ the request and the user approval promise. + * @param subject - The grantee subject. + * @param requestedPermissions - The requested permissions. + * @param options - Additional options. + * @param options.id - The id of the permissions request. Defaults to a unique + * id. + * @param options.metadata - Additional metadata about the permission request. + * @returns The granted permissions and request metadata. + */ + async requestPermissionsIncremental( + subject: PermissionSubjectMetadata, + requestedPermissions: RequestedPermissions, + options: { + id?: string; + metadata?: Record; + } = {}, + ): Promise< + | [ + Partial< + SubjectPermissions< + ExtractPermission< + ControllerPermissionSpecification, + ControllerCaveatSpecification + > + > + >, + ApprovedPermissionsMetadata, + ] + | [] + > { + const { origin } = subject; + const { id = nanoid() } = options; + this.validateRequestedPermissions(origin, requestedPermissions); - if (Object.values(sideEffects.permittedHandlers).length > 0) { - const sideEffectsData = await this.executeSideEffects( - sideEffects, - approvedRequest, - ); - const mappedData = Object.keys(sideEffects.permittedHandlers).reduce( - (acc, permission, i) => ({ [permission]: sideEffectsData[i], ...acc }), - {}, + const currentPermissions = this.getPermissions(origin) ?? {}; + const [newPermissions, permissionDiffMap] = + this.#mergeIncrementalPermissions( + currentPermissions, + requestedPermissions, ); - return [ - this.grantPermissions({ - subject, - approvedPermissions, - preserveExistingPermissions, - requestData, - }), - { data: mappedData, ...metadata }, - ]; + // The second undefined check is just for type narrowing purposes. These values + // will always be jointly defined or undefined. + if (newPermissions === undefined || permissionDiffMap === undefined) { + return []; } - return [ - this.grantPermissions({ - subject, - approvedPermissions, - preserveExistingPermissions, - requestData, - }), + try { + // It does not spark joy to run this validation again after the merger operation. + // But, optimizing this procedure is probably not worth it, especially considering + // that the worst-case scenario for validation degrades to the below function call. + this.validateRequestedPermissions(origin, newPermissions); + } catch (error) { + if (error instanceof Error) { + throw new InvalidMergedPermissionsError( + origin, + error, + permissionDiffMap, + ); + } + /* istanbul ignore next: This should be impossible */ + throw internalError('Unrecognized error type', { error }); + } + + const metadata = { + ...options.metadata, + id, + origin, + }; + + const permissionsRequest: PermissionsRequest = { metadata, - ]; + permissions: newPermissions, + diff: { + currentPermissions, + permissionDiffMap, + }, + }; + + const approvedRequest = await this.requestUserApproval(permissionsRequest); + return await this.#handleApprovedPermissions({ + subject, + metadata, + preserveExistingPermissions: false, + approvedRequest, + }); } /** @@ -1991,6 +2237,172 @@ export class PermissionController< } } + /** + * Merges a set of incrementally requested permissions into the existing permissions of + * the requesting subject. The merge is a right-biased union, where the existing + * permissions are the left-hand side, and the incrementally requested permissions are + * the right-hand side. + * + * @param existingPermissions - The subject's existing permissions. + * @param incrementalRequestedPermissions - The requested permissions to merge. + * @returns The merged permissions and the resulting diff. + */ + #mergeIncrementalPermissions( + existingPermissions: Exclude< + ReturnType, + undefined + >, + incrementalRequestedPermissions: RequestedPermissions, + ): + | [ + SubjectPermissions< + ValidPermission> + >, + PermissionDiffMap, + ] + | [] { + const permissionDiffMap: PermissionDiffMap = {}; + + // Use immer's produce as a convenience for calculating the new permissions + // without mutating the existing permissions or committing the results to state. + const newPermissions = immerProduce( + existingPermissions, + (draftExistingPermissions) => { + const leftPermissions = + draftExistingPermissions as RequestedPermissions; + + Object.entries(incrementalRequestedPermissions).forEach( + ([targetName, rightPermission]) => { + const leftPermission: Partial | undefined = + leftPermissions[targetName]; + + const [newPermission, caveatsDiff] = this.#mergePermission( + leftPermission ?? {}, + rightPermission, + ); + + if ( + leftPermission === undefined || + Object.keys(caveatsDiff).length > 0 + ) { + leftPermissions[targetName] = newPermission; + permissionDiffMap[targetName] = caveatsDiff; + } + // Otherwise, leave the left permission as-is; its authority has + // not changed. + }, + ); + }, + ); + + if (Object.keys(permissionDiffMap).length === 0) { + return []; + } + return [newPermissions, permissionDiffMap]; + } + + /** + * Performs a right-biased union between two permissions. The task of merging caveats + * of the same type between the two permissions is delegated to the corresponding + * caveat type's merger implementation. + * + * Throws if the left-hand and right-hand permissions both have a caveat whose + * specification does not provide a caveat value merger function. + * + * @param leftPermission - The left-hand permission to merge. + * @param rightPermission - The right-hand permission to merge. + * @returns The merged permission. + */ + #mergePermission< + T extends Partial | PermissionConstraint, + >( + leftPermission: T | undefined, + rightPermission: T, + ): [T, CaveatDiffMap] { + const { caveatPairs, leftUniqueCaveats, rightUniqueCaveats } = + collectUniqueAndPairedCaveats(leftPermission, rightPermission); + + const [mergedCaveats, caveatDiffMap] = caveatPairs.reduce( + ([caveats, diffMap], [leftCaveat, rightCaveat]) => { + const [newCaveat, diff] = this.#mergeCaveat(leftCaveat, rightCaveat); + + if (newCaveat !== undefined && diff !== undefined) { + caveats.push(newCaveat); + diffMap[newCaveat.type] = diff; + } else { + caveats.push(leftCaveat); + } + + return [caveats, diffMap]; + }, + [[], {}] as [CaveatConstraint[], CaveatDiffMap], + ); + + const mergedRightUniqueCaveats = rightUniqueCaveats.map((caveat) => { + const [newCaveat, diff] = this.#mergeCaveat(undefined, caveat); + + caveatDiffMap[newCaveat.type] = diff; + return newCaveat; + }); + + const allCaveats = [ + ...mergedCaveats, + ...leftUniqueCaveats, + ...mergedRightUniqueCaveats, + ]; + + const newPermission = { + ...leftPermission, + ...rightPermission, + ...(allCaveats.length > 0 + ? { caveats: allCaveats as NonEmptyArray } + : {}), + }; + + return [newPermission, caveatDiffMap]; + } + + /** + * Merges two caveats of the same type. The task of merging the values of the + * two caveats is delegated to the corresponding caveat type's merger implementation. + * + * @param leftCaveat - The left-hand caveat to merge. + * @param rightCaveat - The right-hand caveat to merge. + * @returns The merged caveat and the diff between the two caveats. + */ + #mergeCaveat( + leftCaveat: U, + rightCaveat: T, + ): MergeCaveatResult { + /* istanbul ignore if: This should be impossible */ + if (leftCaveat !== undefined && leftCaveat.type !== rightCaveat.type) { + throw new CaveatMergeTypeMismatchError(leftCaveat.type, rightCaveat.type); + } + + const merger = this.#expectGetCaveatMerger(rightCaveat.type); + + if (leftCaveat === undefined) { + return [ + { + ...rightCaveat, + }, + rightCaveat.value, + ]; + } + + const [newValue, diff] = merger(leftCaveat.value, rightCaveat.value); + + return newValue !== undefined && diff !== undefined + ? [ + { + type: rightCaveat.type, + value: newValue, + }, + diff, + ] + : ([] as MergeCaveatResult); + } + /** * Adds a request to the {@link ApprovalController} using the * {@link AddApprovalRequest} action. Also validates the resulting approved @@ -2016,6 +2428,60 @@ export class PermissionController< return approvedRequest as PermissionsRequest; } + /** + * Accepts a permissions request that has been approved by the user. This + * method should be called after the user has approved the request and the + * {@link ApprovalController} has resolved the user approval promise. + * + * @param options - Options bag. + * @param options.subject - The subject to grant permissions to. + * @param options.metadata - The metadata of the approved permissions request. + * @param options.preserveExistingPermissions - Whether to preserve the + * subject's existing permissions. + * @param options.approvedRequest - The approved permissions request to handle. + * @returns The granted permissions and request metadata. + */ + async #handleApprovedPermissions({ + subject, + metadata, + preserveExistingPermissions, + approvedRequest, + }: { + subject: PermissionSubjectMetadata; + metadata: PermissionsRequest['metadata']; + preserveExistingPermissions: boolean; + approvedRequest: PermissionsRequest; + }): Promise< + [ReturnType, ApprovedPermissionsMetadata] + > { + const { permissions: approvedPermissions, ...requestData } = + approvedRequest; + const approvedMetadata: ApprovedPermissionsMetadata = { ...metadata }; + + const sideEffects = this.getSideEffects(approvedPermissions); + if (Object.values(sideEffects.permittedHandlers).length > 0) { + const sideEffectsData = await this.executeSideEffects( + sideEffects, + approvedRequest, + ); + + approvedMetadata.data = Object.keys(sideEffects.permittedHandlers).reduce( + (acc, permission, i) => ({ [permission]: sideEffectsData[i], ...acc }), + {}, + ); + } + + return [ + this.grantPermissions({ + subject, + approvedPermissions, + preserveExistingPermissions, + requestData, + }), + approvedMetadata, + ]; + } + /** * Reunites all the side-effects (onPermitted and onFailure) of the requested permissions inside a record of arrays. * @@ -2163,7 +2629,7 @@ export class PermissionController< error instanceof JsonRpcError ? error.data : undefined, ); } - /* istanbul ignore next: We should only throw well-formed errors */ + /* istanbul ignore next: This should be impossible */ throw internalError('Unrecognized error type', { error }); } } diff --git a/packages/permission-controller/src/errors.test.ts b/packages/permission-controller/src/errors.test.ts index 8958859418a..db95eb5aa5e 100644 --- a/packages/permission-controller/src/errors.test.ts +++ b/packages/permission-controller/src/errors.test.ts @@ -1,6 +1,20 @@ -import { EndowmentPermissionDoesNotExistError } from './errors'; +import { + CaveatMergeTypeMismatchError, + EndowmentPermissionDoesNotExistError, +} from './errors'; describe('error', () => { + describe('CaveatMergeTypeMismatchError', () => { + it('has the expected shape', () => { + expect(new CaveatMergeTypeMismatchError('foo', 'bar').data).toStrictEqual( + { + leftCaveatType: 'foo', + rightCaveatType: 'bar', + }, + ); + }); + }); + describe('EndowmentPermissionDoesNotExistError', () => { it('adds origin argument to data property', () => { expect( diff --git a/packages/permission-controller/src/errors.ts b/packages/permission-controller/src/errors.ts index 43a80b7d87f..ed9fec3e77b 100644 --- a/packages/permission-controller/src/errors.ts +++ b/packages/permission-controller/src/errors.ts @@ -6,7 +6,9 @@ import { JsonRpcError, } from '@metamask/rpc-errors'; +import type { CaveatConstraint } from './Caveat'; import type { PermissionType } from './Permission'; +import type { PermissionDiffMap } from './PermissionController'; type UnauthorizedArg = { data?: Record; @@ -104,6 +106,32 @@ export class UnrecognizedSubjectError extends Error { } } +export class CaveatMergerDoesNotExistError extends Error { + constructor(caveatType: string) { + super(`Caveat value merger does not exist for type: "${caveatType}"`); + } +} + +export class InvalidMergedPermissionsError extends Error { + public cause: Error; + + public data: { + diff: PermissionDiffMap; + }; + + constructor( + origin: string, + cause: Error, + diff: PermissionDiffMap, + ) { + super( + `Invalid merged permissions for subject "${origin}":\n${cause.message}`, + ); + this.cause = cause; + this.data = { diff }; + } +} + export class InvalidApprovedPermissionError extends Error { public data: { origin: string; @@ -289,6 +317,20 @@ export class DuplicateCaveatError extends Error { } } +export class CaveatMergeTypeMismatchError extends Error { + public data: { + leftCaveatType: string; + rightCaveatType: string; + }; + + constructor(leftCaveatType: string, rightCaveatType: string) { + super( + `Cannot merge caveats of different types: "${leftCaveatType}" and "${rightCaveatType}".`, + ); + this.data = { leftCaveatType, rightCaveatType }; + } +} + export class CaveatSpecificationMismatchError extends Error { public data: { caveatSpec: Record; diff --git a/packages/permission-controller/src/index.ts b/packages/permission-controller/src/index.ts index 0d3f11a4806..693f3b46617 100644 --- a/packages/permission-controller/src/index.ts +++ b/packages/permission-controller/src/index.ts @@ -2,6 +2,12 @@ export * from './Caveat'; export * from './errors'; export * from './Permission'; export * from './PermissionController'; -export * from './utils'; +export type { + ExtractSpecifications, + HandlerMiddlewareFunction, + HookNames, + PermittedHandlerExport, +} from './utils'; +export { MethodNames } from './utils'; export * as permissionRpcMethods from './rpc-methods'; export * from './SubjectMetadataController'; diff --git a/packages/permission-controller/src/utils.ts b/packages/permission-controller/src/utils.ts index 39b9c8829c0..dd032fbbcc8 100644 --- a/packages/permission-controller/src/utils.ts +++ b/packages/permission-controller/src/utils.ts @@ -10,10 +10,12 @@ import type { } from '@metamask/utils'; import type { + CaveatConstraint, CaveatSpecificationConstraint, CaveatSpecificationMap, } from './Caveat'; import type { + PermissionConstraint, PermissionSpecificationConstraint, PermissionSpecificationMap, } from './Permission'; @@ -75,3 +77,44 @@ export type PermittedHandlerExport< hookNames: HookNames; methodNames: string[]; }; + +/** + * Given two permission objects, computes 3 sets: + * - The set of caveat pairs that are common to both permissions. + * - The set of caveats that are unique to the existing permission. + * - The set of caveats that are unique to the requested permission. + * + * Assumes that the caveat arrays of both permissions are valid. + * + * @param leftPermission - The left-hand permission. + * @param rightPermission - The right-hand permission. + * @returns The sets of caveat pairs and unique caveats. + */ +export function collectUniqueAndPairedCaveats( + leftPermission: Partial | undefined, + rightPermission: Partial, +) { + const leftCaveats = leftPermission?.caveats?.slice() ?? []; + const rightCaveats = rightPermission.caveats?.slice() ?? []; + const leftUniqueCaveats: CaveatConstraint[] = []; + const caveatPairs: [CaveatConstraint, CaveatConstraint][] = []; + + leftCaveats.forEach((leftCaveat) => { + const rightCaveatIndex = rightCaveats.findIndex( + (rightCaveat) => rightCaveat.type === leftCaveat.type, + ); + + if (rightCaveatIndex === -1) { + leftUniqueCaveats.push(leftCaveat); + } else { + caveatPairs.push([leftCaveat, rightCaveats[rightCaveatIndex]]); + rightCaveats.splice(rightCaveatIndex, 1); + } + }); + + return { + caveatPairs, + leftUniqueCaveats, + rightUniqueCaveats: [...rightCaveats], + }; +} diff --git a/packages/phishing-controller/CHANGELOG.md b/packages/phishing-controller/CHANGELOG.md index 7568d5f0e6a..d3c15c4dadb 100644 --- a/packages/phishing-controller/CHANGELOG.md +++ b/packages/phishing-controller/CHANGELOG.md @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [9.0.3] + +### Changed + +- Update phishing detection API endpoint from `*.metafi.codefi.network` to `*.api.cx.metamask.io` ([#4301](https://github.com/MetaMask/core/pull/4301)) + ## [9.0.2] ### Changed @@ -178,7 +184,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 All changes listed after this point were applied to this package following the monorepo conversion. -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/phishing-controller@9.0.2...HEAD +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/phishing-controller@9.0.3...HEAD +[9.0.3]: https://github.com/MetaMask/core/compare/@metamask/phishing-controller@9.0.2...@metamask/phishing-controller@9.0.3 [9.0.2]: https://github.com/MetaMask/core/compare/@metamask/phishing-controller@9.0.1...@metamask/phishing-controller@9.0.2 [9.0.1]: https://github.com/MetaMask/core/compare/@metamask/phishing-controller@9.0.0...@metamask/phishing-controller@9.0.1 [9.0.0]: https://github.com/MetaMask/core/compare/@metamask/phishing-controller@8.0.2...@metamask/phishing-controller@9.0.0 diff --git a/packages/phishing-controller/package.json b/packages/phishing-controller/package.json index 66f11fe34f0..2c9049751d2 100644 --- a/packages/phishing-controller/package.json +++ b/packages/phishing-controller/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/phishing-controller", - "version": "9.0.2", + "version": "9.0.3", "description": "Maintains a periodically updated list of approved and unapproved website origins", "keywords": [ "MetaMask", diff --git a/packages/phishing-controller/src/PhishingController.ts b/packages/phishing-controller/src/PhishingController.ts index 11523feeb01..35e37ab6e72 100644 --- a/packages/phishing-controller/src/PhishingController.ts +++ b/packages/phishing-controller/src/PhishingController.ts @@ -7,7 +7,7 @@ import { toASCII } from 'punycode/'; import { applyDiffs, fetchTimeNow } from './utils'; export const PHISHING_CONFIG_BASE_URL = - 'https://phishing-detection.metafi.codefi.network'; + 'https://phishing-detection.api.cx.metamask.io'; export const METAMASK_STALELIST_FILE = '/v1/stalelist'; diff --git a/packages/preferences-controller/src/PreferencesController.test.ts b/packages/preferences-controller/src/PreferencesController.test.ts index 7bd10b23765..16e31543197 100644 --- a/packages/preferences-controller/src/PreferencesController.test.ts +++ b/packages/preferences-controller/src/PreferencesController.test.ts @@ -37,6 +37,7 @@ describe('PreferencesController', () => { acc[curr] = true; return acc; }, {} as { [chainId in EtherscanSupportedHexChainId]: boolean }), + smartTransactionsOptInStatus: false, }); }); @@ -417,6 +418,12 @@ describe('PreferencesController', () => { expect(controller.state.showIncomingTransactions['0x1']).toBe(false); }); + it('should set smartTransactionsOptInStatus', () => { + const controller = setupPreferencesController(); + controller.setSmartTransactionsOptInStatus(true); + expect(controller.state.smartTransactionsOptInStatus).toBe(true); + }); + it('should set useTransactionSimulations', () => { const controller = setupPreferencesController(); controller.setUseTransactionSimulations(false); diff --git a/packages/preferences-controller/src/PreferencesController.ts b/packages/preferences-controller/src/PreferencesController.ts index 4d8305deaff..151ebc604da 100644 --- a/packages/preferences-controller/src/PreferencesController.ts +++ b/packages/preferences-controller/src/PreferencesController.ts @@ -108,6 +108,10 @@ export type PreferencesState = { * Controls whether token detection is enabled */ useTokenDetection: boolean; + /** + * Controls whether smart transactions are opted into + */ + smartTransactionsOptInStatus: boolean; /** * Controls whether transaction simulations are enabled */ @@ -129,6 +133,7 @@ const metadata = { showIncomingTransactions: { persist: true, anonymous: true }, useNftDetection: { persist: true, anonymous: true }, useTokenDetection: { persist: true, anonymous: true }, + smartTransactionsOptInStatus: { persist: true, anonymous: false }, useTransactionSimulations: { persist: true, anonymous: true }, }; @@ -202,6 +207,7 @@ export function getDefaultPreferencesState() { showTestNetworks: false, useNftDetection: false, useTokenDetection: true, + smartTransactionsOptInStatus: false, useTransactionSimulations: true, }; } @@ -504,6 +510,17 @@ export class PreferencesController extends BaseController< } } + /** + * A setter for the user to opt into smart transactions + * + * @param smartTransactionsOptInStatus - true to opt into smart transactions + */ + setSmartTransactionsOptInStatus(smartTransactionsOptInStatus: boolean) { + this.update((state) => { + state.smartTransactionsOptInStatus = smartTransactionsOptInStatus; + }); + } + /** * A setter for the user preferences to enable/disable transaction simulations. * diff --git a/packages/profile-sync-controller/src/sdk/__fixtures__/test-utils.ts b/packages/profile-sync-controller/src/sdk/__fixtures__/test-utils.ts index de32ae785ba..8af8560c638 100644 --- a/packages/profile-sync-controller/src/sdk/__fixtures__/test-utils.ts +++ b/packages/profile-sync-controller/src/sdk/__fixtures__/test-utils.ts @@ -4,7 +4,7 @@ import type { AuthStorageOptions, } from '../authentication-jwt-bearer/types'; import { AuthType } from '../authentication-jwt-bearer/types'; -import { Env } from '../env'; +import { Env, Platform } from '../env'; // Alias mocking variables with ANY to test runtime safety. // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -62,6 +62,7 @@ export function arrangeAuth( const auth = new JwtBearerAuth( { env: Env.DEV, + platform: Platform.EXTENSION, type: AuthType.SRP, }, { @@ -85,6 +86,7 @@ export function arrangeAuth( const auth = new JwtBearerAuth( { env: Env.DEV, + platform: Platform.EXTENSION, type: AuthType.SiWE, }, { diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/constants.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/constants.ts deleted file mode 100644 index 59a8e2d6c2a..00000000000 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/constants.ts +++ /dev/null @@ -1 +0,0 @@ -export const SESSION_LIFETIME_MS = 45 * 60 * 1000; // 45 minutes in milliseconds diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-siwe.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-siwe.ts index 0b68230e1cd..2bf6765f6ad 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-siwe.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-siwe.ts @@ -1,7 +1,6 @@ import { SiweMessage } from 'siwe'; import { ValidationError } from '../errors'; -import { SESSION_LIFETIME_MS } from './constants'; import { SIWE_LOGIN_URL, authenticate, @@ -77,6 +76,7 @@ export class SIWEJwtBearerAuth implements IBaseAuth { this.#signer = signer; } + // convert expiresIn from seconds to milliseconds and use 90% of expiresIn async #getAuthSession(): Promise { const auth = await this.#options.storage.getLoginResponse(); if (!auth) { @@ -85,7 +85,9 @@ export class SIWEJwtBearerAuth implements IBaseAuth { const currentTime = Date.now(); const sessionAge = currentTime - auth.token.obtainedAt; - if (sessionAge < SESSION_LIFETIME_MS) { + const refreshThreshold = auth.token.expiresIn * 1000 * 0.9; + + if (sessionAge < refreshThreshold) { return auth; } return null; @@ -112,6 +114,7 @@ export class SIWEJwtBearerAuth implements IBaseAuth { const tokenResponse = await authorizeOIDC( authResponse.token, this.#config.env, + this.#config.platform, ); // Save diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts index 2ceddfaf42c..7c0d825c004 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/flow-srp.ts @@ -1,7 +1,6 @@ import { ValidationError } from '../errors'; import { getMetaMaskProviderEIP6963 } from '../utils/eip-6963-metamask-provider'; import { MESSAGE_SIGNING_SNAP } from '../utils/messaging-signing-snap-requests'; -import { SESSION_LIFETIME_MS } from './constants'; import { authenticate, authorizeOIDC, getNonce } from './services'; import type { AuthConfig, @@ -83,6 +82,7 @@ export class SRPJwtBearerAuth implements IBaseAuth { return await this.#options.signing.signMessage(message); } + // convert expiresIn from seconds to milliseconds and use 90% of expiresIn async #getAuthSession(): Promise { const auth = await this.#options.storage.getLoginResponse(); if (!auth) { @@ -91,7 +91,9 @@ export class SRPJwtBearerAuth implements IBaseAuth { const currentTime = Date.now(); const sessionAge = currentTime - auth.token.obtainedAt; - if (sessionAge < SESSION_LIFETIME_MS) { + const refreshThreshold = auth.token.expiresIn * 1000 * 0.9; + + if (sessionAge < refreshThreshold) { return auth; } return null; @@ -120,6 +122,7 @@ export class SRPJwtBearerAuth implements IBaseAuth { const tokenResponse = await authorizeOIDC( authResponse.token, this.#config.env, + this.#config.platform, ); // Save diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts index b46acc24998..a0487a47397 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/services.ts @@ -1,4 +1,4 @@ -import type { Env } from '../env'; +import type { Env, Platform } from '../env'; import { getEnvUrls, getOidcClientId } from '../env'; import { NonceRetrievalError, @@ -134,11 +134,13 @@ export async function getNonce(id: string, env: Env): Promise { * * @param jwtToken - The original token received from Authentication. This is traded for the Access Token. (the authentication token is single-use) * @param env - server environment + * @param platform - SDK platform * @returns Access Token from Authorization server */ export async function authorizeOIDC( jwtToken: string, env: Env, + platform: Platform, ): Promise { const grantType = 'urn:ietf:params:oauth:grant-type:jwt-bearer'; const headers = new Headers({ @@ -147,7 +149,7 @@ export async function authorizeOIDC( const urlEncodedBody = new URLSearchParams(); urlEncodedBody.append('grant_type', grantType); - urlEncodedBody.append('client_id', getOidcClientId(env)); + urlEncodedBody.append('client_id', getOidcClientId(env, platform)); urlEncodedBody.append('assertion', jwtToken); try { diff --git a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/types.ts b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/types.ts index 66aef881ba5..621e7ef419f 100644 --- a/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/types.ts +++ b/packages/profile-sync-controller/src/sdk/authentication-jwt-bearer/types.ts @@ -1,4 +1,4 @@ -import type { Env } from '../env'; +import type { Env, Platform } from '../env'; export const enum AuthType { /* sign in using a private key derived from your secret recovery phrase (SRP). @@ -11,6 +11,7 @@ export const enum AuthType { export type AuthConfig = { env: Env; + platform: Platform; type: AuthType; }; diff --git a/packages/profile-sync-controller/src/sdk/authentication.test.ts b/packages/profile-sync-controller/src/sdk/authentication.test.ts index e09e5043e2e..538b57caa31 100644 --- a/packages/profile-sync-controller/src/sdk/authentication.test.ts +++ b/packages/profile-sync-controller/src/sdk/authentication.test.ts @@ -7,7 +7,7 @@ import type { MockVariable } from './__fixtures__/test-utils'; import { arrangeAuth } from './__fixtures__/test-utils'; import { JwtBearerAuth } from './authentication'; import type { LoginResponse, Pair } from './authentication-jwt-bearer/types'; -import { Env } from './env'; +import { Env, Platform } from './env'; import { NonceRetrievalError, PairError, @@ -136,6 +136,7 @@ describe('Authentication - constructor()', () => { new JwtBearerAuth( { env: Env.PRD, + platform: Platform.EXTENSION, type: 'some fake type' as MockVariable, }, {} as MockVariable, diff --git a/packages/profile-sync-controller/src/sdk/env.test.ts b/packages/profile-sync-controller/src/sdk/env.test.ts index b1ca47293ad..16b0c49d2e0 100644 --- a/packages/profile-sync-controller/src/sdk/env.test.ts +++ b/packages/profile-sync-controller/src/sdk/env.test.ts @@ -1,5 +1,5 @@ import type { MockVariable } from './__fixtures__/test-utils'; -import { getEnvUrls, Env } from './env'; +import { getEnvUrls, Env, Platform, getOidcClientId } from './env'; describe('getEnvUrls', () => { it('should return URLs if given a valid environment', () => { @@ -15,3 +15,25 @@ describe('getEnvUrls', () => { ); }); }); + +describe('getOidcClientId', () => { + it('should return client id if given a valid environment and platform', () => { + const clientId = getOidcClientId(Env.PRD, Platform.EXTENSION); + expect(clientId).toBeDefined(); + expect(clientId).toBe('1132f10a-b4e5-4390-a5f2-d9c6022db564'); + }); + + it('should throw an error if the environment is invalid', () => { + expect(() => + getOidcClientId('invalid_env' as MockVariable, Platform.EXTENSION), + ).toThrow('invalid env invalid_env: cannot determine oidc client id'); + }); + + it('should throw an error if the platform is invalid', () => { + expect(() => + getOidcClientId(Env.DEV, 'invalid_platform' as MockVariable), + ).toThrow( + 'invalid env dev and platform invalid_platform combination: cannot determine oidc client id', + ); + }); +}); diff --git a/packages/profile-sync-controller/src/sdk/env.ts b/packages/profile-sync-controller/src/sdk/env.ts index c6fb3e9de34..0269924994c 100644 --- a/packages/profile-sync-controller/src/sdk/env.ts +++ b/packages/profile-sync-controller/src/sdk/env.ts @@ -4,6 +4,12 @@ export const enum Env { PRD = 'prd', } +export const enum Platform { + MOBILE = 'mobile', + EXTENSION = 'extension', + PORTFOLIO = 'portfolio', +} + type EnvUrlsEntry = { authApiUrl: string; oidcApiUrl: string; @@ -46,20 +52,37 @@ export function getEnvUrls(env: Env): EnvUrlsEntry { * Returns the valid OIDC Client ID (used during authorization) * * @param env - environment field + * @param platform - platform field * @returns the OIDC client id for the environment */ -export function getOidcClientId(env: Env): string { - switch (env) { - case Env.DEV: - return 'f1a963d7-50dc-4cb5-8d81-f1f3654f0df3'; - /* istanbul ignore next */ - case Env.UAT: - return 'a9de167c-c9a6-43d8-af39-d301fd44c485'; - /* istanbul ignore next */ - case Env.PRD: - return '1132f10a-b4e5-4390-a5f2-d9c6022db564'; - /* istanbul ignore next */ - default: - throw new Error('invalid env: cannot determine oidc client id'); +export function getOidcClientId(env: Env, platform: Platform): string { + const clientIds = { + [Env.DEV]: { + [Platform.PORTFOLIO]: 'c7ca94a0-5d52-4635-9502-1a50a9c410cc', + [Platform.MOBILE]: 'e83c7cc9-267d-4fb4-8fec-f0e3bbe5ae8e', + [Platform.EXTENSION]: 'f1a963d7-50dc-4cb5-8d81-f1f3654f0df3', + }, + [Env.UAT]: { + [Platform.PORTFOLIO]: '8f2dd4ac-db07-4819-9ba5-1ee0ec1b56d1', + [Platform.MOBILE]: 'c3cfdcd2-51d6-4fae-ad2c-ff238c8fef53', + [Platform.EXTENSION]: 'a9de167c-c9a6-43d8-af39-d301fd44c485', + }, + [Env.PRD]: { + [Platform.PORTFOLIO]: '35e1cd62-49c5-4be8-8b6e-a5212f2d2cfb', + [Platform.MOBILE]: '75fa62a3-9ca0-4b91-9fe5-76bec86b0257', + [Platform.EXTENSION]: '1132f10a-b4e5-4390-a5f2-d9c6022db564', + }, + }; + + if (!clientIds[env]) { + throw new Error(`invalid env ${env}: cannot determine oidc client id`); } + + if (!clientIds[env][platform]) { + throw new Error( + `invalid env ${env} and platform ${platform} combination: cannot determine oidc client id`, + ); + } + + return clientIds[env][platform]; } diff --git a/packages/transaction-controller/CHANGELOG.md b/packages/transaction-controller/CHANGELOG.md index 615fecc1e67..d73fae04a2a 100644 --- a/packages/transaction-controller/CHANGELOG.md +++ b/packages/transaction-controller/CHANGELOG.md @@ -7,6 +7,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [29.1.0] + +### Changed + +- handle Swap+Send transactions as Swaps transactions sub-category; add typing ([#4298](https://github.com/MetaMask/core/pull/4298)) + +## [29.0.2] + +### Fixed + +- fix incorrect token balance changes for simulations of multiple tokens that include an NFT mint ([#4290](https://github.com/MetaMask/core/pull/4290)) + ## [29.0.1] ### Changed @@ -827,7 +839,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 All changes listed after this point were applied to this package following the monorepo conversion. -[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@29.0.1...HEAD +[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@29.1.0...HEAD +[29.1.0]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@29.0.2...@metamask/transaction-controller@29.1.0 +[29.0.2]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@29.0.1...@metamask/transaction-controller@29.0.2 [29.0.1]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@29.0.0...@metamask/transaction-controller@29.0.1 [29.0.0]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@28.1.1...@metamask/transaction-controller@29.0.0 [28.1.1]: https://github.com/MetaMask/core/compare/@metamask/transaction-controller@28.1.0...@metamask/transaction-controller@28.1.1 diff --git a/packages/transaction-controller/package.json b/packages/transaction-controller/package.json index c1d57668ac2..9e8e32d5c1b 100644 --- a/packages/transaction-controller/package.json +++ b/packages/transaction-controller/package.json @@ -1,6 +1,6 @@ { "name": "@metamask/transaction-controller", - "version": "29.0.1", + "version": "29.1.0", "description": "Stores transactions alongside their periodically updated statuses and manages interactions such as approval and cancellation", "keywords": [ "MetaMask", diff --git a/packages/transaction-controller/src/TransactionController.ts b/packages/transaction-controller/src/TransactionController.ts index 52793f1fe02..7a7ce01cf61 100644 --- a/packages/transaction-controller/src/TransactionController.ts +++ b/packages/transaction-controller/src/TransactionController.ts @@ -455,6 +455,14 @@ export type TransactionControllerTransactionNewSwapEvent = { payload: [{ transactionMeta: TransactionMeta }]; }; +/** + * Represents the `TransactionController:transactionNewSwapApproval` event. + */ +export type TransactionControllerTransactionNewSwapAndSendEvent = { + type: `${typeof controllerName}:transactionNewSwapAndSend`; + payload: [{ transactionMeta: TransactionMeta }]; +}; + /** * Represents the `TransactionController:transactionPublishingSkipped` event. */ @@ -524,6 +532,7 @@ export type TransactionControllerEvents = | TransactionControllerTransactionFinishedEvent | TransactionControllerTransactionNewSwapApprovalEvent | TransactionControllerTransactionNewSwapEvent + | TransactionControllerTransactionNewSwapAndSendEvent | TransactionControllerTransactionPublishingSkipped | TransactionControllerTransactionRejectedEvent | TransactionControllerTransactionStatusUpdatedEvent diff --git a/packages/transaction-controller/src/types.ts b/packages/transaction-controller/src/types.ts index d5fe04b3894..776e5ae559d 100644 --- a/packages/transaction-controller/src/types.ts +++ b/packages/transaction-controller/src/types.ts @@ -123,6 +123,11 @@ type TransactionMetaBase = { */ destinationTokenAddress?: string; + /** + * The raw amount of the destination token + */ + destinationTokenAmount?: string; + /** * The decimals of the token being received of swap transaction. */ @@ -330,11 +335,31 @@ type TransactionMetaBase = { */ submittedTime?: number; + /** + * The address of the token being swapped + */ + sourceTokenAddress?: string; + + /** + * The raw amount of the source swap token + */ + sourceTokenAmount?: string; + + /** + * The decimals of the token being swapped. + */ + sourceTokenDecimals?: number; + /** * The symbol of the token being swapped. */ sourceTokenSymbol?: string; + /** + * The address of the swap recipient. + */ + swapAndSendRecipient?: string; + /** * The metadata of the swap transaction. */ @@ -523,6 +548,11 @@ export enum TransactionType { */ swap = 'swap', + /** + * A transaction swapping one token for another through MetaMask Swaps, then sending the swapped token to a recipient. + */ + swapAndSend = 'swapAndSend', + /** * Similar to the approve type, a swap approval is a special case of ERC20 * approve method that requests an allowance of the token to spend on behalf diff --git a/packages/transaction-controller/src/utils/simulation.test.ts b/packages/transaction-controller/src/utils/simulation.test.ts index 10152b78a9d..4d138b51ab2 100644 --- a/packages/transaction-controller/src/utils/simulation.test.ts +++ b/packages/transaction-controller/src/utils/simulation.test.ts @@ -21,7 +21,8 @@ jest.mock('./simulation-api'); const USER_ADDRESS_MOCK = '0x123'; const OTHER_ADDRESS_MOCK = '0x456'; -const CONTRACT_ADDRESS_MOCK = '0x789'; +const CONTRACT_ADDRESS_1_MOCK = '0x789'; +const CONTRACT_ADDRESS_2_MOCK = '0xDEF'; const BALANCE_1_MOCK = '0x0'; const BALANCE_2_MOCK = '0x1'; const DIFFERENCE_MOCK = '0x1'; @@ -38,23 +39,23 @@ const REQUEST_MOCK: GetSimulationDataRequest = { const PARSED_ERC20_TRANSFER_EVENT_MOCK = { name: 'Transfer', - contractAddress: CONTRACT_ADDRESS_MOCK, + contractAddress: CONTRACT_ADDRESS_2_MOCK, args: [ - OTHER_ADDRESS_MOCK, USER_ADDRESS_MOCK, + OTHER_ADDRESS_MOCK, { toHexString: () => VALUE_MOCK }, ], } as unknown as LogDescription; const PARSED_ERC721_TRANSFER_EVENT_MOCK = { name: 'Transfer', - contractAddress: CONTRACT_ADDRESS_MOCK, + contractAddress: CONTRACT_ADDRESS_1_MOCK, args: [OTHER_ADDRESS_MOCK, USER_ADDRESS_MOCK, TOKEN_ID_MOCK], } as unknown as LogDescription; const PARSED_ERC1155_TRANSFER_SINGLE_EVENT_MOCK = { name: 'TransferSingle', - contractAddress: CONTRACT_ADDRESS_MOCK, + contractAddress: CONTRACT_ADDRESS_1_MOCK, args: [ OTHER_ADDRESS_MOCK, OTHER_ADDRESS_MOCK, @@ -66,7 +67,7 @@ const PARSED_ERC1155_TRANSFER_SINGLE_EVENT_MOCK = { const PARSED_ERC1155_TRANSFER_BATCH_EVENT_MOCK = { name: 'TransferBatch', - contractAddress: CONTRACT_ADDRESS_MOCK, + contractAddress: CONTRACT_ADDRESS_1_MOCK, args: [ OTHER_ADDRESS_MOCK, OTHER_ADDRESS_MOCK, @@ -78,7 +79,7 @@ const PARSED_ERC1155_TRANSFER_BATCH_EVENT_MOCK = { const PARSED_WRAPPED_ERC20_DEPOSIT_EVENT_MOCK = { name: 'Deposit', - contractAddress: CONTRACT_ADDRESS_MOCK, + contractAddress: CONTRACT_ADDRESS_1_MOCK, args: [USER_ADDRESS_MOCK, { toHexString: () => VALUE_MOCK }], } as unknown as LogDescription; @@ -92,7 +93,7 @@ const RESPONSE_NESTED_LOGS_MOCK: SimulationResponse = { calls: [ { calls: [], - logs: [createLogMock(CONTRACT_ADDRESS_MOCK)], + logs: [createLogMock(CONTRACT_ADDRESS_1_MOCK)], }, ], logs: [], @@ -200,7 +201,7 @@ function createBalanceOfResponse( }, })), { - return: '0x', + return: '0xabc', callTrace: { calls: [], logs: [], @@ -379,7 +380,7 @@ describe('Simulation Utils', () => { simulateTransactionsMock .mockResolvedValueOnce( - createEventResponseMock([createLogMock(CONTRACT_ADDRESS_MOCK)]), + createEventResponseMock([createLogMock(CONTRACT_ADDRESS_1_MOCK)]), ) .mockResolvedValueOnce( createBalanceOfResponse(previousBalances, newBalances), @@ -392,7 +393,7 @@ describe('Simulation Utils', () => { tokenBalanceChanges: [ { standard: tokenStandard, - address: CONTRACT_ADDRESS_MOCK, + address: CONTRACT_ADDRESS_1_MOCK, id: tokenId, previousBalance: BALANCE_1_MOCK, newBalance: BALANCE_2_MOCK, @@ -480,8 +481,8 @@ describe('Simulation Utils', () => { simulateTransactionsMock .mockResolvedValueOnce( createEventResponseMock([ - createLogMock(CONTRACT_ADDRESS_MOCK), - createLogMock(CONTRACT_ADDRESS_MOCK), + createLogMock(CONTRACT_ADDRESS_1_MOCK), + createLogMock(CONTRACT_ADDRESS_1_MOCK), ]), ) .mockResolvedValueOnce( @@ -495,7 +496,7 @@ describe('Simulation Utils', () => { tokenBalanceChanges: [ { standard: SimulationTokenStandard.erc20, - address: CONTRACT_ADDRESS_MOCK, + address: CONTRACT_ADDRESS_1_MOCK, id: undefined, previousBalance: BALANCE_2_MOCK, newBalance: BALANCE_1_MOCK, @@ -521,8 +522,8 @@ describe('Simulation Utils', () => { simulateTransactionsMock .mockResolvedValueOnce( createEventResponseMock([ - createLogMock(CONTRACT_ADDRESS_MOCK), - createLogMock(CONTRACT_ADDRESS_MOCK), + createLogMock(CONTRACT_ADDRESS_1_MOCK), + createLogMock(CONTRACT_ADDRESS_1_MOCK), ]), ) .mockResolvedValueOnce( @@ -539,7 +540,7 @@ describe('Simulation Utils', () => { tokenBalanceChanges: [ { standard: SimulationTokenStandard.erc721, - address: CONTRACT_ADDRESS_MOCK, + address: CONTRACT_ADDRESS_1_MOCK, id: TOKEN_ID_MOCK, previousBalance: BALANCE_1_MOCK, newBalance: BALANCE_2_MOCK, @@ -548,7 +549,7 @@ describe('Simulation Utils', () => { }, { standard: SimulationTokenStandard.erc721, - address: CONTRACT_ADDRESS_MOCK, + address: CONTRACT_ADDRESS_1_MOCK, id: OTHER_TOKEN_ID_MOCK, previousBalance: BALANCE_1_MOCK, newBalance: BALANCE_2_MOCK, @@ -571,28 +572,57 @@ describe('Simulation Utils', () => { }, }); + // Pay for NFT mint with ERC20 token. + mockParseLog({ + erc20: PARSED_ERC20_TRANSFER_EVENT_MOCK, + }); + simulateTransactionsMock .mockResolvedValueOnce( - createEventResponseMock([createLogMock(CONTRACT_ADDRESS_MOCK)]), + createEventResponseMock([ + createLogMock(CONTRACT_ADDRESS_1_MOCK), + createLogMock(CONTRACT_ADDRESS_2_MOCK), + ]), ) .mockResolvedValueOnce( - createBalanceOfResponse([], [USER_ADDRESS_MOCK]), + createBalanceOfResponse( + [BALANCE_2_MOCK], // The ERC20 balance before minting. + [ + USER_ADDRESS_MOCK, // The user is the owner. + BALANCE_1_MOCK, // The ERC20 balance after minting. + ], + ), ); const simulationData = await getSimulationData(REQUEST_MOCK); expect(simulateTransactionsMock).toHaveBeenCalledTimes(2); - // The second call should only simulate the minting of the NFT and - // check the balance after, and not before. + + // The balance of the ERC-20 token is checked before and after the transaction. + // The ERC-721 token balance is only checked after the transaction since it is minted. expect(simulateTransactionsMock).toHaveBeenNthCalledWith( 2, REQUEST_MOCK.chainId, { transactions: [ + // ERC-20 balance before minting. + { + from: REQUEST_MOCK.from, + to: CONTRACT_ADDRESS_2_MOCK, + data: expect.any(String), + }, + // Minting ERC-721 token. REQUEST_MOCK, + // ERC-721 owner after minting. { from: REQUEST_MOCK.from, - to: CONTRACT_ADDRESS_MOCK, + to: CONTRACT_ADDRESS_1_MOCK, + data: expect.any(String), + }, + // ERC-20 balance before minting. + { + from: REQUEST_MOCK.from, + to: CONTRACT_ADDRESS_2_MOCK, data: expect.any(String), }, ], @@ -603,13 +633,22 @@ describe('Simulation Utils', () => { tokenBalanceChanges: [ { standard: SimulationTokenStandard.erc721, - address: CONTRACT_ADDRESS_MOCK, + address: CONTRACT_ADDRESS_1_MOCK, id: TOKEN_ID_MOCK, previousBalance: '0x0', newBalance: '0x1', difference: '0x1', isDecrease: false, }, + { + standard: SimulationTokenStandard.erc20, + address: CONTRACT_ADDRESS_2_MOCK, + id: undefined, + previousBalance: '0x1', + newBalance: '0x0', + difference: '0x1', + isDecrease: true, + }, ], }); }); @@ -619,7 +658,7 @@ describe('Simulation Utils', () => { simulateTransactionsMock .mockResolvedValueOnce( - createEventResponseMock([createLogMock(CONTRACT_ADDRESS_MOCK)]), + createEventResponseMock([createLogMock(CONTRACT_ADDRESS_1_MOCK)]), ) .mockResolvedValueOnce( createBalanceOfResponse([BALANCE_1_MOCK], [BALANCE_2_MOCK]), @@ -646,7 +685,7 @@ describe('Simulation Utils', () => { }); simulateTransactionsMock.mockResolvedValueOnce( - createEventResponseMock([createLogMock(CONTRACT_ADDRESS_MOCK)]), + createEventResponseMock([createLogMock(CONTRACT_ADDRESS_1_MOCK)]), ); const simulationData = await getSimulationData(REQUEST_MOCK); @@ -664,7 +703,7 @@ describe('Simulation Utils', () => { simulateTransactionsMock .mockResolvedValueOnce( - createEventResponseMock([createLogMock(CONTRACT_ADDRESS_MOCK)]), + createEventResponseMock([createLogMock(CONTRACT_ADDRESS_1_MOCK)]), ) .mockResolvedValueOnce( createBalanceOfResponse([BALANCE_1_MOCK], [BALANCE_1_MOCK]), @@ -694,7 +733,7 @@ describe('Simulation Utils', () => { tokenBalanceChanges: [ { standard: SimulationTokenStandard.erc20, - address: CONTRACT_ADDRESS_MOCK, + address: CONTRACT_ADDRESS_1_MOCK, id: undefined, previousBalance: BALANCE_1_MOCK, newBalance: BALANCE_2_MOCK, @@ -741,7 +780,7 @@ describe('Simulation Utils', () => { simulateTransactionsMock .mockResolvedValueOnce( - createEventResponseMock([createLogMock(CONTRACT_ADDRESS_MOCK)]), + createEventResponseMock([createLogMock(CONTRACT_ADDRESS_1_MOCK)]), ) .mockResolvedValueOnce(createBalanceOfResponse([], [])); diff --git a/packages/transaction-controller/src/utils/simulation.ts b/packages/transaction-controller/src/utils/simulation.ts index 776b22c9893..ae8b25cb297 100644 --- a/packages/transaction-controller/src/utils/simulation.ts +++ b/packages/transaction-controller/src/utils/simulation.ts @@ -310,6 +310,7 @@ async function getTokenBalanceChanges( throw new SimulationInvalidResponseError(); } + let prevBalanceTxIndex = 0; return [...balanceTxs.after.keys()] .map((token, index) => { const previousBalanceCheckSkipped = !balanceTxs.before.get(token); @@ -318,7 +319,8 @@ async function getTokenBalanceChanges( : getValueFromBalanceTransaction( request.from, token, - response.transactions[index], + // eslint-disable-next-line no-plusplus + response.transactions[prevBalanceTxIndex++], ); const newBalance = getValueFromBalanceTransaction( diff --git a/packages/transaction-controller/src/utils/swaps.test.ts b/packages/transaction-controller/src/utils/swaps.test.ts index 7a269de13b8..4224146f444 100644 --- a/packages/transaction-controller/src/utils/swaps.test.ts +++ b/packages/transaction-controller/src/utils/swaps.test.ts @@ -72,7 +72,7 @@ describe('updateSwapsTransaction', () => { expect(messenger.call).not.toHaveBeenCalled(); }); - it('should not update if transaction type is not swap or swapApproval', async () => { + it('should not update if transaction type is not swap, swapAndSend, swapApproval', async () => { transactionType = TransactionType.deployContract; jest.spyOn(messenger, 'call'); updateSwapsTransaction(transactionMeta, transactionType, swaps, request); @@ -243,6 +243,138 @@ describe('updateSwapsTransaction', () => { type, }); }); + + it('should update swap and send transaction and publish TransactionController:transactionNewSwapAndSend', async () => { + const sourceTokenSymbol = 'ETH'; + const destinationTokenSymbol = 'DAI'; + const type = TransactionType.swapAndSend; + transactionType = TransactionType.swapAndSend; + const destinationTokenDecimals = '18'; + const destinationTokenAddress = '0x0'; + const swapMetaData = { + meta: 'data', + }; + const swapTokenValue = '0x123'; + const estimatedBaseFee = '0x123'; + const approvalTxId = '0x123'; + + // swap + send properties + const destinationTokenAmount = '0x123'; + const sourceTokenAddress = '0x123'; + const sourceTokenAmount = '0x123'; + const sourceTokenDecimals = '12'; + + const swapAndSendRecipient = '0x123'; + + swaps.meta = { + sourceTokenSymbol, + destinationTokenSymbol, + type, + destinationTokenDecimals, + destinationTokenAddress, + swapMetaData, + swapTokenValue, + estimatedBaseFee, + approvalTxId, + destinationTokenAmount, + sourceTokenAddress, + sourceTokenAmount, + sourceTokenDecimals, + swapAndSendRecipient, + }; + + const transactionNewSwapAndSendEventListener = jest.fn(); + messenger.subscribe( + 'TransactionController:transactionNewSwapAndSend', + transactionNewSwapAndSendEventListener, + ); + + updateSwapsTransaction(transactionMeta, transactionType, swaps, request); + + expect(transactionNewSwapAndSendEventListener).toHaveBeenCalledWith({ + transactionMeta: { + ...transactionMeta, + sourceTokenSymbol, + destinationTokenSymbol, + type, + destinationTokenDecimals, + destinationTokenAddress, + swapMetaData, + swapTokenValue, + estimatedBaseFee, + approvalTxId, + destinationTokenAmount, + sourceTokenAddress, + sourceTokenAmount, + sourceTokenDecimals, + swapAndSendRecipient, + }, + }); + }); + + it('should return the swap and send transaction updated with information', () => { + const sourceTokenSymbol = 'ETH'; + const destinationTokenSymbol = 'DAI'; + const type = TransactionType.swapAndSend; + transactionType = TransactionType.swapAndSend; + const destinationTokenDecimals = '18'; + const destinationTokenAddress = '0x0'; + const swapMetaData = { + meta: 'data', + }; + const swapTokenValue = '0x123'; + const estimatedBaseFee = '0x123'; + const approvalTxId = '0x123'; + + // swap + send properties + const destinationTokenAmount = '0x123'; + const sourceTokenAddress = '0x123'; + const sourceTokenAmount = '0x123'; + const sourceTokenDecimals = '12'; + + const swapAndSendRecipient = '0x123'; + + swaps.meta = { + sourceTokenSymbol, + destinationTokenSymbol, + type, + destinationTokenDecimals, + destinationTokenAddress, + swapMetaData, + swapTokenValue, + estimatedBaseFee, + approvalTxId, + destinationTokenAmount, + sourceTokenAddress, + sourceTokenAmount, + sourceTokenDecimals, + swapAndSendRecipient, + }; + + const updatedSwapsTransaction = updateSwapsTransaction( + transactionMeta, + transactionType, + swaps, + request, + ); + expect(updatedSwapsTransaction).toStrictEqual({ + ...transactionMeta, + sourceTokenSymbol, + destinationTokenSymbol, + type, + destinationTokenDecimals, + destinationTokenAddress, + swapMetaData, + swapTokenValue, + estimatedBaseFee, + approvalTxId, + destinationTokenAmount, + sourceTokenAddress, + sourceTokenAmount, + sourceTokenDecimals, + swapAndSendRecipient, + }); + }); }); describe('updatePostTransactionBalance', () => { diff --git a/packages/transaction-controller/src/utils/swaps.ts b/packages/transaction-controller/src/utils/swaps.ts index 9256ece7a8e..7c69142142f 100644 --- a/packages/transaction-controller/src/utils/swaps.ts +++ b/packages/transaction-controller/src/utils/swaps.ts @@ -108,6 +108,7 @@ export const SWAPS_CHAINID_DEFAULT_TOKEN_MAP = { export const SWAP_TRANSACTION_TYPES = [ TransactionType.swap, + TransactionType.swapAndSend, TransactionType.swapApproval, ]; @@ -182,6 +183,16 @@ export function updateSwapsTransaction( }); } + if (transactionType === TransactionType.swapAndSend) { + updatedTransactionMeta = updateSwapAndSendTransaction( + transactionMeta, + swapsMeta, + ); + messenger.publish('TransactionController:transactionNewSwapAndSend', { + transactionMeta: updatedTransactionMeta, + }); + } + if (transactionType === TransactionType.swap) { updatedTransactionMeta = updateSwapTransaction(transactionMeta, swapsMeta); messenger.publish('TransactionController:transactionNewSwap', { @@ -327,6 +338,71 @@ function updateSwapTransaction( return merge({}, transactionMeta, swapTransaction); } +/** + * Updates the transaction meta object with the swap information + * + * @param transactionMeta - Transaction meta object to update + * @param propsToUpdate - Properties to update + * @param propsToUpdate.approvalTxId - Transaction id of the approval transaction + * @param propsToUpdate.destinationTokenAddress - Address of the token to be received + * @param propsToUpdate.destinationTokenAmount - The raw amount of the destination token + * @param propsToUpdate.destinationTokenDecimals - Decimals of the token to be received + * @param propsToUpdate.destinationTokenSymbol - Symbol of the token to be received + * @param propsToUpdate.estimatedBaseFee - Estimated base fee of the transaction + * @param propsToUpdate.sourceTokenAddress - The address of the source token + * @param propsToUpdate.sourceTokenAmount - The raw amount of the source token + * @param propsToUpdate.sourceTokenDecimals - The decimals of the source token + * @param propsToUpdate.sourceTokenSymbol - Symbol of the token to be swapped + * @param propsToUpdate.swapAndSendRecipient - The recipient of the swap and send transaction + * @param propsToUpdate.swapMetaData - Metadata of the swap + * @param propsToUpdate.swapTokenValue - Value of the token to be swapped – possibly the same as sourceTokenAmount; included for consistency + * @param propsToUpdate.type - Type of the transaction + * @returns The updated transaction meta object. + */ +function updateSwapAndSendTransaction( + transactionMeta: TransactionMeta, + { + approvalTxId, + destinationTokenAddress, + destinationTokenAmount, + destinationTokenDecimals, + destinationTokenSymbol, + estimatedBaseFee, + sourceTokenAddress, + sourceTokenAmount, + sourceTokenDecimals, + sourceTokenSymbol, + swapAndSendRecipient, + swapMetaData, + swapTokenValue, + type, + }: Partial, +): TransactionMeta { + validateIfTransactionUnapproved(transactionMeta, 'updateSwapTransaction'); + + let swapTransaction = { + approvalTxId, + destinationTokenAddress, + destinationTokenAmount, + destinationTokenDecimals, + destinationTokenSymbol, + estimatedBaseFee, + sourceTokenAddress, + sourceTokenAmount, + sourceTokenDecimals, + sourceTokenSymbol, + swapAndSendRecipient, + swapMetaData, + swapTokenValue, + type, + }; + // TODO: Replace `any` with type + // eslint-disable-next-line @typescript-eslint/no-explicit-any + swapTransaction = pickBy(swapTransaction) as any; + + return merge({}, transactionMeta, swapTransaction); +} + /** * Updates the transaction meta object with the swap approval information * diff --git a/packages/user-operation-controller/package.json b/packages/user-operation-controller/package.json index e5ab36ca1fb..5a691d21012 100644 --- a/packages/user-operation-controller/package.json +++ b/packages/user-operation-controller/package.json @@ -51,7 +51,7 @@ "@metamask/network-controller": "^18.1.1", "@metamask/polling-controller": "^6.0.2", "@metamask/rpc-errors": "^6.2.1", - "@metamask/transaction-controller": "^29.0.1", + "@metamask/transaction-controller": "^29.1.0", "@metamask/utils": "^8.3.0", "bn.js": "^5.2.1", "immer": "^9.0.6", diff --git a/packages/user-operation-controller/src/UserOperationController.test.ts b/packages/user-operation-controller/src/UserOperationController.test.ts index 33be07822bc..2004abd0f43 100644 --- a/packages/user-operation-controller/src/UserOperationController.test.ts +++ b/packages/user-operation-controller/src/UserOperationController.test.ts @@ -424,10 +424,15 @@ describe('UserOperationController', () => { smartContractAccount, swaps: { approvalTxId: 'testTxId', + destinationTokenAmount: '0x1', destinationTokenAddress: '0x1', destinationTokenDecimals: 3, destinationTokenSymbol: 'TEST', estimatedBaseFee: '0x2', + sourceTokenAddress: '0x3', + sourceTokenAmount: '0x4', + sourceTokenDecimals: 5, + swapAndSendRecipient: '0x5', sourceTokenSymbol: 'ETH', swapMetaData: { test: 'value' }, swapTokenValue: '0x3', @@ -440,10 +445,15 @@ describe('UserOperationController', () => { expect.objectContaining({ swapsMetadata: { approvalTxId: 'testTxId', + destinationTokenAmount: '0x1', destinationTokenAddress: '0x1', destinationTokenDecimals: 3, destinationTokenSymbol: 'TEST', estimatedBaseFee: '0x2', + sourceTokenAddress: '0x3', + sourceTokenAmount: '0x4', + sourceTokenDecimals: 5, + swapAndSendRecipient: '0x5', sourceTokenSymbol: 'ETH', swapMetaData: { test: 'value' }, swapTokenValue: '0x3', @@ -471,10 +481,15 @@ describe('UserOperationController', () => { swapsMetadata: { approvalTxId: null, destinationTokenAddress: null, + destinationTokenAmount: null, destinationTokenDecimals: null, destinationTokenSymbol: null, estimatedBaseFee: null, + sourceTokenAddress: null, + sourceTokenAmount: null, + sourceTokenDecimals: null, sourceTokenSymbol: null, + swapAndSendRecipient: null, swapMetaData: null, swapTokenValue: null, }, diff --git a/packages/user-operation-controller/src/UserOperationController.ts b/packages/user-operation-controller/src/UserOperationController.ts index 8bd42f7179c..6c4bdb641e9 100644 --- a/packages/user-operation-controller/src/UserOperationController.ts +++ b/packages/user-operation-controller/src/UserOperationController.ts @@ -137,10 +137,15 @@ export type AddUserOperationRequest = { export type AddUserOperationSwapOptions = { approvalTxId?: string; destinationTokenAddress?: string; + destinationTokenAmount?: string; destinationTokenDecimals?: number; destinationTokenSymbol?: string; estimatedBaseFee?: string; + sourceTokenAddress?: string; + sourceTokenAmount?: string; + sourceTokenDecimals?: number; sourceTokenSymbol?: string; + swapAndSendRecipient?: string; swapMetaData?: Record; swapTokenValue?: string; }; @@ -437,10 +442,15 @@ export class UserOperationController extends BaseController< ? { approvalTxId: swaps.approvalTxId ?? null, destinationTokenAddress: swaps.destinationTokenAddress ?? null, + destinationTokenAmount: swaps.destinationTokenAmount ?? null, destinationTokenDecimals: swaps.destinationTokenDecimals ?? null, destinationTokenSymbol: swaps.destinationTokenSymbol ?? null, estimatedBaseFee: swaps.estimatedBaseFee ?? null, + sourceTokenAddress: swaps.sourceTokenAddress ?? null, + sourceTokenAmount: swaps.sourceTokenAmount ?? null, + sourceTokenDecimals: swaps.sourceTokenDecimals ?? null, sourceTokenSymbol: swaps.sourceTokenSymbol ?? null, + swapAndSendRecipient: swaps.swapAndSendRecipient ?? null, swapMetaData: (swaps.swapMetaData as Record) ?? null, swapTokenValue: swaps.swapTokenValue ?? null, } diff --git a/packages/user-operation-controller/src/types.ts b/packages/user-operation-controller/src/types.ts index 4fe084bff0d..0e827cc3c19 100644 --- a/packages/user-operation-controller/src/types.ts +++ b/packages/user-operation-controller/src/types.ts @@ -348,12 +348,27 @@ export type SwapsMetadata = { /** Symbol of the destination token. */ destinationTokenSymbol: string | null; + /** Amount of the destination token. */ + destinationTokenAmount: string | null; + /** Estimated base fee of the swap. */ estimatedBaseFee: string | null; + /** Address of the source token. */ + sourceTokenAddress: string | null; + + /** Amount of the source token. */ + sourceTokenAmount: string | null; + + /** Number of decimals of the source token. */ + sourceTokenDecimals: number | null; + /** Symbol of the source token. */ sourceTokenSymbol: string | null; + /** Recipient of the swap and send transaction. */ + swapAndSendRecipient: string | null; + /** Untyped raw metadata values. */ swapMetaData: Record | null; diff --git a/packages/user-operation-controller/src/utils/transaction.ts b/packages/user-operation-controller/src/utils/transaction.ts index 5bb1161c7e8..b9406cd449e 100644 --- a/packages/user-operation-controller/src/utils/transaction.ts +++ b/packages/user-operation-controller/src/utils/transaction.ts @@ -108,11 +108,16 @@ export function getTransactionMetadata( approvalTxId: swapsMetadata?.approvalTxId ?? undefined, destinationTokenAddress: swapsMetadata?.destinationTokenAddress ?? undefined, + destinationTokenAmount: swapsMetadata?.destinationTokenAmount ?? undefined, destinationTokenDecimals: swapsMetadata?.destinationTokenDecimals ?? undefined, destinationTokenSymbol: swapsMetadata?.destinationTokenSymbol ?? undefined, estimatedBaseFee: swapsMetadata?.estimatedBaseFee ?? undefined, + sourceTokenAddress: swapsMetadata?.sourceTokenAddress ?? undefined, + sourceTokenAmount: swapsMetadata?.sourceTokenAmount ?? undefined, + sourceTokenDecimals: swapsMetadata?.sourceTokenDecimals ?? undefined, sourceTokenSymbol: swapsMetadata?.sourceTokenSymbol ?? undefined, + swapAndSendRecipient: swapsMetadata?.swapAndSendRecipient ?? undefined, swapMetaData: swapsMetadata?.swapMetaData ?? undefined, swapTokenValue: swapsMetadata?.swapTokenValue ?? undefined, }; diff --git a/packages/user-operation-controller/src/utils/validation.test.ts b/packages/user-operation-controller/src/utils/validation.test.ts index 5098185e730..fd789265187 100644 --- a/packages/user-operation-controller/src/utils/validation.test.ts +++ b/packages/user-operation-controller/src/utils/validation.test.ts @@ -267,6 +267,12 @@ describe('validation', () => { 123, 'Expected a string, but received: 123', ], + [ + 'swaps.destinationTokenAmount', + 'wrong type', + 123, + 'Expected a string, but received: 123', + ], [ 'swaps.destinationTokenDecimals', 'wrong type', @@ -285,12 +291,36 @@ describe('validation', () => { 123, 'Expected a string, but received: 123', ], + [ + 'swaps.sourceTokenAddress', + 'wrong type', + 123, + 'Expected a string, but received: 123', + ], + [ + 'swaps.sourceTokenAmount', + 'wrong type', + 123, + 'Expected a string, but received: 123', + ], + [ + 'swaps.sourceTokenDecimals', + 'wrong type', + '123', + 'Expected a number, but received: "123"', + ], [ 'swaps.sourceTokenSymbol', 'wrong type', 123, 'Expected a string, but received: 123', ], + [ + 'swaps.swapAndSendRecipient', + 'wrong type', + 123, + 'Expected a string, but received: 123', + ], [ 'swaps.swapMetaData', 'wrong type', @@ -307,7 +337,7 @@ describe('validation', () => { 'type', 'wrong type', 123, - 'Expected one of `"cancel","contractInteraction","contractDeployment","eth_decrypt","eth_getEncryptionPublicKey","incoming","personal_sign","retry","simpleSend","eth_sign","eth_signTypedData","smart","swap","swapApproval","approve","safetransferfrom","transfer","transferfrom","setapprovalforall","increaseAllowance"`, but received: 123', + 'Expected one of `"cancel","contractInteraction","contractDeployment","eth_decrypt","eth_getEncryptionPublicKey","incoming","personal_sign","retry","simpleSend","eth_sign","eth_signTypedData","smart","swap","swapAndSend","swapApproval","approve","safetransferfrom","transfer","transferfrom","setapprovalforall","increaseAllowance"`, but received: 123', ], ])( 'throws if %s is %s', diff --git a/packages/user-operation-controller/src/utils/validation.ts b/packages/user-operation-controller/src/utils/validation.ts index 10cffda2878..901f1be89f0 100644 --- a/packages/user-operation-controller/src/utils/validation.ts +++ b/packages/user-operation-controller/src/utils/validation.ts @@ -75,6 +75,11 @@ export function validateAddUserOperationOptions( sourceTokenSymbol: optional(string()), swapMetaData: optional(object()), swapTokenValue: optional(string()), + destinationTokenAmount: optional(string()), + sourceTokenAddress: optional(string()), + sourceTokenAmount: optional(string()), + sourceTokenDecimals: optional(number()), + swapAndSendRecipient: optional(string()), }), ), type: optional(enums(Object.values(TransactionType))), diff --git a/yarn.lock b/yarn.lock index 3058ef0a19b..01f67a06572 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1641,13 +1641,13 @@ __metadata: linkType: soft "@metamask/action-utils@npm:^1.0.0": - version: 1.0.0 - resolution: "@metamask/action-utils@npm:1.0.0" + version: 1.1.1 + resolution: "@metamask/action-utils@npm:1.1.1" dependencies: "@types/semver": ^7.3.6 glob: ^7.1.7 semver: ^7.3.5 - checksum: 5cfd5c7f8895f4bc602cb6e7b3e133aae0431ded8f50e136d314b2e54936ae22cc6add226fa6dfabb300addf084e32fd864a87b91af9188fa158b6e99d91e784 + checksum: b13245c89e5fd769ab03d3731f0156b805b55c3af402ce896181682b423e5f3d00470dc258161ff6598f5dad49d06e19a40c9967805a8e7bc1de0d8d2313a9ff languageName: node linkType: hard @@ -2687,12 +2687,12 @@ __metadata: linkType: soft "@metamask/post-message-stream@npm:^8.0.0": - version: 8.0.0 - resolution: "@metamask/post-message-stream@npm:8.0.0" + version: 8.1.0 + resolution: "@metamask/post-message-stream@npm:8.1.0" dependencies: "@metamask/utils": ^8.1.0 readable-stream: 3.6.2 - checksum: 3016d8d5f8a5954fd146ce06c0b5fd7a9a070b43284e2bad140e179ee259146b666d56e6dbefa0277f56fbb67806970c9de3067c75f0e56886d0752e7c0f5e22 + checksum: 84b5f90ee28d3440520088c01fb64c42a2ed3e761bef4285c8dd72f78c3f634d58ac3314c5ebaedabc92e3db369960e17d61b84719f2d6271cd6d4957f2b6704 languageName: node linkType: hard @@ -3012,7 +3012,7 @@ __metadata: languageName: node linkType: hard -"@metamask/transaction-controller@^29.0.1, @metamask/transaction-controller@workspace:packages/transaction-controller": +"@metamask/transaction-controller@^29.1.0, @metamask/transaction-controller@workspace:packages/transaction-controller": version: 0.0.0-use.local resolution: "@metamask/transaction-controller@workspace:packages/transaction-controller" dependencies: @@ -3075,7 +3075,7 @@ __metadata: "@metamask/network-controller": ^18.1.1 "@metamask/polling-controller": ^6.0.2 "@metamask/rpc-errors": ^6.2.1 - "@metamask/transaction-controller": ^29.0.1 + "@metamask/transaction-controller": ^29.1.0 "@metamask/utils": ^8.3.0 "@types/jest": ^27.4.1 bn.js: ^5.2.1