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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

- [#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
- [#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)
Expand Down
18 changes: 16 additions & 2 deletions export.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,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
Expand All @@ -33,7 +39,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, ErrorTreeNil
}
// CV Prevent crash on incrVersionReaders if tree.ndb == nil
if tree.ndb == nil {
return nil, ErrorTreeNodeDbNil
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe we can have one error type: ErrNotIniitalizedTree. And then use wrapping to add more details, eg:

ErrNotIniitalizedTree.Wrap("ndb is nil")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrapped

}

ctx, cancel := context.WithCancel(context.Background())
exporter := &Exporter{
tree: tree,
Expand All @@ -44,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
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