Skip to content
Merged
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 @@ -216,6 +216,7 @@ linkStyle default opacity:0.5
earn_controller --> transaction_controller;
eip_5792_middleware --> transaction_controller;
eip_5792_middleware --> keyring_controller;
eip_7702_internal_rpc_middleware --> controller_utils;
eip1193_permission_middleware --> chain_agnostic_permission;
eip1193_permission_middleware --> controller_utils;
eip1193_permission_middleware --> json_rpc_engine;
Expand Down Expand Up @@ -298,6 +299,7 @@ linkStyle default opacity:0.5
profile_sync_controller --> address_book_controller;
profile_sync_controller --> keyring_controller;
rate_limit_controller --> base_controller;
rate_limit_controller --> messenger;
remote_feature_flag_controller --> base_controller;
remote_feature_flag_controller --> controller_utils;
sample_controllers --> base_controller;
Expand Down
4 changes: 1 addition & 3 deletions eslint-warning-thresholds.json
Original file line number Diff line number Diff line change
Expand Up @@ -408,9 +408,7 @@
"@typescript-eslint/prefer-readonly": 1
},
"packages/rate-limit-controller/src/RateLimitController.ts": {
"jsdoc/check-tag-names": 4,
"jsdoc/require-returns": 1,
"jsdoc/tag-lines": 3
"jsdoc/check-tag-names": 4
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: JSDoc Returns Tag Added Without Warning Threshold

Based on the PR discussion, the ESLint warning thresholds for "jsdoc/require-returns" and "jsdoc/tag-lines" were removed from packages/rate-limit-controller/src/RateLimitController.ts. However, looking at the code changes in RateLimitController.ts, a @returns tag was added to the call method's JSDoc (line 165), which means the "jsdoc/require-returns" warning threshold should still be 1 rather than being removed entirely. This was identified in the PR discussion where a reviewer asked if this should be updated.

Fix in Cursor Fix in Web

},
"packages/remote-feature-flag-controller/src/client-config-api-service/client-config-api-service.test.ts": {
"import-x/order": 1,
Expand Down
6 changes: 6 additions & 0 deletions packages/rate-limit-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Changed

- **BREAKING:** Use new `Messenger` from `@metamask/messenger` ([#6503](https://github.com/MetaMask/core/pull/6503))
- Previously, `RateLimitController` accepted a `RestrictedMessenger` instance from `@metamask/base-controller`.
- **BREAKING:** Metadata property `anonymous` renamed to `includeInDebugSnapshot` ([#6503](https://github.com/MetaMask/core/pull/6503))

## [6.1.1]

### Changed
Expand Down
1 change: 1 addition & 0 deletions packages/rate-limit-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
},
"dependencies": {
"@metamask/base-controller": "^8.4.2",
"@metamask/messenger": "^0.3.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Incorrect Dependency Update Location

The PR discussion indicates this line should be updated in the tsconfig.build.json file, not in package.json. The dependency on @metamask/base-controller should likely be updated to @metamask/base-controller/next to match the import change in RateLimitController.ts (line 1 of the diff shows the controller now imports from @metamask/base-controller/next). This is a discrepancy mentioned in the PR review comments.

Fix in Cursor Fix in Web

"@metamask/rpc-errors": "^7.0.2",
"@metamask/utils": "^11.8.1"
},
Expand Down
91 changes: 57 additions & 34 deletions packages/rate-limit-controller/src/RateLimitController.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import { Messenger, deriveStateFromMetadata } from '@metamask/base-controller';

import type {
RateLimitControllerActions,
RateLimitControllerEvents,
} from './RateLimitController';
import { deriveStateFromMetadata } from '@metamask/base-controller/next';
import {
Messenger,
MOCK_ANY_NAMESPACE,
type MessengerActions,
type MessengerEvents,
type MockAnyNamespace,
} from '@metamask/messenger';

import type { RateLimitMessenger } from './RateLimitController';
import { RateLimitController } from './RateLimitController';

const name = 'RateLimitController';
const controllerName = 'RateLimitController';

const implementations = {
apiWithoutCustomLimit: {
Expand All @@ -21,29 +25,48 @@ const implementations = {

type RateLimitedApis = typeof implementations;

type AllRateLimitControllerActions = MessengerActions<
RateLimitMessenger<RateLimitedApis>
>;

type AllRateLimitControllerEvents = MessengerEvents<
RateLimitMessenger<RateLimitedApis>
>;

type RootMessenger = Messenger<
MockAnyNamespace,
AllRateLimitControllerActions,
AllRateLimitControllerEvents
>;

/**
* Constructs an unrestricted messenger.
* Creates and returns a root messenger for testing
*
* @returns An unrestricted messenger.
* @returns A messenger instance
*/
function getUnrestrictedMessenger() {
return new Messenger<
RateLimitControllerActions<RateLimitedApis>,
RateLimitControllerEvents<RateLimitedApis>
>();
function getRootMessenger(): RootMessenger {
return new Messenger({
namespace: MOCK_ANY_NAMESPACE,
});
}

/**
* Constructs a restricted messenger.
* Constructs a messenger for the RateLimitController.
*
* @param messenger - An optional unrestricted messenger
* @returns A restricted messenger.
* @param rootMessenger - An optional root messenger
* @returns A messenger for the RateLimitController.
*/
function getRestrictedMessenger(messenger = getUnrestrictedMessenger()) {
return messenger.getRestricted({
name,
allowedActions: [],
allowedEvents: [],
function getMessenger(
rootMessenger = getRootMessenger(),
): RateLimitMessenger<RateLimitedApis> {
return new Messenger<
typeof controllerName,
AllRateLimitControllerActions,
AllRateLimitControllerEvents,
RootMessenger
>({
namespace: controllerName,
parent: rootMessenger,
});
}

Expand All @@ -62,8 +85,8 @@ describe('RateLimitController', () => {
});

it('action: RateLimitController:call', async () => {
const unrestricted = getUnrestrictedMessenger();
const messenger = getRestrictedMessenger(unrestricted);
const rootMessenger = getRootMessenger();
const messenger = getMessenger(rootMessenger);

// Registers action handlers
new RateLimitController({
Expand All @@ -72,7 +95,7 @@ describe('RateLimitController', () => {
});

expect(
await unrestricted.call(
await rootMessenger.call(
'RateLimitController:call',
origin,
'apiWithoutCustomLimit',
Expand All @@ -88,7 +111,7 @@ describe('RateLimitController', () => {
});

it('uses apiWithoutCustomLimit method', async () => {
const messenger = getRestrictedMessenger();
const messenger = getMessenger();

const controller = new RateLimitController({
implementations,
Expand All @@ -105,7 +128,7 @@ describe('RateLimitController', () => {
});

it('returns false if rate-limited', async () => {
const messenger = getRestrictedMessenger();
const messenger = getMessenger();
const controller = new RateLimitController({
implementations,
messenger,
Expand Down Expand Up @@ -154,7 +177,7 @@ describe('RateLimitController', () => {
});

it('rate limit is reset after timeout', async () => {
const messenger = getRestrictedMessenger();
const messenger = getMessenger();
const controller = new RateLimitController({
implementations,
messenger,
Expand Down Expand Up @@ -198,7 +221,7 @@ describe('RateLimitController', () => {
});

it('timeout is only applied once per window', async () => {
const messenger = getRestrictedMessenger();
const messenger = getMessenger();
const controller = new RateLimitController({
implementations,
messenger,
Expand All @@ -225,22 +248,22 @@ describe('RateLimitController', () => {
it('includes expected state in debug snapshots', () => {
const controller = new RateLimitController({
implementations,
messenger: getRestrictedMessenger(),
messenger: getMessenger(),
});

expect(
deriveStateFromMetadata(
controller.state,
controller.metadata,
'anonymous',
'includeInDebugSnapshot',
),
).toMatchInlineSnapshot(`Object {}`);
});

it('includes expected state in state logs', () => {
const controller = new RateLimitController({
implementations,
messenger: getRestrictedMessenger(),
messenger: getMessenger(),
});

expect(
Expand All @@ -255,7 +278,7 @@ describe('RateLimitController', () => {
it('persists expected state', () => {
const controller = new RateLimitController({
implementations,
messenger: getRestrictedMessenger(),
messenger: getMessenger(),
});

expect(
Expand All @@ -270,7 +293,7 @@ describe('RateLimitController', () => {
it('exposes expected state to UI', () => {
const controller = new RateLimitController({
implementations,
messenger: getRestrictedMessenger(),
messenger: getMessenger(),
});

expect(
Expand Down
49 changes: 28 additions & 21 deletions packages/rate-limit-controller/src/RateLimitController.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import type {
ActionConstraint,
RestrictedMessenger,
ControllerGetStateAction,
ControllerStateChangeEvent,
} from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import {
BaseController,
type ControllerGetStateAction,
type ControllerStateChangeEvent,
} from '@metamask/base-controller/next';
import type { Messenger, ActionConstraint } from '@metamask/messenger';
import { rpcErrors } from '@metamask/rpc-errors';
import { getKnownPropertyNames } from '@metamask/utils';

/**
* A rate-limited API endpoint.
*
* @property method - The method that is rate-limited.
* @property rateLimitTimeout - The time window in which the rate limit is applied (in ms).
* @property rateLimitCount - The amount of calls an origin can make in the rate limit time window.
Expand All @@ -27,34 +27,42 @@ export type RateLimitedApiMap = Record<string, RateLimitedApi>;

/**
* A map of rate-limited API types to the number of requests made in a given interval for each origin and api type combination.
*
* @template RateLimitedApis - A {@link RateLimitedApiMap} containing the rate-limited API endpoints that is used by the {@link RateLimitController}.
*/
export type RateLimitedRequests<RateLimitedApis extends RateLimitedApiMap> =
Record<keyof RateLimitedApis, Record<string, number>>;

/**
* The state of the {@link RateLimitController}.
*
* @template RateLimitedApis - A {@link RateLimitedApiMap} containing the rate-limited API endpoints that is used by the {@link RateLimitController}.
* @property requests - An object containing the number of requests made in a given interval for each origin and api type combination.
*/
export type RateLimitState<RateLimitedApis extends RateLimitedApiMap> = {
requests: RateLimitedRequests<RateLimitedApis>;
};

const name = 'RateLimitController';
const controllerName = 'RateLimitController';

export type RateLimitControllerStateChangeEvent<
RateLimitedApis extends RateLimitedApiMap,
> = ControllerStateChangeEvent<typeof name, RateLimitState<RateLimitedApis>>;
> = ControllerStateChangeEvent<
typeof controllerName,
RateLimitState<RateLimitedApis>
>;

export type RateLimitControllerGetStateAction<
RateLimitedApis extends RateLimitedApiMap,
> = ControllerGetStateAction<typeof name, RateLimitState<RateLimitedApis>>;
> = ControllerGetStateAction<
typeof controllerName,
RateLimitState<RateLimitedApis>
>;

export type RateLimitControllerCallApiAction<
RateLimitedApis extends RateLimitedApiMap,
> = {
type: `${typeof name}:call`;
type: `${typeof controllerName}:call`;
handler: RateLimitController<RateLimitedApis>['call'];
};

Expand All @@ -69,19 +77,17 @@ export type RateLimitControllerEvents<
> = RateLimitControllerStateChangeEvent<RateLimitedApis>;

export type RateLimitMessenger<RateLimitedApis extends RateLimitedApiMap> =
RestrictedMessenger<
typeof name,
Messenger<
typeof controllerName,
RateLimitControllerActions<RateLimitedApis>,
RateLimitControllerEvents<RateLimitedApis>,
never,
never
RateLimitControllerEvents<RateLimitedApis>
>;

const metadata = {
requests: {
includeInStateLogs: false,
persist: false,
anonymous: false,
includeInDebugSnapshot: false,
usedInUi: false,
},
};
Expand All @@ -92,7 +98,7 @@ const metadata = {
export class RateLimitController<
RateLimitedApis extends RateLimitedApiMap,
> extends BaseController<
typeof name,
typeof controllerName,
RateLimitState<RateLimitedApis>,
RateLimitMessenger<RateLimitedApis>
> {
Expand Down Expand Up @@ -131,7 +137,7 @@ export class RateLimitController<
>((acc, key) => ({ ...acc, [key]: {} }), {} as never),
};
super({
name,
name: controllerName,
metadata,
messenger,
state: { ...defaultState, ...state },
Expand All @@ -140,8 +146,8 @@ export class RateLimitController<
this.rateLimitTimeout = rateLimitTimeout;
this.rateLimitCount = rateLimitCount;

this.messagingSystem.registerActionHandler(
`${name}:call`,
this.messenger.registerActionHandler(
`${controllerName}:call`,
(
origin: string,
type: keyof RateLimitedApis,
Expand All @@ -156,6 +162,7 @@ export class RateLimitController<
* @param origin - The requesting origin.
* @param type - The type of API call to make.
* @param args - Arguments for the API call.
* @returns The result of the API call.
*/
async call<ApiType extends keyof RateLimitedApis>(
origin: string,
Expand Down
5 changes: 4 additions & 1 deletion packages/rate-limit-controller/tsconfig.build.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
"outDir": "./dist",
"rootDir": "./src"
},
"references": [{ "path": "../base-controller/tsconfig.build.json" }],
"references": [
{ "path": "../base-controller/tsconfig.build.json" },
{ "path": "../messenger/tsconfig.build.json" }
],
"include": ["../../types", "./src"]
}
2 changes: 1 addition & 1 deletion packages/rate-limit-controller/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
"compilerOptions": {
"baseUrl": "./"
},
"references": [{ "path": "../base-controller" }],
"references": [{ "path": "../base-controller" }, { "path": "../messenger" }],
"include": ["../../types", "./src"]
}
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4574,6 +4574,7 @@ __metadata:
dependencies:
"@metamask/auto-changelog": "npm:^3.4.4"
"@metamask/base-controller": "npm:^8.4.2"
"@metamask/messenger": "npm:^0.3.0"
"@metamask/rpc-errors": "npm:^7.0.2"
"@metamask/utils": "npm:^11.8.1"
"@types/jest": "npm:^27.4.1"
Expand Down
Loading