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

verify block justification when handling FinalizationMessage #1051

Closed
2 of 3 tasks
noot opened this issue Aug 5, 2020 · 0 comments · Fixed by #1084
Closed
2 of 3 tasks

verify block justification when handling FinalizationMessage #1051

noot opened this issue Aug 5, 2020 · 0 comments · Fixed by #1084
Assignees

Comments

@noot
Copy link
Contributor

noot commented Aug 5, 2020

Expected Behavior

  • need to verify block justification when receiving a finalization message (in grandpa MessageHandler.HandleMessage)
  • if justification is valid, mark block as final (ie call blockState.SetFinalizedHash())
  • since grandpa.Justification includes the authority public key and signature, we need to check that:
  1. the signature is valid (need setID and stage to recreate signed FullVote, stage=precommit, use current grandpa setID, eventually can use block number -> setID map)
  2. the authority was in the set indicated by the setID
  3. the total # of signatures is >=2/3 the number of voters in the set

the justification verification will have overlap with #1070

Current Behavior

  • no justification verification

Checklist

  • I have read CODE_OF_CONDUCT and CONTRIBUTING
  • I have provided as much information as possible and necessary
  • I am planning to submit a pull request to fix this issue myself
@amerameen amerameen added this to the Sprint 08-07-20 milestone Aug 7, 2020
@noot noot changed the title add Verify-Block-Justification on block import verify block justification when handling FinalizationMessage Aug 11, 2020
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 a pull request may close this issue.

3 participants