Skip to content

Conversation

@GarrettJMU
Copy link

@GarrettJMU GarrettJMU commented Mar 3, 2023

Motivation

Coming from issue:
#70

Change Summary

Add slither to the ci workflow

Merge Checklist

Additional Context

n/a

@varunsrin
Copy link
Member

Thanks for making this PR.

Unfortunately, this seems to fail because slither is taking > 10 minutes to run. I don't think its a good idea to have such an expensive test suite run on every commit to a branch. I'm going to explore some options for speeding this up, open to suggestions if you have any

@GarrettJMU
Copy link
Author

GarrettJMU commented Mar 4, 2023

@varunsrin - Hm - I think a few other things. It doesn't look like we're not omitting node_modules but maybe taking a step back, what do you want to test here?

Do we also need to test all the folders in lib? Seems like right now we are just omitting forge-std. I just pushed an update to omit lib altogether and might be interesting to see what time tradeoff that has.

I even tried removing some of the inefficient scans like low-level-calls and still timing out. There is something expensive in their analysis that's chewing away.

@varunsrin
Copy link
Member

varunsrin commented Mar 5, 2023

Yeah, i made another PR where I got it down to 11 minutes, but that's still painfully slow for every PR. Appreciate your work on this but it might not make sense to merge right now given how slow it is and just do the checks manually before contract releases.

@varunsrin
Copy link
Member

closing for now since we're not moving forward, thanks for trying @GarrettJMU

@varunsrin varunsrin closed this Mar 9, 2023
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