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

feat: import pionex trades #277

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

feat: import pionex trades #277

wants to merge 4 commits into from

Conversation

Zahrun
Copy link
Contributor

@Zahrun Zahrun commented Dec 11, 2022

No description provided.

@vercel
Copy link

vercel bot commented Dec 11, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cryptotithe ❌ Failed (Inspect) Dec 18, 2022 at 10:25PM (UTC)

@@ -14,6 +15,8 @@ const parserMapping: {[key in EXCHANGES]: any} = {
[EXCHANGES.Kraken]: krakenParser,
[EXCHANGES.Poloniex]: poloniexParser,
[EXCHANGES.Revolut]: revolutParser,
[EXCHANGES.Pionex]: pionexParser,
Copy link
Owner

Choose a reason for hiding this comment

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

What exactly is the difference between pionex and the dust parser?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dust parser is for the dust collector feature in Pionex, it is in a separate csv file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With a different format, hence the different header

Copy link
Owner

Choose a reason for hiding this comment

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

Understood, I am not familiar with Pionex, I am just trying to understand if its actually an exchange, or if its simple transactions or incomes, or something else entirely?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is an exchange specialized in trading bots, with mostly trades and transactions.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah but what is this dust collector? Are they actually trades too?

I think it might be better(assuming these are trades), to change the format so each exchange supports an array of hashes to match against and then in your parser you can check to see if its dust or not and then process based on that. So users dont need to choose

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"Dust Collector is a feature on Pionex exchange that helps you collect all the small left behind assets into a whole. For example, when you convert btc to usdt there are a little fraction that is left behind. The dust collector helps you collect them."

I have seen that on other exchanges too. I think legally it must be considered a trade, at least in my country.
It allows converting very small amount of crypto into USDT usually when the amount is too small to do a normal trade.

Another use is when there are many different coins left, even if the amount is not that small, it will convert all into one coin.

I like your approach, so I will try to implement that testing against an array.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But that will mess with my other PRs. Let’s try to merge the other ones first.

Copy link
Owner

Choose a reason for hiding this comment

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

Okay going to leave this one then as is and work on reviewing the other ones and getting them merged first

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I included a way to support multiple hashes which is very little breaking, so that it will be easy to merge

src/parsers/trades/pionex/index.ts Outdated Show resolved Hide resolved
src/parsers/trades/pionex/index.ts Show resolved Hide resolved
src/parsers/trades/pionex/index.ts Show resolved Hide resolved
src/parsers/trades/pionex/index.ts Outdated Show resolved Hide resolved
src/parsers/trades/pionex/index.ts Outdated Show resolved Hide resolved
@Zahrun
Copy link
Contributor Author

Zahrun commented Dec 18, 2022

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.

2 participants