-
Notifications
You must be signed in to change notification settings - Fork 24
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
Sync Potentially Completed Challenges With a Max Lookback for Refund Purposes #705
Conversation
@rauljordan, did you already merge |
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.
I'm approving this because all of my comments are really optional and I am okay if it gets merged as it is. But, consider them, please.
require.NoError(t, err) | ||
it, err := cmBindings.FilterEdgeAdded(nil, nil, nil, nil) | ||
require.NoError(t, err) | ||
for it.Next() { |
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.
Why doesn't this one have a similar pattern to above where we have a loop as long as the neutral context is alive and then keep working through the filterEdgeAdded iterator until we find everything we're looking for with a half-second delay to keep from spamming the backend?
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.
We don't know how many essential edges we will have, as it depends on the test. I guess it could be computed from the test params, but given it passes as is, I'd rather keep it simple for now
This PR ensures that an honest validator can "lookback" a set number of blocks on startup to sync any challenges that exist since that point until the present. The current behavior only "syncs" from the latest confirmed assertion. However, changes in #634 mandate that we must be able to continue playing challenge games even after the top-level assertion has been completed such that we can confirm essential edges for refund purposes.
We also add an e2e test that the validator will be able to confirm essential edges by resyncing even if the challenged assertion was confirmed.