Check for placeholder nodes in proof verification

Certain proofs (like those for a sparse Merkle tree) can contain empty nodes which are equal to the EmptyChild, but currently these will cause verification to fail for non-existence proofs if they appear where a real sibling node would be invalid, i.e. on a path that must be the leftmost or rightmost path of a subtree. This adds a check to detect and ignore such nodes during verification of left- and rightmost paths, as well as unit tests.
cosmos · Dec 15, 2021 · 0f8d2d4 · 0f8d2d4
1 parent e736db6
commit 0f8d2d4
Show file tree

Hide file tree

Showing 3 changed files with 194 additions and 0 deletions.
diff --git a/go/proof.go b/go/proof.go
@@ -217,6 +217,10 @@ func IsLeftMost(spec *InnerSpec, path []*InnerOp) bool {
 	// ensure every step has a prefix and suffix defined to be leftmost
 	for _, step := range path {
 		if !hasPadding(step, minPrefix, maxPrefix, suffix) {
+			// if this is a placeholder node, skip it
+			if leftBranchesAreEmpty(spec, step, 0) {
+				continue
+			}
 			return false
 		}
 	}
@@ -231,6 +235,10 @@ func IsRightMost(spec *InnerSpec, path []*InnerOp) bool {
 	// ensure every step has a prefix and suffix defined to be rightmost
 	for _, step := range path {
 		if !hasPadding(step, minPrefix, maxPrefix, suffix) {
+			// if this is a placeholder node, skip it
+			if rightBranchesAreEmpty(spec, step, int32(last)) {
+				continue
+			}
 			return false
 		}
 	}
@@ -309,6 +317,42 @@ 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)
+	// compare the prefix bytes with the appropriate number of empty children
+	leftChildren := len(spec.ChildOrder) - 1 - idx
+	actualPrefix := len(op.Prefix) - leftChildren*int(spec.ChildSize)
+	if actualPrefix < 0 {
+		return false
+	}
+	for i := 0; i < leftChildren; 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)
+	// compare the suffix bytes with the appropriate number of empty children
+	if len(op.Suffix) != idx*int(spec.ChildSize) {
+		return false
+	}
+	for i := 0; i < idx; 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 {

diff --git a/go/proof_data_test.go b/go/proof_data_test.go
@@ -69,3 +69,135 @@ func CheckAgainstSpecTestData(t *testing.T) map[string]CheckAgainstSpecTestStruc
 	}
 	return cases
 }
+
+type EmptyBranchTestStruct struct {
+	Op     *InnerOp
+	Spec   *InnerSpec
+	IsTrue bool
+	IsLeft bool
+}
+
+var InnerSpecWithEmptyChild = InnerSpec{
+	ChildOrder:      []int32{0, 1},
+	ChildSize:       32,
+	MinPrefixLength: 1,
+	MaxPrefixLength: 1,
+	EmptyChild:      []byte("32_empty_child_placeholder_bytes"),
+	Hash:            HashOp_SHA256,
+}
+
+func EmptyBranchTestData(t *testing.T) []EmptyBranchTestStruct {
+	return []EmptyBranchTestStruct{
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: InnerSpecWithEmptyChild.EmptyChild,
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: true,
+			IsLeft: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: append([]byte{1}, make([]byte, 32)...),
+				Suffix: InnerSpecWithEmptyChild.EmptyChild,
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: true,
+			IsLeft: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: make([]byte, 32),
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: false,
+			IsLeft: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: nil,
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: false,
+			IsLeft: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: append(InnerSpecWithEmptyChild.EmptyChild, []byte("xxxx")...),
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: false,
+			IsLeft: false,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: nil,
+			},
+			Spec:   IavlSpec.InnerSpec,
+			IsTrue: false,
+			IsLeft: false,
+		},
+
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: append([]byte{1}, InnerSpecWithEmptyChild.EmptyChild...),
+				Suffix: nil,
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: true,
+			IsLeft: true,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: append([]byte{1}, InnerSpecWithEmptyChild.EmptyChild...),
+				Suffix: make([]byte, 32),
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: true,
+			IsLeft: true,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: append([]byte{1}, make([]byte, 32)...),
+				Suffix: nil,
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: false,
+			IsLeft: true,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: nil,
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: false,
+			IsLeft: true,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: append(
+					append([]byte{1}, InnerSpecWithEmptyChild.EmptyChild...),
+					[]byte("xxxx")...),
+				Suffix: nil,
+			},
+			Spec:   &InnerSpecWithEmptyChild,
+			IsTrue: false,
+			IsLeft: true,
+		},
+		EmptyBranchTestStruct{
+			Op: &InnerOp{
+				Prefix: []byte{1},
+				Suffix: nil,
+			},
+			Spec:   IavlSpec.InnerSpec,
+			IsTrue: false,
+			IsLeft: true,
+		},
+	}
+}
diff --git a/go/proof_test.go b/go/proof_test.go
@@ -54,3 +54,21 @@ func TestCheckAgainstSpec(t *testing.T) {
 		})
 	}
 }
+
+func TestEmptyBranch(t *testing.T) {
+	cases := EmptyBranchTestData(t)
+
+	for i, tc := range cases {
+		var res bool
+		if tc.IsLeft {
+			res = leftBranchesAreEmpty(tc.Spec, tc.Op, 0)
+		} else {
+			res = rightBranchesAreEmpty(tc.Spec, tc.Op, 1)
+		}
+		if tc.IsTrue && !res {
+			t.Errorf("Result should be true, but was false (i=%v)", i)
+		} else if !tc.IsTrue && res {
+			t.Errorf("Result should be false, but was true (i=%v)", i)
+		}
+	}
+}