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
14 changes: 14 additions & 0 deletions packages/rate-limit-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,20 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Added

- Add and export types `RateLimitedApiMap`, `RateLimitedRequests`. ([#3963](https://github.com/MetaMask/core/pull/3963))
- `RateLimitedApiMap` represents the type of the `RateLimitedApis` generic parameter used throughout the controller.
- `RateLimitedRequests` represents the type of the `request` property of `RateLimitState`.

### Changed

- **BREAKING:** Rename types to align with conventions followed by other controllers. ([#3963](https://github.com/MetaMask/core/pull/3963))
- `GetRateLimitState` is now `RateLimitControllerGetStateAction`.
- `RateLimitStateChange` is now `RateLimitControllerStateChangeEvent`.
- `CallApi` is now `RateLimitControllerCallApiAction`.
- Add `@metamask/utils` `^8.3.0` as a dependency. ([#3963](https://github.com/MetaMask/core/pull/3963))

### Fixed

- **BREAKING:** Fix `GetRateLimitState`, `RateLimitStateChange` types by replacing `RateLimitedApis` with `RateLimitState<RateLimitedApis>` as the state type passed in as generic arguments to `ControllerGetStateAction` and `ControllerStateChangeEvent` ([#3949](https://github.com/MetaMask/core/pull/3949))
Expand Down
3 changes: 2 additions & 1 deletion packages/rate-limit-controller/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@
},
"dependencies": {
"@metamask/base-controller": "^4.1.1",
"@metamask/rpc-errors": "^6.2.1"
"@metamask/rpc-errors": "^6.2.1",
"@metamask/utils": "^8.3.0"
},
"devDependencies": {
"@metamask/auto-changelog": "^3.4.4",
Expand Down
103 changes: 60 additions & 43 deletions packages/rate-limit-controller/src/RateLimitController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ import type {
} from '@metamask/base-controller';
import { BaseController } from '@metamask/base-controller';
import { rpcErrors } from '@metamask/rpc-errors';
import { getKnownPropertyNames } from '@metamask/utils';

/**
* @type RateLimitedApi
* 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 @@ -20,47 +21,61 @@ export type RateLimitedApi = {
};

/**
* @type RateLimitState
* @property requests - Object containing number of requests in a given interval for each origin and api type combination
* A map of rate-limited API types to APIs.
*/
export type RateLimitState<
RateLimitedApis extends Record<string, RateLimitedApi>,
> = {
requests: Record<keyof RateLimitedApis, Record<string, number>>;
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';

export type RateLimitStateChange<
RateLimitedApis extends Record<string, RateLimitedApi>,
export type RateLimitControllerStateChangeEvent<
RateLimitedApis extends RateLimitedApiMap,
> = ControllerStateChangeEvent<typeof name, RateLimitState<RateLimitedApis>>;

export type GetRateLimitState<
RateLimitedApis extends Record<string, RateLimitedApi>,
export type RateLimitControllerGetStateAction<
RateLimitedApis extends RateLimitedApiMap,
> = ControllerGetStateAction<typeof name, RateLimitState<RateLimitedApis>>;

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

export type RateLimitControllerActions<
RateLimitedApis extends Record<string, RateLimitedApi>,
> = GetRateLimitState<RateLimitedApis> | CallApi<RateLimitedApis>;
RateLimitedApis extends RateLimitedApiMap,
> =
| RateLimitControllerGetStateAction<RateLimitedApis>
| RateLimitControllerCallApiAction<RateLimitedApis>;

export type RateLimitControllerEvents<
RateLimitedApis extends Record<string, RateLimitedApi>,
> = RateLimitStateChange<RateLimitedApis>;

export type RateLimitMessenger<
RateLimitedApis extends Record<string, RateLimitedApi>,
> = RestrictedControllerMessenger<
typeof name,
RateLimitControllerActions<RateLimitedApis>,
RateLimitControllerEvents<RateLimitedApis>,
never,
never
>;
RateLimitedApis extends RateLimitedApiMap,
> = RateLimitControllerStateChangeEvent<RateLimitedApis>;

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

const metadata = {
requests: { persist: false, anonymous: false },
Expand All @@ -70,7 +85,7 @@ const metadata = {
* Controller with logic for rate-limiting API endpoints per requesting origin.
*/
export class RateLimitController<
RateLimitedApis extends Record<string, RateLimitedApi>,
RateLimitedApis extends RateLimitedApiMap,
> extends BaseController<
typeof name,
RateLimitState<RateLimitedApis>,
Expand Down Expand Up @@ -106,10 +121,9 @@ export class RateLimitController<
implementations: RateLimitedApis;
}) {
const defaultState = {
requests: Object.keys(implementations).reduce(
(acc, key) => ({ ...acc, [key]: {} }),
{} as Record<keyof RateLimitedApis, Record<string, number>>,
),
requests: getKnownPropertyNames(implementations).reduce<
RateLimitedRequests<RateLimitedApis>
>((acc, key) => ({ ...acc, [key]: {} }), {} as never),
};
super({
name,
Expand All @@ -122,11 +136,11 @@ export class RateLimitController<
this.rateLimitCount = rateLimitCount;

this.messagingSystem.registerActionHandler(
`${name}:call` as const,
`${name}:call`,
(
origin: string,
type: keyof RateLimitedApis,
...args: Parameters<RateLimitedApis[keyof RateLimitedApis]['method']>
...args: Parameters<RateLimitedApis[typeof type]['method']>
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

args should represent the result of querying RateLimitedApis with the specific type passed in as the second argument, not all possible Api types stored in state.

keyof RateLimitedApis is only the constraint for the type param, not its type.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ah, good catch.

) => this.call(origin, type, ...args),
);
}
Expand Down Expand Up @@ -183,17 +197,17 @@ export class RateLimitController<
private recordRequest(api: keyof RateLimitedApis, origin: string) {
const rateLimitTimeout =
this.implementations[api].rateLimitTimeout ?? this.rateLimitTimeout;
const previous = this.state.requests[api][origin] ?? 0;
this.update((state) => {
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const previous = (state as any).requests[api][origin] ?? 0;
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(state as any).requests[api][origin] = previous + 1;

if (previous === 0) {
setTimeout(() => this.resetRequestCount(api, origin), rateLimitTimeout);
}
Object.assign(state, {
requests: {
...(state.requests as RateLimitedRequests<RateLimitedApis>),
[api]: { [origin]: previous + 1 },
},
});
});
}

Expand All @@ -205,9 +219,12 @@ export class RateLimitController<
*/
private resetRequestCount(api: keyof RateLimitedApis, origin: string) {
this.update((state) => {
// TODO: Replace `any` with type
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(state as any).requests[api][origin] = 0;
Object.assign(state, {
requests: {
...(state.requests as RateLimitedRequests<RateLimitedApis>),
[api]: { [origin]: 0 },
},
});
});
}
}
14 changes: 13 additions & 1 deletion packages/rate-limit-controller/src/index.ts
Original file line number Diff line number Diff line change
@@ -1 +1,13 @@
export * from './RateLimitController';
export type {
RateLimitedApi,
RateLimitedApiMap,
RateLimitedRequests,
RateLimitControllerActions,
RateLimitControllerCallApiAction,
RateLimitControllerGetStateAction,
RateLimitControllerEvents,
RateLimitControllerStateChangeEvent,
RateLimitMessenger,
RateLimitState,
} from './RateLimitController';
export { RateLimitController } from './RateLimitController';
1 change: 1 addition & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2781,6 +2781,7 @@ __metadata:
"@metamask/auto-changelog": ^3.4.4
"@metamask/base-controller": ^4.1.1
"@metamask/rpc-errors": ^6.2.1
"@metamask/utils": ^8.3.0
"@types/jest": ^27.4.1
deepmerge: ^4.2.2
jest: ^27.5.1
Expand Down