Skip to content
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

fix: handle report resubmit case #601

Conversation

Jeday
Copy link
Contributor

@Jeday Jeday commented Feb 11, 2023

Description

Scenario

  • initial report is submitted at refSlot1
  • next report is submitted at refSlot2, initial report is missed, warning event fired
    • WarnProcessingMissed(refSlot1)
  • next report is re-agreed by consensus and resubmitted with new data
    • expected: WarnProcessingMissed(refSlot1)
    • actual: WarnProcessingMissed(refSlot2)

Solution

At resubmit we lack information about slot missed at prev submitting. So proposed solution is to prevent fire of WarnProcessingMissed event at resubmit. What will be achieved:

  • Upholding 1 missed report - 1 event condition
  • WarnProcessingMissed argument can be trusted to contain actual missed slot

Testing notes

Corresponding tests are located in the branch feature/shapella-upgrade-tests-base-oracle

@ujenjt
Copy link
Member

ujenjt commented Feb 11, 2023

looks valid, @skozin could you check plz

@ujenjt
Copy link
Member

ujenjt commented Feb 11, 2023

also, we don't have an event for slot re-agreement occurrences. Maybe let's add one for monitoring purposes?

Copy link
Contributor

@TheDZhon TheDZhon left a comment

Choose a reason for hiding this comment

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

I think the fix should be applied, though waiting for Sam's feedback

@Jeday
Copy link
Contributor Author

Jeday commented Feb 12, 2023

also, we don't have an event for slot re-agreement occurrences. Maybe let's add one for monitoring purposes?

This would be more relevant in HashConsensus contract. I think it can be implemented fairly easily. Though this brings us to question: if consensus can be reached again with different result and report submitted, why can't report be withdrawn if consensus is lost?

@skozin
Copy link
Member

skozin commented Feb 12, 2023

also, we don't have an event for slot re-agreement occurrences. Maybe let's add one for monitoring purposes?

good idea, will add in a separate PR

@skozin
Copy link
Member

skozin commented Feb 12, 2023

I'd say that firing WarnProcessingMissed(refSlot1) twice is not the expected behavior, it's also incorrect. And the expected one is not firing the event twice, exactly the one this PR implements.

@skozin skozin changed the base branch from feature/shapella-upgrade to feature/shapella-upgrade-tests-base-oracle February 12, 2023 13:31
@skozin skozin merged commit cda3398 into feature/shapella-upgrade-tests-base-oracle Feb 12, 2023
@skozin skozin deleted the fix/repeating-missed-report-event branch February 12, 2023 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants