Implement oracle subscriptions#787
Conversation
Release v1.280.0
The iterative variant massively reduces risk of stack overflows and is probably faster as well.
Move redundant code to functions.
This makes sure that fee reserve of the test contracts is only set when starting from scratch, not after loading a snapshot.
Ensures that notifications are in order of querying
Release v1.281.0
|
|
||
| logToConsole(L"Saving oracle subscriptions ..."); | ||
| sizeToSave = sizeof(OracleSubscription) * MAX_ORACLE_SUBSCRIPTIONS; | ||
| sz = saveLargeFile(ORACLE_SNAPSHOT_SUBSCRIPTIONS_FILENAME, sizeToSave, (unsigned char*)subscriptions, directory); |
There was a problem hiding this comment.
I am not sure, but we have nextSubscriptionIdQueue that built from subscriptions, should we rebuild/reload it or it's already been handled else where.
There was a problem hiding this comment.
Very good point. It had not been handled in the version that you have reviewed. I have cherry-picked the fix for this that I have made yesterday from the testnet branch.
src/oracle_core/net_msg_impl.h
Outdated
| break; | ||
| const uint16_t contractIndex = (uint16_t)request->reqTickOrId; | ||
| response->resType = RespondOracleData::respondSubscriptionIds; | ||
| const int32_t* subscriptionIds = nextSubscriptionIdQueue.data(); |
There was a problem hiding this comment.
This subscriptionIds variable is redundant or you want to use it for the following code ?
There was a problem hiding this comment.
Good catch as well. This is unused, originating from copy & paste. I have removed it.
src/oracle_core/oracle_engine.h
Outdated
| while (nextSubscriptionIdQueue.peek(subscriptionId)) | ||
| { | ||
| // Check if it is time to generate the next query | ||
| ASSERT(subscriptionId >= 0 && subscriptionId < usedSubscriberSlots); |
There was a problem hiding this comment.
subscriptionId < usedSubscriptionSlots instead of subscriptionId < usedSubscriberSlots ?
There was a problem hiding this comment.
Yes, you are right.
| if (subscription.firstSubscriberIndex >= 0) | ||
| { | ||
| // there is at least one additional subscriber -> update timestamp to next subscriber | ||
| subscription.nextQueryTimestamp = subscribers[subscription.firstSubscriberIndex].nextQueryTimestamp; |
There was a problem hiding this comment.
Beside updating the timestamp, do we need to update thing like line 933-935 in oracle_engine ?
I saw we update timestamp and re-add this subscription into nextSubscriptionIdQueue
There was a problem hiding this comment.
Great catch again. Thanks, you did an excellent review, @cyber-pc!
3 participants
Implements #673