From c1fbf91165109bbb1499b037e87b1a2c9f1922e6 Mon Sep 17 00:00:00 2001 From: Ismail Khoffi Date: Thu, 5 May 2022 21:01:14 +0200 Subject: [PATCH] Refactoring: Push(namespace.PrefixedData)->Push(namespace.ID, []byte) for details see: https://github.com/celestiaorg/nmt/issues/55 --- fuzz_test.go | 2 +- namespace/data.go | 22 ------ namespace/doc.go | 4 +- namespace/id.go | 14 +++- namespace/size.go | 10 --- nmt.go | 25 ++++--- nmt_test.go | 174 +++++++++++++++++++++++----------------------- proof_test.go | 2 +- 8 files changed, 118 insertions(+), 135 deletions(-) delete mode 100644 namespace/data.go delete mode 100644 namespace/size.go diff --git a/fuzz_test.go b/fuzz_test.go index e7fe8af..7a1cc80 100644 --- a/fuzz_test.go +++ b/fuzz_test.go @@ -37,7 +37,7 @@ func TestFuzzProveVerifyNameSpace(t *testing.T) { for _, ns := range sortedKeys { leafDataList := nidDataMap[ns] for _, d := range leafDataList { - err := tree.Push(d) + err := tree.Push(d[:size], d[size:]) if err != nil { t.Fatalf("error on Push(): %v", err) } diff --git a/namespace/data.go b/namespace/data.go deleted file mode 100644 index 8eb37ca..0000000 --- a/namespace/data.go +++ /dev/null @@ -1,22 +0,0 @@ -package namespace - -// PrefixedData simply represents a slice of bytes which consists of -// a namespace.ID and raw data. -// The user has to guarantee that the bytes are valid namespace prefixed data. -// Go's type system does not allow enforcing the structure we want: -// [namespaceID, rawData ...], especially as this type does not expect any -// particular size for the namespace. -type PrefixedData []byte - -// PrefixedData8 like PrefixedData is just a slice of bytes. -// It assumes that the slice it represents is at least 8 bytes. -// This assumption is not enforced by the type system though. -type PrefixedData8 []byte - -func (d PrefixedData8) NamespaceID() ID { - return ID(d[:8]) -} - -func (d PrefixedData8) Data() []byte { - return d[8:] -} diff --git a/namespace/doc.go b/namespace/doc.go index 82b6a9f..e70338d 100644 --- a/namespace/doc.go +++ b/namespace/doc.go @@ -1,4 +1,2 @@ -// Package namespace contains the core namespaced data types: -// * PrefixedData represents the leaf data that gets pushed to the NMT (data prefixed with a namespace.ID) -// * IntervalDigest is the result of a namespaced hashing operation (minNs, maxNs, rawRoot). +// Package namespace contains core namespaced data types. package namespace diff --git a/namespace/id.go b/namespace/id.go index 9b29d38..57eb53d 100644 --- a/namespace/id.go +++ b/namespace/id.go @@ -1,7 +1,19 @@ package namespace -import "bytes" +import ( + "bytes" + "math" +) +// IDMaxSize defines the max. allowed namespace ID size in bytes. +const IDMaxSize = math.MaxUint8 + +// IDSize is the number of bytes a namespace uses. +// Valid values are in [0,255]. +type IDSize uint8 + +// ID represents a namespace ID. +// It's just augments byte slices with a few convenience methods. type ID []byte func (nid ID) Less(other ID) bool { diff --git a/namespace/size.go b/namespace/size.go deleted file mode 100644 index 0000833..0000000 --- a/namespace/size.go +++ /dev/null @@ -1,10 +0,0 @@ -package namespace - -import "math" - -// IDSize is the number of bytes a namespace uses. -// Valid values are in [0,255]. -type IDSize uint8 - -// IDMaxSize defines the max. allowed namespace ID size in bytes. -const IDMaxSize = math.MaxUint8 diff --git a/nmt.go b/nmt.go index b168219..bc8ac4b 100644 --- a/nmt.go +++ b/nmt.go @@ -249,14 +249,23 @@ func (n NamespacedMerkleTree) NamespaceSize() namespace.IDSize { // Returns an error if the namespace ID size of the input // does not match the tree's NamespaceSize() or the leaves are not pushed in // order (i.e. lexicographically sorted by namespace ID). -func (n *NamespacedMerkleTree) Push(namespacedData namespace.PrefixedData) error { - nID, err := n.validateAndExtractNamespace(namespacedData) +func (n *NamespacedMerkleTree) Push(nID namespace.ID, data []byte) error { + err := n.validateNamespace(nID) if err != nil { return err } + // TODO(liamsi): this is bad and lazy! We are copying the nid and data here once instead of just + // using the referenced slices we got. Reason is that internally we still treat a leaf as a single slice + // of bytes. We can and should fix this. For now we are living with the copying to have the API updated from + // Push(prefixedData) to Push(nID, data). See: https://github.com/celestiaorg/nmt/issues/55 + // We intentionally do the copying here instead of forcing the user to merge (copy) the nid and data + // externally (bad UX). + leafDataCopy := make([]byte, len(nID)+len(data)) + copy(leafDataCopy[:len(nID)], nID) + copy(leafDataCopy[len(nID):], data) // update relevant "caches": - n.leaves = append(n.leaves, namespacedData) + n.leaves = append(n.leaves, leafDataCopy) n.updateNamespaceRanges() n.updateMinMaxID(nID) n.rawRoot = nil @@ -328,17 +337,13 @@ func (n *NamespacedMerkleTree) updateNamespaceRanges() { } } } -func (n *NamespacedMerkleTree) validateAndExtractNamespace(ndata namespace.PrefixedData) (namespace.ID, error) { +func (n *NamespacedMerkleTree) validateNamespace(nID namespace.ID) error { nidSize := int(n.NamespaceSize()) - if len(ndata) < nidSize { - return nil, fmt.Errorf("%w: got: %v, want >= %v", ErrMismatchedNamespaceSize, len(ndata), nidSize) - } - nID := namespace.ID(ndata[:n.NamespaceSize()]) // ensure pushed data doesn't have a smaller namespace than the previous one: curSize := len(n.leaves) if curSize > 0 { if nID.Less(n.leaves[curSize-1][:nidSize]) { - return nil, fmt.Errorf( + return fmt.Errorf( "%w: last namespace: %x, pushed: %x", ErrInvalidPushOrder, n.leaves[curSize-1][:nidSize], @@ -346,7 +351,7 @@ func (n *NamespacedMerkleTree) validateAndExtractNamespace(ndata namespace.Prefi ) } } - return nID, nil + return nil } func (n *NamespacedMerkleTree) updateMinMaxID(id namespace.ID) { diff --git a/nmt_test.go b/nmt_test.go index 3587393..06103b1 100644 --- a/nmt_test.go +++ b/nmt_test.go @@ -39,16 +39,16 @@ func ExampleNamespacedMerkleTree() { // the tree will use this namespace size nidSize := 1 // the leaves that will be pushed - data := [][]byte{ - append(namespace.ID{0}, []byte("leaf_0")...), - append(namespace.ID{0}, []byte("leaf_1")...), - append(namespace.ID{1}, []byte("leaf_2")...), - append(namespace.ID{1}, []byte("leaf_3")...)} + data := []namespaceDataPair{ + {namespace.ID{0}, []byte("leaf_0")}, + {namespace.ID{0}, []byte("leaf_1")}, + {namespace.ID{1}, []byte("leaf_2")}, + {namespace.ID{1}, []byte("leaf_3")}} // Init a tree with the namespace size as well as // the underlying hash function: tree := New(sha256.New(), NamespaceIDSize(nidSize)) for _, d := range data { - if err := tree.Push(d); err != nil { + if err := tree.Push(d.ID, d.Data); err != nil { panic(fmt.Sprintf("unexpected error: %v", err)) } } @@ -91,29 +91,29 @@ func ExampleNamespacedMerkleTree() { func TestNamespacedMerkleTree_Push(t *testing.T) { tests := []struct { - name string - data namespace.PrefixedData - wantErr bool + name string + nID, data []byte + wantErr bool }{ - {"1st push: always OK", append([]byte{0, 0, 0}, []byte("dummy data")...), false}, - {"push with same namespace: OK", append([]byte{0, 0, 0}, []byte("dummy data")...), false}, - {"push with greater namespace: OK", append([]byte{0, 0, 1}, []byte("dummy data")...), false}, - {"push with smaller namespace: Err", append([]byte{0, 0, 0}, []byte("dummy data")...), true}, - {"push with same namespace: Ok", append([]byte{0, 0, 1}, []byte("dummy data")...), false}, - {"push with greater namespace: Ok", append([]byte{1, 0, 0}, []byte("dummy data")...), false}, - {"push with smaller namespace: Err", append([]byte{0, 0, 1}, []byte("dummy data")...), true}, - {"push with smaller namespace: Err", append([]byte{0, 0, 0}, []byte("dummy data")...), true}, - {"push with smaller namespace: Err", append([]byte{0, 1, 0}, []byte("dummy data")...), true}, - {"push with same as last namespace: OK", append([]byte{1, 0, 0}, []byte("dummy data")...), false}, - {"push with greater as last namespace: OK", append([]byte{1, 1, 0}, []byte("dummy data")...), false}, + {"1st push: always OK", []byte{0, 0, 0}, []byte("dummy data"), false}, + {"push with same namespace: OK", []byte{0, 0, 0}, []byte("dummy data"), false}, + {"push with greater namespace: OK", []byte{0, 0, 1}, []byte("dummy data"), false}, + {"push with smaller namespace: Err", []byte{0, 0, 0}, []byte("dummy data"), true}, + {"push with same namespace: Ok", []byte{0, 0, 1}, []byte("dummy data"), false}, + {"push with greater namespace: Ok", []byte{1, 0, 0}, []byte("dummy data"), false}, + {"push with smaller namespace: Err", []byte{0, 0, 1}, []byte("dummy data"), true}, + {"push with smaller namespace: Err", []byte{0, 0, 0}, []byte("dummy data"), true}, + {"push with smaller namespace: Err", []byte{0, 1, 0}, []byte("dummy data"), true}, + {"push with same as last namespace: OK", []byte{1, 0, 0}, []byte("dummy data"), false}, + {"push with greater as last namespace: OK", []byte{1, 1, 0}, []byte("dummy data"), false}, // This will error, as the NMT will treat the first bytes as the namespace. If the passed data is // too short though, it can't extract the namespace and hence will complain: - {"push with wrong namespace size: Err", []byte{1, 1}, true}, + {"push with wrong namespace size: Err", []byte{1}, []byte{1}, true}, } n := New(sha256.New(), NamespaceIDSize(3)) for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := n.Push(tt.data); (err != nil) != tt.wantErr { + if err := n.Push(tt.nID, tt.data); (err != nil) != tt.wantErr { t.Errorf("Push() error = %v, wantErr %v", err, tt.wantErr) } }) @@ -150,7 +150,7 @@ func TestNamespacedMerkleTreeRoot(t *testing.T) { t.Run(tt.name, func(t *testing.T) { n := New(sha256.New(), NamespaceIDSize(tt.nidLen)) for _, d := range tt.pushedData { - if err := n.Push(namespace.PrefixedData(append(d.ID, d.Data...))); err != nil { + if err := n.Push(d.ID, d.Data); err != nil { t.Errorf("Push() error = %v, expected no error", err) } } @@ -240,7 +240,7 @@ func TestNamespacedMerkleTree_ProveNamespace_Ranges_And_Verify(t *testing.T) { t.Run(tt.name, func(t *testing.T) { n := New(sha256.New(), NamespaceIDSize(tt.nidLen)) for _, d := range tt.pushData { - err := n.Push(namespace.PrefixedData(append(d.ID, d.Data...))) + err := n.Push(d.ID, d.Data) if err != nil { t.Fatalf("invalid test case: %v, error on Push(): %v", tt.name, err) } @@ -313,126 +313,126 @@ func TestIgnoreMaxNamespace(t *testing.T) { tests := []struct { name string ignoreMaxNamespace bool - pushData []namespace.PrefixedData8 + pushData []namespaceDataPair wantRootMaxNID namespace.ID }{ {"single leaf with MaxNID (ignored)", true, - []namespace.PrefixedData8{namespace.PrefixedData8(append(maxNID, []byte("leaf_1")...))}, + []namespaceDataPair{{maxNID, []byte("leaf_1")}}, maxNID, }, {"single leaf with MaxNID (not ignored)", false, - []namespace.PrefixedData8{namespace.PrefixedData8(append(maxNID, []byte("leaf_1")...))}, + []namespaceDataPair{{maxNID, []byte("leaf_1")}}, maxNID, }, {"two leaves, one with MaxNID (ignored)", true, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(secondNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_2")...)), + []namespaceDataPair{ + {secondNID, []byte("leaf_1")}, + {maxNID, []byte("leaf_2")}, }, secondNID, }, {"two leaves, one with MaxNID (not ignored)", false, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(secondNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_2")...)), + []namespaceDataPair{ + {secondNID, []byte("leaf_1")}, + {maxNID, []byte("leaf_2")}, }, maxNID, }, {"two leaves with MaxNID (ignored)", true, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(maxNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_2")...)), + []namespaceDataPair{ + {maxNID, []byte("leaf_1")}, + {maxNID, []byte("leaf_2")}, }, maxNID, }, {"two leaves with MaxNID (not ignored)", false, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(maxNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_2")...)), + []namespaceDataPair{ + {maxNID, []byte("leaf_1")}, + {maxNID, []byte("leaf_2")}, }, maxNID, }, {"two leaves, none with MaxNID (ignored)", true, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(minNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(secondNID, []byte("leaf_2")...)), + []namespaceDataPair{ + {minNID, []byte("leaf_1")}, + {secondNID, []byte("leaf_2")}, }, secondNID, }, {"two leaves, none with MaxNID (not ignored)", false, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(minNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(secondNID, []byte("leaf_2")...)), + []namespaceDataPair{ + {minNID, []byte("leaf_1")}, + {secondNID, []byte("leaf_2")}, }, secondNID, }, {"three leaves, one with MaxNID (ignored)", true, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(minNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(secondNID, []byte("leaf_2")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_2")...)), + []namespaceDataPair{ + {minNID, []byte("leaf_1")}, + {secondNID, []byte("leaf_2")}, + {maxNID, []byte("leaf_2")}, }, secondNID, }, {"three leaves, one with MaxNID (not ignored)", false, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(minNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(secondNID, []byte("leaf_2")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_2")...)), + []namespaceDataPair{ + {minNID, []byte("leaf_1")}, + {secondNID, []byte("leaf_2")}, + {maxNID, []byte("leaf_2")}, }, maxNID, }, {"4 leaves, none maxNID (ignored)", true, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(minNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(minNID, []byte("leaf_2")...)), - namespace.PrefixedData8(append(secondNID, []byte("leaf_3")...)), - namespace.PrefixedData8(append(thirdNID, []byte("leaf_4")...)), + []namespaceDataPair{ + {minNID, []byte("leaf_1")}, + {minNID, []byte("leaf_2")}, + {secondNID, []byte("leaf_3")}, + {thirdNID, []byte("leaf_4")}, }, thirdNID, }, {"4 leaves, half maxNID (ignored)", true, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(minNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(secondNID, []byte("leaf_2")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_3")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_4")...)), + []namespaceDataPair{ + {minNID, []byte("leaf_1")}, + {secondNID, []byte("leaf_2")}, + {maxNID, []byte("leaf_3")}, + {maxNID, []byte("leaf_4")}, }, secondNID, }, {"4 leaves, half maxNID (not ignored)", false, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(minNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(secondNID, []byte("leaf_2")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_3")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_4")...)), + []namespaceDataPair{ + {minNID, []byte("leaf_1")}, + {secondNID, []byte("leaf_2")}, + {maxNID, []byte("leaf_3")}, + {maxNID, []byte("leaf_4")}, }, maxNID, }, {"8 leaves, 4 maxNID (ignored)", true, - []namespace.PrefixedData8{ - namespace.PrefixedData8(append(minNID, []byte("leaf_1")...)), - namespace.PrefixedData8(append(secondNID, []byte("leaf_2")...)), - namespace.PrefixedData8(append(thirdNID, []byte("leaf_3")...)), - namespace.PrefixedData8(append(thirdNID, []byte("leaf_4")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_5")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_6")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_7")...)), - namespace.PrefixedData8(append(maxNID, []byte("leaf_8")...)), + []namespaceDataPair{ + {minNID, []byte("leaf_1")}, + {secondNID, []byte("leaf_2")}, + {thirdNID, []byte("leaf_3")}, + {thirdNID, []byte("leaf_4")}, + {maxNID, []byte("leaf_5")}, + {maxNID, []byte("leaf_6")}, + {maxNID, []byte("leaf_7")}, + {maxNID, []byte("leaf_8")}, }, thirdNID, }, @@ -442,7 +442,7 @@ func TestIgnoreMaxNamespace(t *testing.T) { t.Run(tc.name, func(t *testing.T) { tree := New(hash, NamespaceIDSize(nidSize), IgnoreMaxNamespace(tc.ignoreMaxNamespace)) for _, d := range tc.pushData { - if err := tree.Push(namespace.PrefixedData(d)); err != nil { + if err := tree.Push(d.ID, d.Data); err != nil { panic("unexpected error") } } @@ -451,23 +451,23 @@ func TestIgnoreMaxNamespace(t *testing.T) { t.Fatalf("Case: %v, '%v', root.Max() got: %x, want: %x", i, tc.name, gotRootMaxNID, tc.wantRootMaxNID) } for idx, d := range tc.pushData { - proof, err := tree.ProveNamespace(d.NamespaceID()) + proof, err := tree.ProveNamespace(d.ID) if err != nil { t.Fatalf("ProveNamespace() unexpected error: %v", err) } if gotIgnored := proof.IsMaxNamespaceIDIgnored(); gotIgnored != tc.ignoreMaxNamespace { t.Fatalf("Proof.IsMaxNamespaceIDIgnored() got: %v, want: %v", gotIgnored, tc.ignoreMaxNamespace) } - data := tree.Get(d.NamespaceID()) - if !proof.VerifyNamespace(hash, d.NamespaceID(), data, tree.Root()) { - t.Errorf("VerifyNamespace() failed on ID: %x", d.NamespaceID()) + data := tree.Get(d.ID) + if !proof.VerifyNamespace(hash, d.ID, data, tree.Root()) { + t.Errorf("VerifyNamespace() failed on ID: %x", d.ID) } singleProof, err := tree.Prove(idx) if err != nil { t.Fatalf("ProveNamespace() unexpected error: %v", err) } - if !singleProof.VerifyInclusion(hash, d.NamespaceID(), d.Data(), tree.Root()) { + if !singleProof.VerifyInclusion(hash, d.ID, d.Data, tree.Root()) { t.Errorf("VerifyInclusion() failed on data: %#v with index: %v", d, idx) } if gotIgnored := singleProof.IsMaxNamespaceIDIgnored(); gotIgnored != tc.ignoreMaxNamespace { @@ -492,7 +492,7 @@ func TestNodeVisitor(t *testing.T) { data := generateRandNamespacedRawData(numLeaves, nidSize, leafSize) n := New(sha256.New(), NamespaceIDSize(nidSize), NodeVisitor(collectNodeHashes)) for j := 0; j < numLeaves; j++ { - if err := n.Push(data[j]); err != nil { + if err := n.Push(data[j][:nidSize], data[j][nidSize:]); err != nil { t.Errorf("err: %v", err) } } @@ -523,7 +523,7 @@ func TestNamespacedMerkleTree_ProveErrors(t *testing.T) { t.Run(tt.name, func(t *testing.T) { n := New(sha256.New(), NamespaceIDSize(tt.nidLen), InitialCapacity(len(tt.pushData))) for _, d := range tt.pushData { - err := n.Push(namespace.PrefixedData(append(d.ID, d.Data...))) + err := n.Push(d.ID, d.Data) if err != nil { t.Fatalf("invalid test case: %v, error on Push(): %v", tt.name, err) } @@ -604,7 +604,7 @@ func BenchmarkComputeRoot(b *testing.B) { for i := 0; i < b.N; i++ { n := New(sha256.New()) for j := 0; j < tt.numLeaves; j++ { - if err := n.Push(data[j]); err != nil { + if err := n.Push(data[j][:tt.nidSize], data[j][tt.nidSize:]); err != nil { b.Errorf("err: %v", err) } } @@ -618,7 +618,7 @@ func BenchmarkComputeRoot(b *testing.B) { func Test_Root_RaceCondition(t *testing.T) { // this is very similar to: https://github.com/HuobiRDCenter/huobi_Golang/pull/9 tree := New(sha256.New()) - _ = tree.Push([]byte("some data is good enough here")) + _ = tree.Push([]byte("nid007"), []byte("some data is good enough here")) numRoutines := 200 wg := sync.WaitGroup{} wg.Add(numRoutines) diff --git a/proof_test.go b/proof_test.go index a496a2a..4ec9853 100644 --- a/proof_test.go +++ b/proof_test.go @@ -19,7 +19,7 @@ func TestProof_VerifyNamespace_False(t *testing.T) { generateLeafData(testNidLen, 0, 9, []byte("data"))..., ), newNamespaceDataPair([]byte{0, 0, 8}, []byte("last leaf"))) for _, d := range data { - err := n.Push(namespace.PrefixedData(append(d.ID, d.Data...))) + err := n.Push(d.ID, d.Data) if err != nil { t.Fatalf("invalid test setup: error on Push(): %v", err) }