Merge pull request #145 from LNP-BP/fix/verify_api_alt

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<Msg, Protocol: CommitmentProtocol>
 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<Msg, Protocol: CommitmentProtocol>
 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<Self, Self::Error>;
 
-    /// 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<bool, Self::Error> {
-        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_err());
                 });
 
                 // Detecting collision

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<Msg, Source, Protocol>
 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<bool, Source::CommitError>
+    ) -> 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(())
     }
 }
 

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<E: std::error::Error> {
+    /// 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<Container, Container::VerifyError>;
+    ) -> Result<Container, EmbedVerifyError<Container::CommitError>>;
 }
 
 /// 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<Msg, Self, Protocol>;
 
     /// 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<Self::CommitError>;
-
     /// 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<bool, Self::VerifyError>
+    /// 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<Self::CommitError>>
     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_err());
                 });
 
                 // 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_err());
                 });
 
                 // Detecting collision: each message should produce a unique
@@ -303,7 +325,10 @@ mod test {
     impl<T> EmbedCommitProof<T, DummyVec, TestProtocol> for DummyProof
     where T: AsRef<[u8]> + Clone + CommitEncode
     {
-        fn restore_original_container(&self, _: &DummyVec) -> Result<DummyVec, Error> {
+        fn restore_original_container(
+            &self,
+            _: &DummyVec,
+        ) -> Result<DummyVec, EmbedVerifyError<Error>> {
             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<Self::Proof, Self::CommitError> {
             let proof = self.0.clone();

commit_verify/src/lib.rs

@@ -58,11 +58,11 @@ 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 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;