add idavoll_network-milestone_1

[jasonberger0]

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.

[alxs]

Thanks for the delivery @jasonberger0. We'll look into it as soon as possible.

[mmagician]

@jasonberger0 Apologies for the delay in assessing your milestone. I just wanted to let you know that I've started looking into your project today. If all goes well, you should hear back from me by the end of the week.

[mmagician]

I've taken the liberty to write a couple of minor fixes for your project. Feel free to selectively merge my commits as you see fit. Here's the PR.

Apart from what I found and fixed above, here are some further issues/comments:

1. Multiple spelling/grammar errors. While I'm not a native speaker myself, I do find it much easier to read documentation when it's written well. (e.g. the following is a hint in Polkadot-JS UI when I select the vote_proposal method: "voting on the proposal by the members in the organization,user must be lock it's 'value'". ). Is there any chance you could collaborate with someone to ensure that at least the rustdoc's are polished please?
2. add_member_by_owner name is confusing (even though you explain it in your README). After a new user is added to the org, they have the power to add further members (even though they were not the original creator of the org). So the name owner is misleading IMO.
3. add_member_by_owner has misleading rustdoc (related to voting)
4. Plenty of code duplication between the helper methods for tests in lib.rs and mock.rs: get/set_block_number, create_org etc. Is there any way you could unify these?
5. Code formatting seems to be inconsistent throughout the project. Could you stick to one style please?
6. Perhaps the add_member_by_owner function could include an optional parameter for transferring the org tokens to the newly created user? While this is not the only option that new users would get tokens, it's likely to be a common approach. Members without tokens can't really do much otherwise.
7. Perhaps to prevent "spamming", creating a new proposal could require some token lockup? Just an idea.
8. UI doesn't seem to work well for me when doing anything other than create_organization. For example, I use Alice's account to create an org and then try to add Bob to it. I get 1010: Invalid Transaction: Transaction has a bad signature. (I'm using latest master of polkadot-js apps built locally). Could you verify whether this works for you? Perhaps I'm missing something in my setup?

Due to ^8. I can't test much through the UI. Therefore, so far I've been relying only on the unit tests and code reading. Once we manage to resolve the UI issue I will do another round of testing there.

Let me know if any of the above issues are not clear to you. Happy fixing!

[jasonberger0]

I've taken the liberty to write a couple of minor fixes for your project. Feel free to selectively merge my commits as you see fit. Here's the PR.

Apart from what I found and fixed above, here are some further issues/comments:

1. Multiple spelling/grammar errors. While I'm not a native speaker myself, I do find it much easier to read documentation when it's written well. (e.g. the following is a hint in Polkadot-JS UI when I select the vote_proposal method: "voting on the proposal by the members in the organization,user must be lock it's 'value'". ). Is there any chance you could collaborate with someone to ensure that at least the rustdoc's are polished please?
2. add_member_by_owner name is confusing (even though you explain it in your README). After a new user is added to the org, they have the power to add further members (even though they were not the original creator of the org). So the name owner is misleading IMO.
3. add_member_by_owner has misleading rustdoc (related to voting)
4. Plenty of code duplication between the helper methods for tests in lib.rs and mock.rs: get/set_block_number, create_org etc. Is there any way you could unify these?
5. Code formatting seems to be inconsistent throughout the project. Could you stick to one style please?
6. Perhaps the add_member_by_owner function could include an optional parameter for transferring the org tokens to the newly created user? While this is not the only option that new users would get tokens, it's likely to be a common approach. Members without tokens can't really do much otherwise.
7. Perhaps to prevent "spamming", creating a new proposal could require some token lockup? Just an idea.
8. UI doesn't seem to work well for me when doing anything other than create_organization. For example, I use Alice's account to create an org and then try to add Bob to it. I get 1010: Invalid Transaction: Transaction has a bad signature. (I'm using latest master of polkadot-js apps built locally). Could you verify whether this works for you? Perhaps I'm missing something in my setup?

Due to ^8. I can't test much through the UI. Therefore, so far I've been relying only on the unit tests and code reading. Once we manage to resolve the UI issue I will do another round of testing there.

Let me know if any of the above issues are not clear to you. Happy fixing!

Thanks for your PR,that is very good, I will fix these as soon as possible

[jasonberger0]

I've taken the liberty to write a couple of minor fixes for your project. Feel free to selectively merge my commits as you see fit. Here's the PR.
Apart from what I found and fixed above, here are some further issues/comments:

1. Multiple spelling/grammar errors. While I'm not a native speaker myself, I do find it much easier to read documentation when it's written well. (e.g. the following is a hint in Polkadot-JS UI when I select the vote_proposal method: "voting on the proposal by the members in the organization,user must be lock it's 'value'". ). Is there any chance you could collaborate with someone to ensure that at least the rustdoc's are polished please?
2. add_member_by_owner name is confusing (even though you explain it in your README). After a new user is added to the org, they have the power to add further members (even though they were not the original creator of the org). So the name owner is misleading IMO.
3. add_member_by_owner has misleading rustdoc (related to voting)
4. Plenty of code duplication between the helper methods for tests in lib.rs and mock.rs: get/set_block_number, create_org etc. Is there any way you could unify these?
5. Code formatting seems to be inconsistent throughout the project. Could you stick to one style please?
6. Perhaps the add_member_by_owner function could include an optional parameter for transferring the org tokens to the newly created user? While this is not the only option that new users would get tokens, it's likely to be a common approach. Members without tokens can't really do much otherwise.
7. Perhaps to prevent "spamming", creating a new proposal could require some token lockup? Just an idea.
8. UI doesn't seem to work well for me when doing anything other than create_organization. For example, I use Alice's account to create an org and then try to add Bob to it. I get 1010: Invalid Transaction: Transaction has a bad signature. (I'm using latest master of polkadot-js apps built locally). Could you verify whether this works for you? Perhaps I'm missing something in my setup?

Due to ^8. I can't test much through the UI. Therefore, so far I've been relying only on the unit tests and code reading. Once we manage to resolve the UI issue I will do another round of testing there.
Let me know if any of the above issues are not clear to you. Happy fixing!

Thanks for your PR,that is very good, I will fix these as soon as possible

@mmagician i fixed all them, please check again, You can reset the UI to use types.json to make it work

[mmagician]

The fixes look good, except that you didn't check your unit tests at the end :)
It was a quick fix, there you go: idavollnetwork/idavoll#65

When submitting the extrinsics I was slightly confused by the method naming in the beginning, and had to dig into source code to understand the details. The issue was that both idavoll as well as idvAsset contain a transfer extrinsic. I tried creating a proposal with the a call to the latter, but then found out that there was not enough balance. That's where I had to check the source code to find out that to transfer value from the org value, I needed to use the method from idavoll.

Perhaps you could expand the documentation a bit to explain the difference (or even better, rename - just make sure to check your unit tests afterwards!). Also, a brief summary of chain storage of the idvAsset module would be helpful, e.g. of lockedBalance, balance, finances - as these are slightly confusing to me as well. Ideally an end-user can just read your documentation and be able to understand how to use the modules, without having to read your source code.

Otherwise the code looks good and the interaction with your pallets worked well in the end. I also tried a few edge cases (e.g. voting for non-existing proposals, etc.) which were handled properly. I'll be happy to accept the milestone after you've expanded your documentation a little. Keep up the good work!

[jasonberger0]

The fixes look good, except that you didn't check your unit tests at the end :)
It was a quick fix, there you go: idavollnetwork/idavoll#65

When submitting the extrinsics I was slightly confused by the method naming in the beginning, and had to dig into source code to understand the details. The issue was that both idavoll as well as idvAsset contain a transfer extrinsic. I tried creating a proposal with the a call to the latter, but then found out that there was not enough balance. That's where I had to check the source code to find out that to transfer value from the org value, I needed to use the method from idavoll.

Perhaps you could expand the documentation a bit to explain the difference (or even better, rename - just make sure to check your unit tests afterwards!). Also, a brief summary of chain storage of the idvAsset module would be helpful, e.g. of lockedBalance, balance, finances - as these are slightly confusing to me as well. Ideally an end-user can just read your documentation and be able to understand how to use the modules, without having to read your source code.

Otherwise the code looks good and the interaction with your pallets worked well in the end. I also tried a few edge cases (e.g. voting for non-existing proposals, etc.) which were handled properly. I'll be happy to accept the milestone after you've expanded your documentation a little. Keep up the good work!

@mmagician Thanks for your PR, i has expanded the documentation, please check again

[mmagician]

Congratulations on completing the milestone! You can find the full evaluation merged here.

[jasonberger0]

Congratulations on completing the milestone! You can find the full evaluation merged here.

Thanks so much @mmagician and the entire W3F team!