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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Milestone Delivery: Green Lemon Protocol馃崑 Milestone 2 #604

Merged
merged 2 commits into from
Nov 22, 2022

Conversation

wuyahuang
Copy link
Contributor

@wuyahuang wuyahuang commented Oct 18, 2022

Milestone Delivery Checklist

Link to the application pull request: w3f/Grants-Program#1096

@Noc2
Copy link
Collaborator

Noc2 commented Oct 21, 2022

Thanks for the delivery. We will look into it as soon as possible. An external evaluator might also take a look at your delivery.

@keeganquigley keeganquigley self-assigned this Oct 26, 2022
@keeganquigley
Copy link
Contributor

Hi @wuyahuang apologies for the delay, we are on a company retreat this week so it will take me some time to review your milestone but I will reach out to you soon with any questions I may have.

@keeganquigley
Copy link
Contributor

keeganquigley commented Nov 9, 2022

Hi again @wuyahuang sorry for the delay.

I am running into an error when testing all contracts with sh ./test-all.sh UPDATE: See comment below.

I ran cargo +nightly test for each contract individually and realized it's happening for all contracts.

Relayer:

error[E0158]: associated consts cannot be referenced in patterns
   --> lib.rs:128:5
    |
128 | /     #[derive(SpreadAllocate)]
129 | |     pub struct Relayer {
130 | |         // Stores the address of contract verifier
131 | |         pub verifier: AccountId,
...   |
145 | |         pub next_index: u32,
146 | |     }
    | |_____^

For more information about this error, try `rustc --explain E0158`.
error: could not compile `relayer` due to previous error

Verifier:

error[E0158]: associated consts cannot be referenced in patterns
  --> lib.rs:30:5
   |
30 |     pub struct Verifier {}
   |     ^^^^^^^^^^^^^^^^^^^^^^

For more information about this error, try `rustc --explain E0158`.
error: could not compile `verifier` due to previous error

ERC721:

error[E0158]: associated consts cannot be referenced in patterns
  --> lib.rs:68:3
   |
68 | /   #[derive(Default, SpreadAllocate)]
69 | |   pub struct Erc721 {
70 | |     /// Total supply
71 | |     total_supply: u32,
...  |
83 | |     base_uri: String,
84 | |   }
   | |___^

For more information about this error, try `rustc --explain E0158`.
error: could not compile `erc721` due to previous error

I believe it has to do with this known issue where ink! 1.5.0 wasn't compatible with the latest +nightly updates. Using the solution there allowed the contracts to test and build successfully. Here was the original issue.

This issue has the patch to fix the contracts. Are you able to fix these? Thanks!

@keeganquigley
Copy link
Contributor

UPDATE: I apologize I spoke too soon @wuyahuang, simply running rustup update fixed the issue for me. 馃榾

Please disregard my above comment and I will keep working on the evaluation.

@wuyahuang
Copy link
Contributor Author

Hi, @keeganquigley I submitted the same issue to cargo-contract last month. I'm happy to see the issue is addressed.

Please let me know if you have any further questions.馃槃

@keeganquigley
Copy link
Contributor

keeganquigley commented Nov 15, 2022

