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

fix: lower occurenceFloor for linea mainnet to 1 #4253

Merged
merged 7 commits into from
May 7, 2024

Conversation

sahar-fehri
Copy link
Contributor

@sahar-fehri sahar-fehri commented May 3, 2024

Explanation

Lower occurenceFloor for linea mainnet to only 1 list instead of 3.
Add a filter for lineaTeam aggregator when we fetch token list from token-api when the user is on linea-mainnet.

References

Changelog

@metamask/assets-controllers

  • CHANGED: Changed query parameter occurenceFloor in getTokensURL to equal 1 for linea-mainnet and default to 3 for other networks.
  • Added: Added a filter for lineaTeam aggregator on the returned list by fetchTokenListByChainId when the user is on linea mainnet.

Checklist

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

@sahar-fehri sahar-fehri requested a review from a team as a code owner May 3, 2024 18:10
@sahar-fehri sahar-fehri marked this pull request as draft May 3, 2024 18:12
@sahar-fehri sahar-fehri marked this pull request as ready for review May 6, 2024 16:26
hurricanenara
hurricanenara previously approved these changes May 6, 2024
hurricanenara
hurricanenara previously approved these changes May 6, 2024
return parseJsonResponse(response);
const result = await parseJsonResponse(response);
if (Array.isArray(result) && chainId === ChainId['linea-mainnet']) {
return result.filter((elm) => elm.aggregators.includes('lineaTeam'));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return result.filter((elm) => elm.aggregators.includes('lineaTeam'));
return result.filter((elm) => elm.aggregators.includes('lineaTeam') || elm.aggregators.length >= 3);

What do you think about this? This still includes 100% of whats in the lineaTeam list. But continues allowing tokens that made it into 3 sources. Otherwise we could be excluding tokens that used to appear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you 🙏 for record, added the OR statement to avoid disappearing tokens that used to appear to users before because they were on +3 lists different than lineaTeam

@sahar-fehri sahar-fehri merged commit 68b8de7 into main May 7, 2024
143 checks passed
@sahar-fehri sahar-fehri deleted the feat/update-occurenceFloor-for-linea branch May 7, 2024 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants