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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amend parami protocol #521

Closed
wants to merge 3 commits into from

Conversation

didateckck2013
Copy link

@didateckck2013 didateckck2013 commented Jul 23, 2021

@CLAassistant
Copy link

CLAassistant commented Jul 23, 2021

CLA assistant check
All committers have signed the CLA.

@alxs
Copy link
Contributor

alxs commented Jul 23, 2021

Thanks for the amendment @hellotrongo. As I mentioned, could you please include deliverables 0a-d as in the template in all milestones, and 0e at least in the last one? Also I think the price should be split evenly across all milestones. You're quoting a similar workload for each of them and if anything, it seems to me the last milestone would be the most extensive one.

@alxs alxs changed the title Add parami protocol Amend parami protocol Jul 23, 2021
@didateckck2013
Copy link
Author

didateckck2013 commented Jul 24, 2021

Hello @alxs , thanks for the suggestions. I've updated the application.

@didateckck2013 didateckck2013 requested a review from alxs July 24, 2021 04:55
@alxs alxs added the changes requested The team needs to clarify a few things first. label Jul 26, 2021
@DorianRust
Copy link
Contributor

Hi @alx, sorry to disturb you again. The cost has been adjusted. Besides this part, I believe I have amended everything based on your suggestion. What else did I miss? Thank you.

@alxs
Copy link
Contributor

alxs commented Jul 28, 2021

Hi Dorian. Could you please include the same deliverables 0a-d as in the template in every milestone? Eventually you can change the wording etc. But they should be in the spirit of those in the template i.e. 0a is the license, 0b is documentation etc. These are "meta-deliverables" which is why they are labelled with 0. Your other deliverables should each have their own number and can be split into a, b, c, if they clearly belong together or are part of the same component.

@DorianRust
Copy link
Contributor

Hi @alxs, thanks for your suggestions. I have revised the PR accordingly.

@alxs
Copy link
Contributor

alxs commented Jul 29, 2021

Thanks, could you label the deliverables according to what they are? I.e. instead of 10 times "substrate chain" please name the pallet(s)/interface that you will develop. The rest looks good to me.

@didateckck2013
Copy link
Author

didateckck2013 commented Jul 30, 2021

Hi @alxs, please kindly find the amended version. Thanks for all the suggestions and support.

Comment on lines 203 to 205
| 4. | Parami Wallet | Parami wallet supports blind signature algorithm, which are used to obtain reward evidence for users; the data layer supports homomorphic encryption algorithms to provide privacy protection of users' data. |
| 5. | Parami Wallet | Integrate zero-knowledge proof technology for users' private data update, verifiable computaion of reward amount and the implementation of anonymous transactions to fully archive privacy protection. |
| 6. | Parami Wallet | Wallet development, optimization of new functions, improve the smoothness of users using wallets, and enhance users' experience; (finished)|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you do the same here please?

| 1. | Pallet(s)/did | Build Parami ParaChain based on Substrate3.0.0, the core modules that have been developed include DID(Decentralized Identity) pallet, Decentralised identity is an emerging concept that gives back control of identity to consumers. |
| 2. | Pallet(s)/bridge | Completed the bridge development between ETH ERC20 and Parami chain. Please refer to https://github.com/parami-protocol/parami/tree/main/pallets/bridge |
| 3. | Pallet(s)/nft | Complete NFT pallets, please refer to https://github.com/parami-protocol/parami/tree/main/pallets/nft |
| 4. | Testnet | Setup and launch Parami testnet, which has been running stably for about a month. Parami testnet has has become a parachain to Rococo. Please refer to (https://polkadot.js.org/apps/?rpc=wss://rococo-rpc.polkadot. io#/parachains/parathreads). |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We only fund deliverables that can be reused others in the ecosystem. Could you remove this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove Testnet?

| 0d. | Docker | We will provide a Dockerfile(s) that can be used to test all the functionality for wallet. |
| 0e. | Article | We will publish an medium that explains what was done as part of the grant). |
| 1. | Pallet(s)/offchain | Develop Offchain-worker to verify ads and incentives, rewards and withdrawals, etc.
| 2. | Pallet(s)/socialcoin | Support Social coin and introduce NFT support for advertisers.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This "social coin" isn't described anywhere in the application.

@alxs
Copy link
Contributor

alxs commented Jul 30, 2021

Sorry @hellotrongo, I'm gonna go through it one more time. The new deliverables diverge considerably from the original application.

| 0c. | Testing Guide | The code will have unit-test coverage (min. 70%) to ensure functionality and robustness. In the guide we will describe how to run these tests |
| 0d. | Docker | We will provide a Dockerfile(s) that can be used to test all the functionality delivered with this milestone. |
| 0e. | Article | We will publish an article or medium that explains what was done as part of the grant). |
| 1. | Pallet(s)/did | Build Parami ParaChain based on Substrate3.0.0, the core modules that have been developed include DID(Decentralized Identity) pallet, Decentralised identity is an emerging concept that gives back control of identity to consumers. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Description doesn't match deliverable name.

Comment on lines 166 to 167
| 2. | Pallet(s)/bridge | Completed the bridge development between ETH ERC20 and Parami chain. Please refer to https://github.com/parami-protocol/parami/tree/main/pallets/bridge |
| 3. | Pallet(s)/nft | Complete NFT pallets, please refer to https://github.com/parami-protocol/parami/tree/main/pallets/nft |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no mention of either of these anywhere else in the application, and to be honest I don't think we're interested to fund either of them (there are already plenty of implementations for both).

| 0c. | Testing Guide | The code will have unit-test coverage (min. 70%) to ensure functionality and robustness. In the guide we will describe how to run these tests |
| 0d. | Docker | We will provide a Dockerfile(s) that can be used to test all the functionality for wallet. |
| 0e. | Article | We will publish an medium that explains what was done as part of the grant). |
| 1. | Pallet(s)/offchain | Develop Offchain-worker to verify ads and incentives, rewards and withdrawals, etc.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please describe precisely what the implemented functionality will be.

| 0e. | Article | We will publish an achieved that explains what was done). |
| 1. | Pallet(s)/onchain | Develop on-chain governance logic to build a more decentralized platform. |
| 2. | AD3-Maker | encode/decode the avatar of Telegram or Wechat users, and create the binding of DID and social user's identity image. Social users can transfer token through the avatar. Please refer to: https://github.com/parami-protocol/ad3-marker |
| 3. | Parami AMM | Implement Parami AMM, support AD3, Social coin. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No description of the AMM in the rest of the application. Also here, plenty of implementations exist and you can likely reuse any of them.

| 0c. | Testing Guide | The code will have unit-test coverage (min. 80%) to ensure functionality and robustness. In the guide we will describe how to run these tests |
| 0d. | Docker | We will provide a Dockerfile(s) that can be used to test all the functionality. |
| 0e. | Article | We will publish an achieved that explains what was done). |
| 1. | Pallet(s)/onchain | Develop on-chain governance logic to build a more decentralized platform. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Governance modules already exist. We won't fund "governance logic to build a more decentralized platform".

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi @alxs , We spent quite a lot time exploring Zero Knowledge Proof and its adoption to advertising at the beginning of development, including studying cryptography with researchers in academia, which extended the duration. And we also finished some milestone in the last application.

@alxs alxs added the amendment This PR proposes changes to an existing application. label Jul 30, 2021
@didateckck2013
Copy link
Author

Hi @alxs , Thanks for your support along the way. Given that I received very informative suggestions on your side last time, I dived into the deliverables and reconstructed the whole stuff. The current version is more concise because I removed some unnecessary parts based on your suggestions. If any further advice, please feel free to contact me. Thanks.

@alxs
Copy link
Contributor

alxs commented Aug 2, 2021

As you will. The delivery would still be overdue with the updated duration, could you calculate it from the date your original application was accepted or just include an ETA? Thanks.

@didateckck2013
Copy link
Author

Hi @alxs, thanks for the reminder. The deadline is updated to until December, which would be 12 months since the original application was accepted.

@alxs
Copy link
Contributor

alxs commented Aug 2, 2021

@hellotrongo thanks. That's a very long timeframe, I personally think in that case and seeing as your roadmap doesn't seem to be exactly aligned with the deliverables, it would make more sense for everyone involved to terminate the current grant and that you submit a new application. Let's see what the rest of the committee thinks.

@alxs alxs added ready for review The project is ready to be reviewed by the committee members. and removed changes requested The team needs to clarify a few things first. labels Aug 2, 2021
@didateckck2013 didateckck2013 requested a review from alxs August 2, 2021 23:50
@didateckck2013
Copy link
Author

didateckck2013 commented Aug 3, 2021

Hi @alxs, we didn’t change the basic structure, only to make some minor adjustments and a few additions. Given the only major change is time duration alone, do we have to submit a new application? We’d be happy to re-apply, while it would be better if we can make amendments to the original one. And re-apply cost a lot of time.

Copy link
Collaborator

@Noc2 Noc2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hellotrongo Just for clarification: Do the 6 or 12 month start with the amendment or do you think the 6 or 12 months start with the original contract?

@Noc2
Copy link
Collaborator

Noc2 commented Aug 3, 2021

Thanks. In this case, my recommendation is also to cancel the current contract and potentially sign a new contract at the end of the year. A lot of things might change in half a year, especially in the crypto space. For example other teams might already have implemented the features, or you might no longer want to work on this, etc.

@didateckck2013
Copy link
Author

Hi @alxs, After some careful evaluation with the tech team, we found that 8-month duration is doable. I have amended the duration here. Thanks.

@Tailalat
Copy link

#160

@alxs
Copy link
Contributor

alxs commented Aug 16, 2021

@hellotrongo it seems the rest of the committee isn't convinced by the amendment either, despite the shortened duration. I'm closing the PR and putting the grant forward for termination again (which we shouldn't have undone without negotiating the amendment first, apologies for the oversight). As mentioned, feel free to apply again with your current roadmap.

@alxs alxs closed this Aug 16, 2021
@alxs alxs mentioned this pull request Aug 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
amendment This PR proposes changes to an existing application. ready for review The project is ready to be reviewed by the committee members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants