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.

Sign up for GitHub

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

Jump to bottom

Fix justification #1491

Merged
merged 10 commits into from
Feb 21, 2023
Merged

Fix justification #1491

merged 10 commits into from
Feb 21, 2023

Conversation

turuslan
Copy link
Contributor

@turuslan turuslan commented Feb 17, 2023 •
edited
Loading

Referenced issues

Description of the Change

  • Add GrandpaJustification::votes_ancestries field.
  • Always finalize blocks with scheduled/forced change digest.
  • Ignore BlockBuilder_inherent_extrinsics changes.
  • HasChange { scheduled, forced }, because warp sync needs to distinguish change type

Benefits

  • Fix substrate couldn't finalize after sync.
  • Fix substrate Import failed: error decoding justification for header
  • Fix kagome BlockBuilder Extrinsic 04360008 cannot be applied and was not pushed to the block. (InvalidTransaction response, code 8)

Possible Drawbacks

@turuslan
ignore BlockBuilder_inherent_extrinsics changes
0b14ef9 
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
@turuslan
GrandpaJustification votes_ancestries
fa7e399 
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
@turuslan
grandpa HasChange
4f54ab4 
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
@turuslan
finalize change digest
2582213 
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
@turuslan
fix test
9e512ad 
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
igor-egorov
igor-egorov approved these changes Feb 17, 2023
View reviewed changes
Copy link
Contributor

@igor-egorov igor-egorov left a comment

Choose a reason for hiding this comment

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

👍

core/consensus/grandpa/has_change.hpp Outdated Show resolved Hide resolved
@turuslan turuslan requested review from Harrm and removed request for kamilsa February 17, 2023 08:34
Harrm
Harrm reviewed Feb 17, 2023
View reviewed changes
core/consensus/grandpa/has_change.hpp Outdated Show resolved Hide resolved
core/consensus/grandpa/has_change.hpp Outdated Show resolved Hide resolved
core/consensus/grandpa/has_change.hpp Outdated Show resolved Hide resolved
core/network/impl/sync_protocol_observer_impl.cpp Outdated Show resolved Hide resolved
core/runtime/runtime_api/impl/block_builder.cpp Outdated Show resolved Hide resolved
@turuslan
rename
2b1e2c4 
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
@turuslan
required field
0bcf017 
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
@turuslan
comment
2993694 
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
@turuslan
log error
1264a46 
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
@turuslan turuslan merged commit 4ffd7b2 into master Feb 21, 2023
@turuslan turuslan deleted the fix/justification branch February 21, 2023 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kamilsa kamilsa kamilsa approved these changes

@Harrm Harrm Harrm approved these changes

@igor-egorov igor-egorov Awaiting requested review from igor-egorov

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@turuslan @kamilsa @Harrm @igor-egorov