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

Use single conf.verb, warn users on zero-address use #686

Merged
merged 6 commits into from
Mar 20, 2025

Conversation

msooseth
Copy link
Collaborator

@msooseth msooseth commented Mar 18, 2025

Description

This attempts to fix #671

We need to add a general verbosity flag, otherwise libraries that use us will have all sorts of junk printed on the console. Default is zero (is it's seamless for libraries), but we set it to 1 when running from the command line.

Note: hevm test's --verbosity flag has been rolled into this, and it is now part of App rather than UnitTestOptions. This makes it more general, and makes it such that global options are truly in a single place (App).

The hevm test --verbose X seems to have been broken. When it was Nothing, then nothing extra was printed, but when it was Just then stuff got printed. However, the description seems to have talked about a difference between 1 and 2 levels. This doesn't seem to exist. Now it's set to print more failure info when it's 2 or above. This preserves the default "silent" operation when running from the command line (where verbosity is 1 by default).

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

@msooseth msooseth force-pushed the warn-user-zero-addr branch from 04216a2 to 7b87a4e Compare March 18, 2025 15:23
Comment on lines -301 to -316
passOutput :: VM t s -> UnitTestOptions s -> Text -> Text
passOutput vm UnitTestOptions { .. } testName =
let ?context = DappContext { info = dapp
, contracts = vm.env.contracts
, labels = vm.labels }
in let v = fromMaybe 0 verbose
in if (v > 1) then
mconcat
[ "Success: "
, fromMaybe "" (stripSuffix "()" testName)
, "\n"
, if (v > 2) then indentLines 2 (showTraceTree dapp vm) else ""
, indentLines 2 (formatTestLogs dapp.eventMap vm.logs)
, "\n"
]
else ""
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not used anywhere, removing.

@@ -59,7 +59,6 @@ testOpts solvers root buildOutput match maxIter allowFFI rpcinfo = do
, askSmtIters = 1
, smtTimeout = Nothing
, solver = Nothing
, verbose = Just 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is now conf and in cli.hs is set to by default 1. In API library use mode it's set to 0 by default.

@msooseth msooseth marked this pull request as ready for review March 18, 2025 15:42
@msooseth msooseth force-pushed the warn-user-zero-addr branch from 88870e5 to 7a75500 Compare March 18, 2025 17:05
@msooseth msooseth changed the title [DRAFT] Use single conf.verb, warn users on zero-address use Use single conf.verb, warn users on zero-address use Mar 18, 2025
@msooseth msooseth requested a review from blishko March 18, 2025 17:09
@msooseth msooseth force-pushed the warn-user-zero-addr branch from 65a1161 to d412468 Compare March 19, 2025 10:04
@msooseth msooseth force-pushed the warn-user-zero-addr branch from d412468 to 5640901 Compare March 19, 2025 10:08
Copy link
Collaborator

@blishko blishko left a comment

Choose a reason for hiding this comment

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

LGMT!

@msooseth msooseth merged commit 936c175 into main Mar 20, 2025
9 checks passed
@msooseth msooseth deleted the warn-user-zero-addr branch March 20, 2025 13:23
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.

Maybe emit warning when trying to call contract at address zero
2 participants