Restart tweaks #161
Restart tweaks #161
Conversation
sdk+ (zkevm)
| if (px != undefined) { | ||
| this.idxPrices.set(indices[k], px); | ||
| } | ||
| updatedIndices.push(indices[k]); |
There was a problem hiding this comment.
do we need to add indices[k] to updatedIndices if the price px is undefined?
There was a problem hiding this comment.
px can only be undefined if ts.get call fails, which would trigger try/catch with ERR TSDB: the key does not exist
There was a problem hiding this comment.
ok so then the lines 140 to 142 are useless and they should be removed
There was a problem hiding this comment.
px_ts can actually be null if there are no entries in the timeseries, but the key exists. Updated.
There was a problem hiding this comment.
what is the actual change in this file other than formatting?
There was a problem hiding this comment.
nothing, this is the default prettier formatting
There was a problem hiding this comment.
https://stackoverflow.com/questions/2214480/committing-when-changing-source-formatting
"23
Yes. If you need to do whitespace changes, doing them in a separate commit that contains only this kind of cleanup is the best practice. This avoids problems with trying to see what part of a giant diff is actual code changes, and what part is just formatting (cosmetic) changes.
That said, you should try to keep these kind of changes to a minimum, and only do it at all when it is necessary and compatible with whatever coding standards are used in your company / community / project / etc."
There was a problem hiding this comment.
moved to different PR
There was a problem hiding this comment.
as discussed we should make this thread safe:
- this.isInitialized=true only after update
- change update accordingly for example with an optional argument
- this.isInitialized should be checked on price updates (and potentially other functions that would send out funding rates)
There was a problem hiding this comment.
This is not needed, since initial update runs before we start http or websockets server
There was a problem hiding this comment.
what is the change in this file other than formatting?
| console.log("onUpdateMarkPrice: eventListener not initialized"); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
if this is possible, then it is also possible that we receive the event between assigning this.isInitialized=true and calling this._update
There was a problem hiding this comment.
this is now changed, we set isInitialized only after first update call
| // Do not allow 0 funding rate to be sent out | ||
| if (fundingRate === 0) { | ||
| console.log(`[onUpdateMarkPrice] funding rate is 0 for ${perpetualId}`); | ||
| return; |
There was a problem hiding this comment.
this should be an error. Because if the code is correct we never run into this situation
| console.log(`[updateMarkPrice] funding rate is 0 for ${perpetualId}`); | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
same as above, should be an error
There was a problem hiding this comment.
@b1674927 are you sure it is not possible to have 0 fundingRate for a perpetual? On arbitrum sepolia perp 30004 currently has 0 funding rate:
{
"id": 300004,
"state": "NORMAL",
"baseCurrency": "WIF",
"quoteCurrency": "USD",
"indexPrice": 1,
"collToQuoteIndexPrice": 1.000005,
"markPrice": 1,
"midPrice": 1,
"currentFundingRateBps": 0,
"openInterestBC": 0,
"isMarketClosed": false
}This would basically crash the backend
There was a problem hiding this comment.
Good observation, as soon as the perp is traded for the first time, the funding rate is non-zero.
| // | ||
| // 2024-07-02: this exchangeInfo call does not actually issue the | ||
| // "update" call, since initialization is not yet done in | ||
| // EventListener when priceInterfaceInitialize is called. | ||
| const info = await this.sdkInterface.exchangeInfo(); |
There was a problem hiding this comment.
so should we remove it then?
There was a problem hiding this comment.
no, the info is needed for _initIdxNamesToPerpetualIds
This PR fixes a bug where fundingRate and openInterest values are not set on service start and additionally fixes potential restarts on redis
TS.GETcalls when key does not exist