Skip to content

fix: lower occurenceFloor for linea mainnet to 1#4253

Merged
sahar-fehri merged 7 commits intomainfrom
feat/update-occurenceFloor-for-linea
May 7, 2024
Merged

fix: lower occurenceFloor for linea mainnet to 1#4253
sahar-fehri merged 7 commits intomainfrom
feat/update-occurenceFloor-for-linea

Conversation

@sahar-fehri
Copy link
Copy Markdown
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 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
Copy Markdown
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
Copy Markdown
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
@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