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

feat(consensus/grandpa): Add warp sync proof verification #4240

Open
wants to merge 18 commits into
base: development
Choose a base branch
from

Conversation

dimartiro
Copy link
Contributor

@dimartiro dimartiro commented Oct 9, 2024

Changes

Implement warp sync proof verification.
The warp proof is verified by traversing the warp proof fragments (with the block headers and justifications), then we verify the justifications against the authorities based on the genesis authorities and the scheduled changes.
If we are able to verify all the fragments, then the warp proof is valid.

  • Add warp sync proof verify method
  • Minor fixes in proof generation
  • Use right grandpa types
  • Tests
    • Improve tests
    • Test both proof generation and verification

Reference parity code

Note: I'd rather to not use generics in this initial phase because that will imply more changes (eg: in BlockState and GrandpaState) we can revisit this later to make the structs generic.

Tests

make test

Issues

#4237

dot/network/warp_sync.go Outdated Show resolved Hide resolved
dot/network/warp_sync.go Show resolved Hide resolved
@dimartiro dimartiro self-assigned this Oct 14, 2024
@dimartiro dimartiro marked this pull request as ready for review October 15, 2024 21:21
@dimartiro dimartiro changed the title feat(consensus/grandpa): Add warp sync verification feat(consensus/grandpa): Add warp sync proof verification Oct 15, 2024
Copy link
Contributor

@timwu20 timwu20 left a comment

Choose a reason for hiding this comment

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

Do you mind moving the internal/client/consensus/grandpa/warp_sync.go file to lib/grandpa package? I think this is more inline with the "old" way of doing this with the local interfaces for the older block BlockState and GrandpaState. I'd like to keep things more analogous to substrate in the internal/primitives and internal/client code.

@dimartiro
Copy link
Contributor Author

Do you mind moving the internal/client/consensus/grandpa/warp_sync.go file to lib/grandpa package? I think this is more inline with the "old" way of doing this with the local interfaces for the older block BlockState and GrandpaState. I'd like to keep things more analogous to substrate in the internal/primitives and internal/client code.

Moved!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants