Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions packages/chomp-api-service/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
121 changes: 103 additions & 18 deletions packages/chomp-api-service/src/chomp-api-service.test.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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);
});
});
});

/**
Expand Down
32 changes: 30 additions & 2 deletions packages/chomp-api-service/src/chomp-api-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 ===

/**
Expand Down Expand Up @@ -262,7 +290,7 @@ export class ChompApiService extends BaseDataService<
name: serviceName,
messenger,
queryClientConfig,
policyOptions,
policyOptions: { ...DEFAULT_POLICY_OPTIONS, ...policyOptions },
});

this.#baseUrl = baseUrl;
Expand Down
13 changes: 13 additions & 0 deletions packages/money-account-upgrade-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,23 @@ 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))
- **BREAKING:** `UpgradeConfig` no longer includes `musdTokenAddress` (now derived internally from the Veda protocol service details). ([#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
Expand Down
9 changes: 3 additions & 6 deletions packages/money-account-upgrade-controller/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,15 @@
* https://jestjs.io/docs/configuration
*/

const merge = require('deepmerge');
const path = require('path');

const baseConfig = require('../../jest.config.packages');

const displayName = path.basename(__dirname);

module.exports = merge(baseConfig, {
// The display name when running multiple projects
module.exports = {
...baseConfig,
displayName,

// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 100,
Expand All @@ -23,4 +20,4 @@ module.exports = merge(baseConfig, {
statements: 100,
},
},
});
};
4 changes: 4 additions & 0 deletions packages/money-account-upgrade-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Loading
Loading