wip

lib/ics23/src/existence_proof.rs

@@ -1,25 +1,25 @@
 use unionlabs::cosmos::ics23::{existence_proof::ExistenceProof, proof_spec::ProofSpec};
 
-use crate::{inner_op, leaf_op};
+use crate::ops::{inner_op, leaf_op};
 
 #[derive(Debug, PartialEq, thiserror::Error)]
 pub enum SpecMismatchError {
     #[error("leaf spec mismatch ({0})")]
-    LeafSpecMismatch(super::leaf_op::SpecMismatchError),
+    LeafSpecMismatch(leaf_op::SpecMismatchError),
     #[error("inner op spec mismatch ({0})")]
-    InnerOpSpecMismatch(super::inner_op::SpecMismatchError),
+    InnerOpSpecMismatch(inner_op::SpecMismatchError),
     #[error("inner path depth too short, got ({path_len}) while the min depth is ({min_depth})")]
-    InnerDepthTooShort { path_len: usize, min_depth: i32 },
+    InnerDepthTooShort { path_len: usize, min_depth: usize },
     #[error("inner path depth too long, got ({path_len}) while the max depth is ({max_depth})")]
-    InnerDepthTooLong { path_len: usize, max_depth: i32 },
+    InnerDepthTooLong { path_len: usize, max_depth: usize },
 }
 
 #[derive(Debug, PartialEq, thiserror::Error)]
 pub enum CalculateRootError {
     #[error("leaf op hash ({0})")]
-    LeafOpHash(super::leaf_op::ApplyError),
+    LeafOpHash(leaf_op::ApplyError),
     #[error("inner op hash ({0})")]
-    InnerOpHash(super::inner_op::ApplyError),
+    InnerOpHash(inner_op::ApplyError),
     #[error("inner op hash does not match the spec")]
     InnerOpHashAndSpecMismatch,
 }
@@ -31,21 +31,26 @@ pub fn check_against_spec(
     leaf_op::check_against_spec(&existence_proof.leaf, spec)
         .map_err(SpecMismatchError::LeafSpecMismatch)?;
 
-    if spec.min_depth > 0 && existence_proof.path.len() < spec.min_depth as usize {
-        return Err(SpecMismatchError::InnerDepthTooShort {
-            path_len: existence_proof.path.len(),
-            min_depth: spec.min_depth,
-        });
+    if let Some(min_depth) = spec.min_depth {
+        if existence_proof.path.len() < min_depth.inner() {
+            return Err(SpecMismatchError::InnerDepthTooShort {
+                path_len: existence_proof.path.len(),
+                min_depth: min_depth.inner(),
+            });
+        }
     }
-    if spec.max_depth > 0 && existence_proof.path.len() > spec.max_depth as usize {
-        return Err(SpecMismatchError::InnerDepthTooLong {
-            path_len: existence_proof.path.len(),
-            max_depth: spec.max_depth,
-        });
+
+    if let Some(max_depth) = spec.max_depth {
+        if existence_proof.path.len() < max_depth.inner() {
+            return Err(SpecMismatchError::InnerDepthTooLong {
+                path_len: existence_proof.path.len(),
+                max_depth: max_depth.inner(),
+            });
+        }
     }
 
     for (index, inner) in existence_proof.path.iter().enumerate() {
-        inner_op::check_against_spec(inner, spec, index as i32 + 1)
+        inner_op::check_against_spec(inner, spec, index)
             .map_err(SpecMismatchError::InnerOpSpecMismatch)?;
     }
 
@@ -59,28 +64,33 @@ pub fn calculate_root(existence_proof: &ExistenceProof) -> Result<Vec<u8>, Calcu
     calculate(existence_proof, None)
 }
 
+/// Calculates the hash of the given proof, validating against the given spec if provided.
 pub(crate) fn calculate(
     existence_proof: &ExistenceProof,
     spec: Option<&ProofSpec>,
 ) -> Result<Vec<u8>, CalculateRootError> {
-    let res = leaf_op::apply(
+    let leaf_hash = leaf_op::apply(
         &existence_proof.leaf,
         &existence_proof.key,
         &existence_proof.value,
     )
     .map_err(CalculateRootError::LeafOpHash)?;
 
-    existence_proof.path.iter().try_fold(res, |res, step| {
-        let res = inner_op::apply(step, res).map_err(CalculateRootError::InnerOpHash)?;
+    existence_proof
+        .path
+        .iter()
+        .try_fold(leaf_hash, |res, step| {
+            let leaf_hash = inner_op::apply(step, &res).map_err(CalculateRootError::InnerOpHash)?;
 
-        if let Some(proof_spec) = spec {
-            if res.len() > proof_spec.inner_spec.child_size as usize
-                && proof_spec.inner_spec.child_size >= 32
-            {
-                return Err(CalculateRootError::InnerOpHashAndSpecMismatch);
+            if let Some(proof_spec) = spec {
+                if leaf_hash.len() > proof_spec.inner_spec.child_size.inner()
+                    // REVIEW: WHy is this >= 32 check here? Taken directly from https://github.com/cosmos/ics23/blob/master/go/proof.go#L140
+                    && proof_spec.inner_spec.child_size.inner() >= 32
+                {
+                    return Err(CalculateRootError::InnerOpHashAndSpecMismatch);
+                }
             }
-        }
 
-        Ok(res)
-    })
+            Ok(leaf_hash)
+        })
 }

lib/ics23/src/ibc_api.rs

@@ -39,7 +39,7 @@ pub fn verify_membership(
     proof: &MerkleProof,
     specs: &[ProofSpec],
     consensus_root: &MerkleRoot,
-    path: MerklePath,
+    path: &MerklePath,
     value: Vec<u8>,
 ) -> Result<(), VerifyMembershipError> {
     if proof.proofs.len() != specs.len() {
@@ -70,9 +70,10 @@ pub fn verify_non_membership(
     proof: &MerkleProof,
     specs: &[ProofSpec],
     consensus_root: &MerkleRoot,
-    path: MerklePath,
+    path: &MerklePath,
 ) -> Result<(), VerifyMembershipError> {
-    // this will also assert `specs` and `path.key_path` is not empty
+    // this will also assert `specs` and `path.key_path` is not empty, since they are all asserted
+    // to be the same length
     if proof.proofs.is_empty() {
         return Err(VerifyMembershipError::EmptyProof);
     }
@@ -97,13 +98,13 @@ pub fn verify_non_membership(
 
     // Even both are `Some`, still calculate the left branch
     let subroot = match (&nonexist.left, &nonexist.right) {
-        (Some(ep), None) | (None, Some(ep)) | (Some(ep), Some(_)) => {
+        (Some(ep), _) | (None, Some(ep)) => {
             existence_proof::calculate_root(ep).map_err(VerifyMembershipError::RootCalculation)?
         }
         _ => return Err(VerifyMembershipError::EmptyNonExistenceProof),
     };
 
-    let key = path.key_path[path.key_path.len() - 1].as_bytes();
+    let key = path.key_path.last().expect("len is >= 1").as_bytes();
     verify::verify_non_membership(&specs[0], &subroot, nonexist, key)
         .map_err(VerifyMembershipError::InnerVerification)?;
 
@@ -121,7 +122,7 @@ fn verify_chained_membership_proof(
     root: &[u8],
     specs: &[ProofSpec],
     proofs: &[CommitmentProof],
-    keys: MerklePath,
+    keys: &MerklePath,
     value: Vec<u8>,
     index: usize,
 ) -> Result<(), VerifyMembershipError> {
@@ -139,7 +140,9 @@ fn verify_chained_membership_proof(
 
             let key = keys
                 .key_path
-                .get(keys.key_path.len() - 1 - i)
+                .len()
+                .checked_sub(1 + i)
+                .and_then(|i| keys.key_path.get(i))
                 .ok_or(VerifyMembershipError::InvalidIndexing)?;
 
             verify::verify_membership(&specs[i], &subroot, existence_proof, key.as_bytes(), &value)
@@ -180,16 +183,16 @@ mod tests {
         path: &[&str],
     ) -> Result<(), VerifyMembershipError> {
         let path = MerklePath {
-            key_path: path.into_iter().map(|item| item.to_string()).collect(),
+            key_path: path.iter().map(ToString::to_string).collect(),
         };
-        let proofs = MerkleProof::try_from_proto_bytes(&proof).unwrap();
+        let proofs = MerkleProof::try_from_proto_bytes(proof).unwrap();
         verify_membership(
             &proofs,
             &SDK_SPECS,
             &MerkleRoot {
                 hash: H256::try_from(root).unwrap(),
             },
-            path,
+            &path,
             value.into(),
         )
     }
@@ -326,12 +329,8 @@ mod tests {
                 &proof,
                 &SDK_SPECS,
                 &root,
-                MerklePath {
-                    key_path: vec!["acc".to_string(), unsafe {
-                        String::from_utf8_unchecked(
-                            hex!("014152090b0c95c948edc407995560feed4a9df888").to_vec(),
-                        )
-                    }]
+                &MerklePath {
+                    key_path: vec!["acc".to_string(), "oogabooga".to_string()]
                 }
             ),
             Ok(())

lib/ics23/src/lib.rs

@@ -1,8 +1,9 @@
 #![feature(error_in_core)]
+#![warn(clippy::pedantic)]
+#![allow(clippy::missing_errors_doc)]
 
 pub mod existence_proof;
 pub mod ibc_api;
-mod ops;
+pub mod ops;
 pub mod proof_specs;
 pub mod verify;
-pub use ops::*;

lib/ics23/src/ops/inner_op.rs

@@ -1,8 +1,13 @@
-use unionlabs::cosmos::ics23::{hash_op::HashOp, inner_op::InnerOp, proof_spec::ProofSpec};
+use unionlabs::cosmos::ics23::{
+    hash_op::HashOp,
+    inner_op::InnerOp,
+    inner_spec::{InnerSpecMaxPrefixLen, InnerSpecMinPrefixLen},
+    proof_spec::ProofSpec,
+};
 
 use super::{hash_op, validate_iavl_ops};
 use crate::{
-    hash_op::HashError,
+    ops::hash_op::HashError,
     proof_specs::{self, IAVL_PROOF_SPEC},
 };
 
@@ -13,9 +18,15 @@ pub enum SpecMismatchError {
     #[error("prefix ({prefix}) is not the prefix of ({full})", prefix = serde_utils::to_hex(prefix), full = serde_utils::to_hex(full))]
     PrefixMismatch { full: Vec<u8>, prefix: Vec<u8> },
     #[error("inner prefix too short, got ({prefix_len}) while the min length is ({min_len})")]
-    InnerOpPrefixTooShort { prefix_len: usize, min_len: i32 },
+    InnerOpPrefixTooShort {
+        prefix_len: usize,
+        min_len: InnerSpecMinPrefixLen,
+    },
     #[error("inner prefix too long, got ({prefix_len}) while the max length is ({max_len})")]
-    InnerOpPrefixTooLong { prefix_len: usize, max_len: i32 },
+    InnerOpPrefixTooLong {
+        prefix_len: usize,
+        max_len: InnerSpecMaxPrefixLen,
+    },
     #[error("malformed inner op suffix ({0})")]
     InnerOpSuffixMalformed(usize),
     #[error("validate iavl ops ({0})")]
@@ -35,23 +46,23 @@ pub enum ApplyError {
 pub fn check_against_spec(
     inner_op: &InnerOp,
     spec: &ProofSpec,
-    b: i32,
+    b: usize,
 ) -> Result<(), SpecMismatchError> {
     if inner_op.hash != spec.inner_spec.hash {
         return Err(SpecMismatchError::UnexpectedHashOp(inner_op.hash));
     }
 
     if proof_specs::compatible(spec, &IAVL_PROOF_SPEC) {
         match validate_iavl_ops(&inner_op.prefix, b) {
-            Ok(remaining) => {
-                if remaining != 1 && remaining != 34 {
-                    return Err(SpecMismatchError::BadPrefix(remaining));
-                }
-
+            // REVIEW: What?
+            Ok(1 | 34) => {
                 if inner_op.hash != HashOp::Sha256 {
                     return Err(SpecMismatchError::UnexpectedHashOp(inner_op.hash));
                 }
             }
+            Ok(remaining) => {
+                return Err(SpecMismatchError::BadPrefix(remaining));
+            }
             Err(e) => return Err(SpecMismatchError::ValidateIavlOps(e)),
         }
     }
@@ -63,15 +74,15 @@ pub fn check_against_spec(
         });
     }
 
-    if inner_op.prefix.len() < spec.inner_spec.min_prefix_length as usize {
+    if inner_op.prefix.len() < spec.inner_spec.min_prefix_length.inner() {
         return Err(SpecMismatchError::InnerOpPrefixTooShort {
             prefix_len: inner_op.prefix.len(),
             min_len: spec.inner_spec.min_prefix_length,
         });
     }
 
-    let max_prefix_length = spec.inner_spec.max_prefix_length as usize
-        + (spec.inner_spec.child_order.len() - 1) * spec.inner_spec.child_size as usize;
+    let max_prefix_length = spec.inner_spec.max_prefix_length.inner()
+        + (spec.inner_spec.child_order.len() - 1) * spec.inner_spec.child_size.inner();
 
     if inner_op.prefix.len() > max_prefix_length {
         return Err(SpecMismatchError::InnerOpPrefixTooLong {
@@ -80,7 +91,7 @@ pub fn check_against_spec(
         });
     }
 
-    if inner_op.suffix.len() % (spec.inner_spec.child_size as usize) != 0 {
+    if inner_op.suffix.len() % spec.inner_spec.child_size.inner() != 0 {
         return Err(SpecMismatchError::InnerOpSuffixMalformed(
             inner_op.suffix.len(),
         ));
@@ -89,13 +100,13 @@ pub fn check_against_spec(
     Ok(())
 }
 
-pub fn apply(inner_op: &InnerOp, child: Vec<u8>) -> Result<Vec<u8>, ApplyError> {
+pub fn apply(inner_op: &InnerOp, child: &[u8]) -> Result<Vec<u8>, ApplyError> {
     if child.is_empty() {
         return Err(ApplyError::InnerOpNeedsChildValue);
     }
 
     let mut preimage = inner_op.prefix.clone();
-    preimage.extend_from_slice(&child);
+    preimage.extend_from_slice(child);
     preimage.extend_from_slice(&inner_op.suffix);
 
     Ok(hash_op::do_hash(inner_op.hash, &preimage)?)