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

Two checks can be summarized in one #2175

Closed
andrijamitrovic23 opened this issue Jul 27, 2023 · 0 comments · Fixed by #2179
Closed

Two checks can be summarized in one #2175

andrijamitrovic23 opened this issue Jul 27, 2023 · 0 comments · Fixed by #2179
Assignees
@rootulp
Labels
audit item is from an audit refactor optional label for items that are related to implementation work and do not change functionality

Comments

@andrijamitrovic23
Copy link

This check within roundUpByMultipleOf function, which treats cursor == 0 and cursor %v==0 as two different cases can be summarized into a single cursor % v == 0 (because 0 % i is always 0, for any i).
@rootulp rootulp self-assigned this Jul 27, 2023
@rootulp rootulp mentioned this issue Jul 27, 2023
Merged
@rootulp rootulp added refactor optional label for items that are related to implementation work and do not change functionality audit item is from an audit and removed needs:triage labels Jul 27, 2023
@rootulp rootulp added this to the Post-mainnet milestone Jul 27, 2023
rootulp added a commit that referenced this issue Jul 28, 2023
@rootulp
refactor: simplify roundUpByMultipleOf (#2179)
90f303c 
Closes #2175

This [test
case](https://github.com/rootulp/celestia-app/blob/e9f5c157e56e7aff232250ae4a1aac58dfa2336f/pkg/shares/blob_share_commitment_rules_test.go#L312-L316)
still passes.
cmwaters pushed a commit to celestiaorg/go-square that referenced this issue Dec 14, 2023
@rootulp
refactor: simplify roundUpByMultipleOf (#2179)
aaf22bb 
Closes celestiaorg/celestia-app#2175

This [test
case](https://github.com/rootulp/celestia-app/blob/e9f5c157e56e7aff232250ae4a1aac58dfa2336f/pkg/shares/blob_share_commitment_rules_test.go#L312-L316)
still passes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rootulp rootulp

Labels
audit item is from an audit refactor optional label for items that are related to implementation work and do not change functionality
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

refactor: simplify roundUpByMultipleOf rootulp/celestia-app
2 participants
@rootulp @andrijamitrovic23