Skip to content

Commit

Permalink
[assets-contract-controller] Apply messenger pattern and remove inher…
Browse files Browse the repository at this point in the history
…itance from `BaseControllerV1` (#4397)

## Motivation

As part of the Wallet Framework Team's Q2 2024 OKRs (O3KR1: "100%
completion of all core controllers to BaseControllerV2"), the
`AssetsContractController` needs to be converted to a V2 controller.

However, because `AssetsContractController` has an empty state object,
it is a
[non-controller](https://github.com/MetaMask/decisions/blob/main/decisions/core/0001-messaging-non-controllers.md),
which must be able to communicate with other controllers using a
`messagingSystem`, but should not inherit from the `BaseControllerV2`
class.

## Explanation

This commit introduces and applies the messenger pattern to the
`AssetsContractController` class:
- Convert its public methods into messenger actions.
- Define the `AssetsContractControllerMessenger` and associated types. 
- Remove elements associated with `BaseControllerV1`. 
- Avoid creating a new inheritance relationship with `BaseControllerV2`.

The newly defined messenger actions/events are applied downstream in
`NftController` and `TokenBalancesController`.

## References

- Closes #4072

## Changelog

### `@metamask/assets-controllers`

```md
### Added

- **BREAKING:** `TokenBalancesControllerMessenger` must allow the `AssetsContractController:getERC20BalanceOf` action in addition to its previous allowed actions. ([#4397](#4397))
- **BREAKING:** `NftControllerMessenger` must allow the following actions in addition to its previous allowed actions: `AssetsContractController:getERC721AssetName`, `AssetsContractController:getERC721AssetSymbol`, `AssetsContractController:getERC721TokenURI`, `AssetsContractController:getERC721OwnerOf`, `AssetsContractController:getERC1155BalanceOf`, `AssetsContractController:getERC1155TokenURI`. ([#4397](#4397))
- **BREAKING:** Add elements to the `AssetsContractController` class: ([#4397](#4397))
  - **BREAKING:** Add required constructor option `messenger`.
  - Add class field `messagingSystem`.
  - Add getters for `ipfsGateway` and `chainId`. As corresponding setters have not been defined, these properties are not externally mutable.
- Add and export the `AssetsContractControllerMessenger` type ([#4397](#4397))
  - `AssetsContractControllerMessenger` must allow the external actions `NetworkController:getNetworkClientById`, `NetworkController:getNetworkConfigurationByNetworkClientId`, `NetworkController:getSelectedNetworkClient`, `NetworkController:getState`.
  - `AssetsContractControllerMessenger` must allow the external events `PreferencesController:stateChange`, `NetworkController:networkDidChange`. 
- Add and export new types: `AssetsContractControllerActions`, `AssetsContractControllerEvents`, `AssetsContractControllerGetERC20StandardAction`, `AssetsContractControllerGetERC721StandardAction`, `AssetsContractControllerGetERC1155StandardAction`, `AssetsContractControllerGetERC20BalanceOfAction`, `AssetsContractControllerGetERC20TokenDecimalsAction`, `AssetsContractControllerGetERC20TokenNameAction`, `AssetsContractControllerGetERC721NftTokenIdAction`, `AssetsContractControllerGetERC721TokenURIAction`, `AssetsContractControllerGetERC721AssetNameAction`, `AssetsContractControllerGetERC721AssetSymbolAction`, `AssetsContractControllerGetERC721OwnerOfAction`, `AssetsContractControllerGetERC1155TokenURIAction`, `AssetsContractControllerGetERC1155BalanceOfAction`, `AssetsContractControllerTransferSingleERC1155Action`, `AssetsContractControllerGetTokenStandardAndDetailsAction`, `AssetsContractControllerGetBalancesInSingleCallAction`. ([#4397](#4397))
- Add a new `setProvider` method to `AssetsContractController`. ([#4397](#4397))
  - Replaces the removed `provider` setter method, and widens the `provider` function parameter type from `Provider` to `Provider | undefined`.
  
### Changed

- **BREAKING:** The type of `SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID` is narrowed from `Record<Hex, string>` to the const-asserted literal properties of the `SINGLE_CALL_BALANCES_ADDRESS_BY_CHAINID` object. ([#4397](#4397))
  - The index signature is restricted to the union of the enum keys of `SupportedTokenDetectionNetworks`.
  - The property value type is restricted to the type union of the addresses defined in the object.
  - The object type is constrained by `Record<Hex, string>` using the `satisfies` keyword.
- **BREAKING:** Convert the `BalanceMap` type from an `interface` into a type alias. ([#4397](#4397))
  - Type aliases have an index signature of `string` by default, and are compatible with the `StateConstraint` type defined in the `@metamask/base-controller` package.

### Removed

- **BREAKING:** Remove elements from the `AssetsContractController` class: ([#4397](#4397))
  - **BREAKING:** `AssetsContractController` no longer inherits from `BaseControllerV1`.
  - **BREAKING:** Remove constructor option callbacks `onPreferencesStateChange`, `onNetworkDidChange`, `getNetworkClientById`, and replace with corresponding messenger actions and events. 
  - **BREAKING:** Remove class fields: `name`, `config` (along with its properties `provider`, `ipfsGateway`, `chainId`).
  - **BREAKING:** Remove methods: `getProvider`, `getChainId`.
  - **BREAKING:** Remove the `provider` setter method.
- **BREAKING:** Remove the `getERC20BalanceOf` constructor option callback from the `TokenBalancesControllerOptions` type and the `TokenBalancesController` constructor. ([#4397](#4397))
- **BREAKING:** Remove `NftController` constructor option callbacks: `getERC721AssetName`, `getERC721AssetSymbol`, `getERC721TokenURI`, `getERC721OwnerOf`, `getERC1155BalanceOf`, `getERC1155TokenURI`. ([#4397](#4397))
- **BREAKING:** Remove the `AssetsContractConfig` type. ([#4397](#4397))
- **BREAKING:** Remove export for `MISSING_PROVIDER_ERROR`. ([#4397](#4397))

### Fixed

- **BREAKING:** Convert the `getERC721NftTokenId` method of the `AssetsContractController` into an async function. ([#4397](#4397))
```

## Checklist

- [x] I've updated the test suite for new or updated code as appropriate
- [x] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [x] I've highlighted breaking changes using the "BREAKING" category
above as appropriate

---------

Co-authored-by: Elliot Winkler <elliot.winkler@gmail.com>
  • Loading branch information
MajorLift and mcmire authored Jul 19, 2024
1 parent e441444 commit 4485e8a
Show file tree
Hide file tree
Showing 9 changed files with 1,137 additions and 795 deletions.
8 changes: 4 additions & 4 deletions packages/assets-controllers/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ module.exports = merge(baseConfig, {
// An object that configures minimum threshold enforcement for coverage results
coverageThreshold: {
global: {
branches: 90.35,
functions: 96.74,
lines: 97.34,
statements: 97.36,
branches: 91.07,
functions: 97.51,
lines: 98.12,
statements: 98.03,
},
},

Expand Down
Loading

0 comments on commit 4485e8a

Please sign in to comment.