Skip to content

Feat/rework mutators#458

Open
0xalpharush wants to merge 13 commits intomasterfrom
feat/rework-mutators
Open

Feat/rework mutators#458
0xalpharush wants to merge 13 commits intomasterfrom
feat/rework-mutators

Conversation

@0xalpharush
Copy link
Contributor

@0xalpharush 0xalpharush commented Aug 26, 2024

replaces #445

@0xalpharush 0xalpharush force-pushed the feat/rework-mutators branch from 589ab86 to 59a2334 Compare August 26, 2024 16:06
@0xalpharush 0xalpharush linked an issue Sep 9, 2024 that may be closed by this pull request
@anishnaik anishnaik marked this pull request as ready for review January 14, 2025 18:05
@dguido
Copy link
Member

dguido commented Jan 20, 2026

@0xalpharush - this mutator rework looks valuable! The PR has conflicts with recent master changes. Could you rebase when you have a chance? We'd like to review and merge this.

dguido and others added 2 commits January 20, 2026 18:49
Resolve conflict in sequence_generator.go by keeping the new
AppendAndMutate naming while using the corrected "tail" spelling
from master (was "tao;" typo in feature branch).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… strategies

- Fix swapRandList, spliceAtRandom, and interleaveAtRandom to use
  provider.Intn instead of global rand.Intn for deterministic fuzzing
- Add empty slice bounds checks to spliceAtRandom and interleaveAtRandom
  to prevent panics when called with empty sequences
- Wire up SpliceAndMutate and InterleaveAndMutate config fields to the
  mutationStrategyChooser (they were defined but never used)
- Clean up test file: remove debug print statements, fix import path
  (medusa-geth vs ethereum), add tests for empty slice edge cases
- Rename test from TestSplice to TestSequenceGenerationStrategies for
  clarity

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@dguido dguido requested a review from samalws-tob as a code owner January 20, 2026 23:53
@dguido
Copy link
Member

dguido commented Jan 20, 2026

PR Review and Fixes

I've reviewed this PR and pushed fixes for several issues I found.

Merge Conflict Resolution

Resolved conflict in fuzzing/sequence_generator.go - kept the feature branch's AppendAndMutate naming while using the corrected "tail" spelling from master (was "tao;" typo in the feature branch).

Bug Fixes

1. Non-deterministic random number generation (Critical)

swapRandList, spliceAtRandom, and interleaveAtRandom were using global rand.Intn instead of the passed provider.Intn:

// Before (non-deterministic)
i, j := rand.Intn(len(xs)), rand.Intn(len(xs))

// After (deterministic)
i, j := provider.Intn(len(xs)), provider.Intn(len(xs))

This defeated the purpose of seeded deterministic fuzzing - test results would not be reproducible.

2. Potential panics on empty slices

spliceAtRandom and interleaveAtRandom would panic with rand.Intn(0) when passed empty slices. Added bounds checks:

func spliceAtRandom[T any](provider *rand.Rand, xs1, xs2 []*T) []*T {
    if len(xs1) == 0 {
        return xs2
    }
    if len(xs2) == 0 {
        return xs1
    }
    // ... rest of function
}

3. Missing mutation strategies

SpliceAndMutate and InterleaveAndMutate config fields were defined in CallSequenceGeneratorConfig but never wired up to the mutation strategy chooser in NewCallSequenceGenerator. Added them:

randomutils.NewWeightedRandomChoice(
    CallSequenceGeneratorMutationStrategy{
        CallSequenceGeneratorFunc: spliceCorpus,
        Mutate:                    true,
    },
    new(big.Int).SetUint64(config.SpliceAndMutate),
),
randomutils.NewWeightedRandomChoice(
    CallSequenceGeneratorMutationStrategy{
        CallSequenceGeneratorFunc: interleaveCorpus,
        Mutate:                    true,
    },
    new(big.Int).SetUint64(config.InterleaveAndMutate),
),

Code Quality Improvements

4. Test file cleanup

  • Removed debug fmt.Println statements
  • Removed commented-out code
  • Fixed import path (github.com/crytic/medusa-geth/common instead of github.com/ethereum/go-ethereum/common)
  • Renamed test from TestSplice to TestSequenceGenerationStrategies for clarity
  • Added edge case tests for empty slice handling (TestSpliceAtRandomEmptySlices, TestInterleaveAtRandomEmptySlices)

Verification

  • All tests pass ✅
  • Build succeeds ✅
  • go vet passes ✅

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.

add call sequence mutation that copies and repeats a call

3 participants