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.

Sign up for GitHub

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

Jump to bottom

[token-detection-controller] Remove limit on number of tokens that can be detected #3661

Closed
MajorLift opened this issue Dec 13, 2023 · 1 comment · Fixed by #3938
Closed

[token-detection-controller] Remove limit on number of tokens that can be detected #3661

MajorLift opened this issue Dec 13, 2023 · 1 comment · Fixed by #3938
Assignees
@MajorLift
Labels
bug Something isn't working team-wallet-framework

Comments

@MajorLift
Copy link
Contributor

MajorLift commented Dec 13, 2023 •
edited
Loading

Currently, the detectTokens method in TokenDetectionController has a hardcoded upper limit of 2000 on the number of tokens it can process.

  • This logic should be rewritten so that any number of tokens can be processed in batches of 1000.
  • Unit tests need to be written.
@MajorLift MajorLift added bug Something isn't working team-wallet-framework labels Dec 13, 2023
@MajorLift MajorLift mentioned this issue Dec 13, 2023
Merged
3 tasks
@DhifallahOussema
Copy link

Hello, I'm intrested in this issue and wanted to know if I'm still able to solve it.
I thought about a possible approach where we run a loop that splits the tokensToDetect into slices of 1000 token each.
Please let me know if it's still available so I can add a PR.

@MajorLift MajorLift mentioned this issue Feb 23, 2024
Merged
3 tasks
@bergeron bergeron mentioned this issue Feb 27, 2024
Merged
3 tasks
MajorLift added a commit that referenced this issue Feb 27, 2024
@MajorLift @bergeron
Enable detection of arbitrary number of tokens in batches of 1000
f5c3194 
- Fixes #3661

Co-authored-by: Brian Bergeron <[email protected]>
MajorLift added a commit that referenced this issue Feb 27, 2024
@MajorLift @bergeron
Enable detection of arbitrary number of tokens in batches of 1000
f060ced 
- Fixes #3661

Co-authored-by: Brian Bergeron <[email protected]>
@MajorLift MajorLift mentioned this issue Feb 27, 2024
Open
@MajorLift MajorLift self-assigned this Mar 1, 2024
MajorLift added a commit that referenced this issue Mar 4, 2024
@MajorLift @bergeron @mcmire
[token-detection-controller] Refactor detectTokens method (#3938)
34bf8f0 
## 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
Assignees

@MajorLift MajorLift

Labels
bug Something isn't working team-wallet-framework
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[token-detection-controller] Refactor detectTokens method MetaMask/core
2 participants
@MajorLift @DhifallahOussema