Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.

Add support for GrandPa warp sync in the node template#8922

Closed
tomaka wants to merge 5 commits intoparitytech:masterfrom
tomaka:template-warp-sync
Closed

Add support for GrandPa warp sync in the node template#8922
tomaka wants to merge 5 commits intoparitytech:masterfrom
tomaka:template-warp-sync

Conversation

@tomaka
Copy link
Contributor

@tomaka tomaka commented May 27, 2021

GrandPa warp sync is quite an important protocol nowadays, for both light clients and for #8884.

This PR adds GrandPa warp sync in the node template, as it was missing.

@tomaka tomaka added A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels May 27, 2021
Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

However, I think in general a tutorial would be better.

@adoerr adoerr self-requested a review May 27, 2021 12:08
Copy link
Contributor

@adoerr adoerr left a comment

Choose a reason for hiding this comment

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

Or at least add a comment, what this is used for and that it is basically optional.

@tomaka
Copy link
Contributor Author

tomaka commented May 27, 2021

The node-template is basically a thousand lines of completely undocumented code that nobody outside of Parity understands, and you want me to write a tutorial or an explanation specifically for these specific 9 lines of code that I'm adding? 😶

@adoerr
Copy link
Contributor

adoerr commented May 27, 2021

The node-template is basically a thousand lines of completely undocumented code

Fair enough. Those thousand lines of undocumented code probably came into existence by merging one 'it's just those few lines" PR after another.

@tomaka
Copy link
Contributor Author

tomaka commented May 27, 2021

Those thousand lines of undocumented code probably came into existence by merging one 'it's just those few lines" PR after another.

The code in the node-template is initially just the same code as the regular node that we have copy-pasted. It has changed very little since then, and is just kept up to date with the rest of the node from time to time.

Nobody has ever bothered to document anything because even today we're still not sure about what the API of the service stuff should look like. Everyone agrees that this code should change in a major way, and nobody documents anything because they anticipate that the API will change in a major way anyway.

@stale
Copy link

stale bot commented Jul 14, 2021

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jul 14, 2021
@bkchr
Copy link
Member

bkchr commented Jul 19, 2021

Needs a master merge

@stale stale bot removed the A5-stale Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jul 19, 2021
@arkpar
Copy link
Member

arkpar commented Jul 19, 2021

Obsoleted by #9284 anyway

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants