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

Feat: Fail on revert #281

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/fuzz.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ var fuzzCmd = &cobra.Command{
ValidArgsFunction: cmdValidFuzzArgs,
RunE: cmdRunFuzz,
SilenceUsage: true,
SilenceErrors: true,
SilenceErrors: false,
}

func init() {
Expand Down
3 changes: 3 additions & 0 deletions fuzzing/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,9 @@ type AssertionTestingConfig struct {

// PanicCodeConfig describes the various panic codes that can be enabled and be treated as a failing assertion test
type PanicCodeConfig struct {
// FailOnRevert describes whether a revert should be treated as a failing case
FailOnRevert bool `json:"failOnRevert"`

// FailOnCompilerInsertedPanic describes whether a generic compiler inserted panic should be treated as a failing case
FailOnCompilerInsertedPanic bool `json:"failOnCompilerInsertedPanic"`

Expand Down
1 change: 1 addition & 0 deletions fuzzing/config/config_defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ func GetDefaultProjectConfig(platform string) (*ProjectConfig, error) {
TestViewMethods: false,
PanicCodeConfig: PanicCodeConfig{
FailOnAssertion: true,
FailOnRevert: false,
},
},
PropertyTesting: PropertyTestingConfig{
Expand Down
30 changes: 30 additions & 0 deletions fuzzing/fuzzer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,36 @@ func TestAssertionsNotRequire(t *testing.T) {
})
}

// TestAssertionFailOnRevert runs a test to ensure that a revert failure causes a test to fail if FailOnRevert is set to true
func TestAssertionFailOnRevert(t *testing.T) {
runFuzzerTest(t, &fuzzerSolcFileTest{
filePath: "testdata/contracts/assertions/assert_fail_on_revert.sol",
configUpdates: func(config *config.ProjectConfig) {
config.Fuzzing.TargetContracts = []string{"TestContract"}
config.Fuzzing.TestLimit = 500

// enable assertion checking mode only
config.Fuzzing.Testing.PropertyTesting.Enabled = false
config.Fuzzing.Testing.AssertionTesting.Enabled = true

// enabling fail on revert
config.Fuzzing.Testing.AssertionTesting.PanicCodeConfig.FailOnRevert = true

// to ensure all tests fail before fuzzer stops
config.Fuzzing.Testing.StopOnFailedTest = false
},
method: func(f *fuzzerTestContext) {
// Start the fuzzer
err := f.fuzzer.Start()
assert.NoError(t, err)

// Check for any failed tests and verify coverage was captured
assertFailedTestsExpected(f, true)
assert.EqualValues(t, 4, len(f.fuzzer.TestCasesWithStatus(TestCaseStatusFailed)))
},
})
}

// TestAssertionsAndProperties runs a test to property testing and assertion testing can both run in parallel.
// This test does not stop on first failure and expects a failure from each after timeout.
func TestAssertionsAndProperties(t *testing.T) {
Expand Down
10 changes: 9 additions & 1 deletion fuzzing/test_case_assertion_provider.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
package fuzzing

import (
"errors"
"math/big"
"sync"

"github.com/crytic/medusa/compilation/abiutils"
"github.com/crytic/medusa/fuzzing/calls"
"github.com/crytic/medusa/fuzzing/config"
"github.com/crytic/medusa/fuzzing/contracts"

"github.com/ethereum/go-ethereum/core/vm"
"golang.org/x/exp/slices"
)

Expand Down Expand Up @@ -65,6 +66,13 @@ func (t *AssertionTestCaseProvider) checkAssertionFailures(callSequence calls.Ca
// want to be backwards compatible with older Solidity which simply hit an invalid opcode and did not actually
// have a panic code.
lastExecutionResult := lastCall.ChainReference.MessageResults().ExecutionResult

// Check for revert or require failures if FailOnRevert is set to true
if t.fuzzer.config.Fuzzing.Testing.AssertionTesting.PanicCodeConfig.FailOnRevert {
if errors.Is(lastExecutionResult.Err, vm.ErrExecutionReverted) {
return &methodId, true, nil
}
}
panicCode := abiutils.GetSolidityPanicCode(lastExecutionResult.Err, lastExecutionResult.ReturnData, true)
failure := false
if panicCode != nil {
Expand Down
22 changes: 22 additions & 0 deletions fuzzing/testdata/contracts/assertions/assert_fail_on_revert.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// This contract ensures the fuzzer will report an error when it encounters a revert.
contract TestContract {
function failRequire(uint value) public {
// This should trigger a test failure due to a failing require statement (without an error message)
require(false);
}

function failRequireWithErrorString(uint value) public {
// This should trigger a test failure due to a failing require statement (with an error message)
require(false, "Require error");
}

function failRevert(uint value) public {
// This should trigger a test failure on encountering a revert instruction (without an error message)
revert();
}

function failRevertWithErrorString(uint value) public {
// This should trigger a test failure on encountering a revert instruction (with an error message)
revert("Function reverted");
}
}