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: don't check the "done" state when processing a Cancelled event #121

Conversation

gustavogama-cll
Copy link
Contributor

The RBACTimelock contract does not mark an operation as done when it's cancelled. So the isDone(operationID) check we had in the "Cancelled" event handler actually prevented the timelock worker service from properly removing the operation from the scheduler.

This PR simply removes the isDone() check and adds an integration test to verify that we're properly de-scheduling the operation.

(depends on PR #118)

@gustavogama-cll gustavogama-cll marked this pull request as ready for review November 30, 2024 04:20
@gustavogama-cll gustavogama-cll requested a review from a team as a code owner November 30, 2024 04:20
Base automatically changed from chore-convert-operation-tests-into-unit-tests to develop December 2, 2024 02:12
@graham-chainlink
Copy link

Got a merge conflict now that the other is merged.

@gustavogama-cll gustavogama-cll force-pushed the fix-dont-check-done-state-when-processing-cancelled-event branch from 93ad248 to 8e580a6 Compare December 2, 2024 15:41
@gustavogama-cll
Copy link
Contributor Author

Got a merge conflict now that the other is merged.

Thanks. The conflict is now fixed.

The RBACTimelock contract does not mark an operation as `done` when it's
cancelled. So the `isDone(operationID)` check we had in the "Cancelled"
event handler actually prevented the timelock worker service from
properly removing the operation from the scheduler.

This PR simply removes the `isDone()` check and adds an integration test
to verify that we're properly de-scheduling the operation.
@gustavogama-cll gustavogama-cll force-pushed the fix-dont-check-done-state-when-processing-cancelled-event branch from 8e580a6 to f74d80d Compare December 13, 2024 16:09
@gustavogama-cll gustavogama-cll added this pull request to the merge queue Dec 27, 2024
Merged via the queue into develop with commit 3676476 Dec 27, 2024
5 checks passed
@gustavogama-cll gustavogama-cll deleted the fix-dont-check-done-state-when-processing-cancelled-event branch December 27, 2024 02:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants