Merge pull request cosmos#61 from roysc/handle-placeholder-nodes

Check for placeholder nodes in proof verification

go/proof.go

@@ -210,27 +210,27 @@ func (p *NonExistenceProof) Verify(spec *ProofSpec, root CommitmentRoot, key []b
 	return nil
 }
 
-// IsLeftMost returns true if this is the left-most path in the tree
+// IsLeftMost returns true if this is the left-most path in the tree, excluding placeholder (empty child) nodes
 func IsLeftMost(spec *InnerSpec, path []*InnerOp) bool {
 	minPrefix, maxPrefix, suffix := getPadding(spec, 0)
 
-	// ensure every step has a prefix and suffix defined to be leftmost
+	// ensure every step has a prefix and suffix defined to be leftmost, unless it is a placeholder node
 	for _, step := range path {
-		if !hasPadding(step, minPrefix, maxPrefix, suffix) {
+		if !hasPadding(step, minPrefix, maxPrefix, suffix) && !leftBranchesAreEmpty(spec, step, 0) {
 			return false
 		}
 	}
 	return true
 }
 
-// IsRightMost returns true if this is the left-most path in the tree
+// IsRightMost returns true if this is the left-most path in the tree, excluding placeholder (empty child) nodes
 func IsRightMost(spec *InnerSpec, path []*InnerOp) bool {
 	last := len(spec.ChildOrder) - 1
 	minPrefix, maxPrefix, suffix := getPadding(spec, int32(last))
 
-	// ensure every step has a prefix and suffix defined to be rightmost
+	// ensure every step has a prefix and suffix defined to be rightmost, unless it is a placeholder node
 	for _, step := range path {
-		if !hasPadding(step, minPrefix, maxPrefix, suffix) {
+		if !hasPadding(step, minPrefix, maxPrefix, suffix) && !rightBranchesAreEmpty(spec, step, int32(last)) {
 			return false
 		}
 	}
@@ -285,6 +285,7 @@ func isLeftStep(spec *InnerSpec, left *InnerOp, right *InnerOp) bool {
 	return rightidx == leftidx+1
 }
 
+// checks if an op has the expected padding
 func hasPadding(op *InnerOp, minPrefix, maxPrefix, suffix int) bool {
 	if len(op.Prefix) < minPrefix {
 		return false
@@ -309,6 +310,51 @@ func getPadding(spec *InnerSpec, branch int32) (minPrefix, maxPrefix, suffix int
 	return
 }
 
+// leftBranchesAreEmpty returns true if the padding bytes correspond to all empty children
+// on the left side of this branch, ie. it's a valid placeholder on a leftmost path
+func leftBranchesAreEmpty(spec *InnerSpec, op *InnerOp, branch int32) bool {
+	idx := getPosition(spec.ChildOrder, branch)
+	// count branches to left of this
+	leftBranches := idx
+	if leftBranches == 0 {
+		return false
+	}
+	// compare prefix with the expected number of empty branches
+	actualPrefix := len(op.Prefix) - leftBranches*int(spec.ChildSize)
+	if actualPrefix < 0 {
+		return false
+	}
+	for i := 0; i < leftBranches; i++ {
+		from := actualPrefix + i*int(spec.ChildSize)
+		if !bytes.Equal(spec.EmptyChild, op.Prefix[from:from+int(spec.ChildSize)]) {
+			return false
+		}
+	}
+	return true
+}
+
+// rightBranchesAreEmpty returns true if the padding bytes correspond to all empty children
+// on the right side of this branch, ie. it's a valid placeholder on a rightmost path
+func rightBranchesAreEmpty(spec *InnerSpec, op *InnerOp, branch int32) bool {
+	idx := getPosition(spec.ChildOrder, branch)
+	// count branches to right of this one
+	rightBranches := len(spec.ChildOrder) - 1 - idx
+	if rightBranches == 0 {
+		return false
+	}
+	// compare suffix with the expected number of empty branches
+	if len(op.Suffix) != rightBranches*int(spec.ChildSize) {
+		return false // sanity check
+	}
+	for i := 0; i < rightBranches; i++ {
+		from := i * int(spec.ChildSize)
+		if !bytes.Equal(spec.EmptyChild, op.Suffix[from:from+int(spec.ChildSize)]) {
+			return false
+		}
+	}
+	return true
+}
+
 // getPosition checks where the branch is in the order and returns
 // the index of this branch
 func getPosition(order []int32, branch int32) int {

go/proof_data_test.go

@@ -69,3 +69,115 @@ func CheckAgainstSpecTestData(t *testing.T) map[string]CheckAgainstSpecTestStruc
 	}
 	return cases
 }
+
+var SpecWithEmptyChild = &ProofSpec{
+	LeafSpec: &LeafOp{
+		Prefix:       []byte{0},
+		Hash:         HashOp_SHA256,
+		PrehashValue: HashOp_SHA256,
+	},
+	InnerSpec: &InnerSpec{
+		ChildOrder:      []int32{0, 1},
+		ChildSize:       32,
+		MinPrefixLength: 1,
+		MaxPrefixLength: 1,
+		EmptyChild:      []byte("32_empty_child_placeholder_bytes"),
+		Hash:            HashOp_SHA256,
+	},
+}
+
+type EmptyBranchTestStruct struct {
+	Op      *InnerOp
+	Spec    *ProofSpec
+	IsLeft  bool
+	IsRight bool
+}
+
+func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct {
+	var emptyChild = SpecWithEmptyChild.InnerSpec.EmptyChild
+
+	return []EmptyBranchTestStruct{
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: append([]byte{1}, emptyChild...),
+				Suffix: nil,
+				Hash:   SpecWithEmptyChild.InnerSpec.Hash,
+			},
+			Spec:    SpecWithEmptyChild,
+			IsLeft:  true,
+			IsRight: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: emptyChild,
+				Hash:   SpecWithEmptyChild.InnerSpec.Hash,
+			},
+			Spec:    SpecWithEmptyChild,
+			IsLeft:  false,
+			IsRight: true,
+		},
+		// non-empty cases
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: append([]byte{1}, make([]byte, 32)...),
+				Suffix: nil,
+				Hash:   SpecWithEmptyChild.InnerSpec.Hash,
+			},
+			Spec:    SpecWithEmptyChild,
+			IsLeft:  false,
+			IsRight: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: make([]byte, 32),
+				Hash:   SpecWithEmptyChild.InnerSpec.Hash,
+			},
+			Spec:    SpecWithEmptyChild,
+			IsLeft:  false,
+			IsRight: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: append(append([]byte{1}, emptyChild[0:28]...), []byte("xxxx")...),
+				Suffix: nil,
+				Hash:   SpecWithEmptyChild.InnerSpec.Hash,
+			},
+			Spec:    SpecWithEmptyChild,
+			IsLeft:  false,
+			IsRight: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: append(append([]byte(nil), emptyChild[0:28]...), []byte("xxxx")...),
+				Hash:   SpecWithEmptyChild.InnerSpec.Hash,
+			},
+			Spec:    SpecWithEmptyChild,
+			IsLeft:  false,
+			IsRight: false,
+		},
+		// some cases using a spec with no empty child
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: append([]byte{1}, make([]byte, 32)...),
+				Suffix: nil,
+				Hash:   TendermintSpec.InnerSpec.Hash,
+			},
+			Spec:    TendermintSpec,
+			IsLeft:  false,
+			IsRight: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: make([]byte, 32),
+				Hash:   TendermintSpec.InnerSpec.Hash,
+			},
+			Spec:    TendermintSpec,
+			IsLeft:  false,
+			IsRight: false,
+		},
+	}
+}

go/proof_test.go

@@ -54,3 +54,25 @@ func TestCheckAgainstSpec(t *testing.T) {
 		})
 	}
 }
+
+func TestEmptyBranch(t *testing.T) {
+	cases := EmptyBranchTestData(t)
+
+	for _, tc := range cases {
+		t.Run("case", func(t *testing.T) {
+			if err := tc.Op.CheckAgainstSpec(tc.Spec); err != nil {
+				t.Errorf("Invalid InnerOp: %v", err)
+			}
+			order, err := orderFromPadding(tc.Spec.InnerSpec, tc.Op)
+			if err != nil {
+				t.Errorf("Cannot get orderFromPadding: %v", err)
+			}
+			if leftBranchesAreEmpty(tc.Spec.InnerSpec, tc.Op, order) != tc.IsLeft {
+				t.Errorf("Expected leftBranchesAreEmpty to be %t but it wasn't", tc.IsLeft)
+			}
+			if rightBranchesAreEmpty(tc.Spec.InnerSpec, tc.Op, order) != tc.IsRight {
+				t.Errorf("Expected rightBranchesAreEmpty to be %t but it wasn't", tc.IsRight)
+			}
+		})
+	}
+}

go/vectors_test.go

@@ -48,23 +48,26 @@ func TestBatchVectors(t *testing.T) {
 				// non-existence
 				valid := VerifyNonMembership(tc.Spec, tc.Ref.RootHash, tc.Proof, tc.Ref.Key)
 				if valid == tc.Invalid {
-					t.Fatalf("Expected proof validity: %t", !tc.Invalid)
+					t.Logf("name: %+v", name)
+					t.Logf("ref: %+v", tc.Ref)
+					t.Logf("spec: %+v", tc.Spec)
+					t.Errorf("Expected proof validity: %t", !tc.Invalid)
 				}
 				keys := [][]byte{tc.Ref.Key}
 				valid = BatchVerifyNonMembership(tc.Spec, tc.Ref.RootHash, tc.Proof, keys)
 				if valid == tc.Invalid {
-					t.Fatalf("Expected batch proof validity: %t", !tc.Invalid)
+					t.Errorf("Expected batch proof validity: %t", !tc.Invalid)
 				}
 			} else {
 				valid := VerifyMembership(tc.Spec, tc.Ref.RootHash, tc.Proof, tc.Ref.Key, tc.Ref.Value)
 				if valid == tc.Invalid {
-					t.Fatalf("Expected proof validity: %t", !tc.Invalid)
+					t.Errorf("Expected proof validity: %t", !tc.Invalid)
 				}
 				items := make(map[string][]byte)
 				items[string(tc.Ref.Key)] = tc.Ref.Value
 				valid = BatchVerifyMembership(tc.Spec, tc.Ref.RootHash, tc.Proof, items)
 				if valid == tc.Invalid {
-					t.Fatalf("Expected batch proof validity: %t", !tc.Invalid)
+					t.Errorf("Expected batch proof validity: %t", !tc.Invalid)
 				}
 			}
 		})