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
171 changes: 171 additions & 0 deletions docs/code-guidelines/controller-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,177 @@ The name of the controller should reflect its responsibility. If, when creating

Each public method and each state property of a controller should have a purpose, and the name of the method or state property should be readable and should reflect the purpose clearly. If something does not need to be public, it should be made private; if it is unnecessary, it should be removed.

## Define, but do not export, a name for the controller

Every controller has a name, which is used to namespace not only the controller's messenger actions and events, but also the controller's state data when composed with other controllers.

The name should be defined in a constant called `CONTROLLER_NAME` so that it can be easily changed if the need arises. The name should be used to initialize the messenger, and it should also be passed to the `BaseController` constructor.

The constant should be used to define actions and events. It may be exported from the file in which it is defined, but should not listed as an export of the package.

🚫 **The messenger namespace is not defined as a constant, but is repeated**

```typescript
export type TransactionsControllerStateChangedEvent =
ControllerStateChangedEvent<
// 🚫 Name is repeated.
'TransactionsController',
TransactionsControllerState
>;

export type TransactionsControllerTransactionApprovedEvent = {
// 🚫 Name is repeated.
type: 'TransactionsController:transactionApprovedEvent';
payload: [transaction: Transaction];
};

export type TransactionsControllerEvents =
| TransactionsControllerStateChangedEvent
| TransactionsControllerTransactionApprovedEvent;

export type TransactionsControllerMessenger = Messenger<
// 🚫 Name is repeated.
'TransactionsController',
never,
TransactionsControllerEvents
>;

// 🚫 Name is repeated.
export class TransactionsController extends BaseController<'TransactionsController' /*, ... */> {
constructor(/* ... */) {
// 🚫 Name is repeated.
super({ name: 'TransactionsController' /* ... */ });

// ...
}
}
```

🚫 **The messenger namespace is defined as a constant, but it is called `controllerName` (legacy name)**

```typescript
// 🚫 Uses legacy name.
const controllerName = 'TransactionsController';

export type TransactionsControllerStateChangedEvent = ControllerStateChangedEvent<
typeof controllerName,
TransactionsControllerState
>;

export type TransactionsControllerTransactionApprovedEvent = {
type: `${typeof controllerName}:transactionApprovedEvent`;
payload: [transaction: Transaction]
};

export type TransactionsControllerEvents =
| TransactionsControllerStateChangedEvent
| TransactionsControllerTransactionApprovedEvent

export type TransactionsControllerMessenger = Messenger<
typeof controllerName,
never,
TransactionsControllerEvents
>;

export class TransactionsController extends BaseController<typeof controllerName /*, ... */ {
constructor(/* ... */) {
super({ name: controllerName, /* ... */ })

// ...
}
}
```

🚫 **The messenger namespace is defined as a constant, but it is exported from the package**

```typescript
/* packages/transactions-controller/src/transactions-controller.ts */

export const CONTROLLER_NAME = 'TransactionsController';

export type TransactionsControllerStateChangedEvent = ControllerStateChangedEvent<
typeof CONTROLLER_NAME,
TransactionsControllerState
>;

export type TransactionsControllerTransactionApprovedEvent = {
type: `${typeof CONTROLLER_NAME}:transactionApprovedEvent`;
payload: [transaction: Transaction]
};

export type TransactionsControllerEvents =
| TransactionsControllerStateChangedEvent
| TransactionsControllerTransactionApprovedEvent

export type TransactionsControllerMessenger = Messenger<
typeof CONTROLLER_NAME,
never,
TransactionsControllerEvents
>;

export class TransactionsController extends BaseController<typeof CONTROLLER_NAME /*, ... */ {
constructor(/* ... */) {
super({ name: CONTROLLER_NAME, /* ... */ })

// ...
}
}

/* packages/transactions-controller/src/index.ts */

export {
// 🚫 The controller name should not be exported from the package.
CONTROLLER_NAME,
TransactionsController,
// ...
} from './transactions-controller';
```

✅ **The messenger namespace is defined as a constant, and it is kept internal instead of being exported**

