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

Fix 'pragma solidity' parsing #1887

Merged
merged 13 commits into from
Jan 10, 2025
Merged

Conversation

gsalzer
Copy link
Contributor

@gsalzer gsalzer commented Oct 30, 2024

remove comments and strings before searching for a line containing 'pragma solidity'

remove comments and strings before searching for a line containing 'pragma solidity'
@gsalzer
Copy link
Contributor Author

gsalzer commented Oct 30, 2024

The patch tries to be minimally invasive; you may want to integrate it better. The comment citing the source can be removed (no need to give credits), it is just to inspire trust (?) that the code works since we used it for processing source codes in one of our studies.

@norhh
Copy link
Collaborator

norhh commented Nov 5, 2024

For some reason, I’ve lost access to the CircleCI pipeline results over the past month. @psantos-consensys, could you look into the cause of CircleCI’s failure? Additionally, could you change the permissions to view Mythril’s CircleCI results? Since Mythril is open-source, these results should ideally be public as they were previously.

@gsalzer
Copy link
Contributor Author

gsalzer commented Dec 17, 2024

@norhh @psantos-consensys Any chance that something is going to happen in the near future? I'm fine with the pull request being rejected but this CircleCI pipeline issue occurring with every pull request now seems to be unhealthy.

@norhh
Copy link
Collaborator

norhh commented Dec 17, 2024

I don't know when it will be fixed/visible. I've asked someone to look into it.

@psantos-consensys
Copy link
Collaborator

@norhh can you please double check if it is working as expected now?

@norhh
Copy link
Collaborator

norhh commented Jan 6, 2025

@gsalzer, the test failures seem to be due to the test case related to these files: version_2.sol, version_3.sol.
The comments should be removed from these files.

@gsalzer
Copy link
Contributor Author

gsalzer commented Jan 7, 2025

@gsalzer, the test failures seem to be due to the test case related to these files: version_2.sol, version_3.sol. The comments should be removed from these files.

@norhh Please check the updated code. Now, it first searches for a regular pragma that does not occur in a comment or string, and if this fails, falls back to how Mythril searches the pragma currently (i.e., searching within comments as well).

@gsalzer
Copy link
Contributor Author

gsalzer commented Jan 7, 2025

What is the proper way to execute the tests in the tests folder?

Copy link
Collaborator

@norhh norhh left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the PR. Can you make these quick refactoring changes?

mythril/ethereum/util.py Outdated Show resolved Hide resolved
mythril/ethereum/util.py Show resolved Hide resolved
mythril/ethereum/util.py Outdated Show resolved Hide resolved
mythril/ethereum/util.py Outdated Show resolved Hide resolved
@gsalzer
Copy link
Contributor Author

gsalzer commented Jan 8, 2025

@norhh Refactoring done.

Copy link
Collaborator

@norhh norhh left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@psantos-consensys psantos-consensys merged commit acecf97 into Consensys:develop Jan 10, 2025
2 checks passed
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