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

Remove substrate-parachain-PoS-template Milestone 2 #1412

Merged
merged 5 commits into from
Jan 5, 2023
Merged

Remove substrate-parachain-PoS-template Milestone 2 #1412

merged 5 commits into from
Jan 5, 2023

Conversation

octavei
Copy link
Contributor

@octavei octavei commented Jan 3, 2023

Remove the second milestone, for the application of aband-network grant. The development of aband-network will be promoted in two weeks.
The first milestone is a fully functional pos parachain development template, which is also used by aband-network.

@octavei
Copy link
Contributor Author

octavei commented Jan 3, 2023

Delete the second milestone, for the application of aband-network grant. The development of aband-network will be promoted in two weeks. The first milestone is a fully functional pos parachain development template, which is also used by aband-network.

Delete the second milestone, for the application of aband-network grant. The development of aband-network will be promoted in two weeks. The first milestone is a fully functional pos parachain development template, which is also used by aband-network.

We are very willing to complete the development of the second milestone, it is just a bit conflicting with our plan at present. We will be back to apply for it soon.

@octavei
Copy link
Contributor Author

octavei commented Jan 3, 2023

These two milestones are completely independent of each other, that is, two PoS templates based on different consensuses.

@octavei octavei mentioned this pull request Jan 3, 2023
9 tasks
@Noc2 Noc2 self-assigned this Jan 3, 2023
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.

For reference: #1410

And I have one question: Why does removing the second milestone increase the price of the first milestone?

@Noc2 Noc2 added amendment This PR proposes changes to an existing application. ready for review The project is ready to be reviewed by the committee members. labels Jan 3, 2023
@octavei
Copy link
Contributor Author

octavei commented Jan 3, 2023

For reference: #1410

And I have one question: Why does removing the second milestone increase the price of the first milestone?

We will have more time to add more detailed development documentation, and more detailed manual testing.

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.

But you didn't add any additional details to the delivery. Therefore, I'm personally not willing to increase the price and approve this amendment. Be aware others might have a different opinion.

@octavei
Copy link
Contributor Author

octavei commented Jan 3, 2023

For reference: #1410

And I have one question: Why does removing the second milestone increase the price of the first milestone?

But you didn't add any additional details to the delivery. Therefore, I'm personally not willing to increase the price and approve this amendment. Be aware others might have a different opinion.

Of course, there is no need to add these details, but I personally think that it will be more friendly to developers. If it is not necessary, I will change the amount back.

@octavei
Copy link
Contributor Author

octavei commented Jan 3, 2023

For reference: #1410
And I have one question: Why does removing the second milestone increase the price of the first milestone?

But you didn't add any additional details to the delivery. Therefore, I'm personally not willing to increase the price and approve this amendment. Be aware others might have a different opinion.

Of course, there is no need to add these details, but I personally think that it will be more friendly to developers. If it is not necessary, I will change the amount back.

For reference: #1410
And I have one question: Why does removing the second milestone increase the price of the first milestone?

But you didn't add any additional details to the delivery. Therefore, I'm personally not willing to increase the price and approve this amendment. Be aware others might have a different opinion.

Of course, there is no need to add these details, but I personally think that it will be more friendly to developers. If it is not necessary, I will change the amount back.

Thank you very much for your work, I have made changes to the amount. This will not prevent me from providing detailed documentation. just add extra hours to my work. Thanks again!

@octavei
Copy link
Contributor Author

octavei commented Jan 3, 2023

In order to achieve a more perfect PoS function, I added more substarte native modules to the runtime, which increased my workload in code and manual testing. So I decided to add 1000USD. But it doesn't matter if keep 4000USD.

@keeganquigley
Copy link
Contributor

In order to achieve a more perfect PoS function, I added more substarte native modules to the runtime, which increased my workload in code and manual testing. So I decided to add 1000USD. But it doesn't matter if keep 4000USD.

Thanks @octavei

  • I see that you changed M1 & the overall price back to 4k, but I'm not sure what you mean here. Are you saying you are okay with the current price?
  • Can you add a bit more detail to the deliverables? For example in 2. Modules are you wanting to simply import the existing pallets? Also in section 3. Collator module, what exactly does "provide collators" mean?

@octavei
Copy link
Contributor Author

octavei commented Jan 4, 2023

In order to achieve a more perfect PoS function, I added more substarte native modules to the runtime, which increased my workload in code and manual testing. So I decided to add 1000USD. But it doesn't matter if keep 4000USD.

Thanks @octavei

  • I see that you changed M1 & the overall price back to 4k, but I'm not sure what you mean here. Are you saying you are okay with the current price?
  • Can you add a bit more detail to the deliverables? For example in 2. Modules are you wanting to simply import the existing pallets? Also in section 3. Collator module, what exactly does "provide collators" mean?

@keeganquigley Thank you very much for your reply. I'm very sorry that my English is a little rusty.

This is a PoS development template, so I try to use the native modules of substrate as much as possible. Just like the PoA template. But this template needs to do more things. If you want to perfect the PoS function, you need to add modules as much as possible, so I have added all PoS-related modules here. I mean it cost me more time, such as more manual testing to prove that these modules are fully working together, and more team energy to continue to upgrade later. Although our aband-parachain is using this template, we will continue to upgrade it for this reason, but I personally think that 5000USD may be more reasonable. We are happy to make valuable developments for the Polkadot ecosystem. Of course, we are very willing to adopt your opinions, if you think 4000USD is enough, then I will not change the price, this is just a discussion.

@octavei
Copy link
Contributor Author

octavei commented Jan 4, 2023

In order to achieve a more perfect PoS function, I added more substarte native modules to the runtime, which increased my workload in code and manual testing. So I decided to add 1000USD. But it doesn't matter if keep 4000USD.

Thanks @octavei

  • I see that you changed M1 & the overall price back to 4k, but I'm not sure what you mean here. Are you saying you are okay with the current price?
  • Can you add a bit more detail to the deliverables? For example in 2. Modules are you wanting to simply import the existing pallets? Also in section 3. Collator module, what exactly does "provide collators" mean?

@keeganquigley Thank you very much for your reply. I'm very sorry that my English is a little rusty.

This is a PoS development template, so I try to use the native modules of substrate as much as possible. Just like the PoA template. But this template needs to do more things. If you want to perfect the PoS function, you need to add modules as much as possible, so I have added all PoS-related modules here. I mean it cost me more time, such as more manual testing to prove that these modules are fully working together, and more team energy to continue to upgrade later. Although our aband-parachain is using this template, we will continue to upgrade it for this reason, but I personally think that 5000USD may be more reasonable. We are happy to make valuable developments for the Polkadot ecosystem. Of course, we are very willing to adopt your opinions, if you think 4000USD is enough, then I will not change the price, this is just a discussion.

The role of the Collators pallet is to provide a validator set for consensus. The validator can come from the staking module, which can also be set by ensure_root in this module, which means that with this template, you can also use the Staking function in the case of PoA, which is very useful if you just only want to reward collators.

Noc2
Noc2 previously approved these changes Jan 4, 2023
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.

Thanks for the update and that you changed the price back to 4k.

@keeganquigley
Copy link
Contributor

keeganquigley commented Jan 4, 2023

Thanks for the clarifications @octavei

Regarding the 2nd deliverable, can you remove this since it sounds like you are re-using the existing pallets with no changes? Typically it's not something we pay for.

In my opinion, since the grant application was already accepted, I say let's stick with 4k to get you started and then you can always argue for more in the next phase if you'd like to put more time into making improvements.

| 2| Substrate Modlue: babe-ext | This pallets extends the Substrate Babe pallet to make it compatible with parachains. |
| 3 | Substrate Modlue: cumulus-client-consensus-babe | This extends the Substrate provided Babe consensus implementation to make it compatible for parachains. |
| 2 | modules | Imp all staking-related substrate native existing pallets(ElectionProviderMultiPhase, VoterList, Authorship, Utility, ImOnline, Offences, Session and NominationPools) for the runtime. |
| 3 | Substrate Modlue: Collators | The role of the Collators pallet is to provide a validator set for consensus. The validator can come from the staking module, which can also be set by ensure_root in this module, which means that with this template, you can also use the Staking function in the case of PoA, which is very useful if you just only want to reward collators. |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| 3 | Substrate Modlue: Collators | The role of the Collators pallet is to provide a validator set for consensus. The validator can come from the staking module, which can also be set by ensure_root in this module, which means that with this template, you can also use the Staking function in the case of PoA, which is very useful if you just only want to reward collators. |
| 3 | Substrate Module: Collators | The role of the Collators pallet is to provide a validator set for consensus. The validator can come from the staking module, which can also be set by ensure_root in this module, which means that with this template, you can also use the Staking function in the case of PoA, which is very useful if you just only want to reward collators. |

@keeganquigley keeganquigley added changes requested The team needs to clarify a few things first. and removed ready for review The project is ready to be reviewed by the committee members. labels Jan 4, 2023
@octavei
Copy link
Contributor Author

octavei commented Jan 5, 2023

In my opinion, since the grant application was already accepted, I say let's stick with 4k to get you started and then you can always argue for more in the next phase if you'd like to put more time into making improvements.

Thanks for the clarifications @octavei

Regarding the 2nd deliverable, can you remove this since it sounds like you are re-using the existing pallets with no changes? Typically it's not something we pay for.

In my opinion, since the grant application was already accepted, I say let's stick with 4k to get you started and then you can always argue for more in the next phase if you'd like to put more time into making improvements.

Thanks for the suggestion, I have removed it.

@keeganquigley keeganquigley removed the changes requested The team needs to clarify a few things first. label Jan 5, 2023
@keeganquigley keeganquigley added the ready for review The project is ready to be reviewed by the committee members. label Jan 5, 2023
Copy link
Contributor

@keeganquigley keeganquigley left a comment

Choose a reason for hiding this comment

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

Thanks for the changes! LGTM

@Noc2 Noc2 merged commit 33d75b9 into w3f:master Jan 5, 2023
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.

3 participants