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

Simplify Echidna.ABI #545

Merged
merged 1 commit into from
Nov 10, 2020
Merged

Simplify Echidna.ABI #545

merged 1 commit into from
Nov 10, 2020

Conversation

arcz
Copy link
Member

@arcz arcz commented Nov 10, 2020

No semantic changes. Replacing MonadReader with an explicit argument and some lens with bare accessors. This effort is towards #487

@arcz arcz requested review from incertia and ggrieco-tob November 10, 2020 16:33
Copy link

@lojikil lojikil left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@arcz arcz merged commit ffaf587 into master Nov 10, 2020
@arcz arcz deleted the simplify-abi branch November 10, 2020 18:30
genAbiCallM abi = genWithDict (fmap toList . view wholeCalls) (traverse $ traverse genAbiValueM) abi >>= mutateAbiCall
genAbiCallM :: MonadRandom m => GenDict -> SolSignature -> m SolCall
genAbiCallM genDict abi =
genWithDict
Copy link
Contributor

Choose a reason for hiding this comment

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

if we are to make this multiline can we use do

@@ -159,7 +156,7 @@ getRandomUint n = join $ R.fromList [(getRandomR (0, 1023), 1), (getRandomR (0,
-- Note that we define the dictionary case ('genAbiValueM') first (below), so recursive types can be
-- be generated using the same dictionary easily
genAbiValue :: MonadRandom m => AbiType -> m AbiValue
genAbiValue = flip runReaderT defaultDict . genAbiValueM
genAbiValue = genAbiValueM defaultDict
Copy link
Contributor

Choose a reason for hiding this comment

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

we could always just give explicit parameters here to avoid runReaderT

genAbiValue t = genAbiValueM t defaultDict

dictValues :: GenDict -> [Integer]
dictValues g = catMaybes $ concatMap (\(_,h) -> map fromValue $ H.toList h) $ M.toList $ g ^. constants
dictValues :: GenDict -> [Integer]
dictValues g = catMaybes $ concatMap (\(_,h) -> map fromValue $ H.toList h) $ M.toList $ _constants g
Copy link
Contributor

Choose a reason for hiding this comment

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

i would try to use lens wherever possible to promote usage and keep things consistent

fromMaybe <$> g t <*> (bool (pure Nothing) (fromDict c) . (c ^. pSynthA >=) =<< getRandom)
genWithDict :: (Eq a, Hashable a, MonadRandom m)
=> GenDict -> HashMap a [b] -> (a -> m b) -> a -> m b
genWithDict genDict f g t = do
Copy link
Contributor

Choose a reason for hiding this comment

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

we should rename the f parameter here because its meaning has changed and is no longer a function

=> GenDict -> HashMap a [b] -> (a -> m b) -> a -> m b
genWithDict genDict f g t = do
let fromDict = uniformMay (M.lookupDefault [] t f)
fromMaybe <$> g t <*> (bool (pure Nothing) fromDict . (_pSynthA genDict >=) =<< getRandom)
Copy link
Contributor

Choose a reason for hiding this comment

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

if we want to target #545 this is a good line to target

do
  r <- getRandom
  fromMaybe <$> g t <*> bool (pure Nothing) fromDict (genDict ^. pSynthA >= r)

genAbiValueM :: (MonadReader x m, Has GenDict x, MonadRandom m) => AbiType -> m AbiValue
genAbiValueM = genWithDict (fmap toList . view constants) $ \case
genAbiValueM :: MonadRandom m => GenDict -> AbiType -> m AbiValue
genAbiValueM genDict = genWithDict genDict (toList <$> _constants genDict) $ \case
Copy link
Contributor

Choose a reason for hiding this comment

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

this is why i want to avoid removing MonadReader from things. we now have to pass this around everywhere

>>= flip V.replicateM (genAbiValueM t)
(AbiArrayType n t) -> AbiArray n t <$> V.replicateM n (genAbiValueM t)
(AbiTupleType v) -> AbiTuple <$> traverse genAbiValueM v
>>= flip V.replicateM (genAbiValueM genDict t)
Copy link
Contributor

Choose a reason for hiding this comment

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

like here

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