Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Reinstate false positive filtering of Mythril's dynamic array #101

Merged
merged 2 commits into from Mar 7, 2019

Conversation

rocky
Copy link
Contributor

@rocky rocky commented Mar 5, 2019

@nbanmp - This code got accidentally orphaned here and then got stale as a result of MythX changes. For the last 6 months or so it has been and is slated to go into MythX.

When it eventually does, it will finally get removed altogether. However for exposition purposes in say a Medium article, it is useful to have working as it shows an example of what kinds of synergy MythX is capable of by mixing tool output with static information.

@daniyarchambylov I spent some time trying to figure out how to reinstate the 3 isIgnorable tests, and ran out of patience. In tracing there seems to be something weird in the way the AST traversal was working with I guess with one of the mocking/stubbing functions.

Separate from that, I found that by changing one interface to add a paramter, 15 places in test_issueslint.js needed to be changed. This means that the tests are not DRY (do not repeat yourself). I started pulling out the creation routine in to a single function so that this wouldn't happen again. But this is not complete, There is a lot of other duplicated code is mock and stub setup, that now fails as a result of this.

I'd greatly appreciate it if you would fix up the isIgnorable test and DRY the code so that if interfaces change again, we don't have massive changes to the tests (even if it is merely a single search and replace done 15 times). Thanks.

lib/srcmap.js Outdated
@@ -43,7 +43,7 @@ module.exports = {
/**
* Takes a bytecode hexstring and returns a map indexed by offset
* that give the instruction number for that offset.
*
*ok
Copy link
Contributor

Choose a reason for hiding this comment

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

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No doubt that was meant for some discord user -- I have too many Emacs windows.

Should be fixed in 6dc1164

@rocky rocky changed the base branch from separate-mythx-artifacts to master March 7, 2019 00:30
@rocky rocky merged commit 478d9f1 into master Mar 7, 2019
@nbanmp nbanmp deleted the remove-dynamic-array-false-positive branch June 12, 2019 18:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants