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

Fix off by one tipset in searchBackForMsg #4367

Merged
merged 3 commits into from Oct 13, 2020
Merged

Conversation

Kubuxu
Copy link
Contributor

@Kubuxu Kubuxu commented Oct 13, 2020

This way we will return the tipset the message was executed in

This way we will return the tipset the message was executed in

Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
Jakub Sztandera added 2 commits October 13, 2020 20:02
Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai>
Comment on lines +81 to +88
var ser []byte
var err error
if sm.Signature.Type == crypto.SigTypeBLS {
// BLS chain message length doesn't include signature
ser, err = sm.Message.Serialize()
} else {
ser, err = sm.Serialize()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a fix for a different issue, mildly fishy consensus-wise, though looking at usages I believe it's fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it is a fix for the mempool. Doesn't touch consensus as we have no signed BLS message in VM.

Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@magik6k magik6k merged commit d5cea9f into master Oct 13, 2020
@magik6k magik6k deleted the fix/msg-search-off-by-one branch October 13, 2020 20:11
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.

Filecoin.StateSearchMsg returns inconsistent Height results
3 participants