From bd36418f8eff49c61ef65ee4d40057ebf5a04162 Mon Sep 17 00:00:00 2001 From: Thane Thomson Date: Thu, 25 Mar 2021 08:47:58 -0400 Subject: [PATCH 1/3] Fix consensus state vote summary deserialization Signed-off-by: Thane Thomson --- rpc/src/endpoint/consensus_state.rs | 32 ++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 10 deletions(-) diff --git a/rpc/src/endpoint/consensus_state.rs b/rpc/src/endpoint/consensus_state.rs index 9e34b3e9a..1771baa98 100644 --- a/rpc/src/endpoint/consensus_state.rs +++ b/rpc/src/endpoint/consensus_state.rs @@ -2,7 +2,6 @@ use crate::{Error, Method}; use serde::{Deserialize, Deserializer, Serialize, Serializer}; -use std::convert::TryFrom; use std::fmt; use std::str::FromStr; use subtle_encoding::hex; @@ -241,17 +240,18 @@ impl FromStr for VoteSummary { e )) })?; - let vote_type_num: Vec<&str> = height_round_type[2].split('(').collect(); - let vote_type_id = i32::from_str(vote_type_num[0]).map_err(|e| { - Self::Err::client_internal_error(format!( - "failed to parse vote type from consensus state vote summary: {} ({})", - e, vote_type_num[0], - )) - })?; - let vote_type = vote::Type::try_from(vote_type_id).map_err(|e| { + let vote_type_parts: Vec<&str> = height_round_type[2].split('(').collect(); + if vote_type_parts.len() != 2 { + return Err(Self::Err::client_internal_error(format!( + "invalid structure for vote type in consensus state vote summary: {}", + height_round_type[2] + ))); + } + let vote_type_str = vote_type_parts[1].trim_end_matches(')'); + let vote_type = vote::Type::from_str(vote_type_str).map_err(|e| { Self::Err::client_internal_error(format!( "failed to parse vote type from consensus state vote summary: {} ({})", - e, vote_type_id, + e, vote_type_str )) })?; let block_id_hash_fingerprint = Fingerprint::from_str(parts[2]).map_err(|e| { @@ -344,6 +344,11 @@ pub struct ValidatorInfo { mod test { use super::*; + // Cases we've discovered in integration testing where deserialization failed. + const DESERIALIZATION_REGRESSIONS: &[&str] = &[ + "Vote{0:2DA21E474F57 384/00/SIGNED_MSG_TYPE_PREVOTE(Prevote) 8FA9FD23F590 2987C33E8F87 @ 2021-03-25T12:12:03.693870115Z}", + ]; + #[test] fn deserialize_vote_summary() { let src = "Vote{0:000001E443FD 1262197/00/1(Prevote) 634ADAF1F402 7BB974E1BA40 @ 2019-08-01T11:52:35.513572509Z}"; @@ -384,4 +389,11 @@ mod test { }; assert_eq!(summary.to_string(), "Vote{0:000001E443FD 1262197/00/1(Prevote) 634ADAF1F402 7BB974E1BA40 @ 2019-08-01T11:52:35.513572509Z}"); } + + #[test] + fn deserialization_regressions() { + for s in DESERIALIZATION_REGRESSIONS { + let _ = VoteSummary::from_str(s).unwrap(); + } + } } From 7b1b4096dff6590e47466fce7059dfd10d28cbc3 Mon Sep 17 00:00:00 2001 From: Thane Thomson Date: Thu, 25 Mar 2021 08:59:30 -0400 Subject: [PATCH 2/3] Update CHANGELOG Signed-off-by: Thane Thomson --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e3673954..40518bda1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,9 +28,15 @@ * `[tendermint-rpc]` A `tendermint-rpc` CLI has been added to simplify interaction with RPC endpoints from the command line ([#820]) +### BUG FIXES + +* `[tendermint-rpc]` Fix intermittent deserialization failures of the consensus + state response ([#836]) + [#794]: https://github.com/informalsystems/tendermint-rs/pull/794 [#812]: https://github.com/informalsystems/tendermint-rs/pull/812 [#820]: https://github.com/informalsystems/tendermint-rs/pull/820 +[#836]: https://github.com/informalsystems/tendermint-rs/issues/836 ## v0.18.1 From de37908c7790f1f6aede287692793e98742b8435 Mon Sep 17 00:00:00 2001 From: Thane Thomson Date: Mon, 29 Mar 2021 13:05:43 -0400 Subject: [PATCH 3/3] Streamline vote summary de/serialization Signed-off-by: Thane Thomson --- rpc/src/endpoint/consensus_state.rs | 89 +++++++++++++++-------------- 1 file changed, 45 insertions(+), 44 deletions(-) diff --git a/rpc/src/endpoint/consensus_state.rs b/rpc/src/endpoint/consensus_state.rs index 110d3cccc..565a4c28f 100644 --- a/rpc/src/endpoint/consensus_state.rs +++ b/rpc/src/endpoint/consensus_state.rs @@ -342,57 +342,58 @@ pub struct ValidatorInfo { #[cfg(test)] mod test { use super::*; - - // Cases we've discovered in integration testing where deserialization failed. - const DESERIALIZATION_REGRESSIONS: &[&str] = &[ - "Vote{0:2DA21E474F57 384/00/SIGNED_MSG_TYPE_PREVOTE(Prevote) 8FA9FD23F590 2987C33E8F87 @ 2021-03-25T12:12:03.693870115Z}", - ]; + use lazy_static::lazy_static; + + lazy_static! { + // An array of (received, deserialized, serialized) vote summaries + static ref TEST_VOTE_SUMMARIES: Vec<(String, VoteSummary, String)> = vec![ + ( + "Vote{0:000001E443FD 1262197/00/1(Prevote) 634ADAF1F402 7BB974E1BA40 @ 2019-08-01T11:52:35.513572509Z}".to_owned(), + VoteSummary { + validator_index: 0, + validator_address_fingerprint: Fingerprint(vec![0, 0, 1, 228, 67, 253]), + height: Height::from(1262197_u32), + round: Round::from(0_u8), + vote_type: vote::Type::Prevote, + block_id_hash_fingerprint: Fingerprint(vec![99, 74, 218, 241, 244, 2]), + signature_fingerprint: Fingerprint(vec![123, 185, 116, 225, 186, 64]), + timestamp: "2019-08-01T11:52:35.513572509Z".parse().unwrap(), + }, + "Vote{0:000001E443FD 1262197/00/1(Prevote) 634ADAF1F402 7BB974E1BA40 @ 2019-08-01T11:52:35.513572509Z}".to_owned(), + ), + ( + // See https://github.com/informalsystems/tendermint-rs/issues/836 + "Vote{0:2DA21E474F57 384/00/SIGNED_MSG_TYPE_PREVOTE(Prevote) 8FA9FD23F590 2987C33E8F87 @ 2021-03-25T12:12:03.693870115Z}".to_owned(), + VoteSummary { + validator_index: 0, + validator_address_fingerprint: Fingerprint(vec![45, 162, 30, 71, 79, 87]), + height: Height::from(384_u32), + round: Round::from(0_u8), + vote_type: vote::Type::Prevote, + block_id_hash_fingerprint: Fingerprint(vec![143, 169, 253, 35, 245, 144]), + signature_fingerprint: Fingerprint(vec![41, 135, 195, 62, 143, 135]), + timestamp: "2021-03-25T12:12:03.693870115Z".parse().unwrap(), + }, + "Vote{0:2DA21E474F57 384/00/1(Prevote) 8FA9FD23F590 2987C33E8F87 @ 2021-03-25T12:12:03.693870115Z}".to_owned(), + ) + ]; + } #[test] fn deserialize_vote_summary() { - let src = "Vote{0:000001E443FD 1262197/00/1(Prevote) 634ADAF1F402 7BB974E1BA40 @ 2019-08-01T11:52:35.513572509Z}"; - let summary = VoteSummary::from_str(src).unwrap(); - assert_eq!(summary.validator_index, 0); - assert_eq!( - summary.validator_address_fingerprint.0, - vec![0, 0, 1, 228, 67, 253] - ); - assert_eq!(summary.height.value(), 1262197); - assert_eq!(summary.round.value(), 0); - assert_eq!(summary.vote_type, vote::Type::Prevote); - assert_eq!( - summary.block_id_hash_fingerprint.0, - vec![99, 74, 218, 241, 244, 2] - ); - assert_eq!( - summary.signature_fingerprint.0, - vec![123, 185, 116, 225, 186, 64] - ); - assert_eq!( - summary.timestamp.to_rfc3339(), - "2019-08-01T11:52:35.513572509Z" - ); + for (vote_summary_str, expected, _) in TEST_VOTE_SUMMARIES.iter() { + let actual = VoteSummary::from_str(vote_summary_str); + assert!(actual.is_ok(), "{}", vote_summary_str); + let actual = actual.unwrap(); + assert_eq!(expected.clone(), actual); + } } #[test] fn serialize_vote_summary() { - let summary = VoteSummary { - validator_index: 0, - validator_address_fingerprint: Fingerprint(vec![0, 0, 1, 228, 67, 253]), - height: Height::from(1262197_u32), - round: Round::from(0_u8), - vote_type: vote::Type::Prevote, - block_id_hash_fingerprint: Fingerprint(vec![99, 74, 218, 241, 244, 2]), - signature_fingerprint: Fingerprint(vec![123, 185, 116, 225, 186, 64]), - timestamp: Time::parse_from_rfc3339("2019-08-01T11:52:35.513572509Z").unwrap(), - }; - assert_eq!(summary.to_string(), "Vote{0:000001E443FD 1262197/00/1(Prevote) 634ADAF1F402 7BB974E1BA40 @ 2019-08-01T11:52:35.513572509Z}"); - } - - #[test] - fn deserialization_regressions() { - for s in DESERIALIZATION_REGRESSIONS { - let _ = VoteSummary::from_str(s).unwrap(); + for (_, vote_summary, expected) in TEST_VOTE_SUMMARIES.iter() { + let actual = vote_summary.to_string(); + assert_eq!(expected.clone(), actual); } } }