fix: Prevent newExporter crash if tree.ndb is nil (backport cosmos#622)…

… (cosmos#631) Co-authored-by: Chill Validation <[email protected]> Co-authored-by: Marko Baricevic <[email protected]>
yihuang · Dec 8, 2022 · 2af006a · 2af006a
1 parent 43560dc
commit 2af006a
Show file tree

Hide file tree

Showing 7 changed files with 37 additions and 15 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,9 @@
 # Changelog
 
+### Breaking Changes
+
+- [#622](https://github.com/cosmos/iavl/pull/622) `export/newExporter()` and `ImmutableTree.Export()` returns error for nil arguements
+
 ## Unreleased
 
 - [#640](https://github.com/cosmos/iavl/pull/640) commit `NodeDB` batch in `LoadVersionForOverwriting`.

diff --git a/Makefile b/Makefile
@@ -15,10 +15,8 @@ all: lint test install
 install:
 ifeq ($(COLORS_ON),)
 	go install ./cmd/iaviewer
-	go install ./cmd/iavlserver
 else
 	go install $(CMDFLAGS) ./cmd/iaviewer
-	go install $(CMDFLAGS) ./cmd/iavlserver
 endif
 .PHONY: install
 

diff --git a/benchmarks/cosmos-exim/main.go b/benchmarks/cosmos-exim/main.go
@@ -128,7 +128,10 @@ func runExport(dbPath string) (int64, map[string][]*iavl.ExportNode, error) {
 			return 0, nil, err
 		}
 		start := time.Now().UTC()
-		exporter := itree.Export()
+		exporter, err := itree.Export()
+		if err != nil {
+			return 0, nil, err
+		}
 		defer exporter.Close()
 		for {
 			node, err := exporter.Next()

diff --git a/export.go b/export.go
@@ -2,8 +2,8 @@ package iavl
 
 import (
 	"context"
-
-	"github.com/pkg/errors"
+	"errors"
+	"fmt"
 )
 
 // exportBufferSize is the number of nodes to buffer in the exporter. It improves throughput by
@@ -15,6 +15,9 @@ const exportBufferSize = 32
 // nolint:revive
 var ExportDone = errors.New("export is complete") // nolint:golint
 
+// ErrNotInitalizedTree when chains introduce a store without initializing data
+var ErrNotInitalizedTree = errors.New("iavl/export newExporter failed to create")
+
 // ExportNode contains exported node data.
 type ExportNode struct {
 	Key     []byte
@@ -35,7 +38,15 @@ type Exporter struct {
 }
 
 // NewExporter creates a new Exporter. Callers must call Close() when done.
-func newExporter(tree *ImmutableTree) *Exporter {
+func newExporter(tree *ImmutableTree) (*Exporter, error) {
+	if tree == nil {
+		return nil, fmt.Errorf("tree is nil: %w", ErrNotInitalizedTree)
+	}
+	// CV Prevent crash on incrVersionReaders if tree.ndb == nil
+	if tree.ndb == nil {
+		return nil, fmt.Errorf("tree.ndb is nil: %w", ErrNotInitalizedTree)
+	}
+
 	ctx, cancel := context.WithCancel(context.Background())
 	exporter := &Exporter{
 		tree:   tree,
@@ -46,7 +57,7 @@ func newExporter(tree *ImmutableTree) *Exporter {
 	tree.ndb.incrVersionReaders(tree.version)
 	go exporter.export(ctx)
 
-	return exporter
+	return exporter, nil
 }
 
 // export exports nodes

diff --git a/export_test.go b/export_test.go
@@ -160,7 +160,8 @@ func TestExporter(t *testing.T) {
 	}
 
 	actual := make([]*ExportNode, 0, len(expect))
-	exporter := tree.Export()
+	exporter, err := tree.Export()
+	require.NoError(t, err)
 	defer exporter.Close()
 	for {
 		node, err := exporter.Next()
@@ -189,7 +190,8 @@ func TestExporter_Import(t *testing.T) {
 		t.Run(desc, func(t *testing.T) {
 			t.Parallel()
 
-			exporter := tree.Export()
+			exporter, err := tree.Export()
+			require.NoError(t, err)
 			defer exporter.Close()
 
 			newTree, err := NewMutableTree(db.NewMemDB(), 0, false)
@@ -234,7 +236,8 @@ func TestExporter_Import(t *testing.T) {
 
 func TestExporter_Close(t *testing.T) {
 	tree := setupExportTreeSized(t, 4096)
-	exporter := tree.Export()
+	exporter, err := tree.Export()
+	require.NoError(t, err)
 
 	node, err := exporter.Next()
 	require.NoError(t, err)
@@ -273,7 +276,8 @@ func TestExporter_DeleteVersionErrors(t *testing.T) {
 
 	itree, err := tree.GetImmutable(2)
 	require.NoError(t, err)
-	exporter := itree.Export()
+	exporter, err := itree.Export()
+	require.NoError(t, err)
 	defer exporter.Close()
 
 	err = tree.DeleteVersion(2)
@@ -291,7 +295,8 @@ func BenchmarkExport(b *testing.B) {
 	tree := setupExportTreeSized(b, 4096)
 	b.StartTimer()
 	for n := 0; n < b.N; n++ {
-		exporter := tree.Export()
+		exporter, err := tree.Export()
+		require.NoError(b, err)
 		for {
 			_, err := exporter.Next()
 			if err == ExportDone {

diff --git a/immutable_tree.go b/immutable_tree.go
@@ -155,7 +155,7 @@ func (t *ImmutableTree) Hash() ([]byte, error) {
 
 // Export returns an iterator that exports tree nodes as ExportNodes. These nodes can be
 // imported with MutableTree.Import() to recreate an identical tree.
-func (t *ImmutableTree) Export() *Exporter {
+func (t *ImmutableTree) Export() (*Exporter, error) {
 	return newExporter(t)
 }
 

diff --git a/import_test.go b/import_test.go
@@ -27,7 +27,7 @@ func ExampleImporter() {
 	if err != nil {
 		// handle err
 	}
-	exporter := itree.Export()
+	exporter, err := itree.Export()
 	defer exporter.Close()
 	exported := []*ExportNode{}
 	for {
@@ -218,7 +218,8 @@ func BenchmarkImport(b *testing.B) {
 	b.StopTimer()
 	tree := setupExportTreeSized(b, 4096)
 	exported := make([]*ExportNode, 0, 4096)
-	exporter := tree.Export()
+	exporter, err := tree.Export()
+	require.NoError(b, err)
 	for {
 		item, err := exporter.Next()
 		if err == ExportDone {