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

chore(apps/whale-api): update poolpairs to ignore token symbols with BURN #2085

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

veralygit
Copy link

@veralygit veralygit commented Mar 22, 2023

What this PR does / why we need it:

Updated poolpair.controller.ts
to ignore symbols with BURN eg BURN, BURN2 and not only BURN

and poolpair.prices.services.ts to include symbols with BURN as untradeableToken

Which issue(s) does this PR fixes?:

Fixes #

Additional comments?:

This PR is to be put on hold as we are exploring on showing BURN without implying that they are tradable

@codeclimate
Copy link

codeclimate bot commented Mar 22, 2023

Code Climate has analyzed commit 60a5a95 and detected 0 issues on this pull request.

View more on Code Climate.

@netlify
Copy link

netlify bot commented Mar 22, 2023

Deploy Preview for jellyfishsdk ready!

Name Link
🔨 Latest commit 60a5a95
🔍 Latest deploy log https://app.netlify.com/sites/jellyfishsdk/deploys/641c0c406afb4b0007d55637
😎 Deploy Preview https://deploy-preview-2085--jellyfishsdk.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov
Copy link

codecov bot commented Mar 22, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +7.53 🎉

Comparison is base (b3a3b03) 83.78% compared to head (60a5a95) 91.31%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2085      +/-   ##
==========================================
+ Coverage   83.78%   91.31%   +7.53%     
==========================================
  Files         363      359       -4     
  Lines       10981    10858     -123     
  Branches     1433     1416      -17     
==========================================
+ Hits         9200     9915     +715     
+ Misses       1680      902     -778     
+ Partials      101       41      -60     
Impacted Files Coverage Δ
...ps/whale-api/src/module.api/poolpair.controller.ts 97.89% <100.00%> (+21.05%) ⬆️
...hale-api/src/module.api/poolpair.prices.service.ts 95.74% <100.00%> (ø)

... and 124 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@kyleleow
Copy link
Member

kyleleow commented Mar 23, 2023

I am thinking about this again, should we consider filtering out the poolpair that has status: false instead?
I checked that currently status: false poolpair are the one after stock split and our burn bot, e.g.: dTSLA-dDUSD/v1, dGOOGL-dDUSD/v1, dBURN2-DUSD

And on product side we are manually filtering again ..v1 poolpair, e.g.:
https://github.com/WavesHQ/walletkit/blob/8a3cc3d66697a0ca483c4cd29100f088ab6b104b/packages/walletkit-ui/src/store/wallet.ts#L224-L226

For backward compatibility, maybe we should by default return all pair, and accept a new param that will filter out status: false? Or the other way round, i.e.: default to return filtered pair, accept a new param to return unfiltered pairs

Update from ain team: status: false in means that poolpair will not be able to execute swap, but it is still free to add/remove liquidity

Update (23/03/2023 16:23): we will not be checking status: false because we have use case to display the poolpair before we want to make it live

@veralygit veralygit changed the title chore(apps/whale-api): update poolpair controller to ignore symbols with BURN chore(apps/whale-api): update poolpair controller and prices service to ignore symbols with BURN Mar 23, 2023
@veralygit veralygit changed the title chore(apps/whale-api): update poolpair controller and prices service to ignore symbols with BURN chore(apps/whale-api): update poolpairs to ignore token symbols with BURN Mar 23, 2023
@kyleleow kyleleow self-requested a review March 23, 2023 08:49
@veralygit veralygit marked this pull request as draft March 24, 2023 02:35
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.

None yet

2 participants