```typescript
/* packages/transactions-controller/src/transactions-controller.ts */

// ✅ Using correct name.
const CONTROLLER_NAME = 'TransactionsController';

export type TransactionsControllerStateChangedEvent = ControllerStateChangedEvent<
typeof CONTROLLER_NAME,
TransactionsControllerState
>;

export type TransactionsControllerTransactionApprovedEvent = {
type: `${typeof CONTROLLER_NAME}:transactionApprovedEvent`;
payload: [transaction: Transaction]
};

export type TransactionsControllerEvents =
| TransactionsControllerStateChangedEvent
| TransactionsControllerTransactionApprovedEvent

export type TransactionsControllerMessenger = Messenger<
typeof CONTROLLER_NAME,
never,
TransactionsControllerEvents
>;

export class TransactionsController extends BaseController<typeof CONTROLLER_NAME /*, ... */ {
constructor(/* ... */) {
super({ name: CONTROLLER_NAME, /* ... */ })

// ...
}
}

/* packages/transactions-controller/src/index.ts */

export {
// ✅ Name is not exported.
TransactionsController,
// ...
} from './transactions-controller';
```

## Accept an optional, partial representation of state

Although `BaseController` requires a full representation of controller state, in practice, controllers should accept a partial version and then supply missing properties with defaults. In fact, the `state` argument should be optional:
Expand Down
14 changes: 14 additions & 0 deletions packages/assets-controllers/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

