Milestone Delivery: Escrow pallet Milestone 2 and 3

[herou]

Milestone Delivery Checklist

• The milestone-delivery-template.md has been copied and updated.
• The invoice form 📝 has been filled out for this milestone.
• This pull request is being made by the same account as the accepted application.
• In case of acceptance, the payment will be transferred to the BTC/ETH payment address provided in the application.
• The delivery is according to the Guidelines for Milestone Deliverables.

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

[herou]

Hi @keeganquigley
Is any progress on this PR?

[keeganquigley]

Hi @keeganquigley Is any progress on this PR?

Hi @herou sorry for the delay, I am starting the eval now. Will let you know if I have any questions.

[herou]

Hi @keeganquigley Is any progress on this PR?

Hi @herou sorry for the delay, I am starting the eval now. Will let you know if I have any questions.

Thank you. I really appreciate it.

[keeganquigley]

Hi again @herou apologies for the delay. Here are my initial notes regarding your delivery. Please let me know if you can make these fixes.

1. The Application Document link at the top needs to link to your application our repo instead of your personal one (otherwise it will fail the GH actions checks). Can you please update this?
2. The readme file is still just the old substrate-node-template text. Can you replace this with your own text instead? A finished grant typically has a nice readme with your project name at the top, along with a blurb about what your pallet does and how to use it. You can remove anything that isn't necessary for your pallet, such as the "Template Structure" section.
3. To go along with the statement above, nothing has been added to your docs since the last milestone. The docs are very sparse and don't give great examples of how to use the pallet. For example, what exactly is a workDay, or a takeActionDay? What should it look like when someone receives a payment? It would be nice if you could explain these components in detail. Screenshots showing steps to take for a few different scenarios would also be helpful. See this readme for an example of what I am looking for.
4. Your deliverable 0d. Docker states that you will create a Dockerfile to easily spin up the node/pallet. I am not finding a Dockerfile, are you planning on implementing one? Also, the docker-compose.yml file is just the original template. Are you going to update this with your own custom image?
5. Running cargo +nightly clippy generates a high number of warnings. Can you take a look at these to see if any of them are able to be fixed?

Please let me know if these issues can be addressed. Thanks!

[herou]

Hi again @herou apologies for the delay. Here are my initial notes regarding your delivery. Please let me know if you can make these fixes.

1. The Application Document link at the top needs to link to your application our repo instead of your personal one (otherwise it will fail the GH actions checks). Can you please update this?
2. The readme file is still just the old substrate-node-template text. Can you replace this with your own text instead? A finished grant typically has a nice readme with your project name at the top, along with a blurb about what your pallet does and how to use it. You can remove anything that isn't necessary for your pallet, such as the "Template Structure" section.
3. To go along with the statement above, nothing has been added to your docs since the last milestone. The docs are very sparse and don't give great examples of how to use the pallet. For example, what exactly is a workDay, or a takeActionDay? What should it look like when someone receives a payment? It would be nice if you could explain these components in detail. Screenshots showing steps to take for a few different scenarios would also be helpful. See this readme for an example of what I am looking for.
4. Your deliverable 0d. Docker states that you will create a Dockerfile to easily spin up the node/pallet. I am not finding a Dockerfile, are you planning on implementing one? Also, the docker-compose.yml file is just the original template. Are you going to update this with your own custom image?
5. Running cargo +nightly clippy generates a high number of warnings. Can you take a look at these to see if any of them are able to be fixed?

Please let me know if these issues can be addressed. Thanks!

Hi @keeganquigley
Thank you for your update.

1. I think the link is fine. Otherwise, the GitHub link check will fail. Please double-check and let me know.
2. The readme has been updated.
3. The docs have been updated.
4. We are not planning to deliver a docker file. The docker file is a bunch of instructions that explain to the docker how runs it. We are keeping it from substrate-template because we used substrate-template to build our pallet. It should be fine.
5. Fixed the warnings.

If you have something else, please let me know.

[keeganquigley]

Thanks for the updates @herou, the docs are looking a lot better now. The readme also looks great!

1. Regarding the docker file, I assumed as much, and that's no problem seeing as the project is a pallet. However, the deliverable will need to be removed from the application since it is no longer a requirement. Can you please submit a new PR to update your application with the 0d. Docker deliverable removed from M2 and M3? (just like you did with the updated payment info). You can also remove it from this delivery.

2. The numbers on the delivery need to match the ones on your application. So instead of starting with 1., 2., 3., etc. it needs to start with 0a. License, 0b. Documentation & Tutorial, 0c. Tests, 0d. Article, 0e. Benchmarking, etc. Can you please adjust these on the delivery?

3. Your application notes that benchmarking will be provided for milestones 2 & 3. After getting rid of the Docker section can you add 0e. Benchmarking back into the delivery and provide a link?

4. Please also add a section for send_funds and withdraw_funds modules and provide links.

5. The filename of this delivery is still the old one: deliveries/escrow-milestone_1.md Can you please change this to escrow-milestone_2_3.md?

6. I talked to the team and the Application Document link does need to link to our repo instead of your personal repo (I should have noticed this on the first milestone). Therefore, after you submit the amended application, please change the delivery to link to https://github.com/w3f/Grants-Program/blob/master/applications/escrow_pallet.md

7. Your submitted invoice for M2 & M3 still has the old Tron payment address. Can you please submit a new invoice with the correct ETH USDT payment address? This will be needed for payment.

I hope all of this makes sense. Please excuse the pedantry here, as the application needs to match the delivery exactly. If you can make these final adjustments I will be ready to submit my final eval. Thanks!

[herou]

Thanks for the updates @herou, the docs are looking a lot better now. The readme also looks great!

1. Regarding the docker file, I assumed as much, and that's no problem seeing as the project is a pallet. However, the deliverable will need to be removed from the application since it is no longer a requirement. Can you please submit a new PR to update your application with the 0d. Docker deliverable removed from M2 and M3? (just like you did with the updated payment info). You can also remove it from this delivery.
2. The numbers on the delivery need to match the ones on your application. So instead of starting with 1., 2., 3., etc. it needs to start with 0a. License, 0b. Documentation & Tutorial, 0c. Tests, 0d. Article, 0e. Benchmarking, etc. Can you please adjust these on the delivery?
3. Your application notes that benchmarking will be provided for milestones 2 & 3. After getting rid of the Docker section can you add 0e. Benchmarking back into the delivery and provide a link?
4. Please also add a section for send_funds and withdraw_funds modules and provide links.
5. The filename of this delivery is still the old one: deliveries/escrow-milestone_1.md Can you please change this to escrow-milestone_2_3.md?
6. I talked to the team and the Application Document link does need to link to our repo instead of your personal repo (I should have noticed this on the first milestone). Therefore, after you submit the amended application, please change the delivery to link to https://github.com/w3f/Grants-Program/blob/master/applications/escrow_pallet.md
7. Your submitted invoice for M2 & M3 still has the old Tron payment address. Can you please submit a new invoice with the correct ETH USDT payment address? This will be needed for payment.

I hope all of this makes sense. Please excuse the pedantry here, as the application needs to match the delivery exactly. If you can make these final adjustments I will be ready to submit my final eval. Thanks!

Hello @keeganquigley all the points mention above are done if there is something pls let me know. Thanks
1-done
2-done
3-done
4-done
5-done
6-done
7-done

[keeganquigley]

Thanks for the changes @herou you can view my final evaluation here. Note that one benchmarking test is failing if you can take a look at it. Just waiting for a final check. I will let the team know and forward the invoice upon merge. Good luck with the next one!

[herou]

Thanks for the changes @herou you can view my final evaluation here. Note that one benchmarking test is failing if you can take a look at it. Just waiting for a final check. I will let the team know and forward the invoice upon merge. Good luck with the next one!

Hi @keeganquigley Thank you for your feedback. I already pushed and fixed the benchmarking. I hope to get this PR merged as soon as possible to start working on my idea. Thanks again!

[Noc2]

Hi @herou I’m happy to merge this and confirm that the milestone is approved, but as part of this PR you are currently deleting your old delivery. Could you update this and only add the new document?

[herou]

Hi @herou I’m happy to merge this and confirm that the milestone is approved, but as part of this PR you are currently deleting your old delivery. Could you update this and only add the new document?

Hi @Noc2 I created a new PR with the only file you need. Please, close this PR and take this one in consideration: #599

[Noc2]

Thanks for the update!