crowdfundrr_silentcicero_review.md

Review of Crowdfunding DAPP bounty submission

• bounty: https://forum.ethereum.org/discussion/1779/bounty-crowdfunding-dapp
• submission by SilentCicero (https://forum.ethereum.org/profile/3697/SilentCicero)
• Source: https://github.com/CrowdFundrr/CrowdFundrr (commit https://github.com/CrowdFundrr/CrowdFundrr/tree/a1cb55776d524ba78ad899f1f485970ead98b6ee)
• Website: http://crowdfundrr.github.io/
• review by Joris Bontje on 2015-02-16

TLDR; Review summary:

Nice first impression, good presentation & companion website.

Solidity contract compiles, but has a few bugs; I wasn't able to deploy & test it; the contract in easydeploy.html doesn't get mined. No testnet deployment to try; unable to verify that the provided functionality actually works.

Javascript is unstructured 'oldskool' jquery, mixing UI and logic. Not using a web application framework.

Very promising progress, but needs a little work to make this demonstrable!

Also please provide your bitcoin address for future bounty payout ;)

Review of core functionality:

Link to a campaign via the URL

Possible via http://crowdfundrr.github.io/tracker.html?id=ID and implemented in Javascript https://github.com/CrowdFundrr/CrowdFundrr/blob/a1cb55776d524ba78ad899f1f485970ead98b6ee/js/CrowdFundrr.js#L376-L410

Wasn't able to actually test it. Links for the the demo campaigns on the site all point to the same "Jerry's Ice Cream Shop Campaign".

Create a campaign

Implemented on http://crowdfundrr.github.io/index.html#new

Nice instructions in UI!

Javascript code: https://github.com/CrowdFundrr/CrowdFundrr/blob/a1cb55776d524ba78ad899f1f485970ead98b6ee/js/CrowdFundrr.js#L133-L202 Solidity: https://github.com/CrowdFundrr/CrowdFundrr/blob/a1cb55776d524ba78ad899f1f485970ead98b6ee/CrowdFundrr.sol#L38-L54

BUG: block.timestamp is unix time (seconds since epoch), however JS Date.getTime() returns milliseconds. https://github.com/CrowdFundrr/CrowdFundrr/blob/a1cb55776d524ba78ad899f1f485970ead98b6ee/js/CrowdFundrr.js#L140

Suggestions:

• Use Ether for goal amount instead of WEI.
• Prefill Beneficiary (typo on site) address with the one of the current user. (the current keyboard logic is not obvious)

Gives javascript alert boxes (unfinished development feedback) upon submission in mist. Would expect nicer feedback.

Contribute to a campaign

Implemented on: https://github.com/CrowdFundrr/CrowdFundrr/blob/a1cb55776d524ba78ad899f1f485970ead98b6ee/js/CrowdFundrr.js#L412-L423 https://github.com/CrowdFundrr/CrowdFundrr/blob/a1cb55776d524ba78ad899f1f485970ead98b6ee/CrowdFundrr.sol#L56-L63

Pages show $ instead of Ether?

Suggestion:

• Use Ether instead of $ or Wei.
• Provide user feedback after contribution

If sufficient funding received, payout to campaign recipient

In the CrowdFundrr implementation this needs to be explicitly triggered, unlike Vitalik's example where this happens automatically: https://github.com/ethereum/serpent/blob/develop/examples/crowdfund.se#L21-L25 There is something to be said for this approach, otherwise the last contributor might have an enormous gas fee.

All records are deleted upon completion, I would suggest not to do this for auditing purposes. https://github.com/CrowdFundrr/CrowdFundrr/blob/a1cb55776d524ba78ad899f1f485970ead98b6ee/CrowdFundrr.sol#L81-L89

If expired, refund to contributors

In the CrowdFundrr implementation this needs to be explicitly triggered, unlike Vitalik's example where this happens automatically: https://github.com/ethereum/serpent/blob/develop/examples/crowdfund.se#L27-L35 There is something to be said for this approach, otherwise the last contributor might have an enormous gas fee.

Alternatively this could be make so that easy user can claim back their own contribution in the end, but that might be more complex to implement.

BUG: the button to trigger this is never shown if the goal isn't reached. https://github.com/CrowdFundrr/CrowdFundrr/blob/a1cb55776d524ba78ad899f1f485970ead98b6ee/js/CrowdFundrr.js#L399-L408

BUG: funders can be refunded multiple times upon expiration https://github.com/CrowdFundrr/CrowdFundrr/blob/a1cb55776d524ba78ad899f1f485970ead98b6ee/CrowdFundrr.sol#L68-L75

Display progress report of a campaign

Seems to be there, not sure it works.

Hint:

• I just learned that get function in Solidity have been made redundant by the addition of accessor functions: https://github.com/ethereum/wiki/wiki/Solidity-Tutorial#accessor-functions

Review of Rules & Criteria:

1) Contract should be implemented using Solidity.

Correct & Compiles!

2) Frontend to use the ethereum.js API

Yes, but not sure it actually works

3) Use a JavaScript framework such as Angular, Backbone, Meteor, React, etc.

No, I would strongly suggest to use a web application framework over mixing logic and UI.

4) Hosting of frontend assets (HTML + JS) on static server (ie GitHub pages)

Yes, on http://crowdfundrr.github.io/

5) No backend required (besides local Ethereum node and static www server)

Correct

6) Deployed on POC-8 (or later) testnet

No, having a working deployment is really useful for demo purposes! Your deployment tool is a good start, but I would actually suggest it to compile the solidity contract as well, see https://github.com/ethereum/ethereum.js/blob/develop/example/contract.html for a demo.

7) UI fully works from within AlethZero, Mist and external browser using Ethereum's JSONRPC API (HttpSyncProvider).

No, seems to only partially work. I realize it is very painful to get a working DAPP out since the clients & API have been such a moving target lately. Good job on filing bug reports for the various clients, that is very helpful!

8) All components open source & published on GitHub

Yes, MIT licensed and published on https://github.com/CrowdFundrr/CrowdFundrr

9) DEV employees & contractors are excluded

Yes, @SilentCicero seems unaffiliated :D