From 99f4cd0999a664679d73d78faf0155f0341df220 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Thu, 22 Sep 2022 18:36:48 +0200 Subject: [PATCH] rpc: Fix deserialization of `DuplicateVoteEvidence` in `/block_results` response (#1195) * Add test for parsing `block_results` incoming response containing a duplicate vote evidence * Fix `serde` annotations on `Vote` and `DuplicateVoteEvidence` protos * Remove duplicate tests in `kvstore_fixtures` test * Add changelog entry * Reword changelog entry --- .../1194-dup-vote-evidence-parsing.md | 2 + proto/src/prost/tendermint.types.rs | 5 +- rpc/tests/kvstore_fixtures.rs | 131 +++++++++----- .../incoming/block_search_evidence.json | 171 ++++++++++++++++++ tendermint/src/evidence.rs | 14 +- tools/proto-compiler/src/constants.rs | 19 +- 6 files changed, 286 insertions(+), 56 deletions(-) create mode 100644 .changelog/unreleased/bug-fixes/1194-dup-vote-evidence-parsing.md create mode 100644 rpc/tests/kvstore_fixtures/incoming/block_search_evidence.json diff --git a/.changelog/unreleased/bug-fixes/1194-dup-vote-evidence-parsing.md b/.changelog/unreleased/bug-fixes/1194-dup-vote-evidence-parsing.md new file mode 100644 index 000000000..a46f538bc --- /dev/null +++ b/.changelog/unreleased/bug-fixes/1194-dup-vote-evidence-parsing.md @@ -0,0 +1,2 @@ +- `[tendermint-rpc]` Fix deserialization of `/block_results` response when it contains evidence for a duplicate vote + ([#1194](https://github.com/informalsystems/tendermint-rs/issues/1194)) diff --git a/proto/src/prost/tendermint.types.rs b/proto/src/prost/tendermint.types.rs index 4e1ab5d7e..3888d22a6 100644 --- a/proto/src/prost/tendermint.types.rs +++ b/proto/src/prost/tendermint.types.rs @@ -146,7 +146,6 @@ pub struct Vote { #[serde(with = "crate::serializers::from_str")] pub height: i64, #[prost(int32, tag="3")] - #[serde(with = "crate::serializers::from_str")] pub round: i32, /// zero if vote is nil. #[prost(message, optional, tag="4")] @@ -158,7 +157,6 @@ pub struct Vote { #[serde(with = "crate::serializers::bytes::hexstring")] pub validator_address: ::prost::alloc::vec::Vec, #[prost(int32, tag="7")] - #[serde(with = "crate::serializers::from_str")] pub validator_index: i32, #[prost(bytes="vec", tag="8")] #[serde(with = "crate::serializers::bytes::base64string")] @@ -478,10 +476,13 @@ pub struct DuplicateVoteEvidence { #[prost(message, optional, tag="2")] pub vote_b: ::core::option::Option, #[prost(int64, tag="3")] + #[serde(alias = "TotalVotingPower", with = "crate::serializers::from_str")] pub total_voting_power: i64, #[prost(int64, tag="4")] + #[serde(alias = "ValidatorPower", with = "crate::serializers::from_str")] pub validator_power: i64, #[prost(message, optional, tag="5")] + #[serde(alias = "Timestamp")] pub timestamp: ::core::option::Option, } /// LightClientAttackEvidence contains evidence of a set of validators attempting to mislead a light client. diff --git a/rpc/tests/kvstore_fixtures.rs b/rpc/tests/kvstore_fixtures.rs index ba0f3f536..4796b244c 100644 --- a/rpc/tests/kvstore_fixtures.rs +++ b/rpc/tests/kvstore_fixtures.rs @@ -4,7 +4,12 @@ use core::str::FromStr; use std::{fs, path::PathBuf}; use subtle_encoding::{base64, hex}; -use tendermint::{abci::transaction::Hash, evidence::Duration, public_key}; +use tendermint::{ + abci::transaction::Hash, + evidence::{Duration, Evidence}, + public_key, + vote::Vote, +}; use tendermint_config::net::Address; use tendermint_rpc::{ endpoint, @@ -500,51 +505,40 @@ fn incoming_fixtures() { assert_eq!(block.block_id.part_set_header.total, 1); } }, - "blockchain_from_1_to_10" => { - let result = endpoint::blockchain::Response::from_string(content).unwrap(); - assert_eq!(result.block_metas.len(), 10); - for block_meta in result.block_metas { - assert!(!block_meta.block_id.hash.is_empty()); - assert!(!block_meta.block_id.part_set_header.hash.is_empty()); - assert_eq!(block_meta.block_id.part_set_header.total, 1); - assert!(block_meta.block_size > 0); - if block_meta.header.height.value() == 1 { - assert!(block_meta.header.app_hash.value().is_empty()); - assert_eq!(block_meta.header.data_hash, empty_merkle_root_hash); - assert_eq!(block_meta.header.evidence_hash, empty_merkle_root_hash); - assert!(block_meta.header.last_block_id.is_none()); - assert_eq!(block_meta.header.last_commit_hash, empty_merkle_root_hash); - assert_eq!(block_meta.header.last_results_hash, empty_merkle_root_hash); + "block_search_evidence" => { + let result = endpoint::block_search::Response::from_string(content).unwrap(); + assert_eq!(result.total_count as usize, result.blocks.len()); + + // Test a few selected attributes of the results. + for block in result.blocks { + let evidence = block.block.evidence.iter().next().unwrap(); + + use tendermint::vote; + + fn check_vote(vote: &Vote) { + assert_eq!(vote.vote_type, vote::Type::Precommit); + assert_eq!(vote.height.value(), 8009); + assert_eq!(vote.round.value(), 0); + assert_eq!( + vote.validator_address, + "9319035301DA526CC78DCF174A47A74F81401291".parse().unwrap(), + ); + assert_eq!(vote.validator_index.value(), 8); + } + + if let Evidence::DuplicateVote(dup) = evidence { + assert_eq!(dup.total_voting_power.value(), 121); + assert_eq!(dup.validator_power.value(), 1); + assert_eq!( + dup.timestamp, + "2022-09-12T19:49:49.984608464Z".parse().unwrap() + ); + + check_vote(&dup.vote_a); + check_vote(&dup.vote_b); } else { - assert!(!block_meta.header.app_hash.value().is_empty()); - assert!(block_meta.header.data_hash.is_some()); - assert!(block_meta.header.evidence_hash.is_some()); - assert!(block_meta.header.last_block_id.is_some()); - assert!(block_meta.header.last_commit_hash.is_some()); - assert!(block_meta.header.last_results_hash.is_some()); + panic!("not a duplicate vote: {:?}", evidence); } - assert_eq!(block_meta.header.chain_id.as_str(), CHAIN_ID); - assert!(!block_meta.header.consensus_hash.is_empty()); - assert!(!block_meta.header.next_validators_hash.is_empty()); - assert_ne!( - block_meta.header.proposer_address.as_bytes(), - [0u8; tendermint::account::LENGTH] - ); - assert!( - block_meta - .header - .time - .duration_since(informal_epoch) - .unwrap() - .as_secs() - > 0 - ); - assert!(!block_meta.header.validators_hash.is_empty()); - assert_eq!( - block_meta.header.version, - tendermint::block::header::Version { block: 11, app: 1 } - ); - assert_eq!(block_meta.num_txs, 0); } }, "broadcast_tx_async" => { @@ -569,7 +563,7 @@ fn incoming_fixtures() { assert!(result.check_tx.events.is_empty()); assert_eq!(result.check_tx.gas_used.value(), 0); // Todo: https://github.com/informalsystems/tendermint-rs/issues/761 - // assert_eq!(result.check_tx.gas_wanted.value(), 1); + //assert_eq!(result.check_tx.gas_wanted.value(), 1); assert!(result.check_tx.info.to_string().is_empty()); assert!(result.check_tx.log.value().is_empty()); assert_eq!(result.deliver_tx.code, tendermint::abci::Code::Ok); @@ -832,6 +826,53 @@ fn incoming_fixtures() { assert!(result.validator_info.pub_key.ed25519().is_some()); assert_eq!(result.validator_info.power.value(), 10); }, + "blockchain_from_1_to_10" => { + let result = endpoint::blockchain::Response::from_string(content).unwrap(); + assert_eq!(result.block_metas.len(), 10); + for block_meta in result.block_metas { + assert!(!block_meta.block_id.hash.is_empty()); + assert!(!block_meta.block_id.part_set_header.hash.is_empty()); + assert_eq!(block_meta.block_id.part_set_header.total, 1); + assert!(block_meta.block_size > 0); + if block_meta.header.height.value() == 1 { + assert!(block_meta.header.app_hash.value().is_empty()); + assert_eq!(block_meta.header.data_hash, empty_merkle_root_hash); + assert_eq!(block_meta.header.evidence_hash, empty_merkle_root_hash); + assert!(block_meta.header.last_block_id.is_none()); + assert_eq!(block_meta.header.last_commit_hash, empty_merkle_root_hash); + assert_eq!(block_meta.header.last_results_hash, empty_merkle_root_hash); + } else { + assert!(!block_meta.header.app_hash.value().is_empty()); + assert!(block_meta.header.data_hash.is_some()); + assert!(block_meta.header.evidence_hash.is_some()); + assert!(block_meta.header.last_block_id.is_some()); + assert!(block_meta.header.last_commit_hash.is_some()); + assert!(block_meta.header.last_results_hash.is_some()); + } + assert_eq!(block_meta.header.chain_id.as_str(), CHAIN_ID); + assert!(!block_meta.header.consensus_hash.is_empty()); + assert!(!block_meta.header.next_validators_hash.is_empty()); + assert_ne!( + block_meta.header.proposer_address.as_bytes(), + [0u8; tendermint::account::LENGTH] + ); + assert!( + block_meta + .header + .time + .duration_since(informal_epoch) + .unwrap() + .as_secs() + > 0 + ); + assert!(!block_meta.header.validators_hash.is_empty()); + assert_eq!( + block_meta.header.version, + tendermint::block::header::Version { block: 11, app: 1 } + ); + assert_eq!(block_meta.num_txs, 0); + } + }, "subscribe_malformed" => { let result = endpoint::subscribe::Response::from_string(content); diff --git a/rpc/tests/kvstore_fixtures/incoming/block_search_evidence.json b/rpc/tests/kvstore_fixtures/incoming/block_search_evidence.json new file mode 100644 index 000000000..ca4d2e648 --- /dev/null +++ b/rpc/tests/kvstore_fixtures/incoming/block_search_evidence.json @@ -0,0 +1,171 @@ +{ + "jsonrpc": "2.0", + "id": "4aa76fe8-0ad1-4888-a610-49e41aab79be", + "result": { + "blocks": [ + { + "block_id": { + "hash": "13B17D2A7FF6F58F14A88D29B53CBA328776E391DE09D73D43CA5BB54B760F4D", + "parts": { + "total": 1, + "hash": "207A23AE9F86FC843DBBCA7BB63164807492EE340C9193ACF8281C8755C5F7E7" + } + }, + "block": { + "header": { + "version": { + "block": "11" + }, + "chain_id": "provider", + "height": "8011", + "time": "2022-09-12T19:49:56.057476304Z", + "last_block_id": { + "hash": "0F7E242B58D28545269E89CD131DFB662F5221166F26E0CF374EA8402EED5200", + "parts": { + "total": 1, + "hash": "57D5DA87B690945E8C6C1C85924C47B8B3A885E4149D12C207CED8EEB0C4FB5B" + } + }, + "last_commit_hash": "7E929B00540980E5D8B6F35200C371A57C3FDDA38F6E16B5058BDDA56CF3FEA2", + "data_hash": "E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855", + "validators_hash": "1BD8973AC098D625531DFAE3B57551E90136667C554ED268C9A8A5E94E9BE948", + "next_validators_hash": "1BD8973AC098D625531DFAE3B57551E90136667C554ED268C9A8A5E94E9BE948", + "consensus_hash": "048091BC7DDC283F77BFBF91D73C44DA58C3DF8A9CBC867405D8B7F3DAADA22F", + "app_hash": "69EF014ECC4C2EFE97EC253CABE132118D9A57A82A06D2ABA777E6E209B233A7", + "last_results_hash": "E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855", + "evidence_hash": "91D5E877F2C5634E2666A7B4E684D1FB545E2B859514E526BE15CF09C6A37AB6", + "proposer_address": "B218725A22B9319FC8340E83601588438FEF11BD" + }, + "data": { + "txs": [] + }, + "evidence": { + "evidence": [ + { + "type": "tendermint/DuplicateVoteEvidence", + "value": { + "vote_a": { + "type": 2, + "height": "8009", + "round": 0, + "block_id": { + "hash": "", + "parts": { + "total": 0, + "hash": "" + } + }, + "timestamp": "2022-09-07T22:52:09.078399096Z", + "validator_address": "9319035301DA526CC78DCF174A47A74F81401291", + "validator_index": 8, + "signature": "wjyIJ6WxLl38O0gKAWPmCjP0c3HhTfyNjRfc5LlDzQ4jw/7XHmu4tNbW8NV1C7DeuceLYT2OGAAgf1g1R7QXDw==" + }, + "vote_b": { + "type": 2, + "height": "8009", + "round": 0, + "block_id": { + "hash": "7A2840217294E52F4E4C2F1E9B3E7DFEC0B34605B66EC042B02EF82B7C5E70FB", + "parts": { + "total": 1, + "hash": "1B20410D8B8876F51E5272A86E2C73CFF00783DD8D1BF09C6638B812606A9CFB" + } + }, + "timestamp": "2022-09-12T19:49:53.28054099Z", + "validator_address": "9319035301DA526CC78DCF174A47A74F81401291", + "validator_index": 8, + "signature": "9Ugzwmw3N4FZrK5VxMmUYLU8MjlT+I03VoQmqt9nDQSHexVDRaZPnMDiP13lci9IKhnBKG8wVTldJKv0ystKDQ==" + }, + "TotalVotingPower": "121", + "ValidatorPower": "1", + "Timestamp": "2022-09-12T19:49:49.984608464Z" + } + } + ] + }, + "last_commit": { + "height": "8010", + "round": 0, + "block_id": { + "hash": "0F7E242B58D28545269E89CD131DFB662F5221166F26E0CF374EA8402EED5200", + "parts": { + "total": 1, + "hash": "57D5DA87B690945E8C6C1C85924C47B8B3A885E4149D12C207CED8EEB0C4FB5B" + } + }, + "signatures": [ + { + "block_id_flag": 2, + "validator_address": "B218725A22B9319FC8340E83601588438FEF11BD", + "timestamp": "2022-09-12T19:49:56.057476304Z", + "signature": "vMva0A+SVY382TuluaOTpnLiahWSFDwLQoSLUyuRGC7QwS4FCZ326kONExt7DMOF2e1DoxfmDv2VbBwsisEzCg==" + }, + { + "block_id_flag": 3, + "validator_address": "A49DAB77379B869F28E2AA35BF4D7F07866B0C10", + "timestamp": "2022-09-12T19:49:56.325571749Z", + "signature": "jhynVaXG3eN69pRG55wRHZX4lUxmQwol2E8Q3W56axYAGd28YxSmCDnC+SRh85+EBBFGhM79OkcP0xbvxNv+CQ==" + }, + { + "block_id_flag": 3, + "validator_address": "ED46DA41B6942355F8308433764A18318223D9EB", + "timestamp": "2022-09-12T19:49:56.147905252Z", + "signature": "WwaxUwcyPBiwOV5XnZBO+rYPWhlnhBG3VIz5cr/c1vd7gtPrdvqYlRlFNr0rGloU9R3FKqdkdMZdGZhCRs+eCw==" + }, + { + "block_id_flag": 3, + "validator_address": "090AA8D83370208D7B5907072666C8CCE92B0AE4", + "timestamp": "2022-09-12T19:49:56.30837796Z", + "signature": "iADSIvtFsCN7LYMoj4sYc7mzn+k5v40twSmRSRpkcA58O3x1zxPjOXHOXi9Q36qCMRtTHhouUWLEgXhOb0/wCg==" + }, + { + "block_id_flag": 3, + "validator_address": "54443EF34125BDBDF74D8E1FD1EC3857160EF3CC", + "timestamp": "2022-09-12T19:49:56.328293863Z", + "signature": "agsLAHg5/7TeKCJM9UAUeUsGW7EVpmo+F84s9IHpSMIaEItNXdP7JOJrb62UNXJzPRdNJYGPfaFKeUUrs9m/DA==" + }, + { + "block_id_flag": 3, + "validator_address": "A2CE10C56A0F44BA30116535DC67409130BF5E7E", + "timestamp": "2022-09-12T19:49:56.273003947Z", + "signature": "jQDmNCeQ+w62W5bMvQ5JpTD/Jv1KyKfwWO9yswQkbI9yodk6j7ElnlfJ8vzlK9a7iea0SEt6YGRWfhH9uq7LDA==" + }, + { + "block_id_flag": 3, + "validator_address": "E4C657BC63B89319BE7C62E48DF5CE85B1543D00", + "timestamp": "2022-09-12T19:49:56.266262774Z", + "signature": "ZHpV742hZPrLvu3jfMofGHUA8MzIDVxQXgfPG+wCwDyiP9NQlpvceUX53nJ9MW58A80soN17KZvt1Np+PUweBw==" + }, + { + "block_id_flag": 3, + "validator_address": "0ED9646BBC2A58146C5F67E66BF4EC33AD83368D", + "timestamp": "2022-09-12T19:49:56.273836856Z", + "signature": "jXeBSvHO43NfLEwZJAeGmBQ/rQHDoHVNZ0c3JjJwt5kOAVOONAG/iL4tZj9/f4qZZFBaueU8zcANQllzL/S7AQ==" + }, + { + "block_id_flag": 3, + "validator_address": "9319035301DA526CC78DCF174A47A74F81401291", + "timestamp": "2022-09-07T22:52:12.094618273Z", + "signature": "BRySEQjgpaqCy2huVg6v+Wpksrql8NMlIaJbHEsCqqmAeb7ieS/SHOPAgho8fQ1ODzWsL4PiXwYO9Fgx0Ll7DQ==" + }, + { + "block_id_flag": 3, + "validator_address": "9901CCDA42B948EDDC835FB5C8AB06730A9C23CD", + "timestamp": "2022-09-12T19:49:56.165888573Z", + "signature": "Uhsp575gJkKG67YB6iuuj8ojp8Su46LRWgH6rK6eizxpG8cFmRdpRWsu/G29zx7r44sxtqa6RjxYdkanQdpSCg==" + }, + { + "block_id_flag": 3, + "validator_address": "F7DF9EF920DFBCDCA1F382151685A69500BAF630", + "timestamp": "2022-09-12T19:49:56.156036054Z", + "signature": "vtyX9kzMUyb2Sksly7fUZkXhs8e29Eu3dmi4mu9zBvYSwc8vI3GTckSutDMb6jgDv3qD7SjYGhX89TbproUiCg==" + } + ] + } + } + } + ], + "total_count": "1" + } +} + diff --git a/tendermint/src/evidence.rs b/tendermint/src/evidence.rs index f2dfa64e5..172a54cfb 100644 --- a/tendermint/src/evidence.rs +++ b/tendermint/src/evidence.rs @@ -70,11 +70,11 @@ impl From for RawEvidence { /// Duplicate vote evidence #[derive(Clone, Debug, PartialEq, Eq)] pub struct DuplicateVoteEvidence { - vote_a: Vote, - vote_b: Vote, - total_voting_power: Power, - validator_power: Power, - timestamp: Time, + pub vote_a: Vote, + pub vote_b: Vote, + pub total_voting_power: Power, + pub validator_power: Power, + pub timestamp: Time, } impl TryFrom for DuplicateVoteEvidence { @@ -138,9 +138,9 @@ impl DuplicateVoteEvidence { #[derive(Clone, Debug, PartialEq, Eq)] pub struct ConflictingHeadersEvidence { //#[serde(rename = "H1")] - h1: SignedHeader, + pub h1: SignedHeader, //#[serde(rename = "H2")] - h2: SignedHeader, + pub h2: SignedHeader, } impl ConflictingHeadersEvidence { diff --git a/tools/proto-compiler/src/constants.rs b/tools/proto-compiler/src/constants.rs index 3474f85fa..4978cea5f 100644 --- a/tools/proto-compiler/src/constants.rs +++ b/tools/proto-compiler/src/constants.rs @@ -35,6 +35,11 @@ const RENAME_DUPLICATEVOTE: &str = r#"#[serde(rename = "tendermint/DuplicateVote const RENAME_LIGHTCLIENTATTACK: &str = r#"#[serde(rename = "tendermint/LightClientAttackEvidence")]"#; const EVIDENCE_VARIANT: &str = r#"#[serde(from = "crate::serializers::evidence::EvidenceVariant", into = "crate::serializers::evidence::EvidenceVariant")]"#; +const ALIAS_VALIDATOR_POWER_QUOTED: &str = + r#"#[serde(alias = "ValidatorPower", with = "crate::serializers::from_str")]"#; +const ALIAS_TOTAL_VOTING_POWER_QUOTED: &str = + r#"#[serde(alias = "TotalVotingPower", with = "crate::serializers::from_str")]"#; +const ALIAS_TIMESTAMP: &str = r#"#[serde(alias = "Timestamp")]"#; const ALIAS_PARTS: &str = r#"#[serde(alias = "parts")]"#; /// Custom type attributes applied on top of protobuf structs @@ -132,9 +137,19 @@ pub static CUSTOM_FIELD_ATTRIBUTES: &[(&str, &str)] = &[ (".tendermint.types.CommitSig.validator_address", HEXSTRING), (".tendermint.types.CommitSig.timestamp", OPTIONAL), (".tendermint.types.CommitSig.signature", BASE64STRING), - (".tendermint.types.Vote.round", QUOTED), + ( + ".tendermint.types.DuplicateVoteEvidence.total_voting_power", + ALIAS_TOTAL_VOTING_POWER_QUOTED, + ), + ( + ".tendermint.types.DuplicateVoteEvidence.validator_power", + ALIAS_VALIDATOR_POWER_QUOTED, + ), + ( + ".tendermint.types.DuplicateVoteEvidence.timestamp", + ALIAS_TIMESTAMP, + ), (".tendermint.types.Vote.height", QUOTED), - (".tendermint.types.Vote.validator_index", QUOTED), (".tendermint.types.Vote.validator_address", HEXSTRING), (".tendermint.types.Vote.signature", BASE64STRING), (".tendermint.types.Vote.timestamp", OPTIONAL),