Skip to content

Commit b34f794

Browse files
authored
feat!: updates the HashNode and HashLeaf methods to return error instead of panic and refactors the code (celestiaorg#136)
## Overview Closes celestiaorg#109 and celestiaorg#99. This pull request ensures that the `HashNode` and `HashLeaf` functions return errors instead of panicking, and that caller functions properly handle any emitted errors. When generating proofs, any hashing errors are propagated with additional context. During verification, hashing errors are treated as unsuccessful proof verification. Note that in this PR, the term `Irrecoverable errors` used to describe errors that any number of retries can correct the them. ## Checklist - [x] New and updated code has appropriate documentation - [x] New and updated code has new and/or updated testing - [x] Required CI checks are passing - [x] Visual proof for any user facing features like CLI or documentation updates - [x] Linked issues closed with keywords
1 parent c63e970 commit b34f794

File tree

7 files changed

+901
-175
lines changed

7 files changed

+901
-175
lines changed

fuzz_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import (
66
"sort"
77
"testing"
88

9+
"github.com/stretchr/testify/require"
10+
911
"github.com/celestiaorg/nmt"
1012
"github.com/celestiaorg/nmt/namespace"
1113
fuzz "github.com/google/gofuzz"
@@ -44,7 +46,8 @@ func TestFuzzProveVerifyNameSpace(t *testing.T) {
4446
}
4547
}
4648

47-
treeRoot := tree.Root()
49+
treeRoot, err := tree.Root()
50+
require.NoError(t, err)
4851
nonEmptyNsCount := 0
4952
leafIdx := 0
5053
for _, ns := range sortedKeys {

hasher.go

Lines changed: 57 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ var _ hash.Hash = (*Hasher)(nil)
1919
var (
2020
ErrUnorderedSiblings = errors.New("NMT sibling nodes should be ordered lexicographically by namespace IDs")
2121
ErrInvalidNodeLen = errors.New("invalid NMT node size")
22+
ErrInvalidLeafLen = errors.New("invalid NMT leaf size")
2223
)
2324

2425
type Hasher struct {
@@ -65,6 +66,8 @@ func (n *Hasher) Size() int {
6566
//
6667
// Requires data of fixed size to match leaf or inner NMT nodes. Only a single
6768
// write is allowed.
69+
// It panics if more than one single write is attempted.
70+
// If the data does not match the format of an NMT non-leaf node or leaf node, an error will be returned.
6871
func (n *Hasher) Write(data []byte) (int, error) {
6972
if n.data != nil {
7073
panic("only a single Write is allowed")
@@ -74,9 +77,19 @@ func (n *Hasher) Write(data []byte) (int, error) {
7477
switch ln {
7578
// inner nodes are made up of the nmt hashes of the left and right children
7679
case n.Size() * 2:
80+
// check the format of the data
81+
leftChild := data[:n.Size()]
82+
rightChild := data[n.Size():]
83+
if err := n.ValidateNodes(leftChild, rightChild); err != nil {
84+
return 0, err
85+
}
7786
n.tp = NodePrefix
7887
// leaf nodes contain the namespace length and a share
7988
default:
89+
// validate the format of the leaf
90+
if err := n.ValidateLeaf(data); err != nil {
91+
return 0, err
92+
}
8093
n.tp = LeafPrefix
8194
}
8295

@@ -86,16 +99,25 @@ func (n *Hasher) Write(data []byte) (int, error) {
8699

87100
// Sum computes the hash. Does not append the given suffix, violating the
88101
// interface.
102+
// It may panic if the data being hashed is invalid. This should never happen since the Write method refuses an invalid data and errors out.
89103
func (n *Hasher) Sum([]byte) []byte {
90104
switch n.tp {
91105
case LeafPrefix:
92-
return n.HashLeaf(n.data)
106+
res, err := n.HashLeaf(n.data)
107+
if err != nil {
108+
panic(err) // this should never happen since the data is already validated in the Write method
109+
}
110+
return res
93111
case NodePrefix:
94112
flagLen := int(n.NamespaceLen) * 2
95113
sha256Len := n.baseHasher.Size()
96114
leftChild := n.data[:flagLen+sha256Len]
97115
rightChild := n.data[flagLen+sha256Len:]
98-
return n.HashNode(leftChild, rightChild)
116+
res, err := n.HashNode(leftChild, rightChild)
117+
if err != nil {
118+
panic(err) // this should never happen since the data is already validated in the Write method
119+
}
120+
return res
99121
default:
100122
panic("nmt node type wasn't set")
101123
}
@@ -120,30 +142,28 @@ func (n *Hasher) EmptyRoot() []byte {
120142
return digest
121143
}
122144

123-
// IsNamespacedData checks whether data is namespace prefixed.
124-
func (n *Hasher) IsNamespacedData(data []byte) (err error) {
145+
// ValidateLeaf verifies if data is namespaced and returns an error if not.
146+
func (n *Hasher) ValidateLeaf(data []byte) (err error) {
125147
nidSize := int(n.NamespaceSize())
126148
lenData := len(data)
127149
if lenData < nidSize {
128-
return fmt.Errorf("%w: got: %v, want >= %v", ErrMismatchedNamespaceSize, lenData, nidSize)
150+
return fmt.Errorf("%w: got: %v, want >= %v", ErrInvalidLeafLen, lenData, nidSize)
129151
}
130152
return nil
131153
}
132154

133155
// HashLeaf computes namespace hash of the namespaced data item `ndata` as
134156
// ns(ndata) || ns(ndata) || hash(leafPrefix || ndata), where ns(ndata) is the
135157
// namespaceID inside the data item namely leaf[:n.NamespaceLen]). Note that for
136-
// leaves minNs = maxNs = ns(leaf) = leaf[:NamespaceLen]. HashLeaf can panic if
137-
// the input is not properly namespaced. To avoid panic, call IsNamespacedData
138-
// on the input data `ndata` before invoking HashLeaf method.
158+
// leaves minNs = maxNs = ns(leaf) = leaf[:NamespaceLen]. HashLeaf can return the ErrInvalidNodeLen error if the input is not namespaced.
139159
//
140160
//nolint:errcheck
141-
func (n *Hasher) HashLeaf(ndata []byte) []byte {
161+
func (n *Hasher) HashLeaf(ndata []byte) ([]byte, error) {
142162
h := n.baseHasher
143163
h.Reset()
144164

145-
if err := n.IsNamespacedData(ndata); err != nil {
146-
panic(err)
165+
if err := n.ValidateLeaf(ndata); err != nil {
166+
return nil, err
147167
}
148168

149169
nID := ndata[:n.NamespaceLen]
@@ -160,12 +180,12 @@ func (n *Hasher) HashLeaf(ndata []byte) []byte {
160180

161181
// compute h(LeafPrefix || ndata) and append it to the minMaxNIDs
162182
nameSpacedHash := h.Sum(minMaxNIDs) // nID || nID || h(LeafPrefix || ndata)
163-
return nameSpacedHash
183+
return nameSpacedHash, nil
164184
}
165185

166-
// validateNodeFormat checks whether the supplied node conforms to the
167-
// namespaced hash format.
168-
func (n *Hasher) validateNodeFormat(node []byte) (err error) {
186+
// ValidateNodeFormat checks whether the supplied node conforms to the
187+
// namespaced hash format and returns an error if it does not. Specifically, it returns ErrInvalidNodeLen if the length of the node is less than the 2*namespace length which indicates it does not match the namespaced hash format.
188+
func (n *Hasher) ValidateNodeFormat(node []byte) (err error) {
169189
totalNamespaceLen := 2 * n.NamespaceLen
170190
nodeLen := len(node)
171191
if nodeLen < int(totalNamespaceLen) {
@@ -177,9 +197,9 @@ func (n *Hasher) validateNodeFormat(node []byte) (err error) {
177197
// validateSiblingsNamespaceOrder checks whether left and right as two sibling
178198
// nodes in an NMT have correct namespace IDs relative to each other, more
179199
// specifically, the maximum namespace ID of the left sibling should not exceed
180-
// the minimum namespace ID of the right sibling. Note that the function assumes
200+
// the minimum namespace ID of the right sibling. It returns ErrUnorderedSiblings error if the check fails. Note that the function assumes
181201
// that the left and right nodes are in correct format, i.e., they are
182-
// namespaced hash values.
202+
// namespaced hash values. Otherwise, it panics.
183203
func (n *Hasher) validateSiblingsNamespaceOrder(left, right []byte) (err error) {
184204
// each NMT node has two namespace IDs for the min and max
185205
totalNamespaceLen := 2 * n.NamespaceLen
@@ -193,55 +213,50 @@ func (n *Hasher) validateSiblingsNamespaceOrder(left, right []byte) (err error)
193213
return nil
194214
}
195215

196-
// ValidateNodes is helper function to be called prior to HashNode to verify the
197-
// validity of the inputs of HashNode and avoid panics. It verifies whether left
216+
// ValidateNodes is a helper function to verify the
217+
// validity of the inputs of HashNode. It verifies whether left
198218
// and right comply by the namespace hash format, and are correctly ordered
199219
// according to their namespace IDs.
200220
func (n *Hasher) ValidateNodes(left, right []byte) error {
201-
if err := n.validateNodeFormat(left); err != nil {
221+
if err := n.ValidateNodeFormat(left); err != nil {
202222
return err
203223
}
204-
if err := n.validateNodeFormat(right); err != nil {
224+
if err := n.ValidateNodeFormat(right); err != nil {
205225
return err
206226
}
207-
if err := n.validateSiblingsNamespaceOrder(left, right); err != nil {
208-
return err
209-
}
210-
return nil
227+
return n.validateSiblingsNamespaceOrder(left, right)
211228
}
212229

213230
// HashNode calculates a namespaced hash of a node using the supplied left and
214-
// right children. The input values, "left" and "right," are namespaced hash
215-
// values with the format "minNID || maxNID || hash." The HashNode function may
216-
// panic if the inputs provided are invalid, i.e., when left and right are not
217-
// in the namespaced hash format or when left.maxNID is greater than
218-
// right.minNID. To avoid causing panic, it is recommended to first call
219-
// ValidateNodes(left, right) to check if the criteria are met before invoking
220-
// the HashNode function. By default, the normal namespace hash calculation is
221-
// followed, which is "res = min(left.minNID, right.minNID) || max(left.maxNID,
222-
// right.maxNID) || H(NodePrefix, left, right)". "res" refers to the return
223-
// value of the HashNode. However, if the "ignoreMaxNs" property of the Hasher
231+
// right children. The input values, `left` and `right,` are namespaced hash
232+
// values with the format `minNID || maxNID || hash.`
233+
// The HashNode function returns an error if the provided inputs are invalid. Specifically, it returns the ErrInvalidNodeLen error if the left and right inputs are not in the namespaced hash format,
234+
// and the ErrUnorderedSiblings error if left.maxNID is greater than right.minNID.
235+
// By default, the normal namespace hash calculation is
236+
// followed, which is `res = min(left.minNID, right.minNID) || max(left.maxNID,
237+
// right.maxNID) || H(NodePrefix, left, right)`. `res` refers to the return
238+
// value of the HashNode. However, if the `ignoreMaxNs` property of the Hasher
224239
// is set to true, the calculation of the namespace ID range of the node
225240
// slightly changes. In this case, when setting the upper range, the maximum
226241
// possible namespace ID (i.e., 2^NamespaceIDSize-1) should be ignored if
227242
// possible. This is achieved by taking the maximum value among only those namespace
228243
// IDs available in the range of its left and right children that are not
229244
// equal to the maximum possible namespace ID value. If all the namespace IDs are equal
230245
// to the maximum possible value, then the maximum possible value is used.
231-
func (n *Hasher) HashNode(left, right []byte) []byte {
246+
func (n *Hasher) HashNode(left, right []byte) ([]byte, error) {
232247
h := n.baseHasher
233248
h.Reset()
234249

235-
if err := n.validateNodeFormat(left); err != nil {
236-
panic(err)
250+
if err := n.ValidateNodeFormat(left); err != nil {
251+
return nil, err
237252
}
238-
if err := n.validateNodeFormat(right); err != nil {
239-
panic(err)
253+
if err := n.ValidateNodeFormat(right); err != nil {
254+
return nil, err
240255
}
241256

242257
// check the namespace range of the left and right children
243258
if err := n.validateSiblingsNamespaceOrder(left, right); err != nil {
244-
panic(err)
259+
return nil, err
245260
}
246261

247262
// the actual hash result of the children got extended (or flagged) by their
@@ -273,7 +288,7 @@ func (n *Hasher) HashNode(left, right []byte) []byte {
273288
data = append(data, right...)
274289
//nolint:errcheck
275290
h.Write(data)
276-
return h.Sum(res)
291+
return h.Sum(res), nil
277292
}
278293

279294
func max(ns []byte, ns2 []byte) []byte {

0 commit comments

Comments
 (0)