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

Improved array generation using generic mutations #357

Merged
merged 16 commits into from Apr 1, 2020
Merged

Conversation

@ggrieco-tob
Copy link
Member

@ggrieco-tob ggrieco-tob commented Feb 18, 2020

This PR implements better mutations for arrays (e.g. strings, dynamic arrays, etc). It will allow Echidna to test complex code more effectively, in particular when a contract parses something on-chain. This PR is needed in order to eventually merge #305.

ggrieco-tob added 2 commits Feb 18, 2020
ggrieco-tob added 7 commits Feb 19, 2020
@ggrieco-tob ggrieco-tob marked this pull request as ready for review Feb 19, 2020
Copy link
Member

@arcz arcz left a comment

Might be good to actually use vector, there is a lot of random access going on.

lib/Echidna/ABI.hs Outdated Show resolved Hide resolved
lib/Echidna/ABI.hs Outdated Show resolved Hide resolved
lib/Echidna/Mutator.hs Outdated Show resolved Hide resolved
lib/Echidna/Mutator.hs Outdated Show resolved Hide resolved
lib/Echidna/Mutator.hs Outdated Show resolved Hide resolved
lib/Echidna/Mutator.hs Outdated Show resolved Hide resolved
@incertia
Copy link
Member

@incertia incertia commented Feb 24, 2020

at merge point, can we squash all the fixes commits to a single commit?

Copy link
Member

@incertia incertia left a comment

can we fix these before i look at the changes more holistically

lib/Echidna/Mutator.hs Outdated Show resolved Hide resolved
lib/Echidna/Mutator.hs Outdated Show resolved Hide resolved
lib/Echidna/Mutator.hs Outdated Show resolved Hide resolved
mutateAbiValue (AbiUInt n x) = getRandomR (0, 9 :: Int) >>= -- 10% of chance of mutation
\case
0 -> (AbiUInt n <$> mutateNum x)
_ -> return $ AbiUInt n x
mutateAbiValue (AbiInt n x) = getRandomR (0, 9 :: Int) >>= -- 10% of chance of mutation
\case
0 -> (AbiInt n <$> mutateNum x)
_ -> return $ AbiInt n x
Comment on lines +236 to +243

This comment has been minimized.

@incertia

incertia Feb 26, 2020
Member

can we make these 10 percents tunable config parameters. if you want to go the Rational route the JSON for that is { "numerator" : 1, "denominator" : 10 }. otherwise choose something like Double.

This comment has been minimized.

@ggrieco-tob

ggrieco-tob Mar 9, 2020
Author Member

Increasing these values will make the tests to fail, because magic numbers are going to be (likely) corrupted. I think the user should be allowed to modify these.

import EVM.ABI (AbiValue(..))

listMutators :: MonadRandom m => m ([a] -> m [a])
listMutators = fromList [(return, 1), (expandRandList, 10), (deleteRandList, 10), (swapRandList, 10)]

This comment has been minimized.

@incertia

incertia Feb 26, 2020
Member

can we config these weights as well?

This comment has been minimized.

@ggrieco-tob

ggrieco-tob Mar 9, 2020
Author Member

I need to experiment with these parameters (with the exception of the return one, which should be small), in order to understand which mutations are more useful. Probably @agroce will be interested on this as well.

xs <- f vs
return $ maybe xs (`take` (xs ++ fs)) mn

mutateBS :: MonadRandom m => Maybe Int -> [Word8] -> ByteString -> m ByteString

This comment has been minimized.

@incertia

incertia Feb 26, 2020
Member

document

listMutators :: MonadRandom m => m ([a] -> m [a])
listMutators = fromList [(return, 1), (expandRandList, 10), (deleteRandList, 10), (swapRandList, 10)]

mutateV :: MonadRandom m => Maybe Int -> [AbiValue] -> [AbiValue] -> m [AbiValue]

This comment has been minimized.

@incertia

incertia Feb 26, 2020
Member

can we document the new parameters

lib/Echidna/ABI.hs Outdated Show resolved Hide resolved
@@ -285,7 +294,7 @@ genAbiValueM = genWithDict (fmap toList . view constants) $ \case

-- | Given a 'SolSignature', generate a random 'SolCalls' with that signature, possibly with a dictionary.
genAbiCallM :: (MonadState x m, Has GenDict x, MonadRandom m) => SolSignature -> m SolCall
genAbiCallM = genWithDict (fmap toList . view wholeCalls) (traverse $ traverse genAbiValueM)
genAbiCallM abi = genWithDict (fmap toList . view wholeCalls) (traverse $ traverse genAbiValueM) abi >>= mutateAbiCall

This comment has been minimized.

@incertia

incertia Feb 26, 2020
Member

why are we doing a mutateAbiCall right after we gen?

This comment has been minimized.

@ggrieco-tob

ggrieco-tob Mar 9, 2020
Author Member

This is an interesting question. I couldn't find an easy way to mutate only the results of the values generated by the dictionaries, but mutating the generated values will perform the same effect. If you have a better way to do it, please let me know.

ggrieco-tob added 3 commits Mar 7, 2020
@ggrieco-tob
Copy link
Member Author

@ggrieco-tob ggrieco-tob commented Mar 7, 2020

@incertia @arcz I re-wrote the mutators using ListLike. Please take another look to this PR.

@incertia incertia merged commit 1456b8c into master Apr 1, 2020
4 checks passed
4 checks passed
test (ubuntu-latest)
Details
hlint
Details
hlint
Details
license/cla Contributor License Agreement is signed.
Details
@incertia incertia deleted the dev-array-mutation branch Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.