-
Notifications
You must be signed in to change notification settings - Fork 3
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
perf(state/store): avoid double-saving ABCI responses #13
perf(state/store): avoid double-saving ABCI responses #13
Conversation
if height != info.GetHeight() { | ||
return nil, fmt.Errorf("expected height %d but last stored abci responses was at height %d", height, info.GetHeight()) | ||
} | ||
return info.AbciResponses, nil |
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 left out the following logic branch because I don't think it matters here, but would like an ACK
if info.FinalizeBlock == nil {
// sanity check
if info.LegacyAbciResponses == nil {
panic("state store contains last abci response but it is empty")
}
return responseFinalizeBlockFromLegacy(info.LegacyAbciResponses), nil
}
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.
This is for an edge case, but I think this backports it wrong
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 will hold off on merging this till we talk next then
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.
oh your right we are fine without this logic branch, sorry about that.
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.
Looks right to me!
PR checklist
.changelog
(we use unclog to manage our changelog)docs/
orspec/
) and code comments