- Expose missing public `CurrencyRateController` methods through its messenger ([#8561](https://github.com/MetaMask/core/pull/8561))
- The following actions are now available:
- `CurrencyRateController:setCurrentCurrency`
- `CurrencyRateController:updateExchangeRate`
- Corresponding action types (e.g. `CurrencyRateControllerSetCurrentCurrencyAction`) are available as well.

### Changed

- **BREAKING:** Standardize names of `CurrencyRateController` messenger action types ([#8561](https://github.com/MetaMask/core/pull/8561))
- The `GetCurrencyRateState` messenger action has been renamed to `CurrencyRateControllerGetStateAction` to follow the convention. You will need to update imports appropriately.
- These changes only affect the types. The action type strings themselves have not changed, so you do not need to update the list of actions you pass when initializing `CurrencyRateController` messenger.

## [104.3.0]

### Added
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* This file is auto generated.
* Do not edit manually.
*/

import type { CurrencyRateController } from './CurrencyRateController';

/**
* Sets a currency to track.
*
* @param currentCurrency - ISO 4217 currency code.
*/
export type CurrencyRateControllerSetCurrentCurrencyAction = {
type: `CurrencyRateController:setCurrentCurrency`;
handler: CurrencyRateController['setCurrentCurrency'];
};

/**
* Updates the exchange rate for the current currency and native currency pairs.
*
* @param nativeCurrencies - The native currency symbols to fetch exchange rates for.
*/
export type CurrencyRateControllerUpdateExchangeRateAction = {
type: `CurrencyRateController:updateExchangeRate`;
handler: CurrencyRateController['updateExchangeRate'];
};

/**
* Union of all CurrencyRateController action types.
*/
export type CurrencyRateControllerMethodActions =
| CurrencyRateControllerSetCurrentCurrencyAction
| CurrencyRateControllerUpdateExchangeRateAction;
17 changes: 15 additions & 2 deletions packages/assets-controllers/src/CurrencyRateController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { StaticIntervalPollingController } from '@metamask/polling-controller';
import type { Hex } from '@metamask/utils';
import { Mutex } from 'async-mutex';

import type { CurrencyRateControllerMethodActions } from './CurrencyRateController-method-action-types';
import type { AbstractTokenPricesService } from './token-prices-service/abstract-token-prices-service';
import { getNativeTokenAddress } from './token-prices-service/codefi-v2';

Expand Down Expand Up @@ -45,19 +46,26 @@ export type CurrencyRateState = {

const name = 'CurrencyRateController';

const MESSENGER_EXPOSED_METHODS = [
'setCurrentCurrency',
'updateExchangeRate',
] as const;

export type CurrencyRateStateChange = ControllerStateChangeEvent<
typeof name,
CurrencyRateState
>;

export type CurrencyRateControllerEvents = CurrencyRateStateChange;

export type GetCurrencyRateState = ControllerGetStateAction<
export type CurrencyRateControllerGetStateAction = ControllerGetStateAction<
typeof name,
CurrencyRateState
>;

export type CurrencyRateControllerActions = GetCurrencyRateState;
export type CurrencyRateControllerActions =
| CurrencyRateControllerGetStateAction
| CurrencyRateControllerMethodActions;

type AllowedActions =
| NetworkControllerGetNetworkClientByIdAction
Expand Down Expand Up @@ -168,6 +176,11 @@ export class CurrencyRateController extends StaticIntervalPollingController<Curr
this.#useExternalServices = useExternalServices;
this.setIntervalLength(interval);
this.#tokenPricesService = tokenPricesService;

this.messenger.registerMethodActionHandlers(
this,
MESSENGER_EXPOSED_METHODS,
);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import type { Draft } from 'immer';
import type {
CurrencyRateState,
CurrencyRateStateChange,
GetCurrencyRateState,
CurrencyRateControllerGetStateAction,
} from '../CurrencyRateController';
import type {
MultichainAssetsControllerGetStateAction,
Expand Down Expand Up @@ -121,7 +121,7 @@ export type MultichainAssetsRatesControllerEvents =
export type AllowedActions =
| SnapControllerHandleRequestAction
| AccountsControllerListMultichainAccountsAction
| GetCurrencyRateState
| CurrencyRateControllerGetStateAction
| MultichainAssetsControllerGetStateAction
| AccountsControllerGetSelectedMultichainAccountAction;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import type { Messenger } from '@metamask/messenger';
import type { Hex } from '@metamask/utils';

import { formatIconUrlWithProxy } from '../assetsUtil';
import type { GetCurrencyRateState } from '../CurrencyRateController';
import type { CurrencyRateControllerGetStateAction } from '../CurrencyRateController';
import type { AbstractTokenPricesService } from '../token-prices-service';
import {
fetchTokenMetadata,
Expand Down Expand Up @@ -61,7 +61,7 @@ export type TokenSearchDiscoveryDataControllerActions =
/**
* All actions that {@link TokenSearchDiscoveryDataController} calls internally.
*/
export type AllowedActions = GetCurrencyRateState;
export type AllowedActions = CurrencyRateControllerGetStateAction;

/**
* The event that {@link TokenSearchDiscoveryDataController} publishes when updating
Expand Down
4 changes: 4 additions & 0 deletions packages/assets-controllers/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ export {
AssetsContractController,
} from './AssetsContractController';
export * from './CurrencyRateController';
export type {
CurrencyRateControllerSetCurrentCurrencyAction,
CurrencyRateControllerUpdateExchangeRateAction,
} from './CurrencyRateController-method-action-types';
export type {
NftControllerState,
NftControllerMessenger,
Expand Down
18 changes: 9 additions & 9 deletions packages/bridge-controller/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [71.0.0]
## [70.2.0]

### Added

Expand Down Expand Up @@ -160,8 +160,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Bump `@metamask/assets-controllers` from `^100.0.3` to `^100.2.0` ([#8107](https://github.com/MetaMask/core/pull/8107)), ([#8107](https://github.com/MetaMask/core/pull/8107)), ([#8140](https://github.com/MetaMask/core/pull/8140))
- Bump `@metamask/transaction-controller` from `^62.19.0` to `^62.21.0` ([#8104](https://github.com/MetaMask/core/pull/8104)), ([#8104](https://github.com/MetaMask/core/pull/8104)), ([#8140](https://github.com/MetaMask/core/pull/8140))
- Bump `@metamask/assets-controllers` from `^100.0.3` to `^100.2.0`,, ([#8107](https://github.com/MetaMask/core/pull/8107), [#8140](https://github.com/MetaMask/core/pull/8140))
- Bump `@metamask/transaction-controller` from `^62.19.0` to `^62.21.0`,, ([#8104](https://github.com/MetaMask/core/pull/8104), [#8140](https://github.com/MetaMask/core/pull/8140))
- Bump `@metamask/accounts-controller` from `36.0.1` to `37.0.0` ([#8140](https://github.com/MetaMask/core/pull/8140))
- Bump `@metamask/assets-controller` from `2.2.0` to `2.3.0` ([#8140](https://github.com/MetaMask/core/pull/8140))
- Bump `@metamask/multichain-network-controller` from `3.0.4` to `3.0.5` ([#8140](https://github.com/MetaMask/core/pull/8140))
Expand Down Expand Up @@ -270,7 +270,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Refresh asset exchange rates each time quotes are fetched ([#7896](https://github.com/MetaMask/core/pull/7896))
- Return checksummed EVM assetIds from the `formatAddressToAssetId` utility ([#7896](https://github.com/MetaMask/core/pull/7896))
- Bump `@metamask/keyring-api` from `^21.0.0` to `^21.5.0` ([#7857](https://github.com/MetaMask/core/pull/7857))
- Bump `@metamask/transaction-controller` from `^62.15.0` to `^62.17.0` ([#7872](https://github.com/MetaMask/core/pull/7872)), ([#7897](https://github.com/MetaMask/core/pull/7897))
- Bump `@metamask/transaction-controller` from `^62.15.0` to `^62.17.0`, ([#7872](https://github.com/MetaMask/core/pull/7872), [#7897](https://github.com/MetaMask/core/pull/7897))
- Bump `@metamask/multichain-network-controller` from `^3.0.2` to `^3.0.3` ([#7897](https://github.com/MetaMask/core/pull/7897))
- Bump `@metamask/assets-controllers` from `^99.3.1` to `^99.3.2` ([#7897](https://github.com/MetaMask/core/pull/7897))
- Bump `@metamask/accounts-controller` from `^35.0.2` to `^36.0.0` ([#7897](https://github.com/MetaMask/core/pull/7897))
Expand Down Expand Up @@ -771,7 +771,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Changed

- Bump `@metamask/assets-controllers` from `^76.0.0` to `^77.0.0` ([#6716](https://github.com/MetaMask/core/pull/6716), [#6629](https://github.com/MetaMask/core/pull/6716))
- Bump `@metamask/assets-controllers` from `^76.0.0` to `^77.0.0` ([#6716](https://github.com/MetaMask/core/pull/6716), [#6629](https://github.com/MetaMask/core/pull/6629))

## [44.0.1]

Expand Down Expand Up @@ -947,7 +947,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- **BREAKING:** Bump peer dependency `@metamask/accounts-controller` from `^31.0.0` to `^32.0.0` ([#6171](https://github.com/MetaMask/core/pull/6171))
- **BREAKING:** Bump peer dependency `@metamask/assets-controllers` from `^72.0.0` to `^73.0.0` ([#6171](https://github.com/MetaMask/core/pull/6171))
- **BREAKING:** Bump peer dependency `@metamask/transaction-controller` from `^58.0.0` to `^59.0.0` ([#6171](https://github.com/MetaMask/core/pull/6171)), ([#6027](https://github.com/MetaMask/core/pull/6027))
- **BREAKING:** Bump peer dependency `@metamask/transaction-controller` from `^58.0.0` to `^59.0.0`, ([#6171](https://github.com/MetaMask/core/pull/6171), [#6027](https://github.com/MetaMask/core/pull/6027))

## [36.2.0]

Expand Down Expand Up @@ -1129,7 +1129,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

- Added optional `isUnifiedUIEnabled` flag to chain-level feature-flag `ChainConfiguration` type and updated the validation schema to accept the new flag ([#5783](https://github.com/MetaMask/core/pull/5783))
- Add and export `calcSlippagePercentage`, a utility that calculates the absolute slippage percentage based on the adjusted return and the sent amount ([#5723](https://github.com/MetaMask/core/pull/5723)).
- Add and export `calcSlippagePercentage`, a utility that calculates the absolute slippage percentage based on the adjusted return and the sent amount. ([#5723](https://github.com/MetaMask/core/pull/5723))
- Error logs for invalid getQuote responses ([#5816](https://github.com/MetaMask/core/pull/5816))

### Changed
Expand Down Expand Up @@ -1376,8 +1376,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Initial release ([#5317](https://github.com/MetaMask/core/pull/5317))

[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/bridge-controller@71.0.0...HEAD
[71.0.0]: https://github.com/MetaMask/core/compare/@metamask/bridge-controller@70.1.1...@metamask/bridge-controller@71.0.0
[Unreleased]: https://github.com/MetaMask/core/compare/@metamask/bridge-controller@70.2.0...HEAD
[70.2.0]: https://github.com/MetaMask/core/compare/@metamask/bridge-controller@70.1.1...@metamask/bridge-controller@70.2.0
[70.1.1]: https://github.com/MetaMask/core/compare/@metamask/bridge-controller@70.1.0...@metamask/bridge-controller@70.1.1
[70.1.0]: https://github.com/MetaMask/core/compare/@metamask/bridge-controller@70.0.1...@metamask/bridge-controller@70.1.0
[70.0.1]: https://github.com/MetaMask/core/compare/@metamask/bridge-controller@70.0.0...@metamask/bridge-controller@70.0.1
Expand Down
Loading
Loading