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

Separated fuzz tests, added pool_creation #542

Merged
merged 37 commits into from
Jul 6, 2022
Merged

Conversation

Chralt98
Copy link
Member

@Chralt98 Chralt98 commented Apr 6, 2022

Related to [Swaps] Improve fuzz tests #290

The approach at the moment is to fuzz test each dispatchable of the swaps pallet with random and valid pool id's.

The challenge is to check multiple combinations of valid pool id's and these dispatchables. Therefore it would be wasted time to check and find invalid pool id's.

@Chralt98 Chralt98 added the s:in-progress The pull requests is currently being worked on label Apr 6, 2022
@sea212 sea212 linked an issue Apr 6, 2022 that may be closed by this pull request
@Chralt98
Copy link
Member Author

Chralt98 commented Apr 14, 2022

The fuzz extrinsics tests need further modifications to not fail at the first error possibility 100% of the time, but for now the valid pool creation works, the code is separated and the fuzz tests should be valid for a review. Later the Shares::free_balance checks need to get the Shares::deposit (balances of an asset) in the fuzz tests.

@Chralt98 Chralt98 marked this pull request as ready for review April 14, 2022 11:02
@Chralt98 Chralt98 added the i:needs-audit The changes contained in this PR require an audit label Apr 14, 2022
@sea212 sea212 added s:review-needed The pull request requires reviews and removed s:in-progress The pull requests is currently being worked on i:needs-audit The changes contained in this PR require an audit labels Apr 14, 2022
@Chralt98 Chralt98 self-assigned this Apr 25, 2022
@maltekliemann maltekliemann added this to the v0.3.4 milestone May 11, 2022
@Chralt98 Chralt98 requested review from maltekliemann and removed request for sea212 May 30, 2022 10:05
Copy link
Member

@maltekliemann maltekliemann left a comment

Choose a reason for hiding this comment

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

At least once the fuzz tests panicked with a duplicate asset. Nevermind, I ran an old build. 😬

There are a couple of low-priority critiques, mostly lack of randomization. But other than that, looks like a very useful abstraction of the workflow. 👍

zrml/swaps/fuzz/utils.rs Outdated Show resolved Hide resolved
zrml/swaps/fuzz/utils.rs Outdated Show resolved Hide resolved
zrml/swaps/fuzz/utils.rs Outdated Show resolved Hide resolved
zrml/swaps/fuzz/pool_join.rs Outdated Show resolved Hide resolved
zrml/swaps/fuzz/utils.rs Show resolved Hide resolved
zrml/swaps/fuzz/utils.rs Show resolved Hide resolved
zrml/swaps/fuzz/utils.rs Outdated Show resolved Hide resolved
zrml/swaps/fuzz/utils.rs Show resolved Hide resolved
scripts/tests/fuzz.sh Show resolved Hide resolved
zrml/swaps/fuzz/utils.rs Outdated Show resolved Hide resolved
Copy link
Member

@maltekliemann maltekliemann left a comment

Choose a reason for hiding this comment

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

Yeah, looks good to me! 👍

@Chralt98 Chralt98 added s:accepted This pull request is ready for merge and removed s:review-needed The pull request requires reviews labels Jul 6, 2022
@Chralt98 Chralt98 merged commit 464180f into main Jul 6, 2022
@sea212 sea212 modified the milestones: v0.3.5, v0.3.4 Jul 7, 2022
@Chralt98 Chralt98 deleted the swaps_tests_improvement branch July 19, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
s:accepted This pull request is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Swaps] Improve fuzz tests
4 participants