-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Normalise GenesisDoc before saving to state #6059
Conversation
Signed-off-by: Silas Davis <silas@monax.io>
0724fb0
to
d022737
Compare
Codecov Report
@@ Coverage Diff @@
## master #6059 +/- ##
==========================================
+ Coverage 60.70% 60.78% +0.07%
==========================================
Files 276 276
Lines 25706 25709 +3
==========================================
+ Hits 15606 15627 +21
+ Misses 8478 8463 -15
+ Partials 1622 1619 -3
|
Hey @silasdavis, thanks for the PR. This does seem to me like a more appropriate place to validate and complete the genesis doc. I will just check if there are any cases we might be missing. It also feels like we are making this whole "get latest state or if none extract it from genesis" logic a lot more convoluted than perhaps it needs to be. |
@@ -335,11 +335,6 @@ func MakeGenesisDocFromFile(genDocFile string) (*types.GenesisDoc, error) { | |||
|
|||
// MakeGenesisState creates state from types.GenesisDoc. | |||
func MakeGenesisState(genDoc *types.GenesisDoc) (State, error) { | |||
err := genDoc.ValidateAndComplete() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd instead leave this because, if the state is partially gone (due to FS corruption or other reasons; stateKey
is gone, but genesisDocKey
is there, although the probability is very low), LoadFromDBOrGenesisDoc
makes sure we always validate genDoc
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes why not. Two rights make a right.
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Co-authored-by: Callum <cmwaters19@gmail.com>
Please feel free to close and replace this is more work or a better approach is needed, just opening to illustrate.
fixes #5986