OriginProtocol / origin-js

We've moved to a monorepo: https://github.com/OriginProtocol/origin
MIT License
81 stars 33 forks source link

Remove Truffle #94

Open wanderingstan opened 6 years ago

wanderingstan commented 6 years ago

There have been several suggestions about removing Truffle from our workflow, notably from @nick . Most recently in demo-dapp issue 105. As a team we've been burned a few times by people having different versions of truffle.

Features around compiling and deploying can be done with solc, and ganache-cli is a better test chain than truffle develop

@jsulinski added list of cons:

[1] https://github.com/trufflesuite/truffle/issues/596 [2] https://github.com/trufflesuite/truffle/issues/693 [3] https://github.com/trufflesuite/truffle/issues/555 [4] https://github.com/trufflesuite/truffle-contract/pull/95/commits [5] https://github.com/trufflesuite/truffle-contract/issues/100 [6] https://github.com/trufflesuite/truffle-contract/issues/42

@joshfraser This is a big one and your input would be valuable here.

DanielVF commented 6 years ago

From a use in API point of view, truffle doesn't add anything that we need.

We'll just need a way to quickly deploy our registry contracts onto our dev blockchain and a way to get the addresses of those to our code.

wanderingstan commented 6 years ago

Maybe dumb question, but how does one deploy/migrate contracts without truffle?

nsimon commented 6 years ago

@wanderingstan -- here's a simple example without truffle

Assumption: ganache-cli (test blockchain with 10 accounts) is running as a background task

    // Create new instance of a Web3 object
    const web3 = new Web3 (new Web3.providers.HttpProvider ("http://localhost:8545"));

    // Extract the 10 test accounts
    const accounts = web3.eth.accounts;

    // Load smart contact source (e.g. announcement.sol)
    const sourceCode = fs.readFileSync ("announcement.sol").toString();

    // Compile the smart contract source
    const compiledCode = solc.compile (sourceCode);

    // Extract the application binary interface
    const contractABI = JSON.parse (compiledCode.contracts [":announcement"].interface);

    // Extract the byteCode
    const byteCode = compiledCode.contracts [":announcement"].bytecode;

    // Instantiate the smart contract
    const announcementContract = web3.eth.contract (contractABI);

    // Deploy to the test blockchain
    const announcementDeployed = announcementContract.new ({data: byteCode, from: web3.eth.accounts [0], gas: 4700000});

    // Allow the smart contract to fully deploy before continuing
    sleep (3000);

    // Extract the deployed contract address
    const deployedAddress = announcementDeployed.address;
nick commented 6 years ago

I added a few deployment related helper scripts here:

https://github.com/OriginProtocol/identity-playground/tree/master/scripts

The actual deployment is just a web3 call:

https://github.com/OriginProtocol/identity-playground/blob/master/test/_helper.js#L116-L151

Once the deploy script is called, we can put the returned address into a simple neworkid:contract mapping similar to how truffle does it...

module.exports  = {
  2: { Listings: "0xabc123" }, // Ropsten
  3: { Listings: "0xabc345" }, // Rinkeby
}

To instantiate the contract in the web app would be something like this:

var Listing = require('./Listing.js')
var deployedContracts = require('./deployedContracts')
var netId = await web3.eth.getId()
var listingInstance = new web3.eth.Contract(Listing.abi, deployedContracts[netId].Listing)
nsimon commented 6 years ago

@nick I defer to your solution, sir :+1: (still learning here)

tyleryasaka commented 6 years ago

I really would like to see us keep Truffle. Moving away from Truffle is going to require us to do a lot more of the heavy lifting ourselves. Obviously @nick has done a lot of great work around this, but it's not a one-time thing. It'll be code that we have to continually maintain ourselves. (What we're doing is ambitious enough as it is šŸ˜….) When there's a good option out there that is used by the broader community, I definitely prefer to use it. Truffle isn't perfect but it offers a lot of conveniences and is under active development.

As far as the versioning - that's an npm thing, not a truffle thing. There are ways to make the Truffle dependency more reliable. One easy solution is to add an npm script that will use a local truffle dependency, rather than requiring the user to install Truffle globally. We can guarantee the specific version of a local Truffle installation.

@wanderingstan Can you elaborate how "ganache-cli is a better test chain than truffle develop"?

Regarding the bullet point cons: this is just my 2 cents, but I don't think any of these are deal-breakers. Truffle is in active development and I think they will continuously be addressing the pain points. That said I do see the concerns, and I think if others think they are a big deal then I can understand the desire to move away from Truffle. Though I'm still not convinced the cons outweigh the pros.

To reiterate the point that I was making at the beginning. I think there's a lot of value in participating in existing solutions in the community, even if they're not perfect. One of Origin's main selling points is providing a shared network effect for marketplaces. I think it's consistent with our philosophy to try to integrate with the community rather than try to roll out our own thing, whenever possible. Then we get the shared network effect of open source collaboration. šŸ˜ƒ

aiham commented 6 years ago

As far as the versioning - that's an npm thing, not a truffle thing. There are ways to make the Truffle dependency more reliable. One easy solution is to add an npm script that will use a local truffle dependency, rather than requiring the user to install Truffle globally. We can guarantee the specific version of a local Truffle installation.

Just wanted to point out that this is already the case:

https://github.com/OriginProtocol/platform/blob/develop/packages/contracts/package.json#L12

Devs should probably stop using their global truffle.

tyleryasaka commented 6 years ago

@aiham Yeah we have the dependency specified, but the truffle command uses the global installation. We'd have to add an npm start command that would execute:

./node_modules/truffle/build/cli.bundled.js develop

So yes, if we update the package.json with that command and place npm run start instead of truffle develop in the readme for the contracts, then we'll be all set.

nick commented 6 years ago

I'm not sure what heavy lifting truffle is actually doing... apart from deploying contracts and putting a wrapper around the contract api. The contract wrapper ties us to web3 0.2 for now and most of the improvements are native in web3 1.0 anyway. The test helper means we can't write watchable tests and use vanilla mocha (writing tests without auto reload is painful, imo). The built JSON outputted by truffle is huge and bloats the final bundle size (all we need is the ABI and data). We have to wait for truffle to update whenever there is a new solidity release. We're also tying all our developers to using truffle.

The only drawback to removing truffle is that everything we already have uses it and re-writing it will be a pain. I'm definitely not a fan of re-writing things that already work fine, but perhaps we can have a hybrid approach and write new code without truffle to ease the transition. Truffle develop is already powered by ganache, so we wouldn't really be 'switching' to it.

Could we list some 'pros' for continuing to use truffle, as we already have a bunch of cons? Honestly my main con is the watchable tests, which is why I wrote an alternative in the first place :-)

tyleryasaka commented 6 years ago

I'm not sure what heavy lifting truffle is actually doing

For starters, I'll quote their docs:

  • Built-in smart contract compilation, linking, deployment and binary management.
  • Automated contract testing for rapid development.
  • Scriptable, extensible deployment & migrations framework.
  • Network management for deploying to any number of public & private networks.
  • Package management with EthPM & NPM, using the ERC190 standard.
  • Interactive console for direct contract communication.
  • Configurable build pipeline with support for tight integration.
  • External script runner that executes scripts within a Truffle environment.

For me, highlights are:

  1. compliation/deployment (especially managing deployments on different networks)
  2. migrations. I think we'll appreciate the migrations much more once we're live on the main net.
  3. built-in testing framework
  4. ability to write your own scripts that can be run with truffle exec (useful for things like seeding the blockchain)
  5. Knowing that we're using a shared platform that will continuously be improving. New features that could be very useful for us may be added in the future, and if we're using Truffle we'll be able to receive all the benefits without having to build them ourselves.
  6. All of these features are provided in a clean, easy to use interface. (Interacting with Ganache directly is much more low-level and requires us to dig into details that we often shouldn't be concerned about. As can be demonstrated in the code for @nick 's test helper.)

Yes we can have watchable tests using ganache-cli directly, but now we have to maintain our own test helper. Smells a bit NIH syndrome to me. The helper seems to work well, at least for our current needs, but we really shouldn't be rolling out our own alternative when the community already has a robust solution.

If watchable tests are the hang-up, let's contribute to Truffle and see if we can't add the ability to have watchable tests.

nick commented 6 years ago

Watchable tests have been a feature request since September 2016 - if it was easy I think someone would have added it by now.

Truffle is still using web3 0.2.x - they are working on upgrading but who knows when that'll actually happen. If we're fine with web3 0.2.x then that's OK... but the more code we write now that uses it, the harder it'll be to upgrade, if we ever choose to do so.

I totally agree with not re-inventing the wheel, but I think ultimately truffle offers us a little convenience in exchange for complete dependence on their eco system.

  1. Compilation/deployment - essentially just remembering what address a contract was deployed to
  2. A 'migration' just refers to keeping track of what contract has already been deployed to a particular network. It's no more complicated than that. The rest can be accomplished with vanilla JS.
  3. Lock in to a particular testing framework. What if you want to use Jest instead of mocha?

All that being said... I don't want to push too hard only to regret it later. When I first started looking at dapps a few months ago I was on the truffle train, but after a number of frustrations and realizing I could have complete control, use the latest developments, and drop a large dependency all by using a 150 line helper script, I concluded that truffle wasn't necessary. However maybe I didn't give it enough of a chance.

aiham commented 6 years ago

@tyleryasaka the command in the readme uses npx which should run the local truffle, and install it on demand when not installed.

tyleryasaka commented 6 years ago

@nick I see your points.

  1. Yes it seems simple but I think it's definitely nice to have a framework that abstracts that away for us. The little things add up when you have to do it all yourself.
  2. Same as above - I do think there is value in keeping up with which contracts have been deployed using the migrations format that is used for managing databases - but maybe it's not that big of a deal in the blockchain space. I don't know.
  3. Yeah, being locked into a specific testing framework kinda sucks. Agreed there.

Yeah, honestly I'm not super knowledgeable about what Truffle has been doing behind the scenes, so maybe I'm overestimating its value to us. At the same time, the fact that I haven't had to know these details seems to indicate that Truffle has done a pretty good job at abstracting them away.

I've made my case - if the consensus from everyone else is to move away from Truffle then I won't fight it. I just want to make sure we're really clear about what we're doing here and like you said @nick , that we don't regret it later.

By the way, regarding the test watching issue. Truffle's docs do mention it, at the alert on the bottom of this page.

Please use external tools to watch for filesystem changes and rerun tests.

I'm sure we could set up a script to do this.

@aiham Oh wow ok. My bad. I just now learned what npx does. That's awesome, TIL šŸ‘

wanderingstan commented 6 years ago

I'm not too passionate about this, but my feel is that it's not worth doing extra work to remove truffle. The main negative is the lack of web3 1.x support, which is not (AFAIK) blocking us at the moment.

The automated file-watching is extremely cool, but I'm happy to have that with or without truffle.

Do we have any sense of what other projects are doing? Or are we so awesome that we're real trailblazers here? :)

jsulinski commented 6 years ago

@tyleryasaka

As far as the versioning - that's an npm thing, not a truffle thing.

I was referring to the web3.js versioning, PR is here for 1.x: https://github.com/trufflesuite/truffle-contract/pull/95/commits

@wanderingstan

For just truffle-contract, some alternatives are raw web3 or ethjs.

https://github.com/polyswarm/perigord is an example of an alternative to Truffle for golang.

I don't have an opinion yet on whether replacing truffle-contract or truffle is worth the effort, but the issues with builds would be concerning for a production application.

tyleryasaka commented 6 years ago

@jsulinski Right, I was addressing this statement rather than yours:

As a team we've been burned a few times by people having different versions of truffle

The fun of ambiguous language. šŸ˜„ In any case we're all on the same page that that issue has been resolved.

And yes I am aware of the upcoming web3 1.0 release, and that it may take some time for Truffle to support it.

tyleryasaka commented 6 years ago

This issue overlaps somewhat with https://github.com/OriginProtocol/platform/issues/89. Removing Truffle could simplify the development setup process without having to introduce shell scripts (as I have started to do in https://github.com/OriginProtocol/platform/pull/95).

I'm taking it upon myself to create an example setup of our codebase without Truffle, just so we can see what it might look like. (I'll rely on @nick 's work in the identity playground as a guide.) Much easier to talk about actual code than in abstract. Will report back once that's done.

gnidan commented 6 years ago

(Watching for research purposes)