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

Runtime fuzzer #4524

Merged
merged 9 commits into from
Jul 20, 2021
Merged

Runtime fuzzer #4524

merged 9 commits into from
Jul 20, 2021

Conversation

posvyatokum
Copy link
Member

Adding creating of an arbitrary Scenario with Transfer, Stake, and CreateAccount transactions. Also adding fuzzing target.
Currently, all the bounds for random numbers are also chosen randomly (by me).
Next pr will add contract calls, but it will only be for contracts from 'runtime/near-test-contracts'

Copy link
Collaborator

@bowenwang1996 bowenwang1996 left a comment

Choose a reason for hiding this comment

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

Adding a readme on how to run the fuzzer could be helpful

test-utils/runtime-tester/src/fuzzing.rs Outdated Show resolved Hide resolved
test-utils/runtime-tester/src/fuzzing.rs Outdated Show resolved Hide resolved
test-utils/runtime-tester/src/run_test.rs Outdated Show resolved Hide resolved
test-utils/runtime-tester/src/fuzzing.rs Outdated Show resolved Hide resolved
test-utils/runtime-tester/src/fuzzing.rs Outdated Show resolved Hide resolved
test-utils/runtime-tester/src/run_test.rs Outdated Show resolved Hide resolved
test-utils/runtime-tester/src/run_test.rs Outdated Show resolved Hide resolved
@bowenwang1996 bowenwang1996 requested a review from matklad July 16, 2021 16:58
Copy link
Contributor

@matklad matklad left a comment

Choose a reason for hiding this comment

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

The code looks good to me, left just a couple of small nits.

Two comments/questions:

  • should we perhaps add this to the nightly pipeline? To run the fuzzing for half-an-hour every night, or something like this? Not sure what's the best strategy?
  • did we try to estimate how well our generator works? Can we, eg, detect an overflow if we replace some of checked_adds with plain +?

test-utils/runtime-tester/fuzz/README.md Outdated Show resolved Hide resolved
test-utils/runtime-tester/fuzz/README.md Outdated Show resolved Hide resolved
test-utils/runtime-tester/src/run_test.rs Outdated Show resolved Hide resolved
test-utils/runtime-tester/src/run_test.rs Outdated Show resolved Hide resolved
@posvyatokum
Copy link
Member Author

  • should we perhaps add this to the nightly pipeline? To run the fuzzing for half-an-hour every night, or something like this? Not sure what's the best strategy?
  • did we try to estimate how well our generator works? Can we, eg, detect an overflow if we replace some of checked_adds with plain +?

At this point, fuzzer is very weak, so probably shouldn't add it to the nightly pipeline yet.
Currently, I mostly create PR's to approve the basic design. Right after merging this PR, I will create a new one with contract deployment, function calls, and a couple of tricks to make scenarios more realistic. And even then overflows will not be detected in a reasonable time -- these scenarios are too simplistic.
Bowen laid out some examples of what we want to catch #4374 (comment). I will follow these suggestions and hopefully fuzzer will start catching at least intentional bugs quick

@posvyatokum posvyatokum merged commit 1a652b3 into master Jul 20, 2021
@posvyatokum posvyatokum deleted the runtime-fuzzer branch July 20, 2021 21:01
@matklad
Copy link
Contributor

matklad commented Jul 21, 2021

At this point, fuzzer is very weak, so probably shouldn't add it to the nightly pipeline yet.

Process-wise, I'd suggest adding stuff to CI as early as possible, and then using CI as a ratchet, to make sure that stuff can progress forward, but can not regress. That is, the benefit of CI at this stage is not that we catch bugs, but that we prevent fuzzing infra itself from regressing.

But this is a "rule of thumb" suggestion, there's little specific benefits in this case.

@bowenwang1996
Copy link
Collaborator

At this point, fuzzer is very weak, so probably shouldn't add it to the nightly pipeline yet.

Process-wise, I'd suggest adding stuff to CI as early as possible, and then using CI as a ratchet, to make sure that stuff can progress forward, but can not regress. That is, the benefit of CI at this stage is not that we catch bugs, but that we prevent fuzzing infra itself from regressing.

But this is a "rule of thumb" suggestion, there's little specific benefits in this case.

@matklad do you mind creating an issue for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants