Skip to content
This repository has been archived by the owner on Sep 20, 2023. It is now read-only.

Submit on FM startup #119

Merged
merged 3 commits into from
Mar 30, 2021
Merged

Submit on FM startup #119

merged 3 commits into from
Mar 30, 2021

Conversation

ebarakos
Copy link

Regarding #117, how about always sending a job request after first results from polling?

@ebarakos ebarakos requested a review from boxhock March 30, 2021 11:08
Copy link
Contributor

@boxhock boxhock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason we can't send a job request after the first polling finishes is that it would get too expensive for the node operator for stale reports. Don't really want to the node ops to waste a few hundred $$ just for restarting their EI 😅

@@ -179,6 +179,7 @@ func (fm *FluxMonitor) eventListener(ch <-chan interface{}) {
}

func (fm *FluxMonitor) canSubmitToRound(initiate bool) bool {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

@@ -256,6 +257,7 @@ func (fm *FluxMonitor) hitTrigger() {

func (fm *FluxMonitor) startPoller() {
fm.poll()
fm.checkAndSendJob(false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably move this to FM startup and when eligibility to submit changes. Although we haven't gotten that far yet, there will be cases where the poller is disabled - so we can't always rely on this to be triggered.

I'll make a new issue for making the poll interval and heartbeat "optional" - but for now I think we should just move this to the FM startup and when the FA state is updated to make the oracle eligible to submit.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But if we call it in an earlier state than here, before polling has started(e.g near the 1st call of canSubmitUpdated), we wont't have a result to send, right?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, so that function will see that it doesn't have any recent results and do it's own poll to get the latest median.

@ebarakos ebarakos requested a review from boxhock March 30, 2021 13:25
Comment on lines 218 to 226
if fm.latestResult.IsZero() {
logger.Warn("Polling because result is zero")
err := fm.poll()
if err != nil {
logger.Error("cannot retrieve result from polling")
return err
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't be necessary, since we're checking the age of the result just below this (L227). If this hasn't been set - it's always going to trigger a new poll.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True + it will start having an actual functionality :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep 👍

@ebarakos ebarakos force-pushed the fm/submit-on-startup branch from 91f813e to 0185493 Compare March 30, 2021 13:36
@ebarakos ebarakos requested a review from boxhock March 30, 2021 13:37
@ebarakos ebarakos merged commit 36087f8 into feature/fluxmonitor Mar 30, 2021
@ebarakos ebarakos deleted the fm/submit-on-startup branch March 30, 2021 13:39
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants