cherrypick #136 (#199)

Author: crodriguezvega

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")
 		}
 	}

go/proofs.pb.go

@@ -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/generated/codecimpl.d.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/generated/codecimpl.js

@@ -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/ics23.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]);
+  });
 });

js/src/proofs.ts

@@ -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;
 }
 
 /*

js/src/testvectors.spec.ts

@@ -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();
 }