feat: UbiquityPoolFacet fuzz tests

[gitcoindev]

Resolves #925

Skeleton based on UbiquityPoolFacet tests.

[gitcoindev]

This pull request is still a draft. It contains a skeleton for fuzz tests, I will keep updating and hopefully make as ready for review during or after the weekend.

[ubiquity-os-deployer]

809f149

eaa29b3

222355d

e9e0e6c

60651d5

478e4df

ab85a33

320f710

7dce7be

565aaa6

[gitcoindev]

I am playing with the fuzzer a lot and discovered an interesting case. When redeem threshold is set to $0.99 , the curve price must be larger than 990000999999999999 due to conversion between 6 digit and 18 digit curve prices in getDollarPriceUsd() <= poolStorage.redeemPriceThreshold comparison, i.e

vm.assume(dollarPriceUsd > 990000999999999999); // 0.99e18 , greater than redeem threshold

I was stuck for some time with the failing fuzz test but after debugging found the edge value. Fuzzer is great, as setting mocked price to 990000999999999998 it almost immediately finds this and reports the case.

[gitcoindev]

I will finish with the whole test suite till Friday EOD and set the pr as ready for review.

[rndquu]

I am playing with the fuzzer a lot and discovered an interesting case. When redeem threshold is set to $0.99 , the curve price must be larger than 990000999999999999 due to conversion between 6 digit and 18 digit curve prices in getDollarPriceUsd() <= poolStorage.redeemPriceThreshold comparison, i.e

vm.assume(dollarPriceUsd > 990000999999999999); // 0.99e18 , greater than redeem threshold

I was stuck for some time with the failing fuzz test but after debugging found the edge value. Fuzzer is great, as setting mocked price to 990000999999999998 it almost immediately finds this and reports the case.

This seems to be a low severity issue but keeping in mind that nobody from the Sherlock audit have reported this bug it seems that nobody fuzzed the UbiquityPoolFacet contract extensively.

[gitcoindev]

I pushed the remaining fuzz tests also for redemption delay, setting as ready for review now.

[rndquu]

@gitcoindev

1. Could you add 2 more fuzz tests:
   a) If user mints X Dollars he gets exactly X Dollars (minus fee)
   b) If user redeems X Dollars exactly X Dollars are subtracted from user balance
2. Pls notice the original issue spec also describe setting up CI

Overall the PR looks great, we will iterate on the fuzzing tests later

[gitcoindev]

@gitcoindev

1. Could you add 2 more fuzz tests:
   a) If user mints X Dollars he gets exactly X Dollars (minus fee)
   b) If user redeems X Dollars exactly X Dollars are subtracted from user balance
2. Pls notice the original issue spec also describe setting up CI

Overall the PR looks great, we will iterate on the fuzzing tests later

Sure, thank you for the feedback.

[gitcoindev]

Hi @rndquu @0x4007 @molecula451 the PR is ready to review, I executed QA against my fork: https://github.com/gitcoindev/ubiquity-dollar/actions/runs/9534134896/job/26278201422 . The CI setup is ready and according to the original spec. Fuzz tests run by default with 256 inputs on PR commits and the 'deep fuzz' workflow with 100000 inputs https://github.com/ubiquity/ubiquity-dollar/blob/development/.github/workflows/deep-fuzz.yml is executed after merge to the development branch.

[molecula451]

hi @gitcoindev can we instead organize the tests inside a "fuzz" folder,

packages/contracts/test/fuzz/diamond/facets/UbiquityPoolFacet.fuzz.t.sol or something similar

i think this is our initial sketching/drawing for fuzzing inside the repo, besides the SMTChecker we've done before, but untouching the .sol files, this is the first so it would be nice the addition of the folder for future development

[gitcoindev]

@molecula451 I moved fuzz tests as requested, could you please have a look once again now?

[molecula451]

the foundry standard library allow us to use assume() as an abstraction to meet certain conditions on functionality, to fuzz input and get a corresponding value according to the data

we have moved the files and structured a new way for proper fuzzing tests file in the future

this PR meets the correct condition at issue #925 implementing

• Correct foundry.toml profile for fuzzing selection

Minting:

ubiquity-dollar/packages/contracts/test/fuzz/diamond/facets/UbiquityPoolFacet.fuzz.t.sol

Line 154 in 7dce7be

function testMintDollar_FuzzCollateralRatio(

Reedeeming:

ubiquity-dollar/packages/contracts/test/fuzz/diamond/facets/UbiquityPoolFacet.fuzz.t.sol

Line 333 in 7dce7be

function testRedeemDollar_FuzzRedemptionDelayBlocks(

of the corresponding user

ubiquity-dollar/packages/contracts/test/fuzz/diamond/facets/UbiquityPoolFacet.fuzz.t.sol

Line 32 in 7dce7be

address user = address(1);

and assuming arbitrary input, therefore i am approving this PR

[rndquu]

@gitcoindev Did you experiment with how long the deep fuzz workflow runs for, let's say, 1_000_000 runs ? I want to find out a value so that deep fuzz workflow could run for the amount of hours close to github max CI limit (6 hours).

[0x4007]

@gitcoindev Did you experiment with how long the deep fuzz workflow runs for, let's say, 1_000_000 runs ? I want to find out a value so that deep fuzz workflow could run for the amount of hours close to github max CI limit (6 hours).

Can try a binary search and just push a few commits.

[molecula451]

i think in part we're going to see the runs with new upcoming PRs and improve the addition workflow from there

[gitcoindev]

@gitcoindev Did you experiment with how long the deep fuzz workflow runs for, let's say, 1_000_000 runs ? I want to find out a value so that deep fuzz workflow could run for the amount of hours close to github max CI limit (6 hours).

Can try a binary search and just push a few commits.

During my experiments and tests implementation I figured out that 100 000 was more than enough to find all edge cases for this Ubiquity pool lib. Before pushing the commit I modified e.g. vm.assume, expected values or lib implementation, with very small values or large values and fuzzer always found the problem with less than 10 000 iterations. I set deep fuzz to 100 000 to be sure that it will find new issues, but of course we can experiment and increase to full 6 hrs and check if it brings any value.

[rndquu]

@gitcoindev Did you experiment with how long the deep fuzz workflow runs for, let's say, 1_000_000 runs ? I want to find out a value so that deep fuzz workflow could run for the amount of hours close to github max CI limit (6 hours).

Can try a binary search and just push a few commits.

During my experiments and tests implementation I figured out that 100 000 was more than enough to find all edge cases for this Ubiquity pool lib. Before pushing the commit I modified e.g. vm.assume, expected values or lib implementation, with very small values or large values and fuzzer always found the problem with less than 10 000 iterations. I set deep fuzz to 100 000 to be sure that it will find new issues, but of course we can experiment and increase to full 6 hrs and check if it brings any value.

#943