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.

Sign up for GitHub

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

Jump to bottom

audit: invert conditions in x/gov in EndBlocker for clarity in reading and later bug fixing #16807

Closed
odeke-em opened this issue Jul 1, 2023 · 0 comments · Fixed by #16850
Closed

audit: invert conditions in x/gov in EndBlocker for clarity in reading and later bug fixing #16807

odeke-em opened this issue Jul 1, 2023 · 0 comments · Fixed by #16850
Labels
Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Comments

@odeke-em
Copy link
Collaborator

odeke-em commented Jul 1, 2023

Summary of Bug

The code in here has this comment

cosmos-sdk/x/gov/abci.go

Lines 90 to 94 in 8c72f66

// If an expedited proposal fails, we do not want to update
// the deposit at this point since the proposal is converted to regular.
// As a result, the deposits are either deleted or refunded in all cases
// EXCEPT when an expedited proposal fails.
if !(proposal.Expedited && !passes) {

But really we can avoid the double inversion and make it super clear as per the comment by:

if passes || !proposal.Expedited {
   ...
}

/cc @elias-orijtech
@odeke-em odeke-em added the Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. label Jul 1, 2023
odeke-em added a commit that referenced this issue Jul 6, 2023
@odeke-em
fix: x/gov: invert & simplify EndBlocker conditions for expedited pro…
61fee74 
…posals

Inverts the condtion for updating deposits for either failing or unexpedited
proposals to make it simpler to read and maintain for the future.

Fixes #16807
@odeke-em odeke-em mentioned this issue Jul 6, 2023
Merged
odeke-em added a commit that referenced this issue Jul 6, 2023
@odeke-em
fix: x/gov: invert & simplify EndBlocker conditions for expedited pro…
2efb2b3 
…posals

Inverts the condtion for updating deposits for either failing or unexpedited
proposals to make it simpler to read and maintain for the future.

Fixes #16807
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

style(x/gov): invert & simplify EndBlocker conditions for expedited proposals cosmos/cosmos-sdk
1 participant
@odeke-em