From 055432eb422e069f1c2e0f177d18ec5cd0a83784 Mon Sep 17 00:00:00 2001 From: chillyvee Date: Sun, 20 Nov 2022 17:25:13 +0100 Subject: [PATCH 1/7] Prevent newExporter crash if tree.ndb is nil --- export.go | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/export.go b/export.go index ae9f294f2..df1f5e199 100644 --- a/export.go +++ b/export.go @@ -3,6 +3,7 @@ package iavl import ( "context" "errors" + "fmt" ) // exportBufferSize is the number of nodes to buffer in the exporter. It improves throughput by @@ -34,6 +35,15 @@ type Exporter struct { // NewExporter creates a new Exporter. Callers must call Close() when done. func newExporter(tree *ImmutableTree) *Exporter { + if tree == nil { + return nil + } + // CV Prevent crash on incrVersionReaders if tree.ndb == nil + if tree.ndb == nil { + fmt.Printf("iavl/export newExporter failed to create because tree.ndb is nil\n") + return nil + } + ctx, cancel := context.WithCancel(context.Background()) exporter := &Exporter{ tree: tree, From 8db8b72f32254d6c7f19522f895c27606081b0df Mon Sep 17 00:00:00 2001 From: chillyvee Date: Tue, 22 Nov 2022 13:44:22 +0100 Subject: [PATCH 2/7] ImmutableTree.Export() returns error if newExporter is given nil arguments --- CHANGELOG.md | 1 + export.go | 16 ++++++++++------ export_test.go | 15 ++++++++++----- immutable_tree.go | 2 +- import_test.go | 5 +++-- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11d12bd60..243a966d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- [#622](https://github.com/cosmos/iavl/pull/622) `export/newExporter()` and `ImmutableTree.Export()` returns error for nil arguements - [#586](https://github.com/cosmos/iavl/pull/586) Remove the `RangeProof` and refactor the ics23_proof to use the internal methods. ## 0.19.4 (October 28, 2022) diff --git a/export.go b/export.go index df1f5e199..b8a6cbf22 100644 --- a/export.go +++ b/export.go @@ -3,7 +3,6 @@ package iavl import ( "context" "errors" - "fmt" ) // exportBufferSize is the number of nodes to buffer in the exporter. It improves throughput by @@ -14,6 +13,12 @@ const exportBufferSize = 32 // ErrorExportDone is returned by Exporter.Next() when all items have been exported. var ErrorExportDone = errors.New("export is complete") +// ErrorTreeNil is a rare occurance, but it is a clear error +var ErrorTreeNil = errors.New("iavl/export newExporter failed to create because tree is nil") + +// ErrorTreeNodeDbNil encountered when chains introduce a store without initializing data +var ErrorTreeNodeDbNil = errors.New("iavl/export newExporter failed to create because tree.ndb is nil") + // ExportNode contains exported node data. type ExportNode struct { Key []byte @@ -34,14 +39,13 @@ 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 + return nil, ErrorTreeNil } // CV Prevent crash on incrVersionReaders if tree.ndb == nil if tree.ndb == nil { - fmt.Printf("iavl/export newExporter failed to create because tree.ndb is nil\n") - return nil + return nil, ErrorTreeNodeDbNil } ctx, cancel := context.WithCancel(context.Background()) @@ -54,7 +58,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 4f380f787..f6b552633 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 == ErrorExportDone { diff --git a/immutable_tree.go b/immutable_tree.go index f8c33c225..f005de9cf 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 39c9863a0..2f6eb730e 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 == ErrorExportDone { From 46067ecc431981d4f150522f7e6a83c6a707c91b Mon Sep 17 00:00:00 2001 From: chillyvee Date: Tue, 22 Nov 2022 19:32:51 +0100 Subject: [PATCH 3/7] Wrap newExporter errors into ErrNotInitalizedTree --- CHANGELOG.md | 5 ++++- export.go | 14 ++++++-------- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 243a966d0..6e2fac254 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,11 @@ # Changelog -## Unreleased +### Breaking Changes - [#622](https://github.com/cosmos/iavl/pull/622) `export/newExporter()` and `ImmutableTree.Export()` returns error for nil arguements + +## Unreleased + - [#586](https://github.com/cosmos/iavl/pull/586) Remove the `RangeProof` and refactor the ics23_proof to use the internal methods. ## 0.19.4 (October 28, 2022) diff --git a/export.go b/export.go index b8a6cbf22..eb84b4054 100644 --- a/export.go +++ b/export.go @@ -2,7 +2,8 @@ package iavl import ( "context" - "errors" + + "github.com/pkg/errors" ) // exportBufferSize is the number of nodes to buffer in the exporter. It improves throughput by @@ -13,11 +14,8 @@ const exportBufferSize = 32 // ErrorExportDone is returned by Exporter.Next() when all items have been exported. var ErrorExportDone = errors.New("export is complete") -// ErrorTreeNil is a rare occurance, but it is a clear error -var ErrorTreeNil = errors.New("iavl/export newExporter failed to create because tree is nil") - -// ErrorTreeNodeDbNil encountered when chains introduce a store without initializing data -var ErrorTreeNodeDbNil = errors.New("iavl/export newExporter failed to create because tree.ndb is nil") +// 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 { @@ -41,11 +39,11 @@ type Exporter struct { // NewExporter creates a new Exporter. Callers must call Close() when done. func newExporter(tree *ImmutableTree) (*Exporter, error) { if tree == nil { - return nil, ErrorTreeNil + return nil, errors.Wrap(ErrNotInitalizedTree, "tree is nil") } // CV Prevent crash on incrVersionReaders if tree.ndb == nil if tree.ndb == nil { - return nil, ErrorTreeNodeDbNil + return nil, errors.Wrap(ErrNotInitalizedTree, "tree.ndb is nil") } ctx, cancel := context.WithCancel(context.Background()) From e856bd2f65050eeac52c39d4ce080f586c37bd4f Mon Sep 17 00:00:00 2001 From: chillyvee Date: Thu, 24 Nov 2022 12:13:30 +0100 Subject: [PATCH 4/7] change from requested Wrap to fmt.Errorf --- export.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/export.go b/export.go index eb84b4054..17472cafc 100644 --- a/export.go +++ b/export.go @@ -2,8 +2,9 @@ package iavl import ( "context" + "fmt" - "github.com/pkg/errors" + "errors" ) // exportBufferSize is the number of nodes to buffer in the exporter. It improves throughput by @@ -39,11 +40,11 @@ type Exporter struct { // NewExporter creates a new Exporter. Callers must call Close() when done. func newExporter(tree *ImmutableTree) (*Exporter, error) { if tree == nil { - return nil, errors.Wrap(ErrNotInitalizedTree, "tree is 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, errors.Wrap(ErrNotInitalizedTree, "tree.ndb is nil") + return nil, fmt.Errorf("tree.ndb is nil: %w", ErrNotInitalizedTree) } ctx, cancel := context.WithCancel(context.Background()) From f62821d539fcf8e52f70cfd650c53d2c4c86959e Mon Sep 17 00:00:00 2001 From: chillyvee Date: Thu, 24 Nov 2022 13:34:46 +0100 Subject: [PATCH 5/7] fix benchmark test --- benchmarks/cosmos-exim/main.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/benchmarks/cosmos-exim/main.go b/benchmarks/cosmos-exim/main.go index 5ff7f30a4..b6e3023fd 100644 --- a/benchmarks/cosmos-exim/main.go +++ b/benchmarks/cosmos-exim/main.go @@ -127,7 +127,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() From 6fb6de01c967cbbf550eef5fbf020751446dc945 Mon Sep 17 00:00:00 2001 From: chillyvee Date: Thu, 24 Nov 2022 14:03:44 +0100 Subject: [PATCH 6/7] go fumpting --- export.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/export.go b/export.go index 17472cafc..e19998d9e 100644 --- a/export.go +++ b/export.go @@ -2,9 +2,8 @@ package iavl import ( "context" - "fmt" - "errors" + "fmt" ) // exportBufferSize is the number of nodes to buffer in the exporter. It improves throughput by From 5b9853c8c73e1fa67bebd1bae526f5637b9205b8 Mon Sep 17 00:00:00 2001 From: chillyvee Date: Thu, 24 Nov 2022 14:09:17 +0100 Subject: [PATCH 7/7] remove build for previously removed iavlserver --- Makefile | 2 -- 1 file changed, 2 deletions(-) diff --git a/Makefile b/Makefile index cc7f75d95..db1f32b1f 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