diff --git a/go/proof.go b/go/proof.go index d1e258f65..8e40d768d 100644 --- a/go/proof.go +++ b/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 { diff --git a/go/proof_data_test.go b/go/proof_data_test.go index 843ecd0a1..2f2fca22b 100644 --- a/go/proof_data_test.go +++ b/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, + }, + } +} diff --git a/go/proof_test.go b/go/proof_test.go index ccc9ae4bc..e67aedf65 100644 --- a/go/proof_test.go +++ b/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) + } + }) + } +} diff --git a/go/vectors_test.go b/go/vectors_test.go index 61f72882a..4054e4eb5 100644 --- a/go/vectors_test.go +++ b/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) } } }) diff --git a/rust/src/verify.rs b/rust/src/verify.rs index f4f3f2291..0d8074a1f 100644 --- a/rust/src/verify.rs +++ b/rust/src/verify.rs @@ -166,21 +166,25 @@ fn ensure_inner(inner: &ics23::InnerOp, spec: &ics23::ProofSpec) -> Result<()> { } } +// ensure_left_most fails unless this is the left-most path in the tree, excluding placeholder (empty child) nodes fn ensure_left_most(spec: &ics23::InnerSpec, path: &[ics23::InnerOp]) -> Result<()> { let pad = get_padding(spec, 0)?; + // ensure every step has a prefix and suffix defined to be leftmost, unless it is a placeholder node for step in path { - if !has_padding(step, &pad) { + if !has_padding(step, &pad) && !left_branches_are_empty(spec, step, 0)? { bail!("step not leftmost") } } Ok(()) } +// ensure_right_most returns true if this is the right-most path in the tree, excluding placeholder (empty child) nodes fn ensure_right_most(spec: &ics23::InnerSpec, path: &[ics23::InnerOp]) -> Result<()> { let idx = spec.child_order.len() - 1; let pad = get_padding(spec, idx as i32)?; + // ensure every step has a prefix and suffix defined to be rightmost, unless it is a placeholder node for step in path { - if !has_padding(step, &pad) { + if !has_padding(step, &pad) && !right_branches_are_empty(spec, step, idx as i32)? { bail!("step not leftmost") } } @@ -258,11 +262,71 @@ fn get_padding(spec: &ics23::InnerSpec, branch: i32) -> Result { } } +// left_branches_are_empty 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 +fn left_branches_are_empty( + spec: &ics23::InnerSpec, + op: &ics23::InnerOp, + branch: i32, +) -> Result { + if let Some(&idx) = spec.child_order.iter().find(|&&x| x == branch) { + // count branches to left of this + let left_branches = idx as usize; + if left_branches == 0 { + return Ok(false); + } + let child_size = spec.child_size as usize; + // compare prefix with the expected number of empty branches + let actual_prefix = match op.prefix.len().checked_sub(left_branches * child_size) { + Some(n) => n, + _ => return Ok(false), + }; + for i in 0..left_branches { + let from = actual_prefix + i * child_size; + if spec.empty_child != op.prefix[from..from + child_size] { + return Ok(false); + } + } + Ok(true) + } else { + bail!("Branch {} not found", branch); + } +} + +// right_branches_are_empty 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 +fn right_branches_are_empty( + spec: &ics23::InnerSpec, + op: &ics23::InnerOp, + branch: i32, +) -> Result { + if let Some(&idx) = spec.child_order.iter().find(|&&x| x == branch) { + // count branches to right of this one + let right_branches = spec.child_order.len() - 1 - idx as usize; + // compare suffix with the expected number of empty branches + if right_branches == 0 { + return Ok(false); + } + if op.suffix.len() != spec.child_size as usize { + return Ok(false); + } + for i in 0..right_branches { + let from = i * spec.child_size as usize; + if spec.empty_child != op.suffix[from..from + spec.child_size as usize] { + return Ok(false); + } + } + Ok(true) + } else { + bail!("Branch {} not found", branch); + } +} + #[cfg(test)] mod tests { use super::*; use crate::api; - use crate::ics23::{ExistenceProof, HashOp, InnerOp, LeafOp, LengthOp, ProofSpec}; + use crate::ics23::{ExistenceProof, HashOp, InnerOp, InnerSpec, LeafOp, LengthOp, ProofSpec}; use std::collections::btree_map::BTreeMap as HashMap; #[cfg(not(feature = "std"))] use std::prelude::*; @@ -526,4 +590,152 @@ mod tests { } } } + + fn spec_with_empty_child() -> ProofSpec { + let leaf = LeafOp { + hash: ics23::HashOp::Sha256.into(), + prehash_key: 0, + prehash_value: ics23::HashOp::Sha256.into(), + length: 0, + prefix: vec![0_u8], + }; + let inner = InnerSpec { + child_order: vec![0, 1], + child_size: 32, + min_prefix_length: 1, + max_prefix_length: 1, + empty_child: b"32_empty_child_placeholder_bytes".to_vec(), + hash: ics23::HashOp::Sha256.into(), + }; + ProofSpec { + leaf_spec: Some(leaf), + inner_spec: Some(inner), + min_depth: 0, + max_depth: 0, + } + } + + struct EmptyBranchCase<'a> { + op: InnerOp, + spec: &'a ProofSpec, + is_left: bool, + is_right: bool, + } + + #[test] + fn check_empty_branch() -> Result<()> { + let spec = &spec_with_empty_child(); + let inner_spec = spec.inner_spec.as_ref().unwrap(); + let empty_child = inner_spec.empty_child.clone(); + + let non_empty_spec = &api::tendermint_spec(); + let non_empty_inner = non_empty_spec.inner_spec.as_ref().unwrap(); + + let cases = vec![ + EmptyBranchCase { + op: ics23::InnerOp { + prefix: [&[1u8], &empty_child[..]].concat().to_vec(), + suffix: vec![], + hash: inner_spec.hash, + }, + spec: spec, + is_left: true, + is_right: false, + }, + EmptyBranchCase { + op: ics23::InnerOp { + prefix: vec![1u8], + suffix: empty_child.clone(), + hash: inner_spec.hash, + }, + spec: spec, + is_left: false, + is_right: true, + }, + // non-empty cases + EmptyBranchCase { + op: ics23::InnerOp { + prefix: [&[1u8], &[0u8; 32] as &[u8]].concat().to_vec(), + suffix: vec![], + hash: inner_spec.hash, + }, + spec: spec, + is_left: false, + is_right: false, + }, + EmptyBranchCase { + op: ics23::InnerOp { + prefix: vec![1u8], + suffix: vec![0u8; 32], + hash: inner_spec.hash, + }, + spec: spec, + is_left: false, + is_right: false, + }, + EmptyBranchCase { + op: ics23::InnerOp { + prefix: [&[1u8], &empty_child[..28], b"xxxx"].concat().to_vec(), + suffix: vec![], + hash: inner_spec.hash, + }, + spec: spec, + is_left: false, + is_right: false, + }, + EmptyBranchCase { + op: ics23::InnerOp { + prefix: vec![1u8], + suffix: [&empty_child[..28], b"xxxx"].concat().to_vec(), + hash: inner_spec.hash, + }, + spec: spec, + is_left: false, + is_right: false, + }, + // some cases using a spec with no empty child + EmptyBranchCase { + op: ics23::InnerOp { + prefix: [&[1u8], &[0u8; 32] as &[u8]].concat().to_vec(), + suffix: vec![], + hash: non_empty_inner.hash, + }, + spec: non_empty_spec, + is_left: false, + is_right: false, + }, + EmptyBranchCase { + op: ics23::InnerOp { + prefix: vec![1u8], + suffix: vec![0u8; 32], + hash: non_empty_inner.hash, + }, + spec: non_empty_spec, + is_left: false, + is_right: false, + }, + ]; + + for (i, case) in cases.iter().enumerate() { + ensure_inner(&case.op, case.spec)?; + let inner = &case.spec.inner_spec.as_ref().unwrap(); + let order = match order_from_padding(inner, &case.op) { + Ok(branch) => branch, + _ => bail!("invalid op"), + }; + assert_eq!( + case.is_left, + left_branches_are_empty(inner, &case.op, order)?, + "case {}", + i + ); + assert_eq!( + case.is_right, + right_branches_are_empty(inner, &case.op, order)?, + "case {}", + i + ); + } + Ok(()) + } }