Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Prevent newExporter crash if tree.ndb is nil #622

Merged
merged 8 commits into from
Nov 25, 2022
Merged
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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
tac0turtle marked this conversation as resolved.
Show resolved Hide resolved

## Unreleased

- [#586](https://github.com/cosmos/iavl/pull/586) Remove the `RangeProof` and refactor the ics23_proof to use the internal methods.
Expand Down
18 changes: 15 additions & 3 deletions export.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -13,6 +14,9 @@ const exportBufferSize = 32
// ErrorExportDone is returned by Exporter.Next() when all items have been exported.
var ErrorExportDone = errors.New("export is complete")

// 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
Expand All @@ -33,7 +37,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, errors.Wrap(ErrNotInitalizedTree, "tree is nil")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we use fmt.Errorf to avoid introducing GitHub.com/pkg/errors as a dependency

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My mistake. I thought the team was looking to call Wrap() specifically. Will change it again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no worries, pkg/errors is deprecated with go 1.18 and beyond because of fmt.errorf and/or errors pkg.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learn something new every day

}
// CV Prevent crash on incrVersionReaders if tree.ndb == nil
if tree.ndb == nil {
return nil, errors.Wrap(ErrNotInitalizedTree, "tree.ndb is nil")
}

ctx, cancel := context.WithCancel(context.Background())
exporter := &Exporter{
tree: tree,
Expand All @@ -44,7 +56,7 @@ func newExporter(tree *ImmutableTree) *Exporter {
tree.ndb.incrVersionReaders(tree.version)
go exporter.export(ctx)

return exporter
return exporter, nil
}

// export exports nodes
Expand Down
15 changes: 10 additions & 5 deletions export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion immutable_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
5 changes: 3 additions & 2 deletions import_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func ExampleImporter() {
if err != nil {
// handle err
}
exporter := itree.Export()
exporter, err := itree.Export()
defer exporter.Close()
exported := []*ExportNode{}
for {
Expand Down Expand Up @@ -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 {
Expand Down