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/fuzz #249

Closed
wants to merge 58 commits into from
Closed

Feat/fuzz #249

wants to merge 58 commits into from

Conversation

joaosantos15
Copy link
Collaborator

This PR implements the Fuzz commands, with the arm, disarm and run sub-comands.

@joaosantos15 joaosantos15 requested a review from dmuhs March 30, 2021 18:50
Copy link

@blitz-1306 blitz-1306 left a comment

Choose a reason for hiding this comment

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

Good work. A few suggestions and questions.

mythx_cli/analyze/command.py Outdated Show resolved Hide resolved
mythx_cli/analyze/truffle.py Outdated Show resolved Hide resolved
mythx_cli/cli.py Outdated Show resolved Hide resolved
mythx_cli/fuzz/faas.py Outdated Show resolved Hide resolved
mythx_cli/fuzz/rpc.py Outdated Show resolved Hide resolved
:param remap_import: List of import remappings to pass on to solc
:param solc_version: The solc version to use for Solidity compilation
"""
analyze_config = ctx.get("analyze")
Copy link

@blitz-1306 blitz-1306 Apr 5, 2021

Choose a reason for hiding this comment

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

Was wondering why do we stick to ctx.get("analyze") and analyze_config while we should be in fuzz context. Is there a real reason?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, some properties are defined under the "analyze" key. I don't think we should duplicate them under the fuzz key. So I would just keep it like that. I'll just change the "targets" to be fetched from the "fuzz" key, since the user might want different targets for mythx scans and fuzzing.

mythx_cli/fuzz/arm.py Show resolved Hide resolved
@offset.setter
def offset(self, value: int) -> None:
value = int(value)
if value <= 0:

Choose a reason for hiding this comment

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

Question: do we really restrict assigning 0 for offset?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dmuhs made this so we'll have to wait for the reply

@length.setter
def length(self, value: int) -> None:
value = int(value)
if value <= 0:

Choose a reason for hiding this comment

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

Question: do we really restrict assigning 0 for length?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dmuhs made this so we'll have to wait for the reply

Choose a reason for hiding this comment

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

tests/common.py Outdated Show resolved Hide resolved
blitz-1306 and others added 28 commits April 7, 2021 10:41
Tweak feat/fuzz to follow changes
feat: feature support for hardhat
feat: add api_key as config file parameter
@joaosantos15
Copy link
Collaborator Author

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.

None yet

5 participants