feat: support gate.io trades import#279
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
starsoccer
left a comment
starsoccer
left a comment
There was a problem hiding this comment.
If you have an example CSV that would be useful in order to comment on how we may be able to better do some things here
src/parsers/trades/gate.io/index.ts
Outdated
| if (data.length < 1) { | ||
| return internalFormat; | ||
| } | ||
| let feeTrade = data[0]; |
There was a problem hiding this comment.
Why are we setting all of these to 0?
There was a problem hiding this comment.
I was getting filled trade is read before assigned at line 53 tradeToAdd.boughtCurrency = filledTrade.Currency; so I assign it to something first. It should be assigned in the switch structure but that does not compile.
There was a problem hiding this comment.
Okay so just to confirm Im understanding this right basically the reason your doing this is because 2 lines = 1 trade. One line is for the bought amount, then another for the sold amount right?
There was a problem hiding this comment.
Trades are on three lines and "Points Purchase" are on two lines, yes
There was a problem hiding this comment.
One of the lines of trades is the fee and the fee currency.
I had configured to pay the fee in POINT, which is not the bought currency nor the sold currency.
I thought that transactionFee was for that but you told me it’s not. So how to reflect this trade with 3 currencies?
There was a problem hiding this comment.
Can’t we use transactionFee and transactionFeeCurrency also for tradeFee and tradeFeeCurrency?
The ideas are quite similar. To detect if it is a transactionFee or a tradeFee, we can test if one of the soldCurrency or boughtCurrency are fiat.
There was a problem hiding this comment.
They are similiar but serve different purposes. For instance transactionFee is more intended for on chain exchanges. So like trading USDC to some other token and paying for the transaction with ETH
There was a problem hiding this comment.
Okay then let's implement tradeFee. See a strart in #281
There was a problem hiding this comment.
I am going to make a discussion on it first as I dont want to implement this until some of the wasm stuff Ive done is merged in as that will cause breaking changing
There was a problem hiding this comment.
I pushed a better management of trades over multiples lines. And a basic support for tradeFee, at least for import
src/parsers/trades/gate.io/index.ts
Outdated
| } | ||
| } | ||
| tradeToAdd.ID = createID(tradeToAdd); | ||
| tradeToAdd.exchangeID = tradeToAdd.ID; |
There was a problem hiding this comment.
I believe you should be able to move this up to the top. Also ideally we should set this before we create the ID
There was a problem hiding this comment.
So ID would be set to createID after the initial partial trade setting?
I modified the code from the binance parser and followed that structure.
How come exchangeID = trade.ID?
There was a problem hiding this comment.
Yes, and good point the binance parser should be updated. Its outdated and now not making IDs properly.
Ideally when we fix it we should bump the version and make part of the migration process to recompute all trade ids as they will all change then
There was a problem hiding this comment.
I also plan to update the Binance parser. I have exported the data from Binance with the "all statements" feature.
All kind of operations are there.
In that case, should we import trades, transactions, and income all at once?
Or should I make a parser for each type, all of them taking only the concerned type of operations?
Bump the version you mean from "version":"0.8.1", and in migration recalculate all IDs, ok
There was a problem hiding this comment.
@starsoccer Please if you can tell your opinion about this:
"I also plan to update the Binance parser. I have exported the data from Binance with the "all statements" feature.
It contains operations of all different kinds.
In that case, should we import trades, transactions, and income all at once?
Or should I make a parser for each type, all of them taking only the concerned type of operations?"
It is not the case only with Binance, also with other platforms.
I would say import all at once, but I’m also ok with implementing it as separate, since it is the current structure.
There was a problem hiding this comment.
I cant seem to open that file. Is that a zip or csv?
And regarding the binance question, I am fine with it all being in one parser, but we may need to do some tweaking to the current import design because I believe it asks for type first(transaction, income, or trade)
There was a problem hiding this comment.
|
Please see an example file |
2 participants
No description provided.