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

[CL][test]: Add range fuzz test covering negative interval accum cases #5934

Merged
merged 2 commits into from
Aug 5, 2023

Conversation

AlpinYukseloglu
Copy link
Contributor

Closes: #XXX

What is the purpose of the change

This PR adds a range fuzz test covering the edge case where initial accumulator values are negative. Even though the diff is small, this case covers quite complex behavior beyond what that the units tests in the original fix PRs touch.

The case added in this PR hits both the spread reward case and the incentive case and panics for both prior to the fixes being merged.

Once #5869 and #5872 are merged, we expect this to pass with no issues. Leaving this PR as a draft until then.

Testing and Verifying

This PR adds tests.

Documentation and Release Note

  • Does this pull request introduce a new feature or user-facing behavior changes?
  • Changelog entry added to Unreleased section of CHANGELOG.md?

Where is the change documented?

  • Specification (x/{module}/README.md)
  • Osmosis documentation site
  • Code comments?
  • N/A

@AlpinYukseloglu AlpinYukseloglu added V:state/compatible/no_backport State machine compatible PR, depends on prior breaks A:no-changelog labels Jul 31, 2023
@AlpinYukseloglu AlpinYukseloglu marked this pull request as ready for review August 5, 2023 17:21
Copy link
Member

@ValarDragon ValarDragon left a comment

Choose a reason for hiding this comment

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

LGTM, but we should ensure the functionality is hit in fuzz_test.go

@ValarDragon ValarDragon merged commit 962220f into main Aug 5, 2023
@ValarDragon ValarDragon deleted the alpo/neg-accum-fuzz branch August 5, 2023 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:no-changelog C:x/concentrated-liquidity V:state/compatible/no_backport State machine compatible PR, depends on prior breaks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants