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

Solidity State Corpus Study #295

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Solidity State Corpus Study #295

wants to merge 17 commits into from

Conversation

Tim-Ganger
Copy link
Collaborator

This branch contains work from an unfinished corpus study done in Summer 2020.

Copy link
Owner

@mcoblenz mcoblenz left a comment

Choose a reason for hiding this comment

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

This is hard to review without a description of what works and what is still to do. Can you please add a summary?



# This class is a detector plugin for Slither. It counts the number
# of states in each contract given to it, and if that number is more than
Copy link
Owner

Choose a reason for hiding this comment

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

more than what?

# of states in each contract given to it, and if that number is more than
class StateNum(AbstractDetector):
"""
Detect function named backdoor
Copy link
Owner

Choose a reason for hiding this comment

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

Is this comment stale?

return list(set(list1 + list2))

# Takes in a list of indeces and a list of upper boundaries on those
# indeces and increments indeces in the index list until an index does
Copy link
Owner

Choose a reason for hiding this comment

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

Fix spelling: indices.

@@ -0,0 +1,2 @@
To use these plugins with slither, first follow the instructions at https://github.com/crytic/slither/wiki/Developer-installation to set up the slither development environment.
Copy link
Owner

Choose a reason for hiding this comment

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

pycache files appear to be derived binaries; remove them from the PR. You can add them to .gitignore.


WIKI = 'STATE TEST'

WIKI_TITLE = 'TODO'
Copy link
Owner

Choose a reason for hiding this comment

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

TODO?

WIKI_RECOMMENDATION = 'This is just a check, it does not indicate an error'

# Checks if any contracts that contract inherits were already determined
# to be stateful. Returns the truth value of that check.
Copy link
Owner

Choose a reason for hiding this comment

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

What if we haven't processed those contracts yet? Does Slither process in order of the inheritance hierarchy?

# not reach its limit. After that happens, the rest of the input list is
# returned unchanged and the indeces that went over their limit are set
# to 0. If all elements go over their limits, None is returned instead.
def increment_list(self, index_list, length_list) :
Copy link
Owner

Choose a reason for hiding this comment

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

I find this very confusing. Why is the third argument called "length_list"?

# returned unchanged and the indeces that went over their limit are set
# to 0. If all elements go over their limits, None is returned instead.
def increment_list(self, index_list, length_list) :
new_indeces = []
Copy link
Owner

Choose a reason for hiding this comment

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

I think this whole thing might be easier to read in a functional style (with a map).


# Checks whether or not a given check is stateful using the heuristic
# described in
# http://www.cs.cmu.edu/~aldrich/papers/aldrich-empirical-ecoop11.pdf
Copy link
Owner

Choose a reason for hiding this comment

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

Say here what the heuristic is; I shouldn't have to go find it in the paper.

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.

2 participants