search

iExecBlockchainComputing / iexec-apps

Dockerfile of all iExec apps
18 stars 23 forks source link

Soccer dapp #19

Closed pengiundev closed 5 years ago

Amxx commented 5 years ago

Hi @pengiundev,

Thanks for submitting your pull request. Sorry for the delay reviewing it, we were all pretty busy these days.

So I looked at your code, and here is my feedback:

  1. Don't just return a '1-0' in a string and prefer encoding using a meaningful format. In this case, using two integers to store the result would be much easier to work with afterward from the solidity code. https://github.com/iExecBlockchainComputing/iexec-apps/pull/19/files#diff-f7e3e12dab50308c1f9e05161bf9c2eaR18 [MUST BE FIXED]

  2. Still uses a demo API. Should be compatible with a lie API + support for encrypted apikeys (in a dataset) https://github.com/iExecBlockchainComputing/iexec-apps/pull/19/files#diff-f7e3e12dab50308c1f9e05161bf9c2eaR8 [NICE TO FIXED]

  3. Add a readme that explains what the app does, and how it works (here is an example) [MUST BE FIXED]

  4. Rename the smart contract and modify it to store meaningful results that can be processed onchain. https://github.com/iExecBlockchainComputing/iexec-apps/pull/19/files#diff-5dcbe0a833bd74cf8d649d9fdffcdea0R7 [MUST BE FIXED]

Please let me know if anything isn’t clear! As soon as these are fixed, the bounty is yours. Best, Hadrien