cherrypick #136 (#199)

go/ics23.go

@@ -52,7 +52,7 @@ func VerifyMembership(spec *ProofSpec, root CommitmentRoot, proof *CommitmentPro
 func VerifyNonMembership(spec *ProofSpec, root CommitmentRoot, proof *CommitmentProof, key []byte) bool {
 	// decompress it before running code (no-op if not compressed)
 	proof = Decompress(proof)
-	np := getNonExistProofForKey(proof, key)
+	np := getNonExistProofForKey(spec, proof, key)
 	if np == nil {
 		return false
 	}
@@ -148,27 +148,27 @@ func getExistProofForKey(proof *CommitmentProof, key []byte) *ExistenceProof {
 	return nil
 }
 
-func getNonExistProofForKey(proof *CommitmentProof, key []byte) *NonExistenceProof {
+func getNonExistProofForKey(spec *ProofSpec, proof *CommitmentProof, key []byte) *NonExistenceProof {
 	switch p := proof.Proof.(type) {
 	case *CommitmentProof_Nonexist:
 		np := p.Nonexist
-		if isLeft(np.Left, key) && isRight(np.Right, key) {
+		if isLeft(spec, np.Left, key) && isRight(spec, np.Right, key) {
 			return np
 		}
 	case *CommitmentProof_Batch:
 		for _, sub := range p.Batch.Entries {
-			if np := sub.GetNonexist(); np != nil && isLeft(np.Left, key) && isRight(np.Right, key) {
+			if np := sub.GetNonexist(); np != nil && isLeft(spec, np.Left, key) && isRight(spec, np.Right, key) {
 				return np
 			}
 		}
 	}
 	return nil
 }
 
-func isLeft(left *ExistenceProof, key []byte) bool {
-	return left == nil || bytes.Compare(left.Key, key) < 0
+func isLeft(spec *ProofSpec, left *ExistenceProof, key []byte) bool {
+	return left == nil || bytes.Compare(keyForComparison(spec, left.Key), keyForComparison(spec, key)) < 0
 }
 
-func isRight(right *ExistenceProof, key []byte) bool {
-	return right == nil || bytes.Compare(right.Key, key) > 0
+func isRight(spec *ProofSpec, right *ExistenceProof, key []byte) bool {
+	return right == nil || bytes.Compare(keyForComparison(spec, right.Key), keyForComparison(spec, key)) > 0
 }

go/proof.go

@@ -47,7 +47,7 @@ var TendermintSpec = &ProofSpec{
 var SmtSpec = &ProofSpec{
 	LeafSpec: &LeafOp{
 		Hash:         HashOp_SHA256,
-		PrehashKey:   HashOp_NO_HASH,
+		PrehashKey:   HashOp_SHA256,
 		PrehashValue: HashOp_SHA256,
 		Length:       LengthOp_NO_PREFIX,
 		Prefix:       []byte{0},
@@ -60,7 +60,8 @@ var SmtSpec = &ProofSpec{
 		EmptyChild:      make([]byte, 32),
 		Hash:            HashOp_SHA256,
 	},
-	MaxDepth: 256,
+	MaxDepth:                   256,
+	PrehashKeyBeforeComparison: true,
 }
 
 func encodeVarintProto(l int) []byte {
@@ -204,6 +205,17 @@ func (p *ExistenceProof) CheckAgainstSpec(spec *ProofSpec) error {
 	return nil
 }
 
+// If we should prehash the key before comparison, do so; otherwise, return the key. Prehashing
+// changes lexical comparison, so we do so before comparison if the spec sets
+// `PrehashKeyBeforeComparison`.
+func keyForComparison(spec *ProofSpec, key []byte) []byte {
+	if !spec.PrehashKeyBeforeComparison {
+		return key
+	}
+	hash, _ := doHashOrNoop(spec.LeafSpec.PrehashKey, key)
+	return hash
+}
+
 // Verify does all checks to ensure the proof has valid non-existence proofs,
 // and they ensure the given key is not in the CommitmentState
 func (p *NonExistenceProof) Verify(spec *ProofSpec, root CommitmentRoot, key []byte) error {
@@ -229,13 +241,13 @@ func (p *NonExistenceProof) Verify(spec *ProofSpec, root CommitmentRoot, key []b
 
 	// Ensure in valid range
 	if rightKey != nil {
-		if bytes.Compare(key, rightKey) >= 0 {
+		if bytes.Compare(keyForComparison(spec, key), keyForComparison(spec, rightKey)) >= 0 {
 			return errors.New("key is not left of right proof")
 		}
 	}
 
 	if leftKey != nil {
-		if bytes.Compare(key, leftKey) <= 0 {
+		if bytes.Compare(keyForComparison(spec, key), keyForComparison(spec, leftKey)) <= 0 {
 			return errors.New("key is not right of left proof")
 		}
 	}

js/src/generated/codecimpl.d.ts

@@ -703,6 +703,9 @@ export namespace ics23 {
 
     /** ProofSpec minDepth */
     minDepth?: number | null;
+
+    /** ProofSpec prehashKeyBeforeComparison */
+    prehashKeyBeforeComparison?: boolean | null;
   }
 
   /**
@@ -736,6 +739,9 @@ export namespace ics23 {
     /** ProofSpec minDepth. */
     public minDepth: number;
 
+    /** ProofSpec prehashKeyBeforeComparison. */
+    public prehashKeyBeforeComparison: boolean;
+
     /**
      * Creates a new ProofSpec instance using the specified properties.
      * @param [properties] Properties to set

js/src/ics23.ts

@@ -1,8 +1,8 @@
 import { decompress } from "./compress";
 import { ics23 } from "./generated/codecimpl";
 import { CommitmentRoot, verifyExistence, verifyNonExistence } from "./proofs";
+import { keyForComparison } from "./proofs";
 import { bytesBefore, bytesEqual } from "./specs";
-
 /*
 This implements the client side functions as specified in
 https://github.com/cosmos/ics/tree/master/spec/ics-023-vector-commitments
@@ -59,7 +59,7 @@ export function verifyNonMembership(
   key: Uint8Array
 ): boolean {
   const norm = decompress(proof);
-  const nonexist = getNonExistForKey(norm, key);
+  const nonexist = getNonExistForKey(spec, norm, key);
   if (!nonexist) {
     return false;
   }
@@ -122,14 +122,23 @@ function getExistForKey(
 }
 
 function getNonExistForKey(
+  spec: ics23.IProofSpec,
   proof: ics23.ICommitmentProof,
   key: Uint8Array
 ): ics23.INonExistenceProof | undefined | null {
   const match = (p: ics23.INonExistenceProof | null | undefined): boolean => {
     return (
       !!p &&
-      (!p.left || bytesBefore(p.left.key!, key)) &&
-      (!p.right || bytesBefore(key, p.right.key!))
+      (!p.left ||
+        bytesBefore(
+          keyForComparison(spec, p.left.key!),
+          keyForComparison(spec, key)
+        )) &&
+      (!p.right ||
+        bytesBefore(
+          keyForComparison(spec, key),
+          keyForComparison(spec, p.right.key!)
+        ))
     );
   };
   if (match(proof.nonexist)) {

js/src/proofs.ts

@@ -1,5 +1,6 @@
 import { ics23 } from "./generated/codecimpl";
 import { applyInner, applyLeaf } from "./ops";
+import { doHash } from "./ops";
 import {
   bytesEqual,
   ensureBytesBefore,
@@ -45,7 +46,7 @@ export const tendermintSpec: ics23.IProofSpec = {
 export const smtSpec: ics23.IProofSpec = {
   leafSpec: {
     hash: ics23.HashOp.SHA256,
-    prehashKey: ics23.HashOp.NO_HASH,
+    prehashKey: ics23.HashOp.SHA256,
     prehashValue: ics23.HashOp.SHA256,
     length: ics23.LengthOp.NO_PREFIX,
     prefix: Uint8Array.from([0]),
@@ -59,10 +60,22 @@ export const smtSpec: ics23.IProofSpec = {
     hash: ics23.HashOp.SHA256,
   },
   maxDepth: 256,
+  prehashKeyBeforeComparison: true,
 };
 
 export type CommitmentRoot = Uint8Array;
 
+export function keyForComparison(
+  spec: ics23.IProofSpec,
+  key: Uint8Array
+): Uint8Array {
+  if (!spec.prehashKeyBeforeComparison) {
+    return key;
+  }
+
+  return doHash(spec.leafSpec!.prehashKey!, key);
+}
+
 // verifyExistence will throw an error if the proof doesn't link key, value -> root
 // or if it doesn't fulfill the spec
 export function verifyExistence(
@@ -111,10 +124,16 @@ export function verifyNonExistence(
   }
 
   if (leftKey) {
-    ensureBytesBefore(leftKey, key);
+    ensureBytesBefore(
+      keyForComparison(spec, leftKey),
+      keyForComparison(spec, key)
+    );
   }
   if (rightKey) {
-    ensureBytesBefore(key, rightKey);
+    ensureBytesBefore(
+      keyForComparison(spec, key),
+      keyForComparison(spec, rightKey)
+    );
   }
 
   if (!spec.innerSpec) {

js/src/testvectors.spec.ts

@@ -8,7 +8,7 @@ import {
   verifyMembership,
   verifyNonMembership,
 } from "./ics23";
-import { iavlSpec, tendermintSpec } from "./proofs";
+import { iavlSpec, smtSpec, tendermintSpec } from "./proofs";
 import { fromHex } from "./testhelpers.spec";
 
 describe("calculateExistenceRoot", () => {
@@ -251,4 +251,28 @@ describe("calculateExistenceRoot", () => {
     ]);
     validateBatch(proof, tendermintSpec, data[3]);
   });
+
+  it("should validate smt batch exist", () => {
+    const { proof, data } = loadBatch([
+      "../testdata/smt/exist_left.json",
+      "../testdata/smt/exist_right.json",
+      "../testdata/smt/exist_middle.json",
+      "../testdata/smt/nonexist_left.json",
+      "../testdata/smt/nonexist_right.json",
+      "../testdata/smt/nonexist_middle.json",
+    ]);
+    validateBatch(proof, smtSpec, data[2]);
+  });
+
+  it("should validate smt batch nonexist", () => {
+    const { proof, data } = loadBatch([
+      "../testdata/smt/exist_left.json",
+      "../testdata/smt/exist_right.json",
+      "../testdata/smt/exist_middle.json",
+      "../testdata/smt/nonexist_left.json",
+      "../testdata/smt/nonexist_right.json",
+      "../testdata/smt/nonexist_middle.json",
+    ]);
+    validateBatch(proof, smtSpec, data[3]);
+  });
 });

proofs.proto

@@ -162,6 +162,10 @@ message ProofSpec {
   int32 max_depth = 3;
   // min_depth (if > 0) is the minimum number of InnerOps allowed (mainly for fixed-depth tries)
   int32 min_depth = 4;
+  // prehash_key_before_comparison is a flag that indicates whether to use the
+  // prehash_key specified by LeafOp to compare lexical ordering of keys for
+  // non-existence proofs.
+  bool prehash_key_before_comparison = 5;
 }
 
 /*

rust/codegen/src/main.rs

@@ -13,7 +13,9 @@ fn main() {
     let out_dir: &str = &format!("{}{}", root, "/rust/src");
     let input: &str = &format!("{}{}", root, "/proofs.proto");
 
-    let mut cfg = prost_build::Config::new();
-    cfg.out_dir(&out_dir);
-    cfg.compile_protos(&[input], &[root]).unwrap();
+    prost_build::Config::new()
+        .out_dir(&out_dir)
+        .format(true)
+        .compile_protos(&[input], &[root])
+        .unwrap();
 }

rust/src/api.rs

@@ -4,6 +4,7 @@ use alloc::vec;
 use crate::compress::{decompress, is_compressed};
 use crate::host_functions::HostFunctionsProvider;
 use crate::ics23;
+use crate::ops::do_hash;
 use crate::verify::{verify_existence, verify_non_existence, CommitmentRoot};
 
 // Use CommitmentRoot vs &[u8] to stick with ics naming
@@ -56,7 +57,7 @@ pub fn verify_non_membership<H: HostFunctionsProvider>(
         }
     }
 
-    if let Some(non) = get_nonexist_proof(proof, key) {
+    if let Some(non) = get_nonexist_proof::<H>(proof, key, spec) {
         let valid = verify_non_existence::<H>(non, spec, root, key);
         valid.is_ok()
     } else {
@@ -131,18 +132,53 @@ fn get_exist_proof<'a>(
     }
 }
 
-fn get_nonexist_proof<'a>(
+fn get_nonexist_proof<'a, H: HostFunctionsProvider>(
     proof: &'a ics23::CommitmentProof,
     key: &[u8],
+    spec: &ics23::ProofSpec,
 ) -> Option<&'a ics23::NonExistenceProof> {
+    // If we should prehash the key before comparison, do so; otherwise, return the key. Prehashing
+    // changes lexical comparison, so if the spec sets `prehash_key_before_comparison` to true, we
+    // should compare keys on their hashes by prehashing them before comparison.
+    let key_for_comparison = |key: &[u8]| match spec.prehash_key_before_comparison {
+        true => do_hash::<H>(
+            spec.leaf_spec
+                .as_ref()
+                .map(|leaf_spec| leaf_spec.prehash_key())
+                .unwrap_or_default(),
+            key,
+        ),
+        false => key.to_vec(),
+    };
+
     match &proof.proof {
-        Some(ics23::commitment_proof::Proof::Nonexist(non)) => Some(non),
+        Some(ics23::commitment_proof::Proof::Nonexist(non)) => {
+            // use iter/all - true if None, must check if Some
+            if non
+                .left
+                .iter()
+                .all(|x| key_for_comparison(&x.key) < key_for_comparison(key))
+                && non
+                    .right
+                    .iter()
+                    .all(|x| key_for_comparison(&x.key) > key_for_comparison(key))
+            {
+                return Some(non);
+            }
+            None
+        }
         Some(ics23::commitment_proof::Proof::Batch(batch)) => {
             for entry in &batch.entries {
                 if let Some(ics23::batch_entry::Proof::Nonexist(non)) = &entry.proof {
                     // use iter/all - true if None, must check if Some
-                    if non.left.iter().all(|x| x.key.as_slice() < key)
-                        && non.right.iter().all(|x| x.key.as_slice() > key)
+                    if non
+                        .left
+                        .iter()
+                        .all(|x| key_for_comparison(&x.key) < key_for_comparison(key))
+                        && non
+                            .right
+                            .iter()
+                            .all(|x| key_for_comparison(&x.key) > key_for_comparison(key))
                     {
                         return Some(non);
                     }
@@ -176,6 +212,7 @@ pub fn iavl_spec() -> ics23::ProofSpec {
         inner_spec: Some(inner),
         min_depth: 0,
         max_depth: 0,
+        prehash_key_before_comparison: false,
     }
 }
 
@@ -200,13 +237,14 @@ pub fn tendermint_spec() -> ics23::ProofSpec {
         inner_spec: Some(inner),
         min_depth: 0,
         max_depth: 0,
+        prehash_key_before_comparison: false,
     }
 }
 
 pub fn smt_spec() -> ics23::ProofSpec {
     let leaf = ics23::LeafOp {
         hash: ics23::HashOp::Sha256.into(),
-        prehash_key: 0,
+        prehash_key: ics23::HashOp::Sha256.into(),
         prehash_value: ics23::HashOp::Sha256.into(),
         length: 0,
         prefix: vec![0_u8],
@@ -224,6 +262,7 @@ pub fn smt_spec() -> ics23::ProofSpec {
         inner_spec: Some(inner),
         min_depth: 0,
         max_depth: 0,
+        prehash_key_before_comparison: true,
     }
 }