From a8211b7a8489ae04fea158b74a106d79041041e6 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Thu, 10 Nov 2022 17:06:38 +0100 Subject: [PATCH 01/16] Implement verify_light() and verify_light_trusting() --- .../src/operations/voting_power.rs | 17 ++++- light-client-verifier/src/predicates.rs | 17 +++++ light-client-verifier/src/verifier.rs | 67 ++++++++++++++++++- 3 files changed, 98 insertions(+), 3 deletions(-) diff --git a/light-client-verifier/src/operations/voting_power.rs b/light-client-verifier/src/operations/voting_power.rs index 58dbeec65..d47303e89 100644 --- a/light-client-verifier/src/operations/voting_power.rs +++ b/light-client-verifier/src/operations/voting_power.rs @@ -69,12 +69,12 @@ pub trait VotingPowerCalculator: Send + Sync { /// Check against the given threshold that there is enough signers /// overlap between an untrusted header and untrusted validator set - fn check_signers_overlap( + fn check_enough_signers_overlap( &self, untrusted_header: &SignedHeader, untrusted_validators: &ValidatorSet, + trust_threshold: TrustThreshold, ) -> Result<(), VerificationError> { - let trust_threshold = TrustThreshold::TWO_THIRDS; let voting_power = self.voting_power_in(untrusted_header, untrusted_validators, trust_threshold)?; @@ -87,6 +87,19 @@ pub trait VotingPowerCalculator: Send + Sync { } } + /// Check if there is 2/3rd overlap between an untrusted header and untrusted validator set + fn check_signers_overlap( + &self, + untrusted_header: &SignedHeader, + untrusted_validators: &ValidatorSet, + ) -> Result<(), VerificationError> { + self.check_signers_overlap_level( + untrusted_header, + untrusted_validators, + TrustThreshold::TWO_THIRDS, + ) + } + /// Compute the voting power in a header and its commit against a validator set. /// /// The `trust_threshold` is currently not used, but might be in the future diff --git a/light-client-verifier/src/predicates.rs b/light-client-verifier/src/predicates.rs index 0c8787d4f..f347aa376 100644 --- a/light-client-verifier/src/predicates.rs +++ b/light-client-verifier/src/predicates.rs @@ -185,6 +185,23 @@ pub trait VerificationPredicates: Send + Sync { Ok(()) } + /// Check that there is enough signers overlap between the given, untrusted validator set + /// and the untrusted signed header based on the specified trust threshold. + fn has_specified_signers_overlap( + &self, + untrusted_sh: &SignedHeader, + untrusted_validators: &ValidatorSet, + calculator: &dyn VotingPowerCalculator, + trust_threshold: TrustThreshold, + ) -> Result<(), VerificationError> { + calculator.check_signers_overlap_level( + untrusted_sh, + untrusted_validators, + trust_threshold, + )?; + Ok(()) + } + /// Check that the hash of the next validator set in the trusted block matches /// the hash of the validator set in the untrusted one. fn valid_next_validator_set( diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index 8525cf666..e11c6d2a5 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -11,7 +11,7 @@ use crate::{ }, options::Options, predicates as preds, - types::{Time, TrustedBlockState, UntrustedBlockState}, + types::{Time, TrustThreshold, TrustedBlockState, UntrustedBlockState}, }; /// Represents the result of the verification performed by the @@ -57,6 +57,16 @@ pub trait Verifier: Send + Sync { options: &Options, now: Time, ) -> Verdict; + + /// Perform light verification. + fn verify_light(&self, untrusted: UntrustedBlockState<'_>) -> Verdict; + + /// Perform light verification with specified trust level. + fn verify_light_trusting( + &self, + untrusted: UntrustedBlockState<'_>, + trust_threshold: TrustThreshold, + ) -> Verdict; } macro_rules! verdict { @@ -226,6 +236,61 @@ where Verdict::Success } + + fn verify_light(&self, untrusted: UntrustedBlockState<'_>) -> Verdict { + // Ensure the header matches the commit + verdict!(self.predicates.header_matches_commit( + &untrusted.signed_header.header, + untrusted.signed_header.commit.block_id.hash, + &self.hasher, + )); + + // Additional implementation specific validation + verdict!(self.predicates.valid_commit( + untrusted.signed_header, + untrusted.validators, + &self.commit_validator, + )); + + // Verify that more than 2/3 of the validators correctly committed the block. + verdict!(self.predicates.has_sufficient_signers_overlap( + untrusted.signed_header, + untrusted.validators, + &self.voting_power_calculator, + )); + + Verdict::Success + } + + fn verify_light_trusting( + &self, + untrusted: UntrustedBlockState<'_>, + trust_threshold: TrustThreshold, + ) -> Verdict { + // Ensure the header matches the commit + verdict!(self.predicates.header_matches_commit( + &untrusted.signed_header.header, + untrusted.signed_header.commit.block_id.hash, + &self.hasher, + )); + + // Additional implementation specific validation + verdict!(self.predicates.valid_commit( + untrusted.signed_header, + untrusted.validators, + &self.commit_validator, + )); + + // Verify that more than 2/3 of the validators correctly committed the block. + verdict!(self.predicates.has_specified_signers_overlap( + untrusted.signed_header, + untrusted.validators, + &self.voting_power_calculator, + trust_threshold + )); + + Verdict::Success + } } /// The default production implementation of the [`PredicateVerifier`]. From 01c5112f3625905bb00db13c030254532410adf4 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Tue, 15 Nov 2022 20:13:39 +0900 Subject: [PATCH 02/16] Rename `check_signers_overlap_level()` -> `check_enough_signers_overlap` --- light-client-verifier/src/operations/voting_power.rs | 2 +- light-client-verifier/src/predicates.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/light-client-verifier/src/operations/voting_power.rs b/light-client-verifier/src/operations/voting_power.rs index d47303e89..908965be0 100644 --- a/light-client-verifier/src/operations/voting_power.rs +++ b/light-client-verifier/src/operations/voting_power.rs @@ -93,7 +93,7 @@ pub trait VotingPowerCalculator: Send + Sync { untrusted_header: &SignedHeader, untrusted_validators: &ValidatorSet, ) -> Result<(), VerificationError> { - self.check_signers_overlap_level( + self.check_enough_signers_overlap( untrusted_header, untrusted_validators, TrustThreshold::TWO_THIRDS, diff --git a/light-client-verifier/src/predicates.rs b/light-client-verifier/src/predicates.rs index f347aa376..f3969dee0 100644 --- a/light-client-verifier/src/predicates.rs +++ b/light-client-verifier/src/predicates.rs @@ -194,7 +194,7 @@ pub trait VerificationPredicates: Send + Sync { calculator: &dyn VotingPowerCalculator, trust_threshold: TrustThreshold, ) -> Result<(), VerificationError> { - calculator.check_signers_overlap_level( + calculator.check_enough_signers_overlap( untrusted_sh, untrusted_validators, trust_threshold, From bd89492eeddbe170f17fcdbdb31c3615c1a38a4e Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Tue, 15 Nov 2022 23:28:15 +0530 Subject: [PATCH 03/16] Add unit test for `ProdPredicates::has_specified_signers_overlap()` --- light-client-verifier/src/predicates.rs | 68 +++++++++++++++++++++++++ testgen/src/light_block.rs | 18 +++++++ 2 files changed, 86 insertions(+) diff --git a/light-client-verifier/src/predicates.rs b/light-client-verifier/src/predicates.rs index f3969dee0..2dc790835 100644 --- a/light-client-verifier/src/predicates.rs +++ b/light-client-verifier/src/predicates.rs @@ -712,4 +712,72 @@ mod tests { _ => panic!("expected InsufficientSignersOverlap error"), } } + + #[test] + fn test_has_specified_signers_overlap() { + let validators = [ + Validator::new("1").voting_power(25), + Validator::new("2").voting_power(25), + Validator::new("3").voting_power(25), + Validator::new("4").voting_power(25), + ]; + + let mut light_block: LightBlock = + TestgenLightBlock::new_default_with_validators(2, &validators) + .generate() + .unwrap() + .into(); + + let vp = ProdPredicates::default(); + let voting_power_calculator = ProdVotingPowerCalculator::default(); + + // Test scenarios --> + // 1. >3/4 validators sign + let result_ok = vp.has_specified_signers_overlap( + &light_block.signed_header, + &light_block.validators, + &voting_power_calculator, + TrustThreshold::new(3, 4).unwrap(), + ); + + assert!(result_ok.is_ok()); + + // 2. >2/3 validators sign + light_block.signed_header.commit.signatures.pop(); + + let result_ok = vp.has_specified_signers_overlap( + &light_block.signed_header, + &light_block.validators, + &voting_power_calculator, + TrustThreshold::TWO_THIRDS, + ); + + assert!(result_ok.is_ok()); + + // 3. <1/3 validators sign (i.e. 1/4) + light_block.signed_header.commit.signatures.truncate(1); + + let result_err = vp.has_specified_signers_overlap( + &light_block.signed_header, + &light_block.validators, + &voting_power_calculator, + TrustThreshold::ONE_THIRD, + ); + + let trust_threshold = TrustThreshold::ONE_THIRD; + + match result_err { + Err(VerificationError(VerificationErrorDetail::InsufficientSignersOverlap(e), _)) => { + assert_eq!( + e.tally, + VotingPowerTally { + total: 100, + tallied: 25, + trust_threshold, + } + ); + }, + _ => panic!("expected InsufficientSignersOverlap error"), + } + } } diff --git a/testgen/src/light_block.rs b/testgen/src/light_block.rs index c6cea5ab7..174a4d685 100644 --- a/testgen/src/light_block.rs +++ b/testgen/src/light_block.rs @@ -106,6 +106,24 @@ impl LightBlock { } } + pub fn new_default_with_validators(height: u64, validators: &[Validator]) -> Self { + let header = Header::new(&validators) + .height(height) + .chain_id("test-chain") + .next_validators(&validators) + .time(Time::from_unix_timestamp(height as i64, 0).unwrap()); // just wanted to initialize time with some value + + let commit = Commit::new(header.clone(), 1); + + Self { + header: Some(header), + commit: Some(commit), + validators: Some(validators.to_vec()), + next_validators: Some(validators.to_vec()), + provider: Some(default_peer_id()), + } + } + pub fn new_default_with_time_and_chain_id(chain_id: String, time: Time, height: u64) -> Self { let validators = [ Validator::new("1").voting_power(50), From 1f4d6dcc26fc0c1c6a15da721d868433d1acbc36 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Tue, 15 Nov 2022 23:28:41 +0530 Subject: [PATCH 04/16] Add MBT test for light verifier methods --- light-client/src/tests.rs | 39 ++++++++++++++++++++++++++++----------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/light-client/src/tests.rs b/light-client/src/tests.rs index eb64a13b6..de3b3d56f 100644 --- a/light-client/src/tests.rs +++ b/light-client/src/tests.rs @@ -152,25 +152,42 @@ pub fn verify_single( clock_drift: Duration, now: Time, ) -> Result { + trait IntoResult { + fn into_result(self, ok: T) -> Result; + } + + impl IntoResult for Verdict { + fn into_result(self, ok: T) -> Result { + match self { + Verdict::Success => Ok(ok), + error => Err(error), + } + } + } + let verifier = ProdVerifier::default(); + verifier + .verify_light(input.as_untrusted_state()) + .into_result(())?; + verifier + .verify_light_trusting(input.as_untrusted_state(), trust_threshold) + .into_result(())?; + let options = Options { trust_threshold, trusting_period, clock_drift, }; - let result = verifier.verify( - input.as_untrusted_state(), - trusted_block.as_trusted_state(), - &options, - now, - ); - - match result { - Verdict::Success => Ok(input), - error => Err(error), - } + verifier + .verify( + input.as_untrusted_state(), + trusted_block.as_trusted_state(), + &options, + now, + ) + .into_result(input) } pub fn verify_bisection( From 22690edf8f3be62ba8c5f48b18a482be0d980f72 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Tue, 15 Nov 2022 23:43:52 +0530 Subject: [PATCH 05/16] Clippy fix --- testgen/src/light_block.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testgen/src/light_block.rs b/testgen/src/light_block.rs index 174a4d685..27763e987 100644 --- a/testgen/src/light_block.rs +++ b/testgen/src/light_block.rs @@ -107,10 +107,10 @@ impl LightBlock { } pub fn new_default_with_validators(height: u64, validators: &[Validator]) -> Self { - let header = Header::new(&validators) + let header = Header::new(validators) .height(height) .chain_id("test-chain") - .next_validators(&validators) + .next_validators(validators) .time(Time::from_unix_timestamp(height as i64, 0).unwrap()); // just wanted to initialize time with some value let commit = Commit::new(header.clone(), 1); From 5fccfb6fe44130953a75e5c66df084410a4ba61e Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Sat, 19 Nov 2022 00:23:15 +0530 Subject: [PATCH 06/16] Split verify into 4 component methods --- light-client-verifier/src/verifier.rs | 120 ++++++++++++++++++++------ light-client/src/tests.rs | 22 ++++- 2 files changed, 114 insertions(+), 28 deletions(-) diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index e11c6d2a5..766ca05e2 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -11,7 +11,7 @@ use crate::{ }, options::Options, predicates as preds, - types::{Time, TrustThreshold, TrustedBlockState, UntrustedBlockState}, + types::{Time, TrustedBlockState, UntrustedBlockState}, }; /// Represents the result of the verification performed by the @@ -58,14 +58,23 @@ pub trait Verifier: Send + Sync { now: Time, ) -> Verdict; - /// Perform light verification. - fn verify_light(&self, untrusted: UntrustedBlockState<'_>) -> Verdict; + fn validate_untrusted(&self, untrusted: UntrustedBlockState<'_>) -> Verdict; - /// Perform light verification with specified trust level. - fn verify_light_trusting( + fn verify_commit(&self, untrusted: UntrustedBlockState<'_>) -> Verdict; + + fn validate_trusting( &self, untrusted: UntrustedBlockState<'_>, - trust_threshold: TrustThreshold, + trusted: TrustedBlockState<'_>, + options: &Options, + now: Time, + ) -> Verdict; + + fn verify_commit_trusting( + &self, + untrusted: UntrustedBlockState<'_>, + trusted: TrustedBlockState<'_>, + options: &Options, ) -> Verdict; } @@ -237,7 +246,24 @@ where Verdict::Success } - fn verify_light(&self, untrusted: UntrustedBlockState<'_>) -> Verdict { + fn validate_untrusted(&self, untrusted: UntrustedBlockState<'_>) -> Verdict { + // Ensure the header validator hashes match the given validators + verdict!(self.predicates.validator_sets_match( + untrusted.validators, + untrusted.signed_header.header.validators_hash, + &self.hasher, + )); + + // TODO(thane): Is this check necessary for IBC? + if let Some(untrusted_next_validators) = untrusted.next_validators { + // Ensure the header next validator hashes match the given next validators + verdict!(self.predicates.next_validators_match( + untrusted_next_validators, + untrusted.signed_header.header.next_validators_hash, + &self.hasher, + )); + } + // Ensure the header matches the commit verdict!(self.predicates.header_matches_commit( &untrusted.signed_header.header, @@ -252,6 +278,10 @@ where &self.commit_validator, )); + Verdict::Success + } + + fn verify_commit(&self, untrusted: UntrustedBlockState<'_>) -> Verdict { // Verify that more than 2/3 of the validators correctly committed the block. verdict!(self.predicates.has_sufficient_signers_overlap( untrusted.signed_header, @@ -262,32 +292,70 @@ where Verdict::Success } - fn verify_light_trusting( + fn validate_trusting( &self, untrusted: UntrustedBlockState<'_>, - trust_threshold: TrustThreshold, + trusted: TrustedBlockState<'_>, + options: &Options, + now: Time, ) -> Verdict { - // Ensure the header matches the commit - verdict!(self.predicates.header_matches_commit( - &untrusted.signed_header.header, - untrusted.signed_header.commit.block_id.hash, - &self.hasher, + // Ensure the latest trusted header hasn't expired + verdict!(self.predicates.is_within_trust_period( + trusted.header_time, + options.trusting_period, + now, )); - // Additional implementation specific validation - verdict!(self.predicates.valid_commit( - untrusted.signed_header, - untrusted.validators, - &self.commit_validator, + // Ensure the header isn't from a future time + verdict!(self.predicates.is_header_from_past( + untrusted.signed_header.header.time, + options.clock_drift, + now, )); - // Verify that more than 2/3 of the validators correctly committed the block. - verdict!(self.predicates.has_specified_signers_overlap( - untrusted.signed_header, - untrusted.validators, - &self.voting_power_calculator, - trust_threshold - )); + // Check that the untrusted block is more recent than the trusted state + verdict!(self + .predicates + .is_monotonic_bft_time(untrusted.signed_header.header.time, trusted.header_time,)); + + let trusted_next_height = trusted.height.increment(); + + if untrusted.height() == trusted_next_height { + // If the untrusted block is the very next block after the trusted block, + // check that their (next) validator sets hashes match. + verdict!(self.predicates.valid_next_validator_set( + untrusted.signed_header.header.validators_hash, + trusted.next_validators_hash, + )); + } else { + // Otherwise, ensure that the untrusted block has a greater height than + // the trusted block. + verdict!(self + .predicates + .is_monotonic_height(untrusted.signed_header.header.height, trusted.height)); + } + + Verdict::Success + } + + fn verify_commit_trusting( + &self, + untrusted: UntrustedBlockState<'_>, + trusted: TrustedBlockState<'_>, + options: &Options, + ) -> Verdict { + let trusted_next_height = trusted.height.increment(); + + if untrusted.height() != trusted_next_height { + // Check there is enough overlap between the validator sets of + // the trusted and untrusted blocks. + verdict!(self.predicates.has_sufficient_validators_overlap( + untrusted.signed_header, + trusted.next_validators, + &options.trust_threshold, + &self.voting_power_calculator, + )); + } Verdict::Success } diff --git a/light-client/src/tests.rs b/light-client/src/tests.rs index de3b3d56f..39bd034b9 100644 --- a/light-client/src/tests.rs +++ b/light-client/src/tests.rs @@ -168,10 +168,11 @@ pub fn verify_single( let verifier = ProdVerifier::default(); verifier - .verify_light(input.as_untrusted_state()) + .validate_untrusted(input.as_untrusted_state()) .into_result(())?; + verifier - .verify_light_trusting(input.as_untrusted_state(), trust_threshold) + .verify_commit(input.as_untrusted_state()) .into_result(())?; let options = Options { @@ -180,6 +181,23 @@ pub fn verify_single( clock_drift, }; + verifier + .validate_trusting( + input.as_untrusted_state(), + trusted_block.as_trusted_state(), + &options, + now, + ) + .into_result(())?; + + verifier + .verify_commit_trusting( + input.as_untrusted_state(), + trusted_block.as_trusted_state(), + &options, + ) + .into_result(())?; + verifier .verify( input.as_untrusted_state(), From 1fd3407130a645dfbc568eaa2401d34fd1f2f126 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Sat, 19 Nov 2022 00:52:06 +0530 Subject: [PATCH 07/16] Rewrite ProdVerifier::verify() using component methods --- light-client-verifier/src/verifier.rs | 253 +++++++++----------------- light-client/src/tests.rs | 26 --- 2 files changed, 82 insertions(+), 197 deletions(-) diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index 766ca05e2..8d918e0c2 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -57,25 +57,6 @@ pub trait Verifier: Send + Sync { options: &Options, now: Time, ) -> Verdict; - - fn validate_untrusted(&self, untrusted: UntrustedBlockState<'_>) -> Verdict; - - fn verify_commit(&self, untrusted: UntrustedBlockState<'_>) -> Verdict; - - fn validate_trusting( - &self, - untrusted: UntrustedBlockState<'_>, - trusted: TrustedBlockState<'_>, - options: &Options, - now: Time, - ) -> Verdict; - - fn verify_commit_trusting( - &self, - untrusted: UntrustedBlockState<'_>, - trusted: TrustedBlockState<'_>, - options: &Options, - ) -> Verdict; } macro_rules! verdict { @@ -130,233 +111,163 @@ where hasher, } } -} -impl Verifier for PredicateVerifier -where - P: VerificationPredicates, - C: VotingPowerCalculator, - V: CommitValidator, - H: Hasher, -{ - /// Validate the given light block state. - /// - /// - Ensure the latest trusted header hasn't expired - /// - Ensure the header validator hashes match the given validators - /// - Ensure the header next validator hashes match the given next validators - /// - Additional implementation specific validation via `commit_validator` - /// - Check that the untrusted block is more recent than the trusted state - /// - If the untrusted block is the very next block after the trusted block, check that their - /// (next) validator sets hashes match. - /// - Otherwise, ensure that the untrusted block has a greater height than the trusted block. - /// - /// **NOTE**: If the untrusted state's `next_validators` field is `None`, - /// this will not (and will not be able to) check whether the untrusted - /// state's `next_validators_hash` field is valid. - fn verify( + /// Validates an `UntrustedBlockState`. + pub fn validate_untrusted( &self, - untrusted: UntrustedBlockState<'_>, - trusted: TrustedBlockState<'_>, - options: &Options, - now: Time, - ) -> Verdict { - // Ensure the latest trusted header hasn't expired - verdict!(self.predicates.is_within_trust_period( - trusted.header_time, - options.trusting_period, - now, - )); - - // Ensure the header isn't from a future time - verdict!(self.predicates.is_header_from_past( - untrusted.signed_header.header.time, - options.clock_drift, - now, - )); - + untrusted: &UntrustedBlockState<'_>, + ) -> Result<(), VerificationError> { // Ensure the header validator hashes match the given validators - verdict!(self.predicates.validator_sets_match( + self.predicates.validator_sets_match( untrusted.validators, untrusted.signed_header.header.validators_hash, &self.hasher, - )); + )?; // TODO(thane): Is this check necessary for IBC? if let Some(untrusted_next_validators) = untrusted.next_validators { // Ensure the header next validator hashes match the given next validators - verdict!(self.predicates.next_validators_match( + self.predicates.next_validators_match( untrusted_next_validators, untrusted.signed_header.header.next_validators_hash, &self.hasher, - )); + )?; } // Ensure the header matches the commit - verdict!(self.predicates.header_matches_commit( + self.predicates.header_matches_commit( &untrusted.signed_header.header, untrusted.signed_header.commit.block_id.hash, &self.hasher, - )); + )?; // Additional implementation specific validation - verdict!(self.predicates.valid_commit( + self.predicates.valid_commit( untrusted.signed_header, untrusted.validators, &self.commit_validator, - )); - - // Check that the untrusted block is more recent than the trusted state - verdict!(self - .predicates - .is_monotonic_bft_time(untrusted.signed_header.header.time, trusted.header_time,)); + )?; - let trusted_next_height = trusted.height.increment(); - - if untrusted.height() == trusted_next_height { - // If the untrusted block is the very next block after the trusted block, - // check that their (next) validator sets hashes match. - verdict!(self.predicates.valid_next_validator_set( - untrusted.signed_header.header.validators_hash, - trusted.next_validators_hash, - )); - } else { - // Otherwise, ensure that the untrusted block has a greater height than - // the trusted block. - verdict!(self - .predicates - .is_monotonic_height(untrusted.signed_header.header.height, trusted.height)); - - // Check there is enough overlap between the validator sets of - // the trusted and untrusted blocks. - verdict!(self.predicates.has_sufficient_validators_overlap( - untrusted.signed_header, - trusted.next_validators, - &options.trust_threshold, - &self.voting_power_calculator, - )); - } - - // Verify that more than 2/3 of the validators correctly committed the block. - verdict!(self.predicates.has_sufficient_signers_overlap( - untrusted.signed_header, - untrusted.validators, - &self.voting_power_calculator, - )); - - Verdict::Success - } - - fn validate_untrusted(&self, untrusted: UntrustedBlockState<'_>) -> Verdict { - // Ensure the header validator hashes match the given validators - verdict!(self.predicates.validator_sets_match( - untrusted.validators, - untrusted.signed_header.header.validators_hash, - &self.hasher, - )); - - // TODO(thane): Is this check necessary for IBC? - if let Some(untrusted_next_validators) = untrusted.next_validators { - // Ensure the header next validator hashes match the given next validators - verdict!(self.predicates.next_validators_match( - untrusted_next_validators, - untrusted.signed_header.header.next_validators_hash, - &self.hasher, - )); - } - - // Ensure the header matches the commit - verdict!(self.predicates.header_matches_commit( - &untrusted.signed_header.header, - untrusted.signed_header.commit.block_id.hash, - &self.hasher, - )); - - // Additional implementation specific validation - verdict!(self.predicates.valid_commit( - untrusted.signed_header, - untrusted.validators, - &self.commit_validator, - )); - - Verdict::Success + Ok(()) } - fn verify_commit(&self, untrusted: UntrustedBlockState<'_>) -> Verdict { - // Verify that more than 2/3 of the validators correctly committed the block. - verdict!(self.predicates.has_sufficient_signers_overlap( + /// Verify that more than 2/3 of the validators correctly committed the block. + pub fn verify_commit( + &self, + untrusted: &UntrustedBlockState<'_>, + ) -> Result<(), VerificationError> { + self.predicates.has_sufficient_signers_overlap( untrusted.signed_header, untrusted.validators, &self.voting_power_calculator, - )); + )?; - Verdict::Success + Ok(()) } - fn validate_trusting( + /// Validate a `UntrustedBlockState`, based on the given `TrustedBlockState`, `Options` and + /// current time. + pub fn validate_trusting( &self, - untrusted: UntrustedBlockState<'_>, - trusted: TrustedBlockState<'_>, + untrusted: &UntrustedBlockState<'_>, + trusted: &TrustedBlockState<'_>, options: &Options, now: Time, - ) -> Verdict { + ) -> Result<(), VerificationError> { // Ensure the latest trusted header hasn't expired - verdict!(self.predicates.is_within_trust_period( + self.predicates.is_within_trust_period( trusted.header_time, options.trusting_period, now, - )); + )?; // Ensure the header isn't from a future time - verdict!(self.predicates.is_header_from_past( + self.predicates.is_header_from_past( untrusted.signed_header.header.time, options.clock_drift, now, - )); + )?; // Check that the untrusted block is more recent than the trusted state - verdict!(self - .predicates - .is_monotonic_bft_time(untrusted.signed_header.header.time, trusted.header_time,)); + self.predicates + .is_monotonic_bft_time(untrusted.signed_header.header.time, trusted.header_time)?; let trusted_next_height = trusted.height.increment(); if untrusted.height() == trusted_next_height { // If the untrusted block is the very next block after the trusted block, // check that their (next) validator sets hashes match. - verdict!(self.predicates.valid_next_validator_set( + self.predicates.valid_next_validator_set( untrusted.signed_header.header.validators_hash, trusted.next_validators_hash, - )); + )?; } else { // Otherwise, ensure that the untrusted block has a greater height than // the trusted block. - verdict!(self - .predicates - .is_monotonic_height(untrusted.signed_header.header.height, trusted.height)); + self.predicates + .is_monotonic_height(untrusted.signed_header.header.height, trusted.height)?; } - Verdict::Success + Ok(()) } - fn verify_commit_trusting( + /// Check there is enough overlap between the validator sets of the trusted and untrusted + /// blocks. + pub fn verify_commit_trusting( &self, - untrusted: UntrustedBlockState<'_>, - trusted: TrustedBlockState<'_>, + untrusted: &UntrustedBlockState<'_>, + trusted: &TrustedBlockState<'_>, options: &Options, - ) -> Verdict { + ) -> Result<(), VerificationError> { let trusted_next_height = trusted.height.increment(); if untrusted.height() != trusted_next_height { // Check there is enough overlap between the validator sets of // the trusted and untrusted blocks. - verdict!(self.predicates.has_sufficient_validators_overlap( + self.predicates.has_sufficient_validators_overlap( untrusted.signed_header, trusted.next_validators, &options.trust_threshold, &self.voting_power_calculator, - )); + )?; } + Ok(()) + } +} + +impl Verifier for PredicateVerifier +where + P: VerificationPredicates, + C: VotingPowerCalculator, + V: CommitValidator, + H: Hasher, +{ + /// Validate the given light block state. + /// + /// - Ensure the latest trusted header hasn't expired + /// - Ensure the header validator hashes match the given validators + /// - Ensure the header next validator hashes match the given next validators + /// - Additional implementation specific validation via `commit_validator` + /// - Check that the untrusted block is more recent than the trusted state + /// - If the untrusted block is the very next block after the trusted block, check that their + /// (next) validator sets hashes match. + /// - Otherwise, ensure that the untrusted block has a greater height than the trusted block. + /// + /// **NOTE**: If the untrusted state's `next_validators` field is `None`, + /// this will not (and will not be able to) check whether the untrusted + /// state's `next_validators_hash` field is valid. + fn verify( + &self, + untrusted: UntrustedBlockState<'_>, + trusted: TrustedBlockState<'_>, + options: &Options, + now: Time, + ) -> Verdict { + verdict!(self.validate_untrusted(&untrusted)); + verdict!(self.validate_trusting(&untrusted, &trusted, options, now)); + verdict!(self.verify_commit(&untrusted)); + verdict!(self.verify_commit_trusting(&untrusted, &trusted, options)); Verdict::Success } } diff --git a/light-client/src/tests.rs b/light-client/src/tests.rs index 39bd034b9..8b9f54a67 100644 --- a/light-client/src/tests.rs +++ b/light-client/src/tests.rs @@ -166,38 +166,12 @@ pub fn verify_single( } let verifier = ProdVerifier::default(); - - verifier - .validate_untrusted(input.as_untrusted_state()) - .into_result(())?; - - verifier - .verify_commit(input.as_untrusted_state()) - .into_result(())?; - let options = Options { trust_threshold, trusting_period, clock_drift, }; - verifier - .validate_trusting( - input.as_untrusted_state(), - trusted_block.as_trusted_state(), - &options, - now, - ) - .into_result(())?; - - verifier - .verify_commit_trusting( - input.as_untrusted_state(), - trusted_block.as_trusted_state(), - &options, - ) - .into_result(())?; - verifier .verify( input.as_untrusted_state(), From 56d05e1ca38264d97e8ff3f7a43bc47de05245d4 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Sat, 19 Nov 2022 00:59:02 +0530 Subject: [PATCH 08/16] Remove unused code --- .../src/operations/voting_power.rs | 20 +---- light-client-verifier/src/predicates.rs | 85 ------------------- light-client/src/tests.rs | 32 +++---- testgen/src/light_block.rs | 18 ---- 4 files changed, 14 insertions(+), 141 deletions(-) diff --git a/light-client-verifier/src/operations/voting_power.rs b/light-client-verifier/src/operations/voting_power.rs index 908965be0..442db0510 100644 --- a/light-client-verifier/src/operations/voting_power.rs +++ b/light-client-verifier/src/operations/voting_power.rs @@ -67,14 +67,13 @@ pub trait VotingPowerCalculator: Send + Sync { } } - /// Check against the given threshold that there is enough signers - /// overlap between an untrusted header and untrusted validator set - fn check_enough_signers_overlap( + /// Check if there is 2/3rd overlap between an untrusted header and untrusted validator set + fn check_signers_overlap( &self, untrusted_header: &SignedHeader, untrusted_validators: &ValidatorSet, - trust_threshold: TrustThreshold, ) -> Result<(), VerificationError> { + let trust_threshold = TrustThreshold::TWO_THIRDS; let voting_power = self.voting_power_in(untrusted_header, untrusted_validators, trust_threshold)?; @@ -87,19 +86,6 @@ pub trait VotingPowerCalculator: Send + Sync { } } - /// Check if there is 2/3rd overlap between an untrusted header and untrusted validator set - fn check_signers_overlap( - &self, - untrusted_header: &SignedHeader, - untrusted_validators: &ValidatorSet, - ) -> Result<(), VerificationError> { - self.check_enough_signers_overlap( - untrusted_header, - untrusted_validators, - TrustThreshold::TWO_THIRDS, - ) - } - /// Compute the voting power in a header and its commit against a validator set. /// /// The `trust_threshold` is currently not used, but might be in the future diff --git a/light-client-verifier/src/predicates.rs b/light-client-verifier/src/predicates.rs index 2dc790835..0c8787d4f 100644 --- a/light-client-verifier/src/predicates.rs +++ b/light-client-verifier/src/predicates.rs @@ -185,23 +185,6 @@ pub trait VerificationPredicates: Send + Sync { Ok(()) } - /// Check that there is enough signers overlap between the given, untrusted validator set - /// and the untrusted signed header based on the specified trust threshold. - fn has_specified_signers_overlap( - &self, - untrusted_sh: &SignedHeader, - untrusted_validators: &ValidatorSet, - calculator: &dyn VotingPowerCalculator, - trust_threshold: TrustThreshold, - ) -> Result<(), VerificationError> { - calculator.check_enough_signers_overlap( - untrusted_sh, - untrusted_validators, - trust_threshold, - )?; - Ok(()) - } - /// Check that the hash of the next validator set in the trusted block matches /// the hash of the validator set in the untrusted one. fn valid_next_validator_set( @@ -712,72 +695,4 @@ mod tests { _ => panic!("expected InsufficientSignersOverlap error"), } } - - #[test] - fn test_has_specified_signers_overlap() { - let validators = [ - Validator::new("1").voting_power(25), - Validator::new("2").voting_power(25), - Validator::new("3").voting_power(25), - Validator::new("4").voting_power(25), - ]; - - let mut light_block: LightBlock = - TestgenLightBlock::new_default_with_validators(2, &validators) - .generate() - .unwrap() - .into(); - - let vp = ProdPredicates::default(); - let voting_power_calculator = ProdVotingPowerCalculator::default(); - - // Test scenarios --> - // 1. >3/4 validators sign - let result_ok = vp.has_specified_signers_overlap( - &light_block.signed_header, - &light_block.validators, - &voting_power_calculator, - TrustThreshold::new(3, 4).unwrap(), - ); - - assert!(result_ok.is_ok()); - - // 2. >2/3 validators sign - light_block.signed_header.commit.signatures.pop(); - - let result_ok = vp.has_specified_signers_overlap( - &light_block.signed_header, - &light_block.validators, - &voting_power_calculator, - TrustThreshold::TWO_THIRDS, - ); - - assert!(result_ok.is_ok()); - - // 3. <1/3 validators sign (i.e. 1/4) - light_block.signed_header.commit.signatures.truncate(1); - - let result_err = vp.has_specified_signers_overlap( - &light_block.signed_header, - &light_block.validators, - &voting_power_calculator, - TrustThreshold::ONE_THIRD, - ); - - let trust_threshold = TrustThreshold::ONE_THIRD; - - match result_err { - Err(VerificationError(VerificationErrorDetail::InsufficientSignersOverlap(e), _)) => { - assert_eq!( - e.tally, - VotingPowerTally { - total: 100, - tallied: 25, - trust_threshold, - } - ); - }, - _ => panic!("expected InsufficientSignersOverlap error"), - } - } } diff --git a/light-client/src/tests.rs b/light-client/src/tests.rs index 8b9f54a67..1bfeede80 100644 --- a/light-client/src/tests.rs +++ b/light-client/src/tests.rs @@ -152,19 +152,6 @@ pub fn verify_single( clock_drift: Duration, now: Time, ) -> Result { - trait IntoResult { - fn into_result(self, ok: T) -> Result; - } - - impl IntoResult for Verdict { - fn into_result(self, ok: T) -> Result { - match self { - Verdict::Success => Ok(ok), - error => Err(error), - } - } - } - let verifier = ProdVerifier::default(); let options = Options { trust_threshold, @@ -172,14 +159,17 @@ pub fn verify_single( clock_drift, }; - verifier - .verify( - input.as_untrusted_state(), - trusted_block.as_trusted_state(), - &options, - now, - ) - .into_result(input) + let result = verifier.verify( + input.as_untrusted_state(), + trusted_block.as_trusted_state(), + &options, + now, + ); + + match result { + Verdict::Success => Ok(input), + error => Err(error), + } } pub fn verify_bisection( diff --git a/testgen/src/light_block.rs b/testgen/src/light_block.rs index 27763e987..c6cea5ab7 100644 --- a/testgen/src/light_block.rs +++ b/testgen/src/light_block.rs @@ -106,24 +106,6 @@ impl LightBlock { } } - pub fn new_default_with_validators(height: u64, validators: &[Validator]) -> Self { - let header = Header::new(validators) - .height(height) - .chain_id("test-chain") - .next_validators(validators) - .time(Time::from_unix_timestamp(height as i64, 0).unwrap()); // just wanted to initialize time with some value - - let commit = Commit::new(header.clone(), 1); - - Self { - header: Some(header), - commit: Some(commit), - validators: Some(validators.to_vec()), - next_validators: Some(validators.to_vec()), - provider: Some(default_peer_id()), - } - } - pub fn new_default_with_time_and_chain_id(chain_id: String, time: Time, height: u64) -> Self { let validators = [ Validator::new("1").voting_power(50), From 58b46b6c09a27a7f37cfa1739c3842653b3c34a0 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Sat, 19 Nov 2022 01:23:58 +0530 Subject: [PATCH 09/16] Add changelog entry --- .../unreleased/improvements/1222-verify-component-methods.md | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 .changelog/unreleased/improvements/1222-verify-component-methods.md diff --git a/.changelog/unreleased/improvements/1222-verify-component-methods.md b/.changelog/unreleased/improvements/1222-verify-component-methods.md new file mode 100644 index 000000000..ed5b7eb52 --- /dev/null +++ b/.changelog/unreleased/improvements/1222-verify-component-methods.md @@ -0,0 +1,2 @@ +- `[light-client]` Added `validate_untrusted`, `validate_trusting`, `verify_commit` and `verify_commit_trusting` methods to `PredicateVerifier`. + ([#1222](https://github.com/informalsystems/tendermint-rs/issues/1222)) \ No newline at end of file From 841ab0bcd7cc50de367be63714eb06ee0ec16e04 Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Mon, 21 Nov 2022 14:14:34 +0530 Subject: [PATCH 10/16] Apply suggestions from code review Co-authored-by: Anca Zamfir --- light-client-verifier/src/verifier.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index 8d918e0c2..e5a18e647 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -165,7 +165,7 @@ where Ok(()) } - /// Validate a `UntrustedBlockState`, based on the given `TrustedBlockState`, `Options` and + /// Validate an `UntrustedBlockState`, based on the given `TrustedBlockState`, `Options` and /// current time. pub fn validate_trusting( &self, @@ -266,8 +266,8 @@ where ) -> Verdict { verdict!(self.validate_untrusted(&untrusted)); verdict!(self.validate_trusting(&untrusted, &trusted, options, now)); - verdict!(self.verify_commit(&untrusted)); verdict!(self.verify_commit_trusting(&untrusted, &trusted, options)); + verdict!(self.verify_commit(&untrusted)); Verdict::Success } } From 4555afee69b1dd4290fa644fd5ce747bd10f5452 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Mon, 21 Nov 2022 14:21:52 +0530 Subject: [PATCH 11/16] Improve naming --- .../improvements/1222-verify-component-methods.md | 2 +- light-client-verifier/src/verifier.rs | 15 ++++++--------- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/.changelog/unreleased/improvements/1222-verify-component-methods.md b/.changelog/unreleased/improvements/1222-verify-component-methods.md index ed5b7eb52..f2eca7bab 100644 --- a/.changelog/unreleased/improvements/1222-verify-component-methods.md +++ b/.changelog/unreleased/improvements/1222-verify-component-methods.md @@ -1,2 +1,2 @@ -- `[light-client]` Added `validate_untrusted`, `validate_trusting`, `verify_commit` and `verify_commit_trusting` methods to `PredicateVerifier`. +- `[light-client]` Added `validate`, `validate_against_trusted`, `verify_commit` and `verify_commit_against_trusted` methods to `PredicateVerifier`. ([#1222](https://github.com/informalsystems/tendermint-rs/issues/1222)) \ No newline at end of file diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index e5a18e647..01aac58f8 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -113,10 +113,7 @@ where } /// Validates an `UntrustedBlockState`. - pub fn validate_untrusted( - &self, - untrusted: &UntrustedBlockState<'_>, - ) -> Result<(), VerificationError> { + pub fn validate(&self, untrusted: &UntrustedBlockState<'_>) -> Result<(), VerificationError> { // Ensure the header validator hashes match the given validators self.predicates.validator_sets_match( untrusted.validators, @@ -167,7 +164,7 @@ where /// Validate an `UntrustedBlockState`, based on the given `TrustedBlockState`, `Options` and /// current time. - pub fn validate_trusting( + pub fn validate_against_trusted( &self, untrusted: &UntrustedBlockState<'_>, trusted: &TrustedBlockState<'_>, @@ -213,7 +210,7 @@ where /// Check there is enough overlap between the validator sets of the trusted and untrusted /// blocks. - pub fn verify_commit_trusting( + pub fn verify_commit_against_trusted( &self, untrusted: &UntrustedBlockState<'_>, trusted: &TrustedBlockState<'_>, @@ -264,9 +261,9 @@ where options: &Options, now: Time, ) -> Verdict { - verdict!(self.validate_untrusted(&untrusted)); - verdict!(self.validate_trusting(&untrusted, &trusted, options, now)); - verdict!(self.verify_commit_trusting(&untrusted, &trusted, options)); + verdict!(self.validate(&untrusted)); + verdict!(self.validate_against_trusted(&untrusted, &trusted, options, now)); + verdict!(self.verify_commit_against_trusted(&untrusted, &trusted, options)); verdict!(self.verify_commit(&untrusted)); Verdict::Success } From 7df64ac512cf32e100dc4135d82420b726d69d9d Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Tue, 22 Nov 2022 20:34:01 +0530 Subject: [PATCH 12/16] Apply suggestion to remove todo comment Co-authored-by: Anca Zamfir --- light-client-verifier/src/verifier.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index 01aac58f8..024e017c8 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -121,9 +121,8 @@ where &self.hasher, )?; - // TODO(thane): Is this check necessary for IBC? + // Ensure the header next validator hashes match the given next validators if let Some(untrusted_next_validators) = untrusted.next_validators { - // Ensure the header next validator hashes match the given next validators self.predicates.next_validators_match( untrusted_next_validators, untrusted.signed_header.header.next_validators_hash, From c1b5c341f6df258b84d33aa79ade5e4dc554a3a8 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Tue, 22 Nov 2022 20:45:08 +0530 Subject: [PATCH 13/16] Return `Verdict` from all `PredicateVerifier` verification methods --- light-client-verifier/src/verifier.rs | 82 +++++++++++++++------------ 1 file changed, 45 insertions(+), 37 deletions(-) diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index 024e017c8..4904a54ad 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -68,6 +68,15 @@ macro_rules! verdict { }; } +macro_rules! ensure_verdict_success { + ($e:expr) => { + let verdict = $e; + if !matches!(verdict, Verdict::Success) { + return verdict; + } + }; +} + /// Predicate verifier encapsulating components necessary to facilitate /// verification. #[derive(Debug, Clone, PartialEq, Eq)] @@ -113,52 +122,49 @@ where } /// Validates an `UntrustedBlockState`. - pub fn validate(&self, untrusted: &UntrustedBlockState<'_>) -> Result<(), VerificationError> { + pub fn validate(&self, untrusted: &UntrustedBlockState<'_>) -> Verdict { // Ensure the header validator hashes match the given validators - self.predicates.validator_sets_match( + verdict!(self.predicates.validator_sets_match( untrusted.validators, untrusted.signed_header.header.validators_hash, &self.hasher, - )?; + )); // Ensure the header next validator hashes match the given next validators if let Some(untrusted_next_validators) = untrusted.next_validators { - self.predicates.next_validators_match( + verdict!(self.predicates.next_validators_match( untrusted_next_validators, untrusted.signed_header.header.next_validators_hash, &self.hasher, - )?; + )); } // Ensure the header matches the commit - self.predicates.header_matches_commit( + verdict!(self.predicates.header_matches_commit( &untrusted.signed_header.header, untrusted.signed_header.commit.block_id.hash, &self.hasher, - )?; + )); // Additional implementation specific validation - self.predicates.valid_commit( + verdict!(self.predicates.valid_commit( untrusted.signed_header, untrusted.validators, &self.commit_validator, - )?; + )); - Ok(()) + Verdict::Success } /// Verify that more than 2/3 of the validators correctly committed the block. - pub fn verify_commit( - &self, - untrusted: &UntrustedBlockState<'_>, - ) -> Result<(), VerificationError> { - self.predicates.has_sufficient_signers_overlap( + pub fn verify_commit(&self, untrusted: &UntrustedBlockState<'_>) -> Verdict { + verdict!(self.predicates.has_sufficient_signers_overlap( untrusted.signed_header, untrusted.validators, &self.voting_power_calculator, - )?; + )); - Ok(()) + Verdict::Success } /// Validate an `UntrustedBlockState`, based on the given `TrustedBlockState`, `Options` and @@ -169,42 +175,44 @@ where trusted: &TrustedBlockState<'_>, options: &Options, now: Time, - ) -> Result<(), VerificationError> { + ) -> Verdict { // Ensure the latest trusted header hasn't expired - self.predicates.is_within_trust_period( + verdict!(self.predicates.is_within_trust_period( trusted.header_time, options.trusting_period, now, - )?; + )); // Ensure the header isn't from a future time - self.predicates.is_header_from_past( + verdict!(self.predicates.is_header_from_past( untrusted.signed_header.header.time, options.clock_drift, now, - )?; + )); // Check that the untrusted block is more recent than the trusted state - self.predicates - .is_monotonic_bft_time(untrusted.signed_header.header.time, trusted.header_time)?; + verdict!(self + .predicates + .is_monotonic_bft_time(untrusted.signed_header.header.time, trusted.header_time)); let trusted_next_height = trusted.height.increment(); if untrusted.height() == trusted_next_height { // If the untrusted block is the very next block after the trusted block, // check that their (next) validator sets hashes match. - self.predicates.valid_next_validator_set( + verdict!(self.predicates.valid_next_validator_set( untrusted.signed_header.header.validators_hash, trusted.next_validators_hash, - )?; + )); } else { // Otherwise, ensure that the untrusted block has a greater height than // the trusted block. - self.predicates - .is_monotonic_height(untrusted.signed_header.header.height, trusted.height)?; + verdict!(self + .predicates + .is_monotonic_height(untrusted.signed_header.header.height, trusted.height)); } - Ok(()) + Verdict::Success } /// Check there is enough overlap between the validator sets of the trusted and untrusted @@ -214,21 +222,21 @@ where untrusted: &UntrustedBlockState<'_>, trusted: &TrustedBlockState<'_>, options: &Options, - ) -> Result<(), VerificationError> { + ) -> Verdict { let trusted_next_height = trusted.height.increment(); if untrusted.height() != trusted_next_height { // Check there is enough overlap between the validator sets of // the trusted and untrusted blocks. - self.predicates.has_sufficient_validators_overlap( + verdict!(self.predicates.has_sufficient_validators_overlap( untrusted.signed_header, trusted.next_validators, &options.trust_threshold, &self.voting_power_calculator, - )?; + )); } - Ok(()) + Verdict::Success } } @@ -260,10 +268,10 @@ where options: &Options, now: Time, ) -> Verdict { - verdict!(self.validate(&untrusted)); - verdict!(self.validate_against_trusted(&untrusted, &trusted, options, now)); - verdict!(self.verify_commit_against_trusted(&untrusted, &trusted, options)); - verdict!(self.verify_commit(&untrusted)); + ensure_verdict_success!(self.validate(&untrusted)); + ensure_verdict_success!(self.validate_against_trusted(&untrusted, &trusted, options, now)); + ensure_verdict_success!(self.verify_commit_against_trusted(&untrusted, &trusted, options)); + ensure_verdict_success!(self.verify_commit(&untrusted)); Verdict::Success } } From 90bdd30b7724081af6e6a0ffee20958dd4c5cdd2 Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Tue, 22 Nov 2022 20:54:42 +0530 Subject: [PATCH 14/16] Update comment --- light-client-verifier/src/verifier.rs | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index 4904a54ad..5e5c6902e 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -247,16 +247,24 @@ where V: CommitValidator, H: Hasher, { - /// Validate the given light block state. + /// Validate the given light block state by performing the following checks -> /// - /// - Ensure the latest trusted header hasn't expired - /// - Ensure the header validator hashes match the given validators - /// - Ensure the header next validator hashes match the given next validators - /// - Additional implementation specific validation via `commit_validator` - /// - Check that the untrusted block is more recent than the trusted state - /// - If the untrusted block is the very next block after the trusted block, check that their - /// (next) validator sets hashes match. - /// - Otherwise, ensure that the untrusted block has a greater height than the trusted block. + /// - Validate latest untrusted header + /// - Ensure the header validator hashes match the given validators + /// - Ensure the header next validator hashes match the given next validators + /// - Ensure the header matches the commit + /// - Ensure commit is valid + /// - Validate latest untrusted header + /// - Ensure the latest trusted header hasn't expired + /// - Ensure the header isn't from a future time + /// - Check that the untrusted block is more recent than the trusted state + /// - If the untrusted block is the very next block after the trusted block, check that + /// their (next) validator sets hashes match. + /// - Otherwise, ensure that the untrusted block has a greater height than the trusted + /// block. + /// - Check there is enough overlap between the validator sets of the trusted and untrusted + /// blocks. + /// - Verify that more than 2/3 of the validators correctly committed the block. /// /// **NOTE**: If the untrusted state's `next_validators` field is `None`, /// this will not (and will not be able to) check whether the untrusted From 8274a4d36ae4d4ed4128851a1cc2a984ca8d267d Mon Sep 17 00:00:00 2001 From: Shoaib Ahmed Date: Thu, 24 Nov 2022 20:31:26 +0530 Subject: [PATCH 15/16] Apply suggestions from code review Co-authored-by: Anca Zamfir --- light-client-verifier/src/verifier.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index 5e5c6902e..794fa3a58 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -249,12 +249,12 @@ where { /// Validate the given light block state by performing the following checks -> /// - /// - Validate latest untrusted header + /// - Validate the untrusted header /// - Ensure the header validator hashes match the given validators /// - Ensure the header next validator hashes match the given next validators /// - Ensure the header matches the commit /// - Ensure commit is valid - /// - Validate latest untrusted header + /// - Validate the untrusted header against the trusted header /// - Ensure the latest trusted header hasn't expired /// - Ensure the header isn't from a future time /// - Check that the untrusted block is more recent than the trusted state From c0f2469caf9d1f38b1c6af2f4232d22867165fec Mon Sep 17 00:00:00 2001 From: hu55a1n1 Date: Thu, 24 Nov 2022 20:32:04 +0530 Subject: [PATCH 16/16] Rename `validate()` -> `verify_validator_sets()` --- light-client-verifier/src/verifier.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/light-client-verifier/src/verifier.rs b/light-client-verifier/src/verifier.rs index 5e5c6902e..0f7c55ac4 100644 --- a/light-client-verifier/src/verifier.rs +++ b/light-client-verifier/src/verifier.rs @@ -122,7 +122,7 @@ where } /// Validates an `UntrustedBlockState`. - pub fn validate(&self, untrusted: &UntrustedBlockState<'_>) -> Verdict { + pub fn verify_validator_sets(&self, untrusted: &UntrustedBlockState<'_>) -> Verdict { // Ensure the header validator hashes match the given validators verdict!(self.predicates.validator_sets_match( untrusted.validators, @@ -276,7 +276,7 @@ where options: &Options, now: Time, ) -> Verdict { - ensure_verdict_success!(self.validate(&untrusted)); + ensure_verdict_success!(self.verify_validator_sets(&untrusted)); ensure_verdict_success!(self.validate_against_trusted(&untrusted, &trusted, options, now)); ensure_verdict_success!(self.verify_commit_against_trusted(&untrusted, &trusted, options)); ensure_verdict_success!(self.verify_commit(&untrusted));