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

Panic in evidence serializers #782

Closed
romac opened this issue Jan 13, 2021 · 0 comments · Fixed by #798
Closed

Panic in evidence serializers #782

romac opened this issue Jan 13, 2021 · 0 comments · Fixed by #798
Assignees
Labels
bug Something isn't working serialization

Comments

@romac
Copy link
Member

romac commented Jan 13, 2021

The serializers for tendermint::types::{Evidence, evidence::Sum} are incomplete and may cause a panic in the relayer.

The panic happens in this From instance, which should likely be converted to a TryFrom instance:

impl From<Evidence> for EvidenceVariant {
fn from(value: Evidence) -> Self {
let sum = value.sum.unwrap(); // Todo: Error handling
match sum {
Sum::DuplicateVoteEvidence(d) => Self::DuplicateVoteEvidence(d),
Sum::LightClientAttackEvidence(l) => Self::LightClientAttackEvidence(l),
}
}
}

Additionally, these From instances are currently unimplemented!:

impl From<Sum> for EvidenceVariant {
fn from(_: Sum) -> Self {
unimplemented!() // Prost adds extra annotations on top of Sum that are not used.
}
}
impl From<EvidenceVariant> for Sum {
fn from(_: EvidenceVariant) -> Self {
unimplemented!() // Prost adds extra annotations on top of Sum that are not used.
}
}

The comment seems to imply they won't ever be called, so it shouldn't be a problem, but perhaps it would worth looking into removing them altogether while we're at it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working serialization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants