Skip to content

Latest commit

 

History

History
86 lines (57 loc) · 3.72 KB

green-lemon_2_keeganquigley.md

File metadata and controls

86 lines (57 loc) · 3.72 KB
Raw

Evaluation

  • Status: Accepted.
  • Application Document: Green Lemon Protocol
  • Milestone: 2
  • Previously successfully merged evaluation: All by keeganquigley
Number Deliverable Accepted Link Evaluation Notes
0a. License
Link
0b. Documentation
Link Looks good.
0c. Testing Guide
Link All steps were eventually successful. See notes below.
0d. Article
Article, Video Looks good.
1. (ink!)Smart contracts: Anonymous NFT
Link Successfully deployed ERC721 and verifier contracts. See notes below.
2. (Node.js)Relayer service
Link Successfully deployed relayer contract. See notes below.

General Notes

Documentation

Docs were somewhat improved upon request. Great job on inline comments.

Testing Guide

  1. Ran into issue with substrate-greenlemon-node where my mac wouldn't recognize the unix executable file.
    UPDATE: release was fixed by adding new .zip file.

  2. Ran into Zokrates compatibility issue:

sh ./circuits/build.sh
Compiling withdraw.zok

Compilation failed:

withdraw.zok:7:35
	Visibility modifiers on arguments are only allowed on the entrypoint function

withdraw.zok:7:55
	Visibility modifiers on arguments are only allowed on the entrypoint function

withdraw.zok:7:94
	Visibility modifiers on arguments are only allowed on the entrypoint function
mv: abi.json: No such file or directory
mv: rename out to ../build/out: No such file or directory

UPDATE: Issue fixed.

  1. Every time there is a system change (such as a reboot) I have to re-add Zokrates to my path with export PATH=$PATH:/Users/keeganquigley/.zokrates/bin; I would suggest adding this to the testing instructions, otherwise the scripts will fail.

UPDATE: Issue fixed.

  1. Unit tests are successful for all functions, however I noticed that there isn't one for transfer_from in the ERC721 contract. Consider adding it if it makes sense.

UPDATE: Issue explained/fixed.

  1. Minor improvement suggestion for Step 2: take out "baseUri" so that it's clear that just the string needs to be entered:
https://raw.githubusercontent.com/GreenLemonProtocol/assets/main/nft

UPDATE: fixed.

Scripts

All js scripts run successfully. Index service starts successfully.

Contracts

cargo +nightly clippy generates quite a few warnings for each of the contracts. Please take a look at these to see which ones make sense to fix. You can also use the command below to attempt to fix them automatically:

cargo doc --open works to generate all HTML docs.

cargo +nightly clippy –fix

UPDATE: cargo warnings fixed.

Relayer

Deposit, RegisterPublicKeys, Withdrawal, Execute functions all work manually and with automated tests.

Verifier

verify function works manually and with automated tests.

ERC721

mint, burn, transfer, approve functions all work manually and with automated tests.

Please note: No security audits have been conducted as part of this evaluation.