feature: deep fuzzing on newly modified Solidity contracts and pull requests

[gitcoindev]

Resolves #515

[ubiquibot]

• 0e19fc4
• a1ac686
• 2390afa
• cfd27ed
• dc7f74b

[rndquu]

Regarding the issue:

1. We don't have proper fuzzing tests right now so we're getting ahead of ourselves with this PR
2. There is no sense in running fuzzing tests only on modified solidity files because all of the contracts are interconnected so in the end we'd better to deep fuzz the whole protocol

Regarding the PR:

1. If we had proper deep fuzzing tests then it would take for them a decent amount of time to finish. I don't think that we should run this "deep fuzz" workflow on pull_request or push events. Perhaps it is better to run the workflow on workflow_dispatch or on push to the development branch.
2. Isn't it better to use the workflow syntax (example) to check for modified files instead of a shell script?

[gitcoindev]

Hi @rndquu thank you for starting the review and your valuable comments.

2. Isn't it better to use the workflow syntax (example) to check for modified files instead of a shell script?

I used the recommended workflow implementation for listing changed files for https://github.com/tj-actions/changed-files , see https://github.com/tj-actions/changed-files#using-local-git-history . Since deep fuzz will be run on the whole protocol I will use the Slither's example that you provided.

Apart from that I will incorporate your feedback and submit updates so that the pull request can be accepted.
I would also like to keep @pavlovcik in the loop so that he is aware of the changes.

[gitcoindev]

Hi @rndquu , @pavlovcik I incorporated the suggested changes cfd27ed , my QA is available at https://github.com/gitcoindev/ubiquity-dollar/actions/runs/5935230402/job/16093344234 . The pull request is ready for another round of the review.

[0x4007]

There is no sense in running fuzzing tests only on modified solidity files because all of the contracts are interconnected so in the end we'd better to deep fuzz the whole protocol

Warning: I did not read the fuzz documentation on Foundry, but from what I understand about the concepts, here are my beliefs: the concept of fuzzing is testing as many types of inputs as possible. We only need to deep fuzz the entire protocol once, and then afterwards only deep fuzz the changed contracts. It does not matter if the contracts are connected or not with the rest of the protocol, because each contract is fuzz tested in isolation anyways. It will only add more time to fuzz test the entire protocol every time a Solidity file is changed.

see https://github.com/tj-actions/changed-files#using-local-git-history

You should be using the GitHub API version, not relying on local .git history (we are not running locally) which checks for changed files on a pull request: https://github.com/tj-actions/changed-files#using-githubs-api

[gitcoindev]

There is no sense in running fuzzing tests only on modified solidity files because all of the contracts are interconnected so in the end we'd better to deep fuzz the whole protocol

Warning: I did not read the fuzz documentation on Foundry, but from what I understand about the concepts, here are my beliefs: the concept of fuzzing is testing as many types of inputs as possible. We only need to deep fuzz the entire protocol once, and then afterwards only deep fuzz the changed contracts. It does not matter if the contracts are connected or not with the rest of the protocol, because each contract is fuzz tested in isolation anyways. It will only add more time to fuzz test the entire protocol every time a Solidity file is changed.

see https://github.com/tj-actions/changed-files#using-local-git-history

You should be using the GitHub API version, not relying on local .git history (we are not running locally) which checks for changed files on a pull request: https://github.com/tj-actions/changed-files#using-githubs-api

Hi @pavlovcik @rndquu , I executed a few experiments during implementation. Currently 35 out of 267 total tests in packages/contracts are fuzz tests, with default 256 runs for each test case. The deep fuzzing takes about 2-3 minutes for 10000 runs for the whole protocol, so currently this is still manageable.

For my original approach with QA here gitcoindev#2 : I had to find out which tests imported changed contracts and execute 10000 runs on those. When the test code base grows significantly this approach can still be used.

I also already reworked the pull request to use GitHub API version and verified it triggers only on push to development branch when *.sol files change: see https://github.com/gitcoindev/ubiquity-dollar/actions/runs/5935230402

[gitcoindev]

Thank you for approvals and merging!