-
-
Notifications
You must be signed in to change notification settings - Fork 7.3k
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
livecoin: fetchOrder() implemented #3001
Conversation
js/livecoin.js
Outdated
let datetime = undefined; | ||
if ('lastModificationTime' in order) { | ||
timestamp = this.safeInteger (order, 'lastModificationTime'); | ||
if (!timestamp) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this line be like
if (timestamp) {
// if it is defined
// it is an integer already, should multiply by a 1000, if needed
} else {
// otherwise parse it as a string
// parse8601...
}
? ;)
The problem with safeInteger there is that for an ISO-string like this.safeInteger ({ 'foo': '2018-01-01T00:00:00')) }, 'foo')
will return 2018
. So we should check if it's an ISO string first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess, the correct code for it is:
let timestamp = this.safeString (order, 'lastModificationTime')
if (typeof timestamp !== 'undefined') {
if (timestamp.indexOf ('T') >= 0) {
timestamp = this.parse8601 (timestamp);
} else {
timestamp = this.safeInteger (order, 'lastModificationTime');
// or
// timestamp = parseInt (order['lastModificationTime']);
}
}
How does it sound to you?
Fixed. |
js/livecoin.js
Outdated
} else { | ||
side = 'buy'; | ||
let price = this.safeFloat (order, 'price'); | ||
let cost = this.safeFloat (order, 'commissionByTrade'); // commission_rate? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cost
of an order is:
if (status === 'open' and filled === 0) { amount * price }
if (status === 'closed' || status === 'canceled') { filled * price }
The cost
of a trade is amount * price
.
So, an order cost
or a trade cost
means the total quote volume of the order (whereas amount
is the base volume). The cost
field itself is there mostly for convenience and can be deduced from other fields.
I'll add that to the Manual )
As for the unified fee
sub-structure, it should be like this:
'fee': {
'cost': 123.45,
'rate': 0.001,
'currency': 'ETH',
}
@kroitor Fixed. Please review if I have got it right. |
Almost ) But no worries, I'll merge it soon and you'll see the edits I made from there. Thx so much! |
No description provided.