From ffc51f5a38d854e356c2282e706f58b0b8cc0f50 Mon Sep 17 00:00:00 2001 From: John Whiles Date: Fri, 3 Jul 2026 14:53:04 +0100 Subject: [PATCH] wip --- packages/chomp-api-service/CHANGELOG.md | 8 ++ .../chomp-api-service-method-action-types.ts | 23 +++- .../src/chomp-api-service.test.ts | 118 +++++++++++++++- .../src/chomp-api-service.ts | 65 ++++++++- packages/chomp-api-service/src/index.ts | 2 + packages/chomp-api-service/src/types.ts | 14 ++ .../CHANGELOG.md | 7 + .../src/MoneyAccountUpgradeController.test.ts | 7 + .../src/MoneyAccountUpgradeController.ts | 2 + .../src/steps/associate-address.test.ts | 127 +++++++++++++++++- .../src/steps/associate-address.ts | 83 +++++++++--- 11 files changed, 428 insertions(+), 28 deletions(-) diff --git a/packages/chomp-api-service/CHANGELOG.md b/packages/chomp-api-service/CHANGELOG.md index 4889c7b640..f5235958dc 100644 --- a/packages/chomp-api-service/CHANGELOG.md +++ b/packages/chomp-api-service/CHANGELOG.md @@ -7,8 +7,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `getAssociatedAddresses` method, exposed as the `ChompApiService:getAssociatedAddresses` messenger action, which fetches the active address associations of the authenticated profile via `GET /v1/auth/address` + - Also adds the `ProfileAddressEntry` type describing each returned entry and the `ChompApiServiceGetAssociatedAddressesAction` type + - Returned addresses are parsed into canonical lowercase form, entries are guaranteed to have `status: 'active'`, and results are never served from cache (the response is scoped to the authenticated profile, which the cache key cannot capture) + ### Changed +- **BREAKING:** `associateAddress` now throws an `HttpError` on a 409 response instead of returning the parsed body + - A 409 from `POST /v1/auth/address` indicates the address is associated with a _different_ profile; the previous handling attempted to parse the error body as an association result and failed with a confusing validation error. An address already associated with the authenticated profile is reported via a 201 response with `status: 'active'`, which is unchanged. - Bump `@metamask/utils` from `^11.9.0` to `^11.11.0` ([#9074](https://github.com/MetaMask/core/pull/9074)) - Bump `@metamask/controller-utils` from `^12.0.0` to `^12.3.0` ([#8774](https://github.com/MetaMask/core/pull/8774), [#9058](https://github.com/MetaMask/core/pull/9058), [#9083](https://github.com/MetaMask/core/pull/9083), [#9218](https://github.com/MetaMask/core/pull/9218)) - Bump `@metamask/base-data-service` from `^0.1.2` to `^0.1.3` ([#8799](https://github.com/MetaMask/core/pull/8799)) diff --git a/packages/chomp-api-service/src/chomp-api-service-method-action-types.ts b/packages/chomp-api-service/src/chomp-api-service-method-action-types.ts index 33e38df66b..a9517e43a8 100644 --- a/packages/chomp-api-service/src/chomp-api-service-method-action-types.ts +++ b/packages/chomp-api-service/src/chomp-api-service-method-action-types.ts @@ -12,13 +12,33 @@ import type { ChompApiService } from './chomp-api-service'; * * @param params - The association params containing signature, timestamp, * and address. - * @returns The profile association result. Returns on both 201 and 409. + * @returns The profile association result: `status: 'created'` for a new + * association, `status: 'active'` when the address was already associated + * with the authenticated profile. Throws on 409, which indicates the + * address is associated with a different profile. */ export type ChompApiServiceAssociateAddressAction = { type: `ChompApiService:associateAddress`; handler: ChompApiService['associateAddress']; }; +/** + * Fetches the addresses associated with the authenticated profile. + * + * GET /v1/auth/address + * + * The result is never served from cache (`staleTime: 0`): it is scoped to + * the authenticated profile (which the query key cannot capture) and + * consumers use it to decide whether an association already exists. + * + * @returns The active address associations; empty array if none exist. + * Addresses are lowercased. + */ +export type ChompApiServiceGetAssociatedAddressesAction = { + type: `ChompApiService:getAssociatedAddresses`; + handler: ChompApiService['getAssociatedAddresses']; +}; + /** * Creates an account upgrade request. * @@ -120,6 +140,7 @@ export type ChompApiServiceGetServiceDetailsAction = { */ export type ChompApiServiceMethodActions = | ChompApiServiceAssociateAddressAction + | ChompApiServiceGetAssociatedAddressesAction | ChompApiServiceCreateUpgradeAction | ChompApiServiceGetUpgradesAction | ChompApiServiceVerifyDelegationAction diff --git a/packages/chomp-api-service/src/chomp-api-service.test.ts b/packages/chomp-api-service/src/chomp-api-service.test.ts index 0cab2c7e02..e304958fad 100644 --- a/packages/chomp-api-service/src/chomp-api-service.test.ts +++ b/packages/chomp-api-service/src/chomp-api-service.test.ts @@ -45,8 +45,8 @@ describe('ChompApiService', () => { }); }); - it('returns the response on 409 without throwing', async () => { - nock(BASE_URL).post('/v1/auth/address').reply(409, { + it('returns the response when the address is already associated with the profile', async () => { + nock(BASE_URL).post('/v1/auth/address').reply(201, { address: '0xabc', status: 'active', }); @@ -60,7 +60,19 @@ describe('ChompApiService', () => { }); }); - it('throws on non-201/409 status', async () => { + it('throws when the address is associated with another profile (409)', async () => { + nock(BASE_URL).post('/v1/auth/address').reply(409, { + statusCode: 409, + message: 'Address is already associated with another profile', + }); + const { service } = createService(); + + await expect(service.associateAddress(associateParams)).rejects.toThrow( + "POST /v1/auth/address failed with status '409'", + ); + }); + + it('throws on non-OK status', async () => { nock(BASE_URL) .post('/v1/auth/address') .times(DEFAULT_MAX_RETRIES + 1) @@ -84,6 +96,106 @@ describe('ChompApiService', () => { }); }); + describe('getAssociatedAddresses', () => { + const addressEntry = { + profileId: 'p1', + address: '0xabc', + status: 'active', + }; + + it('sends a GET with auth headers and returns the address entries', async () => { + nock(BASE_URL) + .get('/v1/auth/address') + .matchHeader('Authorization', `Bearer ${MOCK_TOKEN}`) + .reply(200, [addressEntry]); + const { rootMessenger } = createService(); + + const result = await rootMessenger.call( + 'ChompApiService:getAssociatedAddresses', + ); + + expect(result).toStrictEqual([addressEntry]); + }); + + it('returns an empty array when no addresses are associated', async () => { + nock(BASE_URL).get('/v1/auth/address').reply(200, []); + const { service } = createService(); + + const result = await service.getAssociatedAddresses(); + + expect(result).toStrictEqual([]); + }); + + it('lowercases returned addresses', async () => { + nock(BASE_URL) + .get('/v1/auth/address') + .reply(200, [ + { + profileId: 'p1', + address: '0xABCdef1234567890ABCdef1234567890ABCdef12', + status: 'active', + }, + ]); + const { service } = createService(); + + const result = await service.getAssociatedAddresses(); + + expect(result).toStrictEqual([ + { + profileId: 'p1', + address: '0xabcdef1234567890abcdef1234567890abcdef12', + status: 'active', + }, + ]); + }); + + it('rejects entries with a non-active status', async () => { + nock(BASE_URL) + .get('/v1/auth/address') + .reply(200, [{ profileId: 'p1', address: '0xabc', status: 'deleted' }]); + const { service } = createService(); + + await expect(service.getAssociatedAddresses()).rejects.toThrow( + 'At path: 0.status', + ); + }); + + it('does not serve results from cache', async () => { + nock(BASE_URL).get('/v1/auth/address').reply(200, []); + nock(BASE_URL).get('/v1/auth/address').reply(200, [addressEntry]); + const { service } = createService(); + + const first = await service.getAssociatedAddresses(); + const second = await service.getAssociatedAddresses(); + + expect(first).toStrictEqual([]); + expect(second).toStrictEqual([addressEntry]); + }); + + it('throws on non-OK status', async () => { + nock(BASE_URL) + .get('/v1/auth/address') + .times(DEFAULT_MAX_RETRIES + 1) + .reply(500); + const { service } = createService(); + + await expect(service.getAssociatedAddresses()).rejects.toThrow( + "GET /v1/auth/address failed with status '500'", + ); + }); + + it('throws on malformed response', async () => { + nock(BASE_URL) + .get('/v1/auth/address') + .reply(200, JSON.stringify([{ bad: 'data' }])); + const { service } = createService(); + + await expect(service.getAssociatedAddresses()).rejects.toThrow( + 'At path: 0.profileId', + ); + }); + }); + describe('createUpgrade', () => { const upgradeParams = { r: '0x1' as const, diff --git a/packages/chomp-api-service/src/chomp-api-service.ts b/packages/chomp-api-service/src/chomp-api-service.ts index b04c683041..766984b63f 100644 --- a/packages/chomp-api-service/src/chomp-api-service.ts +++ b/packages/chomp-api-service/src/chomp-api-service.ts @@ -10,6 +10,7 @@ import type { Messenger } from '@metamask/messenger'; import { array, boolean, + coerce, create, enums, literal, @@ -27,6 +28,7 @@ import type { ChompApiServiceMethodActions } from './chomp-api-service-method-ac import type { AssociateAddressParams, AssociateAddressResponse, + ProfileAddressEntry, CreateUpgradeParams, CreateUpgradeResponse, UpgradeEntry, @@ -56,6 +58,7 @@ export const serviceName = 'ChompApiService'; */ const MESSENGER_EXPOSED_METHODS = [ 'associateAddress', + 'getAssociatedAddresses', 'createUpgrade', 'getUpgrades', 'verifyDelegation', @@ -129,6 +132,24 @@ const AssociateAddressResponseStruct = type({ status: enums(['active', 'created']), }); +/** + * Parses addresses into canonical lowercase form. CHOMP stores and returns + * addresses lowercased, but `StrictHexStruct` alone accepts any casing, so + * this makes the canonical form a guarantee of the parsed data rather than a + * convention consumers must each remember. + */ +const LowercaseHexAddressStruct = coerce(StrictHexStruct, string(), (value) => + value.toLowerCase(), +); + +const ProfileAddressEntryArrayStruct = array( + type({ + profileId: string(), + address: LowercaseHexAddressStruct, + status: enums(['active']), + }), +); + const AccountUpgradeStatusStruct = enums(['pending', 'upgraded']); const AuthorizationDataStruct = type({ @@ -323,7 +344,10 @@ export class ChompApiService extends BaseDataService< * * @param params - The association params containing signature, timestamp, * and address. - * @returns The profile association result. Returns on both 201 and 409. + * @returns The profile association result: `status: 'created'` for a new + * association, `status: 'active'` when the address was already associated + * with the authenticated profile. Throws on 409, which indicates the + * address is associated with a different profile. */ async associateAddress( params: AssociateAddressParams, @@ -342,7 +366,7 @@ export class ChompApiService extends BaseDataService< }, ); - if (!response.ok && response.status !== 409) { + if (!response.ok) { throw new HttpError( response.status, `POST /v1/auth/address failed with status '${response.status}'`, @@ -356,6 +380,43 @@ export class ChompApiService extends BaseDataService< return create(jsonResponse, AssociateAddressResponseStruct); } + /** + * Fetches the addresses associated with the authenticated profile. + * + * GET /v1/auth/address + * + * The result is never served from cache (`staleTime: 0`): it is scoped to + * the authenticated profile (which the query key cannot capture) and + * consumers use it to decide whether an association already exists. + * + * @returns The active address associations; empty array if none exist. + * Addresses are lowercased. + */ + async getAssociatedAddresses(): Promise { + const jsonResponse = await this.fetchQuery({ + queryKey: [`${this.name}:getAssociatedAddresses`], + staleTime: 0, + queryFn: async () => { + const headers = await this.#authHeaders(); + const response = await fetch( + new URL('/v1/auth/address', this.#baseUrl), + { headers }, + ); + + if (!response.ok) { + throw new HttpError( + response.status, + `GET /v1/auth/address failed with status '${response.status}'`, + ); + } + + return response.json(); + }, + }); + + return create(jsonResponse, ProfileAddressEntryArrayStruct); + } + /** * Creates an account upgrade request. * diff --git a/packages/chomp-api-service/src/index.ts b/packages/chomp-api-service/src/index.ts index edf0825cd9..72d6cc07f3 100644 --- a/packages/chomp-api-service/src/index.ts +++ b/packages/chomp-api-service/src/index.ts @@ -9,6 +9,7 @@ export type { } from './chomp-api-service'; export type { ChompApiServiceAssociateAddressAction, + ChompApiServiceGetAssociatedAddressesAction, ChompApiServiceCreateUpgradeAction, ChompApiServiceGetUpgradesAction, ChompApiServiceVerifyDelegationAction, @@ -31,6 +32,7 @@ export type { IntentEntry, IntentMetadataParams, IntentMetadataResponse, + ProfileAddressEntry, SendIntentParams, SendIntentResponse, ServiceDetailsChain, diff --git a/packages/chomp-api-service/src/types.ts b/packages/chomp-api-service/src/types.ts index 6f03ea2d15..e4dba14259 100644 --- a/packages/chomp-api-service/src/types.ts +++ b/packages/chomp-api-service/src/types.ts @@ -69,6 +69,8 @@ export type CreateWithdrawalParams = { * `profileId` is only included when the address was newly associated * (`status: 'created'`). When the address was already associated with the * authenticated profile (`status: 'active'`), only `address` is returned. + * Both cases respond with 201; an address associated with a different + * profile responds with 409, which is surfaced as an error. */ export type AssociateAddressResponse = { profileId?: string; @@ -76,6 +78,18 @@ export type AssociateAddressResponse = { status: 'active' | 'created'; }; +/** + * One entry returned by GET /v1/auth/address. The endpoint returns an array + * of these — the active address associations of the authenticated profile + * (the API filters out soft-deleted associations, so `status` is always + * `'active'`). Addresses are lowercased. + */ +export type ProfileAddressEntry = { + profileId: string; + address: Hex; + status: 'active'; +}; + export type AccountUpgradeStatus = 'pending' | 'upgraded'; export type AuthorizationData = { diff --git a/packages/money-account-upgrade-controller/CHANGELOG.md b/packages/money-account-upgrade-controller/CHANGELOG.md index 91f7e3059a..f914590add 100644 --- a/packages/money-account-upgrade-controller/CHANGELOG.md +++ b/packages/money-account-upgrade-controller/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Changed + +- **BREAKING:** The `associate-address` upgrade step now checks the profile's existing address associations via `ChompApiService:getAssociatedAddresses` before signing, and reports `already-done` without signing or submitting anything when the address is already associated + - `MoneyAccountUpgradeControllerMessenger` consumers must grant the `ChompApiService:getAssociatedAddresses` action alongside the previously required actions, and must provide a `@metamask/chomp-api-service` version that registers it (`>=3.2.0`). + - The lookup is an optimization: if it fails, the step falls through to the previous sign-and-submit behavior. + - A 409 conflict from the association request is disambiguated by re-fetching the associations, so a same-profile create race reports `already-done` instead of failing the upgrade; a genuine conflict (address associated with a different profile) still fails the step. + ## [2.2.1] ### Changed diff --git a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts index b617238242..7a08f3a60f 100644 --- a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts +++ b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.test.ts @@ -70,6 +70,7 @@ type Mocks = { getServiceDetails: jest.Mock; signPersonalMessage: jest.Mock; associateAddress: jest.Mock; + getAssociatedAddresses: jest.Mock; createUpgrade: jest.Mock; signEip7702Authorization: jest.Mock; findNetworkClientIdByChainId: jest.Mock; @@ -115,6 +116,7 @@ function setup(): { address: MOCK_ACCOUNT_ADDRESS, status: 'created', }), + getAssociatedAddresses: jest.fn().mockResolvedValue([]), createUpgrade: jest.fn().mockResolvedValue({ signerAddress: MOCK_ACCOUNT_ADDRESS, address: MAINNET_CONTRACTS.EIP7702StatelessDeleGatorImpl, @@ -155,6 +157,10 @@ function setup(): { 'ChompApiService:associateAddress', mocks.associateAddress, ); + rootMessenger.registerActionHandler( + 'ChompApiService:getAssociatedAddresses', + mocks.getAssociatedAddresses, + ); rootMessenger.registerActionHandler( 'ChompApiService:createUpgrade', mocks.createUpgrade, @@ -206,6 +212,7 @@ function setup(): { 'ChompApiService:getServiceDetails', 'KeyringController:signPersonalMessage', 'ChompApiService:associateAddress', + 'ChompApiService:getAssociatedAddresses', 'ChompApiService:createUpgrade', 'KeyringController:signEip7702Authorization', 'NetworkController:findNetworkClientIdByChainId', diff --git a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts index 7d0556e5ec..b38e095b47 100644 --- a/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts +++ b/packages/money-account-upgrade-controller/src/MoneyAccountUpgradeController.ts @@ -12,6 +12,7 @@ import type { ChompApiServiceAssociateAddressAction, ChompApiServiceCreateIntentsAction, ChompApiServiceCreateUpgradeAction, + ChompApiServiceGetAssociatedAddressesAction, ChompApiServiceGetIntentsByAddressAction, ChompApiServiceGetServiceDetailsAction, ChompApiServiceVerifyDelegationAction, @@ -70,6 +71,7 @@ type AllowedActions = | ChompApiServiceAssociateAddressAction | ChompApiServiceCreateIntentsAction | ChompApiServiceCreateUpgradeAction + | ChompApiServiceGetAssociatedAddressesAction | ChompApiServiceGetIntentsByAddressAction | ChompApiServiceGetServiceDetailsAction | ChompApiServiceVerifyDelegationAction diff --git a/packages/money-account-upgrade-controller/src/steps/associate-address.test.ts b/packages/money-account-upgrade-controller/src/steps/associate-address.test.ts index da1b8491e3..c1fafd7bd8 100644 --- a/packages/money-account-upgrade-controller/src/steps/associate-address.test.ts +++ b/packages/money-account-upgrade-controller/src/steps/associate-address.test.ts @@ -9,7 +9,8 @@ import type { Hex } from '@metamask/utils'; import type { MoneyAccountUpgradeControllerMessenger } from '../MoneyAccountUpgradeController'; import { associateAddressStep } from './associate-address'; -const MOCK_ADDRESS = '0xabcdef1234567890abcdef1234567890abcdef12' as Hex; +const MOCK_ADDRESS = '0xAbCdEf1234567890AbCdEf1234567890AbCdEf12' as Hex; +const MOCK_ADDRESS_LOWERCASE = MOCK_ADDRESS.toLowerCase() as Hex; const MOCK_CHAIN_ID = '0x1' as Hex; const MOCK_DELEGATE = '0x1111111111111111111111111111111111111111' as Hex; const MOCK_DELEGATOR_IMPL = '0x2222222222222222222222222222222222222222' as Hex; @@ -23,12 +24,26 @@ const MOCK_VALUE_LTE_ENFORCER = const MOCK_SIGNATURE = '0xdeadbeefcafebabe'; const MOCK_NOW = new Date('2026-04-17T12:00:00.000Z').getTime(); +/** + * Builds the error `ChompApiService.associateAddress` throws on a 409. + * + * @returns An `Error` carrying `httpStatus: 409`, matching `HttpError` from + * `@metamask/controller-utils`. + */ +function conflictError(): Error { + return Object.assign( + new Error("POST /v1/auth/address failed with status '409'"), + { httpStatus: 409 }, + ); +} + type AllActions = MessengerActions; type AllEvents = MessengerEvents; type Mocks = { signPersonalMessage: jest.Mock; associateAddress: jest.Mock; + getAssociatedAddresses: jest.Mock; }; function setup(): { @@ -39,9 +54,10 @@ function setup(): { signPersonalMessage: jest.fn().mockResolvedValue(MOCK_SIGNATURE), associateAddress: jest.fn().mockResolvedValue({ profileId: 'profile-1', - address: MOCK_ADDRESS, + address: MOCK_ADDRESS_LOWERCASE, status: 'created', }), + getAssociatedAddresses: jest.fn().mockResolvedValue([]), }; const rootMessenger = new Messenger({ @@ -55,6 +71,10 @@ function setup(): { 'ChompApiService:associateAddress', mocks.associateAddress, ); + rootMessenger.registerActionHandler( + 'ChompApiService:getAssociatedAddresses', + mocks.getAssociatedAddresses, + ); const messenger: MoneyAccountUpgradeControllerMessenger = new Messenger({ namespace: 'MoneyAccountUpgradeController', @@ -64,6 +84,7 @@ function setup(): { actions: [ 'KeyringController:signPersonalMessage', 'ChompApiService:associateAddress', + 'ChompApiService:getAssociatedAddresses', ], events: [], messenger, @@ -104,6 +125,52 @@ describe('associateAddressStep', () => { expect(associateAddressStep.name).toBe('associate-address'); }); + it('checks the associated addresses before signing anything', async () => { + const { messenger, mocks } = setup(); + + await run(messenger); + + expect(mocks.getAssociatedAddresses).toHaveBeenCalledTimes(1); + expect( + mocks.getAssociatedAddresses.mock.invocationCallOrder[0], + ).toBeLessThan(mocks.signPersonalMessage.mock.invocationCallOrder[0]); + }); + + it('returns "already-done" without signing or submitting when the address is already associated', async () => { + const { messenger, mocks } = setup(); + // CHOMP lowercases stored addresses; the step receives a checksummed one, + // so this also covers the case-insensitive match. + mocks.getAssociatedAddresses.mockResolvedValue([ + { + profileId: 'profile-1', + address: MOCK_ADDRESS_LOWERCASE, + status: 'active', + }, + ]); + + const result = await run(messenger); + + expect(result).toBe('already-done'); + expect(mocks.signPersonalMessage).not.toHaveBeenCalled(); + expect(mocks.associateAddress).not.toHaveBeenCalled(); + }); + + it('proceeds with association when only other addresses are associated', async () => { + const { messenger, mocks } = setup(); + mocks.getAssociatedAddresses.mockResolvedValue([ + { + profileId: 'profile-1', + address: '0x9999999999999999999999999999999999999999', + status: 'active', + }, + ]); + + const result = await run(messenger); + + expect(result).toBe('completed'); + expect(mocks.associateAddress).toHaveBeenCalled(); + }); + it('signs the CHOMP Authentication message with the given address', async () => { const { messenger, mocks } = setup(); @@ -135,10 +202,10 @@ describe('associateAddressStep', () => { expect(result).toBe('completed'); }); - it('returns "already-done" when CHOMP responds with active (409)', async () => { + it('returns "already-done" when the association was created concurrently', async () => { const { messenger, mocks } = setup(); mocks.associateAddress.mockResolvedValue({ - address: MOCK_ADDRESS, + address: MOCK_ADDRESS_LOWERCASE, status: 'active', }); @@ -147,6 +214,58 @@ describe('associateAddressStep', () => { expect(result).toBe('already-done'); }); + it('falls through to sign-and-submit when the lookup fails', async () => { + const { messenger, mocks } = setup(); + mocks.getAssociatedAddresses.mockRejectedValue(new Error('lookup failed')); + + const result = await run(messenger); + + expect(result).toBe('completed'); + expect(mocks.signPersonalMessage).toHaveBeenCalled(); + expect(mocks.associateAddress).toHaveBeenCalled(); + }); + + it('returns "already-done" when a conflict turns out to be a same-profile race', async () => { + const { messenger, mocks } = setup(); + mocks.associateAddress.mockRejectedValue(conflictError()); + // First lookup (pre-check) misses; second (disambiguation) finds it. + mocks.getAssociatedAddresses + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([ + { + profileId: 'profile-1', + address: MOCK_ADDRESS_LOWERCASE, + status: 'active', + }, + ]); + + const result = await run(messenger); + + expect(result).toBe('already-done'); + }); + + it('rethrows a conflict when the address belongs to another profile', async () => { + const { messenger, mocks } = setup(); + mocks.associateAddress.mockRejectedValue(conflictError()); + mocks.getAssociatedAddresses.mockResolvedValue([]); + + await expect(run(messenger)).rejects.toThrow( + "POST /v1/auth/address failed with status '409'", + ); + }); + + it('rethrows a conflict when the disambiguating lookup also fails', async () => { + const { messenger, mocks } = setup(); + mocks.associateAddress.mockRejectedValue(conflictError()); + mocks.getAssociatedAddresses + .mockResolvedValueOnce([]) + .mockRejectedValueOnce(new Error('lookup failed')); + + await expect(run(messenger)).rejects.toThrow( + "POST /v1/auth/address failed with status '409'", + ); + }); + it('propagates errors from signing and does not submit to the API', async () => { const { messenger, mocks } = setup(); mocks.signPersonalMessage.mockRejectedValue(new Error('signing failed')); diff --git a/packages/money-account-upgrade-controller/src/steps/associate-address.ts b/packages/money-account-upgrade-controller/src/steps/associate-address.ts index ebf4658e18..4bb9c3fec0 100644 --- a/packages/money-account-upgrade-controller/src/steps/associate-address.ts +++ b/packages/money-account-upgrade-controller/src/steps/associate-address.ts @@ -1,23 +1,63 @@ import type { Hex } from '@metamask/utils'; +import { hasProperty } from '@metamask/utils'; +import { equalsIgnoreCase } from './delegation-matchers'; import type { Step } from './step'; +/** + * Determines whether an error is a CHOMP conflict (HTTP 409) response. + * + * @param error - The error to inspect. + * @returns `true` when the error carries a 409 HTTP status. + */ +function isConflictError(error: unknown): boolean { + return ( + error instanceof Error && + hasProperty(error, 'httpStatus') && + error.httpStatus === 409 + ); +} + /** * Associates the Money Account address with the user's CHOMP profile. * - * Signs `CHOMP Authentication {timestamp}` (EIP-191) with the account's key - * and submits the signature to CHOMP, which verifies the timestamp is fresh, - * recovers the signer, and records the profile–address mapping. + * First checks the profile's existing associations via + * `GET /v1/auth/address`; if the address is already associated the step + * reports `'already-done'` without signing anything. The lookup is an + * optimization: if it fails, the step falls through to the POST path below, + * which is authoritative and matches the behavior before the lookup existed. + * + * Otherwise, signs `CHOMP Authentication {timestamp}` (EIP-191) with the + * account's key and submits the signature to CHOMP, which verifies the + * timestamp is fresh, recovers the signer, and records the profile–address + * mapping. CHOMP responds with `status: 'created'` for a new association and + * `status: 'active'` when the address was already associated with this + * profile, so the latter also reports `'already-done'`. * - * CHOMP responds with 201 and `status: 'created'` when a new association is - * made, and 409 with `status: 'active'` when the address is already - * associated with the authenticated profile. The service surfaces both - * responses, so this step reports `'already-done'` for the 409 case and - * `'completed'` otherwise. + * A 409 usually means the address belongs to a different profile, but CHOMP + * also returns it when two same-profile requests race on the initial create + * (the loser's conditional write fails). The step disambiguates by re-fetching + * the associations: if the address is now present the race was benign and the + * step reports `'already-done'`; otherwise the conflict propagates. */ export const associateAddressStep: Step = { name: 'associate-address', async run({ messenger, address }) { + const isAssociated = async (): Promise => { + const entries = await messenger.call( + 'ChompApiService:getAssociatedAddresses', + ); + return entries.some((entry) => equalsIgnoreCase(entry.address, address)); + }; + + try { + if (await isAssociated()) { + return 'already-done'; + } + } catch { + // The lookup is an optimization — the POST path below is authoritative. + } + const timestamp = Date.now(); const message = `CHOMP Authentication ${timestamp}`; @@ -26,16 +66,23 @@ export const associateAddressStep: Step = { { data: message, from: address }, )) as Hex; - const response = await messenger.call('ChompApiService:associateAddress', { - signature, - timestamp, - address, - }); - - if (response.status === 'active') { - return 'already-done'; + try { + const response = await messenger.call( + 'ChompApiService:associateAddress', + { signature, timestamp, address }, + ); + return response.status === 'active' ? 'already-done' : 'completed'; + } catch (error) { + if (isConflictError(error)) { + try { + if (await isAssociated()) { + return 'already-done'; + } + } catch { + // Could not disambiguate — surface the original conflict. + } + } + throw error; } - - return 'completed'; }, };