-
Notifications
You must be signed in to change notification settings - Fork 1.2k
prevent eas from going backward when increasing cgc #15966
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
base: develop
Are you sure you want to change the base?
Conversation
| } | ||
|
|
||
| // When increasing custody group count, prevent earliestAvailableSlot from going backward. | ||
| if storedEarliestAvailableSlot != 0 && earliestAvailableSlot < storedEarliestAvailableSlot { |
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.
Not tested
| // When increasing custody group count, prevent earliestAvailableSlot from going backward. | ||
| if storedEarliestAvailableSlot != 0 && earliestAvailableSlot < storedEarliestAvailableSlot { | ||
| return errors.Errorf( | ||
| "cannot decrease earliest available slot from %d to %d when increasing custody group count from %d to %d. ", |
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.
No need to add a period at the end of the error.
nalepae
left a comment
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 don't see how this PR solves the initial issue.
For sure, if such a thing happens, it will now error.
But the bug will still happen.
What type of PR is this?
Bug fix
What does this PR do? Why is it needed?
Which issues(s) does this PR fix?
#15938
Other notes for review
Currently in the process of reproducing the issue locally (kurtosis or otherwise). Only after that we can be sure if the fix in this PR actually fixes this issue.
Edit: I am yet to reproduce but the change in this PR should be there anyway even if it doesn't solve the bug.
Acknowledgements