From 2af006a167c5eed89039d4109af2e63dea5de821 Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 8 Dec 2022 11:24:07 +0100 Subject: [PATCH] fix: Prevent newExporter crash if tree.ndb is nil (backport #622) (#631) Co-authored-by: Chill Validation <92176880+chillyvee@users.noreply.github.com> Co-authored-by: Marko Baricevic --- CHANGELOG.md | 4 ++++ Makefile | 2 -- benchmarks/cosmos-exim/main.go | 5 ++++- export.go | 19 +++++++++++++++---- export_test.go | 15 ++++++++++----- immutable_tree.go | 2 +- import_test.go | 5 +++-- 7 files changed, 37 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c48e7f17f..3f5d3e246 100644 --- 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 index 0364b7350..ecba76669 100644 --- 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 index 4b7ee3580..5d2b0de30 100644 --- 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 index c65b81b21..e4596f334 100644 --- 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 index 8b7051f2d..166c68a08 100644 --- 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 index 5113873c8..bc857e4ad 100644 --- 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 index 5fbf7d733..52c4b4e23 100644 --- 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 {