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

Add support for filtering printer output #2233

Open
lucas-manuel opened this issue Nov 16, 2023 · 6 comments
Open

Add support for filtering printer output #2233

lucas-manuel opened this issue Nov 16, 2023 · 6 comments
Labels
enhancement New feature or request

Comments

@lucas-manuel
Copy link

Describe the desired feature

It would be great to have more control over outputs from printers, specifically table formatted ones such as function-summary and vars-and-auth.

Using function-summary as an example, it would be great to be able to filter the following (regex would be sufficient for all probably):

  1. Function names
  2. Visibility
  3. Modifiers
  4. Read
  5. Write
  6. Internal Calls
  7. External Calls

Using regex a user could have full flexibility to filter out based on specifics or based on whether something is empty

"Return all functions without get* at the start that are external, have external calls, don't have any modifiers, and modify state."

@lucas-manuel lucas-manuel added the enhancement New feature or request label Nov 16, 2023
@shortdoom
Copy link

shortdoom commented Nov 16, 2023

regex is not needed, all of the listed information is available to slither as formatted py object (string) in either Function or Contract class.

i think the Issue could be generalized to better control over any output as slither relies extensively on prettyTable for formatting cli display and on the other hand complex json data type for output with --json flag. both of those have drawbacks when integrating with other external to slither scripts. amazingly, as much as Slither allows for easy integration directly with Class objects, it's output pipeline is pretty "hardcoded" from what I've seen

some middleware allowing for specifying custom ouput easily is good idea (say, simplified-json). either in form of a standalone method accessible from the core (by user or other slither classes) or as a separate util.

@devtooligan
Copy link
Contributor

hi @lucas-manuel 👋🏽

Great idea.

We have talked about making slither-printer it's own thing which could take some clargs for filtering as well as output options as @shortdoom suggests. Would also be nice to have a toggle for --fully-derived-contracts only, for example, for certain printers.

Making slither-printer a separate command is not a requirement to this but it would be cleaner than trying to tack on more clargs to the base slither command.

I can work on this when I have some time (soon ™️) or if someone else wants to take a shot at implementing this we can provide guidance.

@shortdoom
Copy link

shortdoom commented Nov 17, 2023

@lucas-manuel @devtooligan @0xalpharush

if you are fine with having this as a separate "tool" and not as a "printer", I can make a PR today, just green-light following logic and reasoning behind it:

  • printers are built around Contract class and do not accept any args. it's a problem because some of the data needed is residing in the Function class. basically, how printers are structured disallows, without some changes of AbstractPrinter or creating new type of abstract interface, to implement "filtering printer" effectively. hence why the "tool" and not printer. tooling interface allows for multiple arguments and it's more relaxed.

example use case:

"Return all functions without get* at the start that are external, have external calls, don't have any modifiers, and modify state."

example input:

slither-function-filter contract.sol --viz external --ext-call 1 --modifier 0 --state 1

where,

1 or 0 for true/fals, if --ext-call 1 == include all functions that make AT LEAST 1 external call

returns:

functions matching criteria printed to cli (guidance as to this may be needed, i didn't spent that much time on Output class, PR will just print to cli initially)

@lucas-manuel
Copy link
Author

@shortdoom yes a tool would be totally fine with me, that sounds good!

@0xalpharush
Copy link
Contributor

0xalpharush commented Nov 17, 2023

Just a heads up that we are likely moving towards subcommands and that work may conflict. We may be able to make the filter a global arg and pass that down to the detect command and print command as outlined here:

Instead of filter, maybe we can use exclude and include (or similar names, but that works in both directions). We can then use the same options for detectors and printers, so something like

--exclude-filenames / --include-filenames
--exclude-contracts / --include-contracts
--exclude-functions / --include-functions. Maybe with both support for f(..) and contract.f(..), so we work with top level functions, and can filter similar functions across contracts in case of inheritance

@shortdoom I've been thinking about a way to expose info in a way that's easier to query. If we replaced the printer table output with something more structured that can be parsed we can do something like the following. This example queries for writes to is_killed and can be combined with a logical operator to further filter the query like /-is_killed && Sends ETH: no/. Wdyt of this approach?

Screenshot 2023-11-17 at 2 14 15 PM

@shortdoom
Copy link

shortdoom commented Nov 19, 2023

i made a "draft" PR. it doesn't include requested regex matching but it's straight forward to implement if needed. i followed existing implementation of parser without sub-commands for now as this is the current state of dev branch. regardless, I think slither-function-filter tool still can provide functionality above of (to be updated) output control of printers/detectors. search -> match -> print is the specific flow of this tool.

as for the better control of outputs, I'll comment in the referenced Issue after having some closer look. but I am of a biased opinion that Python API outputs should be prioritized over cli for restructuring. for larger codebases API integration is go-to solution over parsing cli output. for cli I think that standardization of output is enough - making output parsable ie. <name>:<value> - exactly as presented on the screenshot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants