diff --git a/packages/bridge-controller/CHANGELOG.md b/packages/bridge-controller/CHANGELOG.md index 11b9470c708..271f1f096dd 100644 --- a/packages/bridge-controller/CHANGELOG.md +++ b/packages/bridge-controller/CHANGELOG.md @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/remote-feature-flag-controller` from `^4.0.0` to `^4.1.0` ([#8041](https://github.com/MetaMask/core/pull/8041)) - Bump `@metamask/transaction-controller` from `^62.18.0` to `^62.19.0` ([#8031](https://github.com/MetaMask/core/pull/8031)) - Bump `@metamask/assets-controllers` from `^100.0.2` to `^100.0.3` ([#8029](https://github.com/MetaMask/core/pull/8029)) +- Extend quote intent validation to accept optional EIP-712 `typedData` payloads ([#7895](https://github.com/MetaMask/core/pull/7895)). ## [67.2.0] diff --git a/packages/bridge-controller/src/utils/validators.test.ts b/packages/bridge-controller/src/utils/validators.test.ts index 06bfeab351e..da6ff91c0d2 100644 --- a/packages/bridge-controller/src/utils/validators.test.ts +++ b/packages/bridge-controller/src/utils/validators.test.ts @@ -1,4 +1,6 @@ -import { validateFeatureFlagsResponse } from './validators'; +import { is } from '@metamask/superstruct'; + +import { validateFeatureFlagsResponse, IntentSchema } from './validators'; describe('validators', () => { describe('validateFeatureFlagsResponse', () => { @@ -280,9 +282,10 @@ describe('validators', () => { maxRefreshCount: 5, refreshRate: 30000, support: true, - minimumVersion: '0.0', + minimumVersion: '0.0.0', sse: { enabled: true, + minimumVersion: '0.0', }, }, type: 'sse config - malformed minimum version', @@ -316,11 +319,129 @@ describe('validators', () => { type: 'all evm chains active + an extra field not specified in the schema', expected: true, }, - ])( - 'should return $expected if the response is valid: $type', - ({ response, expected }) => { - expect(validateFeatureFlagsResponse(response)).toBe(expected); + ])('should return $expected for: $type', ({ response, expected }) => { + expect(validateFeatureFlagsResponse(response)).toBe(expected); + }); + }); + + describe('IntentSchema', () => { + const validOrder = { + sellToken: '0x0000000000000000000000000000000000000001', + buyToken: '0x0000000000000000000000000000000000000002', + validTo: 1717027200, + appData: 'some-app-data', + appDataHash: '0xabcd', + feeAmount: '100', + kind: 'sell' as const, + partiallyFillable: false, + sellAmount: '1000', + }; + + const validIntent = { + protocol: 'cowswap', + order: validOrder, + typedData: { + types: { Order: [{ name: 'sellToken', type: 'address' }] }, + domain: { name: 'GPv2Settlement', chainId: 1 }, + primaryType: 'Order', + message: { sellToken: '0x01', buyToken: '0x02' }, }, - ); + }; + + it('accepts a valid intent with required fields only', () => { + expect(is(validIntent, IntentSchema)).toBe(true); + }); + + it('accepts intent with optional settlementContract', () => { + expect( + is( + { + ...validIntent, + settlementContract: '0x9008D19f58AAbd9eD0D60971565AA8510560ab41', + }, + IntentSchema, + ), + ).toBe(true); + }); + + it('rejects intent without typedData', () => { + const { typedData: _, ...intentWithoutTypedData } = validIntent; + expect(is(intentWithoutTypedData, IntentSchema)).toBe(false); + }); + + it('rejects intent with typedData missing domain', () => { + expect( + is( + { + ...validIntent, + typedData: { types: {}, primaryType: 'Order', message: {} }, + }, + IntentSchema, + ), + ).toBe(false); + }); + + it('rejects intent with typedData missing message', () => { + expect( + is( + { + ...validIntent, + typedData: { types: {}, domain: {}, primaryType: 'Order' }, + }, + IntentSchema, + ), + ).toBe(false); + }); + + it('rejects intent with typedData missing types', () => { + expect( + is( + { + ...validIntent, + typedData: { domain: {}, primaryType: 'Order', message: {} }, + }, + IntentSchema, + ), + ).toBe(false); + }); + + it('rejects intent with typedData missing primaryType', () => { + expect( + is( + { + ...validIntent, + typedData: { types: {}, domain: {}, message: {} }, + }, + IntentSchema, + ), + ).toBe(false); + }); + + it('rejects intent without protocol', () => { + const { protocol: _, ...intentWithoutProtocol } = validIntent; + expect(is(intentWithoutProtocol, IntentSchema)).toBe(false); + }); + + it('rejects intent without order', () => { + const { order: _, ...intentWithoutOrder } = validIntent; + expect(is(intentWithoutOrder, IntentSchema)).toBe(false); + }); + + it('accepts intent with empty typedData records', () => { + expect( + is( + { + ...validIntent, + typedData: { + types: {}, + domain: {}, + primaryType: 'Order', + message: {}, + }, + }, + IntentSchema, + ), + ).toBe(true); + }); }); }); diff --git a/packages/bridge-controller/src/utils/validators.ts b/packages/bridge-controller/src/utils/validators.ts index ac6577aa45e..4750862c8e2 100644 --- a/packages/bridge-controller/src/utils/validators.ts +++ b/packages/bridge-controller/src/utils/validators.ts @@ -1,6 +1,7 @@ import { isValidHexAddress } from '@metamask/controller-utils'; import type { Infer } from '@metamask/superstruct'; import { + any, string, boolean, number, @@ -329,9 +330,26 @@ export const IntentSchema = type({ settlementContract: optional(HexAddressSchema), /** - * Optional relayer address responsible for order submission. + * Optional EIP-712 typed data payload for signing. + * Must be JSON-serializable and include required EIP-712 fields. */ - relayer: optional(HexAddressSchema), + typedData: type({ + // Keep values as `any()` here. Using `unknown()` in this record causes + // TS2321/TS2589 (excessive type instantiation depth) in bridge state + // inference during build. + types: record( + string(), + array( + type({ + name: string(), + type: string(), + }), + ), + ), + primaryType: string(), + domain: record(string(), any()), + message: record(string(), any()), + }), }); export const QuoteSchema = type({ diff --git a/packages/bridge-status-controller/CHANGELOG.md b/packages/bridge-status-controller/CHANGELOG.md index c35d44ff441..99c1d5eca1b 100644 --- a/packages/bridge-status-controller/CHANGELOG.md +++ b/packages/bridge-status-controller/CHANGELOG.md @@ -18,6 +18,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Bump `@metamask/bridge-controller` from `^67.1.1` to `^67.2.0` ([#8024](https://github.com/MetaMask/core/pull/8024)) - Bump `@metamask/transaction-controller` from `^62.17.1` to `^62.19.0` ([#8005](https://github.com/MetaMask/core/pull/8005), [#8031](https://github.com/MetaMask/core/pull/8031)) +- **BREAKING:** Make `submitIntent` sign intent typed data internally when signature is not provided, keeping support for externally provided signatures ([#7895](https://github.com/MetaMask/core/pull/7895)). - Move `IntentApiImpl` instantation from `BridgeStatusController` to `IntentManager` ([#8015](https://github.com/MetaMask/core/pull/8015/)) ## [67.0.1] diff --git a/packages/bridge-status-controller/package.json b/packages/bridge-status-controller/package.json index 93a3ccc800b..499d9fde8bd 100644 --- a/packages/bridge-status-controller/package.json +++ b/packages/bridge-status-controller/package.json @@ -52,6 +52,7 @@ "@metamask/bridge-controller": "^67.2.0", "@metamask/controller-utils": "^11.19.0", "@metamask/gas-fee-controller": "^26.0.3", + "@metamask/keyring-controller": "^25.1.0", "@metamask/network-controller": "^30.0.0", "@metamask/polling-controller": "^16.0.3", "@metamask/profile-sync-controller": "^27.1.0", diff --git a/packages/bridge-status-controller/src/bridge-status-controller.intent.test.ts b/packages/bridge-status-controller/src/bridge-status-controller.intent.test.ts index f918bf4dafc..62939ead58d 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.intent.test.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.intent.test.ts @@ -69,6 +69,12 @@ const minimalIntentQuoteResponse = (overrides?: Partial): any => { protocol: 'cowswap', order: { some: 'order' }, settlementContract: '0x9008D19f58AAbd9eD0D60971565AA8510560ab41', + typedData: { + types: {}, + domain: {}, + primaryType: 'Order', + message: {}, + }, }, }, sentAmount: { amount: '1', usd: '1' }, @@ -169,6 +175,8 @@ const createMessengerHarness = ( return undefined; case 'GasFeeController:getState': return { gasFeeEstimates: {} }; + case 'KeyringController:signTypedMessage': + return '0xtest-signature'; default: return undefined; } @@ -401,7 +409,6 @@ describe('BridgeStatusController (intent swaps)', () => { const promise = controller.submitIntent({ quoteResponse, - signature: '0xsig', accountAddress, }); expect(await promise.catch((error: any) => error)).toStrictEqual( @@ -448,7 +455,6 @@ describe('BridgeStatusController (intent swaps)', () => { await expect( controller.submitIntent({ quoteResponse, - signature: '0xsig', accountAddress, }), ).resolves.toBeDefined(); @@ -482,7 +488,6 @@ describe('BridgeStatusController (intent swaps)', () => { const promise = controller.submitIntent({ quoteResponse, - signature: '0xsig', accountAddress, }); expect(await promise.catch((error: any) => error)).toStrictEqual( @@ -522,7 +527,6 @@ describe('BridgeStatusController (intent swaps)', () => { const result = await controller.submitIntent({ quoteResponse, - signature: '0xsig', accountAddress, }); @@ -535,6 +539,58 @@ describe('BridgeStatusController (intent swaps)', () => { consoleSpy.mockRestore(); }); + it('submitIntent: signs typedData', async () => { + const { controller, messenger, accountAddress, submitIntentMock } = setup(); + + const orderUid = 'order-uid-signed-in-core-1'; + submitIntentMock.mockResolvedValue({ + id: orderUid, + status: IntentOrderStatus.SUBMITTED, + txHash: undefined, + metadata: { txHashes: [] }, + }); + + const quoteResponse = minimalIntentQuoteResponse(); + quoteResponse.quote.intent.typedData = { + types: {}, + primaryType: 'Order', + domain: {}, + message: {}, + }; + + const originalCallImpl = ( + messenger.call as jest.Mock + ).getMockImplementation(); + (messenger.call as jest.Mock).mockImplementation( + (method: string, ...args: any[]) => { + if (method === 'KeyringController:signTypedMessage') { + return '0xautosigned'; + } + return originalCallImpl?.(method, ...args); + }, + ); + + await controller.submitIntent({ + quoteResponse, + accountAddress, + }); + + expect((messenger.call as jest.Mock).mock.calls).toStrictEqual( + expect.arrayContaining([ + [ + 'KeyringController:signTypedMessage', + expect.objectContaining({ + from: accountAddress, + data: quoteResponse.quote.intent.typedData, + }), + 'V4', + ], + ]), + ); + + expect(submitIntentMock.mock.calls[0]?.[0]?.signature).toBe('0xautosigned'); + }); + it('intent polling: updates history, merges tx hashes, updates TC tx, and stops polling on COMPLETED', async () => { const { controller, @@ -557,7 +613,6 @@ describe('BridgeStatusController (intent swaps)', () => { await controller.submitIntent({ quoteResponse, - signature: '0xsig', accountAddress, }); @@ -602,7 +657,6 @@ describe('BridgeStatusController (intent swaps)', () => { await controller.submitIntent({ quoteResponse, - signature: '0xsig', accountAddress, }); @@ -649,7 +703,6 @@ describe('BridgeStatusController (intent swaps)', () => { await controller.submitIntent({ quoteResponse, - signature: '0xsig', accountAddress, }); diff --git a/packages/bridge-status-controller/src/bridge-status-controller.ts b/packages/bridge-status-controller/src/bridge-status-controller.ts index d84ff9cb3c5..c2076497d8d 100644 --- a/packages/bridge-status-controller/src/bridge-status-controller.ts +++ b/packages/bridge-status-controller/src/bridge-status-controller.ts @@ -5,7 +5,6 @@ import type { RequiredEventContextFromClient, TxData, QuoteResponse, - Intent, Trade, } from '@metamask/bridge-controller'; import { @@ -27,6 +26,7 @@ import { } from '@metamask/bridge-controller'; import type { TraceCallback } from '@metamask/controller-utils'; import { toHex } from '@metamask/controller-utils'; +import { SignTypedDataVersion } from '@metamask/keyring-controller'; import { StaticIntervalPollingController } from '@metamask/polling-controller'; import { TransactionStatus, @@ -1593,26 +1593,23 @@ export class BridgeStatusController extends StaticIntervalPollingController & QuoteMetadata; - signature: string; + quoteResponse: QuoteResponse & QuoteMetadata; accountAddress: string; location?: MetaMetricsSwapsEventSource; abTests?: Record; }): Promise> => { - const { quoteResponse, signature, accountAddress, location, abTests } = - params; + const { quoteResponse, accountAddress, location, abTests } = params; this.messenger.call( 'BridgeController:stopPollingForQuotes', @@ -1631,9 +1628,7 @@ export class BridgeStatusController extends StaticIntervalPollingController[EventName], ): void => { + const { ab_tests: eventAbTests } = + (eventProperties as + | Record | undefined> + | undefined) ?? {}; const historyAbTests = txMetaId ? this.state.txHistory?.[txMetaId]?.abTests : undefined; - const resolvedAbTests = - eventProperties?.ab_tests ?? historyAbTests ?? undefined; + const resolvedAbTests = eventAbTests ?? historyAbTests ?? undefined; const baseProperties = { action_type: MetricsActionType.SWAPBRIDGE_V1, diff --git a/packages/bridge-status-controller/src/types.ts b/packages/bridge-status-controller/src/types.ts index ed410151560..e4fe5193c3d 100644 --- a/packages/bridge-status-controller/src/types.ts +++ b/packages/bridge-status-controller/src/types.ts @@ -14,6 +14,7 @@ import type { MetaMetricsSwapsEventSource, } from '@metamask/bridge-controller'; import type { GetGasFeeState } from '@metamask/gas-fee-controller'; +import type { KeyringControllerSignTypedMessageAction } from '@metamask/keyring-controller'; import type { Messenger } from '@metamask/messenger'; import type { NetworkControllerFindNetworkClientIdByChainIdAction, @@ -310,7 +311,8 @@ type AllowedActions = | GetGasFeeState | AccountsControllerGetAccountByAddressAction | RemoteFeatureFlagControllerGetStateAction - | AuthenticationControllerGetBearerTokenAction; + | AuthenticationControllerGetBearerTokenAction + | KeyringControllerSignTypedMessageAction; /** * The external events available to the BridgeStatusController. diff --git a/yarn.lock b/yarn.lock index 4ad12d57ba5..1c78c2f861c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2787,6 +2787,7 @@ __metadata: "@metamask/bridge-controller": "npm:^67.2.0" "@metamask/controller-utils": "npm:^11.19.0" "@metamask/gas-fee-controller": "npm:^26.0.3" + "@metamask/keyring-controller": "npm:^25.1.0" "@metamask/network-controller": "npm:^30.0.0" "@metamask/polling-controller": "npm:^16.0.3" "@metamask/profile-sync-controller": "npm:^27.1.0"