-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
Delta: fetchSettlementHistory, createExpiredOptionMarket #18675
Conversation
Added fetchSettlementHistory and createExpiredOptionMarket to Delta: ``` delta.fetchSettlementHistory (BTC/USDT:USDT-130723-29000-C) 2023-07-27T03:16:20.747Z iteration 0 passed in 4150 ms symbol | price | timestamp | datetime --------------------------------------------------------------------------------------- BTC/USDT:USDT-130723-29000-C | 1555.784058200865 | 1689249600000 | 2023-07-13T12:00:00Z 1 objects ``` ``` delta.fetchSettlementHistory (C-BTC-29000-130723) 2023-07-27T03:17:03.101Z iteration 0 passed in 4590 ms symbol | price | timestamp | datetime --------------------------------------------------------------------------------------- BTC/USDT:USDT-130723-29000-C | 1555.784058200865 | 1689249600000 | 2023-07-13T12:00:00Z 1 objects ```
const result = this.safeValue (response, 'result', []); | ||
const settlements = this.parseSettlements (result, market); | ||
const sorted = this.sortBy (settlements, 'timestamp'); | ||
return this.filterBySymbolSinceLimit (sorted, market['symbol'], since, limit); |
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.
hum for some reason, the python version returns an empty array
LGTM
|
Delta: fetchSettlementHistory, createExpiredOptionMarket [ci skip]
} else if (symbol in this.markets_by_id) { | ||
const markets = this.markets_by_id[symbol]; | ||
return markets[0]; | ||
} else if ((symbol.indexOf ('-C') > -1) || (symbol.indexOf ('-P') > -1) || (symbol.indexOf ('C')) || (symbol.indexOf ('P'))) { |
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.
@Dan-krm This logic isn't good for a few reasons.
- first, symbol.indexOf ('C') evaluates to
-1
so the boolean version of this is true, so this evaluates to true whenever the letterC
orP
is not present, but it also evaluates to true whenever C/P is at any index other than 0, the only timesymbol.indexOf ('C')
evaluates to false is when C is the first letter of symbol, but then because we also havesymbol.indexOf ('P')
right after, this line will always evaluate to true, because the first letter ofsymbol
can't be bothC
andP
.
you can test this in node
const symbol = "market";
undefined
> Boolean(symbol.indexOf ('P'))
true
- second, you can't just test for the presence of
C
orP
to see if the symbol is an option symbol, by this logic, this symbol would be an option symbolETH/BTC
can you update this line to use some logic that is correct? Thanks
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.
@samgermain Yeah definitely, I'll adjust this
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.
Added fetchSettlementHistory and createExpiredOptionMarket to Delta:
https://www.delta.exchange/app/expired_futures_and_options