feat: [MUSD-445] add build delegations step to money upgrade

README.md

@@ -380,8 +380,10 @@ linkStyle default opacity:0.5
   money_account_controller --> base_controller;
   money_account_controller --> keyring_controller;
   money_account_controller --> messenger;
+  money_account_upgrade_controller --> authenticated_user_storage;
   money_account_upgrade_controller --> base_controller;
   money_account_upgrade_controller --> chomp_api_service;
+  money_account_upgrade_controller --> delegation_controller;
   money_account_upgrade_controller --> keyring_controller;
   money_account_upgrade_controller --> messenger;
   money_account_upgrade_controller --> network_controller;

packages/chomp-api-service/CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Changed
+
+- `ChompApiService` no longer retries HTTP requests that fail with a 4xx response (other than 429), since those responses indicate the request itself is at fault and will not be resolved by re-issuing it. 5xx, 429, and non-HTTP errors (network/timeout) continue to be retried. Consumers can still override this by passing a `retryFilterPolicy` via `policyOptions`. ([#8621](https://github.com/MetaMask/core/pull/8621))
+
 ## [3.0.1]
 
 ### Changed

packages/chomp-api-service/src/chomp-api-service.test.ts

@@ -1,4 +1,4 @@
-import { DEFAULT_MAX_RETRIES } from '@metamask/controller-utils';
+import { DEFAULT_MAX_RETRIES, handleAll } from '@metamask/controller-utils';
 import { Messenger, MOCK_ANY_NAMESPACE } from '@metamask/messenger';
 import type {
   MockAnyNamespace,
@@ -255,10 +255,7 @@ describe('ChompApiService', () => {
     });
 
     it('throws on non-OK status', async () => {
-      nock(BASE_URL)
-        .post('/v1/intent/verify-delegation')
-        .times(DEFAULT_MAX_RETRIES + 1)
-        .reply(400);
+      nock(BASE_URL).post('/v1/intent/verify-delegation').reply(400);
       const { service } = createService();
 
       await expect(service.verifyDelegation(delegationParams)).rejects.toThrow(
@@ -322,10 +319,7 @@ describe('ChompApiService', () => {
     });
 
     it('throws on non-OK status', async () => {
-      nock(BASE_URL)
-        .post('/v1/intent')
-        .times(DEFAULT_MAX_RETRIES + 1)
-        .reply(409);
+      nock(BASE_URL).post('/v1/intent').reply(409);
       const { service } = createService();
 
       await expect(service.createIntents(intentParams)).rejects.toThrow(
@@ -432,10 +426,7 @@ describe('ChompApiService', () => {
     });
 
     it('throws on non-OK status', async () => {
-      nock(BASE_URL)
-        .post('/v1/withdrawal')
-        .times(DEFAULT_MAX_RETRIES + 1)
-        .reply(400);
+      nock(BASE_URL).post('/v1/withdrawal').reply(400);
       const { service } = createService();
 
       await expect(service.createWithdrawal(withdrawalParams)).rejects.toThrow(
@@ -509,11 +500,7 @@ describe('ChompApiService', () => {
     });
 
     it('throws on non-OK status', async () => {
-      nock(BASE_URL)
-        .get('/v1/chomp')
-        .query({ chainId: '0xa4b1' })
-        .times(DEFAULT_MAX_RETRIES + 1)
-        .reply(400);
+      nock(BASE_URL).get('/v1/chomp').query({ chainId: '0xa4b1' }).reply(400);
       const { service } = createService();
 
       await expect(service.getServiceDetails(['0xa4b1'])).rejects.toThrow(
@@ -533,6 +520,104 @@ describe('ChompApiService', () => {
       );
     });
   });
+
+  describe('retry policy', () => {
+    const upgradeParams = {
+      r: '0x1' as const,
+      s: '0x2' as const,
+      v: 27,
+      yParity: 0,
+      address: '0xabc' as const,
+      chainId: '1',
+      nonce: '0',
+    };
+
+    it('retries 5xx responses up to the default retry limit', async () => {
+      let attempts = 0;
+      nock(BASE_URL)
+        .post('/v1/account-upgrade')
+        .times(DEFAULT_MAX_RETRIES + 1)
+        .reply(() => {
+          attempts += 1;
+          return [500];
+        });
+      const { service } = createService();
+
+      await expect(service.createUpgrade(upgradeParams)).rejects.toThrow(
+        "POST /v1/account-upgrade failed with status '500'",
+      );
+      expect(attempts).toBe(DEFAULT_MAX_RETRIES + 1);
+    });
+
+    it.each([400, 401, 403, 404, 409, 422])(
+      'does not retry %i responses',
+      async (status) => {
+        let attempts = 0;
+        nock(BASE_URL)
+          .post('/v1/account-upgrade')
+          .times(DEFAULT_MAX_RETRIES + 1)
+          .reply(() => {
+            attempts += 1;
+            return [status];
+          });
+        const { service } = createService();
+
+        await expect(service.createUpgrade(upgradeParams)).rejects.toThrow(
+          `POST /v1/account-upgrade failed with status '${status}'`,
+        );
+        expect(attempts).toBe(1);
+      },
+    );
+
+    it('retries 429 responses alongside 5xx (rate-limit is transient)', async () => {
+      let attempts = 0;
+      nock(BASE_URL)
+        .post('/v1/account-upgrade')
+        .times(DEFAULT_MAX_RETRIES + 1)
+        .reply(() => {
+          attempts += 1;
+          return [429];
+        });
+      const { service } = createService();
+
+      await expect(service.createUpgrade(upgradeParams)).rejects.toThrow(
+        "POST /v1/account-upgrade failed with status '429'",
+      );
+      expect(attempts).toBe(DEFAULT_MAX_RETRIES + 1);
+    });
+
+    it('retries non-HTTP errors (e.g. network failures)', async () => {
+      const scope = nock(BASE_URL)
+        .post('/v1/account-upgrade')
+        .times(DEFAULT_MAX_RETRIES + 1)
+        .replyWithError('network down');
+      const { service } = createService();
+
+      await expect(service.createUpgrade(upgradeParams)).rejects.toThrow(
+        'network down',
+      );
+      expect(scope.isDone()).toBe(true);
+    });
+
+    it('lets consumer-supplied policyOptions override the default retryFilterPolicy', async () => {
+      let attempts = 0;
+      nock(BASE_URL)
+        .post('/v1/account-upgrade')
+        .times(DEFAULT_MAX_RETRIES + 1)
+        .reply(() => {
+          attempts += 1;
+          return [409];
+        });
+      const { service } = createService({
+        options: { policyOptions: { retryFilterPolicy: handleAll } },
+      });
+
+      await expect(service.createUpgrade(upgradeParams)).rejects.toThrow(
+        "POST /v1/account-upgrade failed with status '409'",
+      );
+      expect(attempts).toBe(DEFAULT_MAX_RETRIES + 1);
+    });
+  });
 });
 
 /**

packages/chomp-api-service/src/chomp-api-service.ts

@@ -5,7 +5,7 @@ import type {
   DataServiceInvalidateQueriesAction,
 } from '@metamask/base-data-service';
 import type { CreateServicePolicyOptions } from '@metamask/controller-utils';
-import { HttpError } from '@metamask/controller-utils';
+import { handleWhen, HttpError } from '@metamask/controller-utils';
 import type { Messenger } from '@metamask/messenger';
 import {
   array,
@@ -223,6 +223,34 @@ const ServiceDetailsResponseStruct = type({
   ),
 });
 
+// === RETRY POLICY ===
+
+/**
+ * Determines whether an error from a CHOMP API call is worth retrying.
+ *
+ * 4xx responses (e.g. 409 "already exists", 400 validation, 401/403 auth) are
+ * caused by the request itself and will not be resolved by re-issuing the same
+ * request, so they bypass the retry loop. 429 is treated as transient and
+ * retried alongside 5xx server errors. Non-HTTP errors (network/timeout) fall
+ * through to the default "retry" behaviour.
+ *
+ * @param error - The error thrown by the query function.
+ * @returns `true` when the error is worth retrying.
+ */
+function isRetryableError(error: unknown): boolean {
+  if (error instanceof HttpError) {
+    if (error.httpStatus === 429) {
+      return true;
+    }
+    return error.httpStatus < 400 || error.httpStatus >= 500;
+  }
+  return true;
+}
+
+const DEFAULT_POLICY_OPTIONS: CreateServicePolicyOptions = {
+  retryFilterPolicy: handleWhen(isRetryableError),
+};
+
 // === SERVICE DEFINITION ===
 
 /**
@@ -262,7 +290,7 @@ export class ChompApiService extends BaseDataService<
       name: serviceName,
       messenger,
       queryClientConfig,
-      policyOptions,
+      policyOptions: { ...DEFAULT_POLICY_OPTIONS, ...policyOptions },
     });
 
     this.#baseUrl = baseUrl;

packages/money-account-upgrade-controller/CHANGELOG.md

@@ -7,10 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Added
+
+- Add remaining steps in money account upgrade process ([#8621](https://github.com/MetaMask/core/pull/8621))
+
 ### Changed
 
+- **BREAKING:** The controller messenger now requires access to six additional allowed actions: `AuthenticatedUserStorageService:listDelegations`, `AuthenticatedUserStorageService:createDelegation`, `ChompApiService:verifyDelegation`, `ChompApiService:getIntentsByAddress`, `ChompApiService:createIntents`, and `DelegationController:signDelegation`. Delegation signing is now delegated to `@metamask/delegation-controller` rather than calling `KeyringController:signTypedMessage` directly; consumers must instantiate `DelegationController` and update their messenger configuration accordingly. ([#8621](https://github.com/MetaMask/core/pull/8621))
+- **BREAKING:** `init()` now takes a `{ chainId, boringVaultAddress }` object instead of an `InitConfig`. The EIP-7702 delegator implementation and caveat enforcer addresses are resolved from `@metamask/delegation-deployments` for the target chain; `init()` throws if the chain is not supported by Delegation Framework 1.3.0. The `InitConfig` type is no longer exported. ([#8621](https://github.com/MetaMask/core/pull/8621))
+- Add `@metamask/authenticated-user-storage`, `@metamask/delegation-controller`, `@metamask/delegation-core`, and `@metamask/delegation-deployments` as dependencies. ([#8621](https://github.com/MetaMask/core/pull/8621))
 - Bump `@metamask/network-controller` from `^31.0.0` to `^31.1.0` ([#8765](https://github.com/MetaMask/core/pull/8765))
 
+### Fixed
+
+- Build-delegation step no longer emits a redundant duplicate `ValueLteEnforcer` caveat; the Delegation Framework treats both as equivalent, but the duplicate was inadvertently inherited from `@metamask/smart-accounts-kit`'s `erc20TransferAmount` scope helper. ([#8621](https://github.com/MetaMask/core/pull/8621))
+- EIP-7702 authorization step now treats a 409 response from `POST /v1/account-upgrade` as `already-done` instead of a fatal error, making the step retry-safe when a prior submission was accepted by CHOMP but has not yet been observed on-chain. ([#8621](https://github.com/MetaMask/core/pull/8621))
+
 ## [1.3.2]
 
 ### Changed

packages/money-account-upgrade-controller/jest.config.js

@@ -11,10 +11,7 @@ const baseConfig = require('../../jest.config.packages');
 const displayName = path.basename(__dirname);
 
 module.exports = merge(baseConfig, {
-  // The display name when running multiple projects
   displayName,
-
-  // An object that configures minimum threshold enforcement for coverage results
   coverageThreshold: {
     global: {
       branches: 100,
@@ -23,4 +20,5 @@ module.exports = merge(baseConfig, {
       statements: 100,
     },
   },
+  testEnvironment: '<rootDir>/jest.environment.js',
 });

packages/money-account-upgrade-controller/jest.environment.js

@@ -0,0 +1,18 @@
+const { TestEnvironment } = require('jest-environment-node');
+
+/**
+ * Some transitive dependencies rely on the Web Crypto API, which is not
+ * exposed as a global by jest-environment-node.
+ */
+class CustomTestEnvironment extends TestEnvironment {
+  async setup() {
+    await super.setup();
+    if (typeof this.global.crypto === 'undefined') {
+      // Only used for testing.
+      // eslint-disable-next-line n/no-unsupported-features/node-builtins
+      this.global.crypto = require('crypto').webcrypto;
+    }
+  }
+}
+
+module.exports = CustomTestEnvironment;

packages/money-account-upgrade-controller/package.json

@@ -53,8 +53,12 @@
     "test:watch": "NODE_OPTIONS=--experimental-vm-modules jest --watch"
   },
   "dependencies": {
+    "@metamask/authenticated-user-storage": "^1.0.1",
     "@metamask/base-controller": "^9.1.0",
     "@metamask/chomp-api-service": "^3.0.1",
+    "@metamask/delegation-controller": "^3.0.0",
+    "@metamask/delegation-core": "^2.0.0",
+    "@metamask/delegation-deployments": "^1.3.0",
     "@metamask/keyring-controller": "^25.5.0",
     "@metamask/messenger": "^1.2.0",
     "@metamask/network-controller": "^31.1.0",
@@ -66,6 +70,7 @@
     "@types/jest": "^29.5.14",
     "deepmerge": "^4.2.2",
     "jest": "^29.7.0",
+    "jest-environment-node": "^29.7.0",
     "ts-jest": "^29.2.5",
     "tsx": "^4.20.5",
     "typedoc": "^0.25.13",