From 0ca16c116f850cfc401041a144969a725786706c Mon Sep 17 00:00:00 2001 From: Naohiro Yoshida Date: Tue, 10 Oct 2023 14:19:48 +0900 Subject: [PATCH] ensure use only trusted validator set Signed-off-by: Naohiro Yoshida --- light-client/src/client.rs | 4 +- light-client/src/errors.rs | 10 +- light-client/src/header/eth_headers.rs | 124 ++++++++++++++++------- light-client/src/header/mod.rs | 112 +++++++++++--------- light-client/src/header/validator_set.rs | 14 ++- 5 files changed, 174 insertions(+), 90 deletions(-) diff --git a/light-client/src/client.rs b/light-client/src/client.rs index 2011eaf..085fc7e 100644 --- a/light-client/src/client.rs +++ b/light-client/src/client.rs @@ -78,7 +78,7 @@ impl LightClient for ParliaLightClient { any_header: Any, ) -> Result { //Ensure header can be verified. - let header = Header::try_from(any_header)?; + let mut header = Header::try_from(any_header)?; let height = header.height(); let timestamp = header.timestamp()?; let trusted_height = header.trusted_height(); @@ -191,7 +191,7 @@ impl ParliaLightClient { client_id: ClientId, any_misbehaviour: Any, ) -> Result { - let misbehaviour = Misbehaviour::try_from(any_misbehaviour)?; + let mut misbehaviour = Misbehaviour::try_from(any_misbehaviour)?; let any_client_state = ctx.client_state(&client_id)?; let any_consensus_state1 = ctx.consensus_state(&client_id, &misbehaviour.header_1.trusted_height())?; diff --git a/light-client/src/errors.rs b/light-client/src/errors.rs index 5ed7212..1558471 100644 --- a/light-client/src/errors.rs +++ b/light-client/src/errors.rs @@ -70,9 +70,10 @@ pub enum Error { UnexpectedPreviousValidatorsHash(Height, Height, Hash, Hash), UnexpectedCurrentValidatorsHash(Height, Height, Hash, Hash), InvalidVerifyingHeaderLength(BlockNumber, usize), + ValidatorNotTrusted(Hash), // Vote attestation - TooManyHeaders(BlockNumber, usize), + UnexpectedTooManyHeadersToFinalize(BlockNumber, usize), UnexpectedVoteRelation(BlockNumber, usize, Option>), UnexpectedSourceInGrandChild(BlockNumber, BlockNumber, Hash, Hash), UnexpectedVoteLength(usize), @@ -262,12 +263,15 @@ impl core::fmt::Display for Error { Error::InvalidVerifyingHeaderLength(e1, e2) => { write!(f, "InvalidVerifyingHeaderLength : {} {}", e1, e2) } - Error::TooManyHeaders(e1, e2) => { - write!(f, "TooManyHeaders : {} {}", e1, e2) + Error::UnexpectedTooManyHeadersToFinalize(e1, e2) => { + write!(f, "UnexpectedTooManyHeadersToFinalize : {} {}", e1, e2) } Error::UnexpectedVoteRelation(e1, e2, e3) => { write!(f, "UnexpectedVoteRelation : {} {} {:?}", e1, e2, e3) } + Error::ValidatorNotTrusted(e1) => { + write!(f, "ValidatorNotTrusted : {:?}", e1) + } } } } diff --git a/light-client/src/header/eth_headers.rs b/light-client/src/header/eth_headers.rs index 12f65b0..7fbd31b 100644 --- a/light-client/src/header/eth_headers.rs +++ b/light-client/src/header/eth_headers.rs @@ -3,6 +3,7 @@ use alloc::vec::Vec; use parlia_ibc_proto::ibc::lightclients::parlia::v1::EthHeader; use crate::errors::Error; +use crate::header::validator_set::ValidatorSet; use crate::misc::{ChainId, Validators}; @@ -19,8 +20,8 @@ impl ETHHeaders { pub fn verify( &self, chain_id: &ChainId, - current_validators: &Validators, - previous_validators: &Validators, + current_validators: &ValidatorSet, + previous_validators: &ValidatorSet, ) -> Result<(), Error> { // Ensure all the headers are successfully chained. for (i, header) in self.all.iter().enumerate() { @@ -30,12 +31,14 @@ impl ETHHeaders { } } + let previous_validators = previous_validators.validators()?; + // Ensure valid seals let epoch = self.target.number / BLOCKS_PER_EPOCH; let checkpoint = epoch * BLOCKS_PER_EPOCH + checkpoint(previous_validators); for h in self.all.iter() { if h.number >= checkpoint { - h.verify_seal(current_validators, chain_id)?; + h.verify_seal(current_validators.validators()?, chain_id)?; } else { h.verify_seal(previous_validators, chain_id)?; } @@ -49,7 +52,7 @@ impl ETHHeaders { for h in headers_for_finalize { let vote = h.get_vote_attestation()?; if h.number > checkpoint { - vote.verify(h.number, current_validators)?; + vote.verify(h.number, current_validators.validators()?)?; } else { vote.verify(h.number, previous_validators)?; } @@ -74,7 +77,10 @@ impl ETHHeaders { Err(e) => last_error = Some(e), Ok(()) => { if i + 2 != self.all.len() - 1 { - return Err(Error::TooManyHeaders(self.target.number, self.all.len())); + return Err(Error::UnexpectedTooManyHeadersToFinalize( + self.target.number, + self.all.len(), + )); } return Ok(vec![child, grand_child]); } @@ -145,45 +151,56 @@ mod test { use crate::errors::Error; use crate::header::eth_headers::ETHHeaders; use crate::header::testdata::*; + use crate::header::validator_set::ValidatorSet; use crate::header::Header; + use crate::misc::Validators; use hex_literal::hex; use light_client::types::Any; + fn trust(mut v: ValidatorSet) -> ValidatorSet { + v.trusted = true; + v + } + + fn empty() -> ValidatorSet { + let validators: Validators = vec![]; + validators.into() + } + #[test] fn test_success_verify_before_checkpoint() { let headers = create_before_checkpoint_headers(); - let p_val = validators_in_31297000(); - headers.verify(&mainnet(), &vec![], &p_val).unwrap(); + let p_val = trust(validators_in_31297000().into()); + headers.verify(&mainnet(), &empty(), &p_val).unwrap(); } #[test] fn test_success_verify_across_checkpoint() { let headers = create_across_checkpoint_headers(); - let c_val = header_31297200().get_validator_bytes().unwrap(); - let p_val = validators_in_31297000(); + let c_val = trust(header_31297200().get_validator_bytes().unwrap().into()); + let p_val = trust(validators_in_31297000().into()); headers.verify(&mainnet(), &c_val, &p_val).unwrap(); } #[test] fn test_success_verify_after_checkpoint() { let headers = create_after_checkpoint_headers(); - let c_val = header_31297200().get_validator_bytes().unwrap(); - headers.verify(&mainnet(), &c_val, &vec![]).unwrap(); + let c_val = trust(header_31297200().get_validator_bytes().unwrap().into()); + headers.verify(&mainnet(), &c_val, &trust(empty())).unwrap(); } #[test] fn test_error_verify_before_checkpoint() { let header = create_before_checkpoint_headers(); - let mainnet = &mainnet(); - let _result = header.verify(mainnet, &vec![], &vec![]); // first block uses previous broken validator set - let mut previous_validator_set = validators_in_31297000(); - for v in previous_validator_set.iter_mut() { + let mut validators = validators_in_31297000(); + for v in validators.iter_mut() { v.remove(0); } - let result = header.verify(mainnet, &vec![], &previous_validator_set); + let previous_validator_set = trust(validators.into()); + let result = header.verify(mainnet, &empty(), &previous_validator_set); match result.unwrap_err() { Error::MissingSignerInValidator(number, _) => { assert_eq!(number, header.target.number) @@ -194,21 +211,17 @@ mod test { #[test] fn test_error_verify_across_checkpoint() { - let mut new_validator_set = header_31297200().get_validator_bytes().unwrap(); - let previous_validator_set = validators_in_31297000(); + let mut new_validators: Validators = header_31297200().get_validator_bytes().unwrap(); + for (i, v) in new_validators.iter_mut().enumerate() { + v[0] = i as u8; + } + let new_validator_set = trust(new_validators.into()); + let previous_validator_set = trust(validators_in_31297000().into()); let mainnet = &mainnet(); - // insufficient header for after checkpoint - let mut header = create_across_checkpoint_headers(); - header.all.pop(); - let _result = header.verify(mainnet, &new_validator_set, &previous_validator_set); - // last block uses new empty validator set let header = create_across_checkpoint_headers(); - for (i, v) in new_validator_set.iter_mut().enumerate() { - v[0] = i as u8; - } let result = header.verify(mainnet, &new_validator_set, &previous_validator_set); match result.unwrap_err() { Error::MissingSignerInValidator(number, _) => { @@ -223,8 +236,8 @@ mod test { fn test_error_verify_non_continuous_header() { let mut headers = create_after_checkpoint_headers(); headers.all[1] = headers.all[0].clone(); - let c_val = header_31297200().get_validator_bytes().unwrap(); - let result = headers.verify(&mainnet(), &c_val, &vec![]); + let c_val = trust(header_31297200().get_validator_bytes().unwrap().into()); + let result = headers.verify(&mainnet(), &c_val, &trust(empty())); match result.unwrap_err() { Error::UnexpectedHeaderRelation(e1, e2) => { assert_eq!(e1, headers.target.number); @@ -235,13 +248,13 @@ mod test { } #[test] - fn test_error_verify_too_many_headers() { + fn test_error_verify_too_many_headers_to_finalize() { let mut headers = create_after_checkpoint_headers(); headers.all.push(header_31297214()); - let c_val = header_31297200().get_validator_bytes().unwrap(); - let result = headers.verify(&mainnet(), &c_val, &vec![]); + let c_val = trust(header_31297200().get_validator_bytes().unwrap().into()); + let result = headers.verify(&mainnet(), &c_val, &trust(empty())); match result.unwrap_err() { - Error::TooManyHeaders(e1, e2) => { + Error::UnexpectedTooManyHeadersToFinalize(e1, e2) => { assert_eq!(e1, headers.target.number, "block error"); assert_eq!(e2, headers.all.len(), "header size"); } @@ -253,8 +266,8 @@ mod test { fn test_error_verify_invalid_header_size() { let mut headers = create_after_checkpoint_headers(); headers.all.pop(); - let c_val = header_31297200().get_validator_bytes().unwrap(); - let result = headers.verify(&mainnet(), &c_val, &vec![]); + let c_val = trust(header_31297200().get_validator_bytes().unwrap().into()); + let result = headers.verify(&mainnet(), &c_val, &trust(empty())); match result.unwrap_err() { Error::InvalidVerifyingHeaderLength(e1, e2) => { assert_eq!(e1, headers.target.number, "block error"); @@ -289,6 +302,49 @@ mod test { } } + #[test] + fn test_error_verify_too_many_headers_to_seal() { + let v = vec![ + header_31297200(), + header_31297201(), + header_31297202(), + header_31297203(), + header_31297204(), + header_31297205(), + header_31297206(), + header_31297207(), + header_31297208(), + header_31297209(), + header_31297210(), + ]; + let mut headers = ETHHeaders { + target: v[0].clone(), + all: v, + }; + + // success ( untrusted validator not used ) + let p_val = trust(validators_in_31297000().into()); + let untrusted_c_val = validators_in_31297000().into(); + let result = headers.verify(&mainnet(), &untrusted_c_val, &p_val); + match result.unwrap_err() { + Error::UnexpectedTooManyHeadersToFinalize(e1, e2) => { + assert_eq!(e1, headers.target.number, "block error"); + assert_eq!(e2, headers.all.len(), "header size"); + } + e => unreachable!("{:?}", e), + } + + // error + headers.all.push(header_31297211()); + let result = headers.verify(&mainnet(), &untrusted_c_val, &p_val); + match result.unwrap_err() { + Error::ValidatorNotTrusted(e1) => { + assert_eq!(e1, untrusted_c_val.hash); + } + e => unreachable!("{:?}", e), + } + } + fn create_before_checkpoint_headers() -> ETHHeaders { let v = vec![header_31297208(), header_31297209(), header_31297210()]; ETHHeaders { diff --git a/light-client/src/header/mod.rs b/light-client/src/header/mod.rs index 847822a..fa05dd3 100644 --- a/light-client/src/header/mod.rs +++ b/light-client/src/header/mod.rs @@ -70,8 +70,8 @@ impl Header { pub fn verify(&self, chain_id: &ChainId) -> Result<(), Error> { self.headers.verify( chain_id, - &self.current_validators.validators, - &self.previous_validators.validators, + &self.current_validators, + &self.previous_validators, ) } @@ -79,14 +79,14 @@ impl Header { &self.headers.target.hash } - pub fn verify_validator_set(&self, consensus_state: &ConsensusState) -> Result<(), Error> { + pub fn verify_validator_set(&mut self, consensus_state: &ConsensusState) -> Result<(), Error> { verify_validator_set( consensus_state, self.headers.target.is_epoch(), self.height(), self.trusted_height, - self.previous_validators_hash(), - self.current_validators_hash(), + &mut self.previous_validators, + &mut self.current_validators, ) } } @@ -96,8 +96,8 @@ fn verify_validator_set( is_epoch: bool, height: Height, trusted_height: Height, - previous_validators_hash: Hash, - current_validators_hash: Hash, + previous_validators: &mut ValidatorSet, + current_validators: &mut ValidatorSet, ) -> Result<(), Error> { let header_epoch = height.revision_height() / BLOCKS_PER_EPOCH; let trusted_epoch = trusted_height.revision_height() / BLOCKS_PER_EPOCH; @@ -109,11 +109,13 @@ fn verify_validator_set( height.revision_height(), )); } - if previous_validators_hash != consensus_state.current_validators_hash { + previous_validators.trusted = + previous_validators.hash == consensus_state.current_validators_hash; + if !previous_validators.trusted { return Err(Error::UnexpectedPreviousValidatorsHash( trusted_height, height, - previous_validators_hash, + previous_validators.hash, consensus_state.current_validators_hash, )); } @@ -124,19 +126,23 @@ fn verify_validator_set( height.revision_height(), )); } - if previous_validators_hash != consensus_state.previous_validators_hash { + previous_validators.trusted = + previous_validators.hash == consensus_state.previous_validators_hash; + if !previous_validators.trusted { return Err(Error::UnexpectedPreviousValidatorsHash( trusted_height, height, - previous_validators_hash, + previous_validators.hash, consensus_state.previous_validators_hash, )); } - if current_validators_hash != consensus_state.current_validators_hash { + current_validators.trusted = + current_validators.hash == consensus_state.current_validators_hash; + if !current_validators.trusted { return Err(Error::UnexpectedCurrentValidatorsHash( trusted_height, height, - current_validators_hash, + current_validators.hash, consensus_state.current_validators_hash, )); } @@ -167,24 +173,22 @@ impl TryFrom for Header { )); } - let previous_validators: ValidatorSet = value.previous_validators.into(); - if previous_validators.validators.is_empty() { + if value.previous_validators.is_empty() { return Err(Error::MissingPreviousTrustedValidators( headers.target.number, )); } // Epoch header contains validator set - let current_validators: ValidatorSet = if headers.target.is_epoch() { + let current_validators = if headers.target.is_epoch() { headers .target .get_validator_bytes() .ok_or_else(|| Error::MissingValidatorInEpochBlock(headers.target.number))? - .into() } else { - value.current_validators.into() + value.current_validators }; - if current_validators.validators.is_empty() { + if current_validators.is_empty() { return Err(Error::MissingCurrentTrustedValidators( headers.target.number, )); @@ -194,8 +198,8 @@ impl TryFrom for Header { account_proof: value.account_proof, headers, trusted_height, - previous_validators, - current_validators, + previous_validators: value.previous_validators.into(), + current_validators: current_validators.into(), }) } } @@ -227,10 +231,18 @@ mod testdata; mod test { use crate::consensus_state::ConsensusState; use crate::errors::Error; + use crate::header::validator_set::ValidatorSet; use crate::header::verify_validator_set; - use crate::misc::new_height; + use crate::misc::{new_height, Hash, Validators}; use light_client::types::Time; + fn to_validator_set(h: Hash) -> ValidatorSet { + let validators: Validators = vec![]; + let mut v: ValidatorSet = validators.into(); + v.hash = h; + v + } + #[test] fn test_success_verify_validator_set() { let cs = ConsensusState { @@ -242,29 +254,29 @@ mod test { let height = new_height(0, 400); let trusted_height = new_height(0, 201); - let current_validators_hash = [3u8; 32]; - let previous_validators_hash = cs.current_validators_hash; + let current_validators = &mut to_validator_set([3u8; 32]); + let previous_validators = &mut to_validator_set(cs.current_validators_hash); verify_validator_set( &cs, true, height, trusted_height, - previous_validators_hash, - current_validators_hash, + previous_validators, + current_validators, ) .unwrap(); let height = new_height(0, 599); let trusted_height = new_height(0, 400); - let current_validators_hash = cs.current_validators_hash; - let previous_validators_hash = cs.previous_validators_hash; + let current_validators = &mut to_validator_set(cs.current_validators_hash); + let previous_validators = &mut to_validator_set(cs.previous_validators_hash); verify_validator_set( &cs, false, height, trusted_height, - previous_validators_hash, - current_validators_hash, + previous_validators, + current_validators, ) .unwrap(); } @@ -280,15 +292,15 @@ mod test { let height = new_height(0, 400); let trusted_height = new_height(0, 199); - let current_validators_hash = [1u8; 32]; - let previous_validators_hash = [2u8; 32]; + let current_validators = &mut to_validator_set([1u8; 32]); + let previous_validators = &mut to_validator_set([2u8; 32]); let err = verify_validator_set( &cs, true, height, trusted_height, - previous_validators_hash, - current_validators_hash, + previous_validators, + current_validators, ) .unwrap_err(); match err { @@ -300,21 +312,21 @@ mod test { } let trusted_height = new_height(0, 200); - let previous_validators_hash = [3u8; 32]; + let previous_validators = &mut to_validator_set([3u8; 32]); let err = verify_validator_set( &cs, true, height, trusted_height, - previous_validators_hash, - current_validators_hash, + previous_validators, + current_validators, ) .unwrap_err(); match err { Error::UnexpectedPreviousValidatorsHash(t, h, hash, cons_hash) => { assert_eq!(t, trusted_height); assert_eq!(h, height); - assert_eq!(hash, previous_validators_hash); + assert_eq!(hash, previous_validators.hash); assert_eq!(cons_hash, cons_hash); } _ => unreachable!("err {:?}", err), @@ -327,8 +339,8 @@ mod test { false, height, trusted_height, - previous_validators_hash, - current_validators_hash, + previous_validators, + current_validators, ) .unwrap_err(); match err { @@ -340,44 +352,44 @@ mod test { } let trusted_height = new_height(0, 400); - let current_validators_hash = [1u8; 32]; - let previous_validators_hash = [3u8; 32]; + let current_validators = &mut to_validator_set([1u8; 32]); + let previous_validators = &mut to_validator_set([3u8; 32]); let err = verify_validator_set( &cs, false, height, trusted_height, - previous_validators_hash, - current_validators_hash, + previous_validators, + current_validators, ) .unwrap_err(); match err { Error::UnexpectedPreviousValidatorsHash(t, h, hash, cons_hash) => { assert_eq!(t, trusted_height); assert_eq!(h, height); - assert_eq!(hash, previous_validators_hash); + assert_eq!(hash, previous_validators.hash); assert_eq!(cons_hash, cons_hash); } _ => unreachable!("err {:?}", err), } let trusted_height = new_height(0, 400); - let current_validators_hash = [3u8; 32]; - let previous_validators_hash = [2u8; 32]; + let current_validators = &mut to_validator_set([3u8; 32]); + let previous_validators = &mut to_validator_set([2u8; 32]); let err = verify_validator_set( &cs, false, height, trusted_height, - previous_validators_hash, - current_validators_hash, + previous_validators, + current_validators, ) .unwrap_err(); match err { Error::UnexpectedCurrentValidatorsHash(t, h, hash, cons_hash) => { assert_eq!(t, trusted_height); assert_eq!(h, height); - assert_eq!(hash, current_validators_hash); + assert_eq!(hash, current_validators.hash); assert_eq!(cons_hash, cons_hash); } _ => unreachable!("err {:?}", err), diff --git a/light-client/src/header/validator_set.rs b/light-client/src/header/validator_set.rs index 73867b6..cd9f31f 100644 --- a/light-client/src/header/validator_set.rs +++ b/light-client/src/header/validator_set.rs @@ -1,11 +1,22 @@ +use crate::errors::Error; use alloc::vec::Vec; use crate::misc::{keccak_256_vec, Hash, Validators}; #[derive(Clone, Debug, PartialEq, serde::Serialize, serde::Deserialize)] pub struct ValidatorSet { - pub validators: Validators, + validators: Validators, pub hash: Hash, + pub trusted: bool, +} + +impl ValidatorSet { + pub fn validators(&self) -> Result<&Validators, Error> { + if !self.trusted { + return Err(Error::ValidatorNotTrusted(self.hash)); + } + Ok(&self.validators) + } } impl From>> for ValidatorSet { @@ -14,6 +25,7 @@ impl From>> for ValidatorSet { Self { validators: value as Validators, hash, + trusted: false, } } }