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

Fix verify functions' return type to be bool #144

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
10 changes: 7 additions & 3 deletions commit_verify/src/commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
/// Verifies commitment against the message; default implementation just
/// repeats the commitment to the message and check it against the `self`.
#[inline]
#[must_use = "the boolean carries the result of the verification"]
fn verify(&self, msg: &Msg) -> bool { Self::commit(msg) == *self }
}

Expand All @@ -59,12 +60,15 @@
/// Tries to create commitment to a byte representation of a given message
fn try_commit(msg: &Msg) -> Result<Self, Self::Error>;

/// Tries to verify commitment against the message; default implementation
/// Verifies commitment against the message; default implementation
/// just repeats the commitment to the message and check it against the
/// `self`.
#[inline]
fn try_verify(&self, msg: &Msg) -> Result<bool, Self::Error> {
Ok(Self::try_commit(msg)? == *self)
#[must_use = "the boolean carries the result of the verification"]
fn verify(&self, msg: &Msg) -> bool {
Self::try_commit(msg)
.map(|other| other == *self)
.unwrap_or(false)

Check warning on line 71 in commit_verify/src/commit.rs

View check run for this annotation

Codecov / codecov/patch

commit_verify/src/commit.rs#L68-L71

Added lines #L68 - L71 were not covered by tests
}
}

Expand Down
37 changes: 11 additions & 26 deletions commit_verify/src/convolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,34 +49,19 @@
/// that the resulting commitment matches the provided one in the
/// `commitment` parameter.
///
/// Errors if the commitment can't be created, i.e. the
/// [`ConvolveCommit::convolve_commit`] procedure for the original,
/// restored from the proof, can't be performed. This means that the
/// verification has failed and the commitment and/or the proof are
/// invalid. The function returns error in this case (ano not simply
/// `false`) since this usually means the software error in managing
/// container and proof data, or selection of a different commitment
/// protocol parameters comparing to the ones used during commitment
/// creation. In all these cases we'd like to provide devs with more
/// information for debugging.
///
/// The proper way of using the function in a well-debugged software should
/// be `if commitment.verify(...).expect("proof managing system") { .. }`.
/// However if the proofs are provided by some sort of user/network input
/// from an untrusted party, a proper form would be
/// `if commitment.verify(...).unwrap_or(false) { .. }`.
fn verify(
&self,
msg: &Msg,
commitment: &Source::Commitment,
) -> Result<bool, Source::CommitError>
where
Self: VerifyEq,
{
/// Returns `false` if the commitment doesn't pass the verification or can't
/// be replicated, i.e. the [`ConvolveCommit::convolve_commit`]
/// procedure for the original, restored from the proof, can't be
/// performed. This means that the verification has failed and the
/// commitment and/or the proof are invalid.
#[must_use = "the boolean carries the result of the verification"]
fn verify(&self, msg: &Msg, commitment: &Source::Commitment) -> bool {
let original = self.restore_original(commitment);
let suppl = self.extract_supplement();
let (commitment_prime, proof) = original.convolve_commit(suppl, msg)?;
Ok(commitment.verify_eq(&commitment_prime) && self.verify_eq(&proof))
let Ok((commitment_prime, proof)) = original.convolve_commit(suppl, msg) else {
return false;

Check warning on line 62 in commit_verify/src/convolve.rs

View check run for this annotation

Codecov / codecov/patch

commit_verify/src/convolve.rs#L62

Added line #L62 was not covered by tests
};
commitment.verify_eq(&commitment_prime) && self.verify_eq(&proof)
}
}

Expand Down
63 changes: 26 additions & 37 deletions commit_verify/src/embed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,9 @@
{
/// Restores original container before the commitment from the proof data
/// and a container containing embedded commitment.
fn restore_original_container(
&self,
commit_container: &Container,
) -> Result<Container, Container::VerifyError>;
///
/// Can fail if the container data are not match proof data.
fn restore_original_container(&self, commit_container: &Container) -> Option<Container>;
}

/// Trait for *embed-commit-verify scheme*, where some data structure (named
Expand Down Expand Up @@ -113,10 +112,6 @@
/// invalid and the commitment can't be re-created.
type CommitError: std::error::Error;

/// Error type that may be reported during [`Self::verify`] procedure.
/// It must be a subset of [`Self::CommitError`].
type VerifyError: std::error::Error + From<Self::CommitError>;

/// Creates a commitment to a message and embeds it into the provided
/// container (`self`) by mutating it and returning commitment proof.
///
Expand All @@ -131,31 +126,26 @@
/// [`Self::embed_commit`] procedure checking that the resulting proof and
/// commitment matches the provided `self` and `proof`.
///
/// Errors if the provided commitment can't be created, i.e. the
/// [`Self::embed_commit`] procedure for the original container, restored
/// from the proof and current container, can't be performed. This means
/// that the verification has failed and the commitment and proof are
/// invalid. The function returns error in this case (ano not simply
/// `false`) since this usually means the software error in managing
/// container and proof data, or selection of a different commitment
/// protocol parameters comparing to the ones used during commitment
/// creation. In all these cases we'd like to provide devs with more
/// information for debugging.
///
/// The proper way of using the function in a well-debugged software should
/// be `if commitment.verify(...).expect("proof managing system") { .. }`.
/// However if the proofs are provided by some sort of user/network input
/// from an untrusted party, a proper form would be
/// `if commitment.verify(...).unwrap_or(false) { .. }`.
/// Returns `false` if the if the commitment doesn't pass the verification
/// or can't be replicated, i.e. the original container can't be restored
/// from the proof, or [`Self::embed_commit`] procedure for the original
/// container, restored from the proof and current container, can't be
/// performed. All of these options mean that the verification has failed
/// and the commitment or the proof is invalid.
#[inline]
fn verify(&self, msg: &Msg, proof: &Self::Proof) -> Result<bool, Self::VerifyError>
#[must_use = "the boolean carries the result of the verification"]
fn verify(&self, msg: &Msg, proof: &Self::Proof) -> bool
where
Self: VerifyEq,
Self::Proof: VerifyEq,
{
let mut container_prime = proof.restore_original_container(self)?;
let proof_prime = container_prime.embed_commit(msg)?;
Ok(proof_prime.verify_eq(proof) && self.verify_eq(&container_prime))
let Some(mut container_prime) = proof.restore_original_container(self) else {
return false;

Check warning on line 143 in commit_verify/src/embed.rs

View check run for this annotation

Codecov / codecov/patch

commit_verify/src/embed.rs#L143

Added line #L143 was not covered by tests
};
let Ok(proof_prime) = container_prime.embed_commit(msg) else {
return false;

Check warning on line 146 in commit_verify/src/embed.rs

View check run for this annotation

Codecov / codecov/patch

commit_verify/src/embed.rs#L146

Added line #L146 was not covered by tests
};
proof_prime.verify_eq(proof) && self.verify_eq(&container_prime)
}

/// Phantom method used to add `Protocol` generic parameter to the trait.
Expand Down Expand Up @@ -207,18 +197,18 @@
});

// Testing verification
assert!(commitment.clone().verify(msg, &proof).unwrap());
assert!(commitment.clone().verify(msg, &proof));

messages.iter().for_each(|m| {
// Testing that commitment verification succeeds only
// for the original message and fails for the rest
assert_eq!(commitment.clone().verify(m, &proof).unwrap(), m == msg);
assert_eq!(commitment.clone().verify(m, &proof), m == msg);
});

acc.iter().for_each(|cmt| {
// Testing that verification against other commitments
// returns `false`
assert!(!cmt.clone().verify(msg, &proof).unwrap());
assert!(!cmt.clone().verify(msg, &proof));
});

// Detecting collision: each message should produce a unique
Expand Down Expand Up @@ -253,18 +243,18 @@
});

// Testing verification
assert!(SUPPLEMENT.verify(msg, &commitment).unwrap());
assert!(SUPPLEMENT.verify(msg, &commitment));

messages.iter().for_each(|m| {
// Testing that commitment verification succeeds only
// for the original message and fails for the rest
assert_eq!(SUPPLEMENT.verify(m, &commitment).unwrap(), m == msg);
assert_eq!(SUPPLEMENT.verify(m, &commitment), m == msg);
});

acc.iter().for_each(|commitment| {
// Testing that verification against other commitments
// returns `false`
assert!(!SUPPLEMENT.verify(msg, commitment).unwrap());
assert!(!SUPPLEMENT.verify(msg, commitment));
});

// Detecting collision: each message should produce a unique
Expand Down Expand Up @@ -303,8 +293,8 @@
impl<T> EmbedCommitProof<T, DummyVec, TestProtocol> for DummyProof
where T: AsRef<[u8]> + Clone + CommitEncode
{
fn restore_original_container(&self, _: &DummyVec) -> Result<DummyVec, Error> {
Ok(DummyVec(self.0.clone()))
fn restore_original_container(&self, _: &DummyVec) -> Option<DummyVec> {
Copy link
Member

Choose a reason for hiding this comment

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

Why change from Result to Option?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is internal method called from verify, which was relying on type VerifyError which is not there anymore. So there is no error type we can return from here.

Some(DummyVec(self.0.clone()))
}
}

Expand All @@ -313,7 +303,6 @@
{
type Proof = DummyProof;
type CommitError = Error;
type VerifyError = Error;

fn embed_commit(&mut self, msg: &T) -> Result<Self::Proof, Self::CommitError> {
let proof = self.0.clone();
Expand Down
Loading