From 62e177b759a077380ddc768701e14a1089a883b3 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Sun, 15 Oct 2023 10:52:34 +0200 Subject: [PATCH 1/4] commit: return enumerated errors for CommitVerify and TryCommitVerify --- commit_verify/src/commit.rs | 47 ++++++++++++++++++++++++------------- commit_verify/src/lib.rs | 2 +- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/commit_verify/src/commit.rs b/commit_verify/src/commit.rs index e0dba900..6daad609 100644 --- a/commit_verify/src/commit.rs +++ b/commit_verify/src/commit.rs @@ -28,9 +28,18 @@ use strict_encoding::{StrictEncode, StrictWriter}; use crate::digest::DigestExt; use crate::CommitmentProtocol; -/// Trait for commit-verify scheme. A message for the commitment may be any -/// structure that can be represented as a byte array (i.e. implements -/// `AsRef<[u8]>`). +/// Error during commitment verification +#[derive(Copy, Clone, Eq, PartialEq, Debug, Display, Error)] +#[display(doc_comments)] +pub enum VerifyError { + /// The verified commitment doesn't commit to the provided message. + InvalidCommitment, + /// The message is invalid since a commitment to it can't be created / + /// exist. + InvalidMessage, +} + +/// Trait for commit-verify scheme. pub trait CommitVerify where Self: Eq + Sized { @@ -43,28 +52,34 @@ where Self: Eq + Sized /// Verifies commitment against the message; default implementation just /// repeats the commitment to the message and check it against the `self`. #[inline] - fn verify(&self, msg: &Msg) -> bool { Self::commit(msg) == *self } + fn verify(&self, msg: &Msg) -> Result<(), VerifyError> { + match Self::commit(msg) == *self { + false => Err(VerifyError::InvalidCommitment), + true => Ok(()), + } + } } -/// Trait for a failable version of commit-verify scheme. A message for the -/// commitment may be any structure that can be represented as a byte array -/// (i.e. implements `AsRef<[u8]>`). +/// Trait for a failable version of commit-verify scheme. pub trait TryCommitVerify where Self: Eq + Sized { - /// Error type that may be reported during [`TryCommitVerify::try_commit`] - /// and [`TryCommitVerify::try_verify`] procedures + /// Error type that may be reported during [`TryCommitVerify::try_commit`]. type Error: std::error::Error; - /// Tries to create commitment to a byte representation of a given message + /// Tries to create commitment to a byte representation of a given message. fn try_commit(msg: &Msg) -> Result; - /// Tries to verify commitment against the message; default implementation + /// Verifies the 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 { - Ok(Self::try_commit(msg)? == *self) + fn verify(&self, msg: &Msg) -> Result<(), VerifyError> { + let other_commitment = Self::try_commit(msg).map_err(|_| VerifyError::InvalidMessage)?; + if other_commitment != *self { + return Err(VerifyError::InvalidCommitment); + } + Ok(()) } } @@ -113,18 +128,18 @@ pub(crate) mod test_helpers { }); // Testing verification - assert!(commitment.verify(msg)); + assert!(commitment.verify(msg).is_ok()); messages.iter().for_each(|m| { // Testing that commitment verification succeeds only // for the original message and fails for the rest - assert_eq!(commitment.verify(m), m == msg); + assert_eq!(commitment.verify(m).is_ok(), m == msg); }); acc.iter().for_each(|cmt| { // Testing that verification against other commitments // returns `false` - assert!(!cmt.verify(msg)); + assert!(!cmt.verify(msg).is_ok()); }); // Detecting collision diff --git a/commit_verify/src/lib.rs b/commit_verify/src/lib.rs index fc5e50f0..2c4d6125 100644 --- a/commit_verify/src/lib.rs +++ b/commit_verify/src/lib.rs @@ -58,7 +58,7 @@ pub mod merkle; pub mod mpc; mod digest; -pub use commit::{CommitVerify, StrictEncodedProtocol, TryCommitVerify}; +pub use commit::{CommitVerify, StrictEncodedProtocol, TryCommitVerify, VerifyError}; pub use conceal::Conceal; pub use convolve::{ConvolveCommit, ConvolveCommitProof}; pub use digest::{Digest, DigestExt, Ripemd160, Sha256}; From b32f00138920ede5c72052545e8258dddaa6819c Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Sun, 15 Oct 2023 11:02:36 +0200 Subject: [PATCH 2/4] commit: return enumerated errors for ConvolveCommit --- commit_verify/src/convolve.rs | 47 +++++++++++++++++++++-------------- commit_verify/src/lib.rs | 2 +- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/commit_verify/src/convolve.rs b/commit_verify/src/convolve.rs index ef58ce96..0bd13168 100644 --- a/commit_verify/src/convolve.rs +++ b/commit_verify/src/convolve.rs @@ -23,6 +23,21 @@ use crate::{CommitEncode, CommitmentProtocol, VerifyEq}; +/// Error during commitment verification +#[derive(Copy, Clone, Eq, PartialEq, Debug, Display, Error)] +#[display(doc_comments)] +#[allow(clippy::enum_variant_names)] +pub enum ConvolveVerifyError { + /// The verified commitment doesn't commit to the provided message. + InvalidCommitment, + /// The message is invalid since a commitment to it can't be created / + /// exist. + InvalidMessage, + /// The proof of the commitment is invalid and the commitment can't be + /// verified. + InvalidProof, +} + /// Proof type used by [`ConvolveCommit`] protocol. pub trait ConvolveCommitProof where @@ -49,34 +64,30 @@ where /// 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. + /// # Errors /// - /// 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) { .. }`. + /// Errors if the commitment doesn't pass the validation (see + /// [`ConvolveVerifyError`] variants for the cases when this may happen). fn verify( &self, msg: &Msg, commitment: &Source::Commitment, - ) -> Result + ) -> Result<(), ConvolveVerifyError> where Self: VerifyEq, { 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 (commitment_prime, proof) = original + .convolve_commit(suppl, msg) + .map_err(|_| ConvolveVerifyError::InvalidMessage)?; + if !self.verify_eq(&proof) { + return Err(ConvolveVerifyError::InvalidProof); + } + if !commitment.verify_eq(&commitment_prime) { + return Err(ConvolveVerifyError::InvalidCommitment); + } + Ok(()) } } diff --git a/commit_verify/src/lib.rs b/commit_verify/src/lib.rs index 2c4d6125..d55bf555 100644 --- a/commit_verify/src/lib.rs +++ b/commit_verify/src/lib.rs @@ -60,7 +60,7 @@ mod digest; pub use commit::{CommitVerify, StrictEncodedProtocol, TryCommitVerify, VerifyError}; pub use conceal::Conceal; -pub use convolve::{ConvolveCommit, ConvolveCommitProof}; +pub use convolve::{ConvolveCommit, ConvolveCommitProof, ConvolveVerifyError}; pub use digest::{Digest, DigestExt, Ripemd160, Sha256}; pub use embed::{EmbedCommitProof, EmbedCommitVerify, VerifyEq}; pub use encode::{strategies, CommitEncode, CommitStrategy}; From c5a3da8574d912358ae979906d51e63b0fd2c9c5 Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Sun, 15 Oct 2023 20:00:47 +0200 Subject: [PATCH 3/4] commit: return enumerated errors for EmbedCommit --- commit_verify/src/embed.rs | 94 ++++++++++++++++++++++++-------------- commit_verify/src/lib.rs | 2 +- 2 files changed, 60 insertions(+), 36 deletions(-) diff --git a/commit_verify/src/embed.rs b/commit_verify/src/embed.rs index 2d1859b5..cb4fb7cb 100644 --- a/commit_verify/src/embed.rs +++ b/commit_verify/src/embed.rs @@ -23,7 +23,30 @@ use crate::{CommitEncode, CommitmentProtocol}; -/// Trait for equivalence verification. Implemented for all types implemeting +/// Error during commitment verification +#[derive(Copy, Clone, Eq, PartialEq, Debug, Display, Error, From)] +#[display(doc_comments)] +pub enum EmbedVerifyError { + /// The verified commitment doesn't commit to the provided message. + CommitmentMismatch, + + /// The message is invalid since a commitment to it can't be created / + /// exist. + /// + /// Details: {0} + #[from] + InvalidMessage(E), + + /// The proof of the commitment is invalid and the commitment can't be + /// verified since the original container can't be restored from it. + InvalidProof, + + /// The proof of the commitment does not match to the proof generated for + /// the same message during the verification. + ProofMismatch, +} + +/// Trait for equivalence verification. Implemented for all types implementing /// `Eq`. For non-`Eq` types this trait provides way to implement custom /// equivalence verification used during commitment verification procedure. pub trait VerifyEq { @@ -47,10 +70,15 @@ where { /// Restores original container before the commitment from the proof data /// and a container containing embedded commitment. + /// + /// # Error + /// + /// If the container can't be restored from the proof returns + /// [`EmbedVerifyError::InvalidProof`]. fn restore_original_container( &self, commit_container: &Container, - ) -> Result; + ) -> Result>; } /// Trait for *embed-commit-verify scheme*, where some data structure (named @@ -82,7 +110,7 @@ where /// combination of the same message and container type (each of each will have /// its own `Proof` type defined as an associated generic). /// -/// Usually represents an uninstantiable type, but may be a structure +/// Usually represents a non-instantiable type, but may be a structure /// containing commitment protocol configuration or context objects. /// /// ``` @@ -109,14 +137,11 @@ where type Proof: EmbedCommitProof; /// Error type that may be reported during [`Self::embed_commit`] procedure. - /// It may also be returned from [`Self::verify`] in case the proof data are - /// invalid and the commitment can't be re-created. + /// It may also be returned from [`Self::verify`] (wrapped into + /// [`EmbedVerifyError`] in case the proof data are 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; - /// Creates a commitment to a message and embeds it into the provided /// container (`self`) by mutating it and returning commitment proof. /// @@ -131,31 +156,28 @@ where /// [`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. + /// # Errors /// - /// 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) { .. }`. - #[inline] - fn verify(&self, msg: &Msg, proof: &Self::Proof) -> Result + /// Errors if the commitment doesn't pass the validation (see + /// [`EmbedVerifyError`] variants for the cases when this may happen). + fn verify( + &self, + msg: &Msg, + proof: &Self::Proof, + ) -> Result<(), EmbedVerifyError> 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)) + if !proof_prime.verify_eq(proof) { + return Err(EmbedVerifyError::InvalidProof); + } + if !self.verify_eq(&container_prime) { + return Err(EmbedVerifyError::CommitmentMismatch); + } + Ok(()) } /// Phantom method used to add `Protocol` generic parameter to the trait. @@ -207,18 +229,18 @@ pub(crate) mod test_helpers { }); // Testing verification - assert!(commitment.clone().verify(msg, &proof).unwrap()); + assert!(commitment.clone().verify(msg, &proof).is_ok()); 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).is_ok(), 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).is_ok()); }); // Detecting collision: each message should produce a unique @@ -253,18 +275,18 @@ pub(crate) mod test_helpers { }); // Testing verification - assert!(SUPPLEMENT.verify(msg, &commitment).unwrap()); + assert!(SUPPLEMENT.verify(msg, &commitment).is_ok()); 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).is_ok(), 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).is_ok()); }); // Detecting collision: each message should produce a unique @@ -303,7 +325,10 @@ mod test { impl EmbedCommitProof for DummyProof where T: AsRef<[u8]> + Clone + CommitEncode { - fn restore_original_container(&self, _: &DummyVec) -> Result { + fn restore_original_container( + &self, + _: &DummyVec, + ) -> Result> { Ok(DummyVec(self.0.clone())) } } @@ -313,7 +338,6 @@ mod test { { type Proof = DummyProof; type CommitError = Error; - type VerifyError = Error; fn embed_commit(&mut self, msg: &T) -> Result { let proof = self.0.clone(); diff --git a/commit_verify/src/lib.rs b/commit_verify/src/lib.rs index d55bf555..cd9830cb 100644 --- a/commit_verify/src/lib.rs +++ b/commit_verify/src/lib.rs @@ -62,7 +62,7 @@ pub use commit::{CommitVerify, StrictEncodedProtocol, TryCommitVerify, VerifyErr pub use conceal::Conceal; pub use convolve::{ConvolveCommit, ConvolveCommitProof, ConvolveVerifyError}; pub use digest::{Digest, DigestExt, Ripemd160, Sha256}; -pub use embed::{EmbedCommitProof, EmbedCommitVerify, VerifyEq}; +pub use embed::{EmbedCommitProof, EmbedCommitVerify, EmbedVerifyError, VerifyEq}; pub use encode::{strategies, CommitEncode, CommitStrategy}; pub use id::CommitmentId; From 059d8d95c71f0752ac1f1eb1f497b7cd0ca9ea5b Mon Sep 17 00:00:00 2001 From: Dr Maxim Orlovsky Date: Tue, 17 Oct 2023 11:18:48 +0200 Subject: [PATCH 4/4] chore: fix clippy lints --- commit_verify/src/commit.rs | 2 +- commit_verify/src/embed.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/commit_verify/src/commit.rs b/commit_verify/src/commit.rs index 6daad609..14deedaa 100644 --- a/commit_verify/src/commit.rs +++ b/commit_verify/src/commit.rs @@ -139,7 +139,7 @@ pub(crate) mod test_helpers { acc.iter().for_each(|cmt| { // Testing that verification against other commitments // returns `false` - assert!(!cmt.verify(msg).is_ok()); + assert!(cmt.verify(msg).is_err()); }); // Detecting collision diff --git a/commit_verify/src/embed.rs b/commit_verify/src/embed.rs index cb4fb7cb..82e49602 100644 --- a/commit_verify/src/embed.rs +++ b/commit_verify/src/embed.rs @@ -240,7 +240,7 @@ pub(crate) mod test_helpers { acc.iter().for_each(|cmt| { // Testing that verification against other commitments // returns `false` - assert!(!cmt.clone().verify(msg, &proof).is_ok()); + assert!(cmt.clone().verify(msg, &proof).is_err()); }); // Detecting collision: each message should produce a unique @@ -286,7 +286,7 @@ pub(crate) mod test_helpers { acc.iter().for_each(|commitment| { // Testing that verification against other commitments // returns `false` - assert!(!SUPPLEMENT.verify(msg, commitment).is_ok()); + assert!(SUPPLEMENT.verify(msg, commitment).is_err()); }); // Detecting collision: each message should produce a unique