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: osnap bot on disputed proposals #4768

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

Reinis-FRP
Copy link
Contributor

Motivation

#4765 introduced side effects on oSnap bot:

  • when proposing, it would not consider proposals that had been disputed (as intended by the fix), but this also extends to proposals from another snapshot votes that had been disputed and happens to have matching tx content with a new snapshot vote that has not been disputed since results of getUndiscardedProposals now can include also discarded proposals and are used as inputs in filterUnblockedProposals
  • when disputing, it would consider all proposals, even those that had been disputed and are still in challenge window. this would result in bot error logging the failed dispute attempts since the proposal has been discarded from prior dispute
  • when executing, it would also consider all disputed proposals that are past their challenge windows. this results in bot info logging the failed execution attempts since the proposal has been discarded by dispute.

Summary

This addresses the issue by splitting the getUndiscardedProposals in 2 functions (getAllProposals and filterUndiscardedProposals) and it is up to the caller which to use depending on context and reproposeDisputed flag.

Testing

Check a box to describe how you tested these changes and list the steps for reviewers to test.

  • Ran end-to-end test, running the code as in production
  • New unit tests created
  • Existing tests adequate, no new tests required
  • All existing tests pass
  • Untested

Issue(s)

Fixes #XXXX

Signed-off-by: Reinis Martinsons <[email protected]>
).flat();
// Filters all proposals that have not been discarded. Discards are most likely due to disputes, but can also occur on
// OOv3 upgrades.
const filterUndiscardedProposals = async (
Copy link
Contributor

@md0x md0x Jul 17, 2024

Choose a reason for hiding this comment

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

nit: Should we call this removeDeletedProposals? I find the current name a bit confusing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

makes sense, pushed the fix for rename

Signed-off-by: Reinis Martinsons <[email protected]>
Copy link
Contributor

@md0x md0x left a comment

Choose a reason for hiding this comment

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

LGTM

@Reinis-FRP Reinis-FRP merged commit 30ab4dd into master Jul 17, 2024
22 checks passed
@Reinis-FRP Reinis-FRP deleted the reinis-frp/filter-disputed-osnap branch July 17, 2024 13:44
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.

2 participants