Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement verify_light() and verify_light_trusting() #1226

Merged
merged 17 commits into from
Nov 24, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions light-client-verifier/src/operations/voting_power.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)?;

Expand All @@ -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_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
Expand Down
85 changes: 85 additions & 0 deletions light-client-verifier/src/predicates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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_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(
Expand Down Expand Up @@ -695,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"),
}
}
}
67 changes: 66 additions & 1 deletion light-client-verifier/src/verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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`].
Expand Down
39 changes: 28 additions & 11 deletions light-client/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,25 +152,42 @@ pub fn verify_single(
clock_drift: Duration,
now: Time,
) -> Result<LightBlock, Verdict> {
trait IntoResult<T, E> {
fn into_result(self, ok: T) -> Result<T, E>;
}

impl<T> IntoResult<T, Verdict> for Verdict {
fn into_result(self, ok: T) -> Result<T, Verdict> {
match self {
Verdict::Success => Ok(ok),
error => Err(error),
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: the purpose of this could look more obvious if this were just a helper function. Then again, method chaining is neat.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, not related to this PR: why did there have to be a non-idiomatic Verdict, rather than a Result<(), VerificationError> where VerificationError would be an enum or otherwise easily matchable to that variant?
Basically, what the existing impl From<Result<(), VerificationError>> for Verdict does could be just how the Result returned value is supposed to be used if you care about the NotEnoughTrust case. Cc @romac

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, let's do that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, let's do that.

I didn't mean this to be done in scope for this PR, which so far has not introduced breaking changes, but if you think it would be good and easy thing to do in the next release, I'm all for it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, let's target this for 0.27.


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(
Expand Down
18 changes: 18 additions & 0 deletions testgen/src/light_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down