Hi @wuyahuang sorry again for the delay, I am currently only part-time and was off for a couple days last week.

  1. I am having trouble getting your custom substrate-greenlemon-node to run. After downloading the source code .zip file, running cargo test fails with:
   Compiling sp-rpc v6.0.0 (https://github.com/paritytech/substrate#021f7126)
error: `sp_trie::recorder::Recorder<H>::as_trie_recorder::{opaque#0}<'_>` does not live long enough
   --> /Users/keeganquigley/.cargo/git/checkouts/substrate-7e08433d4c370a21/021f712/primitives/state-machine/src/trie_backend_essence.rs:181:44
    |
181 |         let recorder = recorder.as_mut().map(|r| r as _);
    |                                                  ^

error: `sp_trie::recorder::Recorder<H>::as_trie_recorder::{opaque#0}<'_>` does not live long enough
   --> /Users/keeganquigley/.cargo/git/checkouts/substrate-7e08433d4c370a21/021f712/primitives/state-machine/src/trie_backend_essence.rs:219:44
    |
219 |         let recorder = recorder.as_mut().map(|r| r as _);
    |                                                  ^

error: could not compile `sp-state-machine` due to 2 previous errors
warning: build failed, waiting for other jobs to finish...

I get the same errors when cloning https://github.com/GreenLemonProtocol/substrate-contracts-node.git and trying to run it that way.

The contracts build and run successfully on a local substrate-contracts-node but of course, due to the weight adjustments, I can't enter the proper amount of gas to be able to execute the calls. Is the node supposed to run out-of-the-box?

  1. The YouTube video is very helpful, however some of the steps you take seem to be missing from the tutorial steps. For example, the step where you copy the "commitmentHex" string from proof-1.json and enter it into the contract call with a value of "1" is not documented in the steps. Can you update the docs to include all the necessary steps? If you could explain the "proofs" that would be great too.

  2. The same goes for the "Client test" section in the video. The step of entering in "Alice" as an alias string isn't covered in the docs. If someone didn't have your video for reference, I'm not sure how they would figure out the steps.

  3. I appreciate the high-level introduction to the four functions, but after this, the docs start to get a bit sparse. Especially for the "Generate commitment and proof manually" section, it would be nice to see descriptions for exactly what these scripts are accomplishing (as it's a bit hard to tell from the inline comments alone).

I know these suggestions might seem minor or pedantic, but seeing as this milestone will complete the grant, it would be great to see more improved, robust docs. They should give a good introduction to the different components of the project and any frameworks used. As it stands now, without any audio in the video, it is hard to tell exactly what is happening.

@wuyahuang
Copy link
Contributor Author

wuyahuang commented Nov 16, 2022

Hi, @keeganquigley Thanks for your valuable feedback.

  1. Please use compiled substrate-green-lemon-node, and download it from here. The cargo build and cargo test both work as expected on my side.馃槀 Actually, I have no idea why the cargo test failed, because I didn't modify that part of the source code, I only increase the MAXIMUM_BLOCK_WEIGHT here.

The version of my DEV environment(macOS 11.3.1)

rustc --version

rustc 1.64.0 (a55dd71d5 2022-09-19)

cargo --version

cargo 1.64.0 (387270bc7 2022-09-16)

  1. As I supposed, the video is part of the tutorial.馃槀 However, issue 2 and issue 3 are added to the text tutorial as you suggested. Theproof means zero-knowledge proof which is used to prove the user's previous deposit record. As README mentioned, For more details about zero-knowledge proof, please click me.

  2. I Already updated the tutorial.

  3. I Already added a description of why we need the generate commitment and proof manually section.

@keeganquigley
Copy link
Contributor

Thank you for the updated documentation @wuyahuang, it is looking much better now! I appreciate you adding the details on overall functionality. Reading through your dksap-polkadot repo also helped to give me a good understanding of your product.

Regarding your response about the node, I am still not sure how to execute it. The release page has a package file substrate-greenlemon-node with no .filetype is it supposed to?

When I download the source code.tar.gz file and extract it, it doesn't produce an artifact like it does when I download the official substrate-contract-node release. Instead it produces the source code files (which then fail to build). Also, does it matter whether I am using a mac or not?

For example, with the official release I download the substrate-contracts-node-mac-universal.tar.gz option. After downloading and extracting the file, I can use ./substrate-contracts-node --dev to start up the node.

But with your node there is no substrate-greenlemon-node artifact to launch. I know these are elementary steps, but typically I just launch the node using substrate-contracts-node --dev or cargo run --release. Or better yet a Docker image which can quickly launch the node. This isn't required since it wasn't in your deliverables, but I appreciate any help you can provide!

@wuyahuang
Copy link
Contributor Author

wuyahuang commented Nov 17, 2022

Hi, @keeganquigley
Please just download the package file substrate-greenlemon-node with no .filetype from release page.

And execute ./substrate-greenlemon-node --dev.

I also uploaded a new file substrate-greenlemon-node-mac-universal.zip to make it looks more easy-understanding. And like the official release, just downloading and extracting it. and execute ./substrate-greenlemon-node --dev to start up the node.

@keeganquigley
Copy link
Contributor

thanks @wuyahuang when I do that it says:

./substrate-greenlemon-node --dev
zsh: permission denied: ./substrate-greenlemon-node

When I try it with sudo it says this:

sudo ./substrate-greenlemon-node --dev
Password:
sudo: ./substrate-greenlemon-node: command not found

However, extracting your new substrate-greenlemon-node-mac-universal.zip works to produce the artifact! Thank you.

I think the issue is that when downloading it directly, the mac doesn't recognize it as a unix executable file, as you can see below (the file with the (1) is the file with no .filename), whereas the zip allows it to be recognized.

Screen Shot 2022-11-16 at 11 58 50 PM

I will continue my evaluation and get back to you.

@wuyahuang
Copy link
Contributor Author

@keeganquigley Thanks for your help with this issue.

I just deleted the original executable file substrate-greenlemon-node for better understanding. And the tutorial is also updated.

@keeganquigley
Copy link
Contributor

@wuyahuang thanks for your help. I was able to get through all the steps up until the Circuits section.

  1. When running sh ./circuits/build.sh I get the following error:
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

Do you know what this might be indicating?

  1. Moving on, in the Generate commitment and proof manually steps:

node scripts/0-generateCommitment.js script works:

The commitment has been generated successfully, located in /Users/keeganquigley/ink/build/commitment.json

However, the next script fails: node scripts/1-compute-witness.js

node scripts/1-compute-witness.js
error: Command failed: cd build; zokrates compute-witness -a 1246573051891109157906697728096281706743063917384028885969089040311658538909 10228216762982226909411447107540944739390092935836241194442745356563891833683 282074524127326628435517427149838065622 173030805087624539772769074318515479165 189659110152103896296452029441145961554 179950539185128938459188096563022031432 500000000000 500000000000 434090476392859288492961792859103468004462980451820851543399067438715806797 324028898526253369467876196695238365669696791339117232792499465164618714289 0 20636625426020718969131298365984859231982649550971729229988535915544421356929 8234632431858659206959486870703726442454087730228411315786216865106603625166 7985001422402102077350925203503698316627789269711557462970266825665867053007 18097266179879782427361438755277450939722755112152115227098348943187633376449 17881168164677037514367869548776650520965052851469330112398906502158797604517 922786292280634969147910688433687283453311471541485803183285293828322638602 14966121255901869775959970702197500594950233358407635238140938902275743163839 15950129931660381885541753302118095863142450307256106174572389060872212753325 16464761340879542328718857346548831929741065470370013028703745046966789709133 0 0 0 0 0 0 0 0 0 0

I tried re-installing Zokrates to no avail. Here is my version:

zokrates --version
ZoKrates 0.8.3

Can you help me out with these errors? I don't believe I've missed any steps. Thanks!

@wuyahuang
Copy link
Contributor Author

wuyahuang commented Nov 18, 2022

@keeganquigley It looks like the compiler of the new zo-krates does not support our source code.
I checked my version:

zokrates --version
ZoKrates 0.8.2

However, the issue is fixed after I upgraded to 0.8.3 and remove Visibility modifiers on arguments as the compiler suggested. Please re-compile the circuits and generate commitment.

@keeganquigley
Copy link
Contributor

Hi @wuyahuang thanks again for your patience. You can find my in-progress eval here. No show stoppers anymore, I was able to get through everything! But please check to see if you can make any of the requested changes there. After which I will submit the eval to be merged. Thanks!

@wuyahuang
Copy link
Contributor Author

Hi, @keeganquigley Thank you for the nice feedback. I updated the source code and tutorial as you requested.

  1. Add a notice about zokrates $PATH.
  2. fn transfer_from is already included in the test case approve_and_transfer, but I still added a separate test case for transfer_from to make it better understood.
  3. Take out baseUri and levels
  4. Fix Clippy warning for 3 contracts.

@keeganquigley
Copy link
Contributor

Thank you for the changes @wuyahuang! I appreciate your quick communication during the process, and I am happy to accept your milestone. Also, nice app! You can find my final eval here.

Once it is merged I will submit your invoice for payment. Cheers!

@Noc2 Noc2 merged commit 29ef65d into w3f:master Nov 22, 2022
@keeganquigley
Copy link
Contributor

@wuyahuang Your invoice has been forwarded for payment. Please allow up to 2 weeks for delivery.

@keeganquigley
Copy link
Contributor

@wuyahuang One other question, are you able to provide an email address that I can use in filling out your information for our Substrate Builder's Program database? If you'd rather not publish it here you can send it to me at keegan@web3.foundation

Thanks!

@wuyahuang
Copy link
Contributor Author

wuyahuang commented Nov 23, 2022

@keeganquigley Thank you for taking the time to review this milestone, I really appreciate your time and effort.
My email address is wuyahuang@gmail.com

Cheers!

@keeganquigley
Copy link
Contributor

keeganquigley commented Nov 23, 2022

@wuyahuang thanks! I have forwarded it to the SBP.

@RouvenP
Copy link

RouvenP commented Nov 25, 2022

hi @wuyahuang we transferred the payment today

@wuyahuang
Copy link
Contributor Author

@Noc2 @keeganquigley @RouvenP Thank you for funding Green Lemon and bringing our idea into reality. 馃槃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants