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

Conversation

@andresilva
Copy link
Contributor

Currently when GRANDPA finalizes a block the client does not always persist the justification. It currently only does it: on authority set change blocks, randomly every N blocks. Since the only justifications that are required to provably sync the chain correctly are for the authority set handoffs it would just contribute to database bloat if we kept justifications for everything that's finalized. Still having a justification at hand for a recent block is useful to prove finality to light clients who don't participate in the GRANDPA protocol.

This PR adds a GRANDPA auxiliary storage entry where we always store the justification for the latest finalized block and overwrite it once a new block gets finalized.

Additionally I updated the warp sync protocol to make sure that we send the latest justification once we have finished warp syncing to the latest authority set.

@andresilva andresilva 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 Mar 18, 2021
@andresilva andresilva requested review from expenses and octol March 18, 2021 12:51
@andresilva
Copy link
Contributor Author

Still want to add some tests for this.

@expenses
Copy link
Contributor

Awesome, nicely done!

Copy link
Contributor

@octol octol left a comment

Choose a reason for hiding this comment

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

Looks good!

@andresilva
Copy link
Contributor Author

Sorry but I forgot about this PR. Should be good to merge now.

@tomaka
Copy link
Contributor

tomaka commented Mar 30, 2021

I'm not qualified to review, but I enjoy the idea of merging this.

@andresilva
Copy link
Contributor Author

Maybe @expenses can approve the warp sync changes. The grandpa changes were already reviewed by @octol.

Copy link
Contributor

@expenses expenses left a comment

Choose a reason for hiding this comment

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

LGTM

@andresilva
Copy link
Contributor Author

bot merge

@ghost
Copy link

ghost commented Apr 1, 2021

Trying merge.

@ghost ghost merged commit 18e8b21 into master Apr 1, 2021
@ghost ghost deleted the andre/grandpa-store-best-justification branch April 1, 2021 09:42
hirschenberger pushed a commit to hirschenberger/substrate that referenced this pull request Apr 14, 2021
…aritytech#8392)

* grandpa: always store justification for best finalized block

* grandpa-warp-sync: add latest justification when finished proving

* grandpa-warp-sync: change logic for sending best justification when finished

* grandpa: test storing best justification

* grandpa: reorder variants in WarpSyncFinished
This pull request was closed.
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