Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 118.0.0 #3918

Closed
wants to merge 4 commits into from
Closed

Release 118.0.0 #3918

wants to merge 4 commits into from

Conversation

MajorLift
Copy link
Contributor

@MajorLift MajorLift commented Feb 14, 2024

Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

Release 116 is out, so this is ready to be rebased. I've also left some suggestions below.

packages/assets-controllers/CHANGELOG.md Outdated Show resolved Hide resolved
packages/assets-controllers/CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems there is a typo on line 27.

Suggestion:

-   - **BREAKING:** The constructor for `TokenDetectionController` expects a new required proprerty `trackMetaMetricsEvent`, which defines the callback that is called in the `detectTokens` method.
+   - **BREAKING:** The constructor for `TokenDetectionController` expects a new required property `trackMetaMetricsEvent`, which defines the callback that is called in the `detectTokens` method.

Copy link
Contributor Author

@MajorLift MajorLift Feb 15, 2024

Choose a reason for hiding this comment

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

Thanks for pointing these out! Applied in 32dbd78

@MajorLift MajorLift changed the title Release 117.0.0 Release 118.0.0 Feb 15, 2024
@MajorLift
Copy link
Contributor Author

Co-authored-by: Elliot Winkler <[email protected]>

Update packages/assets-controllers/CHANGELOG.md

Co-authored-by: Elliot Winkler <[email protected]>

Update changelog
@MajorLift
Copy link
Contributor Author

MajorLift commented Feb 16, 2024

Closing to avoid blocking other more urgent releases. Will reopen once #3923 is merged.

@MajorLift MajorLift closed this Feb 16, 2024
MajorLift added a commit that referenced this pull request Feb 27, 2024
…dates (#3923)

## Explanation

As a preparatory step for fully replacing the extension
`DetectTokensController` with the consolidated core repo
`TokenDetectionController`, `TokenDetectionController` needs to be
updated with changes made to the extension `DetectTokensController`
since #1813 was closed.

#### Diff of `DetectTokensController` state
-
[MetaMask/metamask-extension@`5d285f7be5f7be981995dfa725aad97d81cc990a..85cd1c89039e900b452edb704ec37e9ccbd3e76a`#diff-323d0cf464](https://github.com/MetaMask/metamask-extension/compare/5d285f7be5f7be981995dfa725aad97d81cc990a..85cd1c89039e900b452edb704ec37e9ccbd3e76a#diff-323d0cf46498be3850b971474905354ea5ccf7fa13745ad1e6eba59c5b586830)

### Differences from extension `DetectTokensController`

- Refactors logic for retrieving `chainId`, `networkClientId` into
`this.#getCorrectChainIdAndNetworkClientId`
- Uses `getNetworkConfigurationByNetworkClientId` action instead of
`getNetworkClientById` to retrieve `chainId`.
- If `networkClientId` is not supplied to the method, or it's supplied
but `getNetworkConfigurationByNetworkClientId` returns `undefined`,
finds `chainId` from `providerConfig`.

- `detectTokens` replaces `detectNewTokens`
- `detectTokens` accepts options object `{ selectedAddress,
networkClientId }` instead of `{ selectedAddress, chainId,
networkClientId }`.
- Does not throw error if `getBalancesInSingleCall` fails. Also does not
exit early -- continues looping.
- Passes lists of full `Token` types to
`TokensController:addDetectedTokens` instead of objects containing only
`{ address, decimals, symbol }`.
  
- `#trackMetaMetricsEvents` is a private method instead of protected.
- Passes string literals instead of extension shared constants into
`_trackMetaMetricsEvent`.

## References

- Partially implements #3916
- Blocking #3918
- Changes adopted from:
  - MetaMask/metamask-extension#22898
  - MetaMask/metamask-extension#22814
  - #3914
  - MetaMask/metamask-extension#21437
- Blocking (Followed by) #3938

## Changelog

###
[`@metamask/assets-controllers`](https://github.com/MetaMask/core/pull/3923/files#diff-ee47d03d53776b8dd530799a8047f5e32e36e35765620aeb50b294adc3339fab)

## 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: jiexi <[email protected]>
@MajorLift
Copy link
Contributor Author

Superseded by #4007

MajorLift added a commit that referenced this pull request Mar 4, 2024
## Explanation

Adds refactors and cosmetic, typing fixes to #3923.

- Extracts three methods from `TokenDetectionController`'s
`detectTokens` method:
  - `#getCorrectChainIdAndNetworkClientId`
  - `#getTokenListAndSlicesOfTokensToDetect`
  - `#addDetectedTokens`
- Maintains distinction between class fields `#selectedAddress`,
`#networkClientId` and corresponding parameters used in `detectTokens`
and its helper methods, so that `detectTokens` method can be used
independently of polling/passive detection.
- [Refactor
`#getCorrectChainIdAndNetworkClientId`](c75fb3b)
to remove `findNetworkClientIdByChainId` which might return
inconsistent/unexpected results, and replace it with `getState`,
`getNetworkClientById`
- [Add missing method return
types](5c2e887)
- [Type networkClientId as
`NetworkClientId`](05be5d2)
- [Fix excess properties from legacy token list, define `LegacyToken`,
`TokenDetectionMap`](ea4c5b8)
- Removes `#chainId` class field.

## References

- Closes #1614
- Fixes #3661
- Blocked by (Follows from) #3923
- Blocking #3916
- Blocking #3918

## Changelog

## 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: Brian Bergeron <[email protected]>
Co-authored-by: Elliot Winkler <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bring TokenDetectionController up-to-date and release new version of assets-controllers
2 participants