search

celestiaorg / smt

A Go library that implements a Sparse Merkle tree for a key-value map.
https://godoc.org/github.com/celestiaorg/smt
MIT License
138 stars 53 forks source link

(*SparseMerkleTree).HasDescend should firstly check the error from GetDescend before value comparisons #48

Closed odeke-em closed 3 years ago

odeke-em commented 3 years ago

A minor nit, but if we look at the code in https://github.com/celestiaorg/smt/blob/2c9076637cd90849e6e37a0be715973586568329/deepsubtree.go#L123-L126

it can potentially return (true, non-nil error) which is an odd return composite, we’d expect to have either: (true, nil error) (false, non-nil error)

If at any point defaultValue changes, the current assumption of bytes.Equal((*bytes)(nil), []byte{}) will fail so it is safer to fool proof that code by just checking the error from GetDescend firstly.

adlerjohn commented 3 years ago

Good catch. To document for posterity, #39 introduced non-trivial changes to the logic for getting leaves, so not unexpected there's still a few kinks to iron out.