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

Integrate with Truffle #18

Closed
dguido opened this issue Sep 14, 2018 · 26 comments
Closed

Integrate with Truffle #18

dguido opened this issue Sep 14, 2018 · 26 comments
Labels
help wanted Extra attention is needed translation ux

Comments

@dguido
Copy link
Member

dguido commented Sep 14, 2018

Truffle is the most popular development framework for smart contracts, and building this feature would enable the largest possible audience to have instant access to Slither.

At Trail of Bits, we are not experts in the internals of Truffle, and we try to avoid writing anything in JavaScript whenever possible. Our expertise here is limited and we're looking for help to implement this feature the "right" way. A successful implementation should use the lowest number of installation and configuration steps. It should just work.

@dguido dguido changed the title Support the Truffle AST Integrate with Truffle Oct 19, 2018
@dguido dguido added the help wanted Extra attention is needed label Oct 19, 2018
@dguido
Copy link
Member Author

dguido commented Oct 22, 2018

It should be more straightforward to support Truffle once we have the solc compact AST support merged in. truffle compile creates one AST json file per contract, which we can merge into a single json that Slither can analyze.

It's likely that we could do this as a package that does the json merging, and then hook into truffle via a truffle slither-analyze or similar command.

Or, we could be totally wrong and there is a simpler way to build this feature.

@mkosowsk
Copy link

@montyly you and @dguido have disagreements on what is an appropriate bounty size for this issue :)

I'll let you two hash it out on what makes sense for the best approach and then come back in later when scoping is figured out 👍

@montyly
Copy link
Member

montyly commented Oct 25, 2018

We have merged in support for the compact AST in #54.

Slither's dev branch supports now Truffle if you run the following:

truffle compile
slither .

I agree with Dan, we should integrate with truffle directly, like truffle slither-analyze, where the command runs the two commands above and prints the result to the user.

It seems feasible through the truffle package system: https://github.com/trufflesuite/truffle/blob/next/CONTRIBUTING.md#add-a-new-command-in-truffle

I am pushing this issue to high priority. We're going to stop working on other issues to focus on this one.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to it as part of the Ethereum Community Fund via ECF Web 3.0 Infrastructure Fund fund.

@gitcoinbot
Copy link

gitcoinbot commented Oct 25, 2018

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 7 months, 2 weeks from now.
Please review their action plans below:

1) anukul has been approved to start work.

I'm quite comfortable with python and javascript, and I've started looking into how Truffle works and can be extended further with new commands so as to accommodate Slither.
I realize that this is high priority, so I'll work fast to keep pace with the requirement.

Learn more on the Gitcoin Issue Details page.

@gitcoinbot
Copy link

@anukul Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

1 similar comment
@gitcoinbot
Copy link

@anukul Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@ghost
Copy link

ghost commented Oct 31, 2018

[In progress]

Does slither automatically have to be installed alongwith truffle?
I don't think that would be feasible since truffle is an npm package, and we have no reason to assume that those users would have python / pip installed.

Is it okay if truffle slither-analyze errors out with an appropriate message (and perhaps a suggestion to install slither) if it can't find the slither binary in the PATH?

@montyly
Copy link
Member

montyly commented Oct 31, 2018

If slither is not found, printing a message with the slither installation commands works for me.

@gitcoinbot
Copy link

@anukul Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

1 similar comment
@gitcoinbot
Copy link

@anukul Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


@anukul due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

1 similar comment
@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


@anukul due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@rmshea
Copy link

rmshea commented Nov 8, 2018

Hey @anukul, Ryan from Gitcoin checking in here. How's the progress coming on this bounty? Is there anything I can do to help move it along? Thanks 😸 🎉

@anukul
Copy link

anukul commented Nov 12, 2018

@ryan-shea @montyly Given the response on the PR, do we keep this on hold?

@montyly
Copy link
Member

montyly commented Nov 14, 2018

Yes, unfortunately, we won't have a truffle slither-analyze.

We will see how the truffle run <command-plugin> works, maybe we can still have a npm package to integrate slither within truffle, though I may not sure that it will provide any advantage over just running slither .

I will integrate the execution of truffle compile with slither, and we will see what makes more sense for the users

@anukul
Copy link

anukul commented Nov 17, 2018

Cool, is there anything else I can do to complete the bounty?

@spm32
Copy link

spm32 commented Nov 28, 2018

Hey @montyly any thoughts on @anukul's comment?

@montyly
Copy link
Member

montyly commented Nov 28, 2018

@ceresstation yes you can validate the bounty, thanks!

@anukul: thank you for your contribution.
Truffle just release the version 0.5.0 which includes truffle run <cmd> (https://github.com/trufflesuite/truffle/releases#truffle-run), it looks like we can adapt your code to work with it!

@mkosowsk
Copy link

mkosowsk commented Dec 4, 2018

@anukul please submit work on the Gitcoin issues detail page and we will pay out ASAP. Thanks and my apologies about the delay! :)

@anukul
Copy link

anukul commented Dec 4, 2018

@mkosowsk I can't seem to log in to Gitcoin.
I had reported the issue and it is being tracked on their repo - gitcoinco/web#2743

Can you proceed with the payment from your end?
Or I'll have to find some time this weekend and try to fix that bug myself. :P

@gitcoinbot
Copy link

⚡️ A tip worth 200.00000 DAI (200.0 USD @ $1.0/DAI) has been granted to @anukul for this issue from @. ⚡️

Nice work @anukul! Your tip has automatically been deposited in the ETH address we have on file.

@mkosowsk
Copy link

mkosowsk commented Dec 4, 2018

@anukul should be paid out, thanks!

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This Bounty has been completed.

Additional Tips for this Bounty:

  • tipped 200.0000 DAI worth 200.0 USD to anukul.

@BlinkyStitt
Copy link

I think this should be closed.

@dguido
Copy link
Member Author

dguido commented Jan 7, 2019

Yes, I think this is sufficiently addressed for now.

@dguido dguido closed this as completed Jan 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed translation ux
Projects
None yet
Development

No branches or pull requests

8 participants