From 20b1da7a2ecad161d40db4d3f8dbf4821ddf6d3d Mon Sep 17 00:00:00 2001 From: Aleksandr Bezobchuk Date: Mon, 8 Jan 2024 14:26:10 -0500 Subject: [PATCH] refactor(store/v2): Use Core Iterator (#18957) Co-authored-by: marbar3778 Co-authored-by: cool-developer <51834436+cool-develope@users.noreply.github.com> --- core/store/store.go | 52 +++++++++++++---------------- store/database.go | 7 ++-- store/go.mod | 2 ++ store/iterator.go | 38 --------------------- store/kv/branch/iterator.go | 35 +++++++++++-------- store/kv/branch/store.go | 13 ++++---- store/kv/branch/store_test.go | 16 --------- store/kv/mem/iterator.go | 14 ++++---- store/kv/mem/store.go | 5 +-- store/kv/mem/store_test.go | 9 +---- store/kv/trace/iterator.go | 15 +++++---- store/kv/trace/store.go | 5 +-- store/kv/trace/store_test.go | 2 -- store/storage/database.go | 5 +-- store/storage/pebbledb/db.go | 5 +-- store/storage/pebbledb/iterator.go | 29 ++++++++-------- store/storage/rocksdb/db.go | 5 +-- store/storage/rocksdb/db_test.go | 2 -- store/storage/rocksdb/iterator.go | 14 ++++---- store/storage/sqlite/db.go | 7 ++-- store/storage/sqlite/db_test.go | 2 -- store/storage/sqlite/iterator.go | 16 +++++---- store/storage/storage_test_suite.go | 2 -- store/storage/store.go | 5 +-- store/store.go | 5 +-- 25 files changed, 131 insertions(+), 179 deletions(-) delete mode 100644 store/iterator.go diff --git a/core/store/store.go b/core/store/store.go index 2969ef29b9..8943390cc5 100644 --- a/core/store/store.go +++ b/core/store/store.go @@ -30,32 +30,15 @@ type KVStore interface { ReverseIterator(start, end []byte) (Iterator, error) } -// Iterator represents an iterator over a domain of keys. Callers must call Close when done. -// No writes can happen to a domain while there exists an iterator over it, some backends may take -// out database locks to ensure this will not happen. +// Iterator represents an iterator over a domain of keys. Callers must call +// Close when done. No writes can happen to a domain while there exists an +// iterator over it. Some backends may take out database locks to ensure this +// will not happen. // -// Callers must make sure the iterator is valid before calling any methods on it, otherwise -// these methods will panic. This is in part caused by most backend databases using this convention. -// -// As with DB, keys and values should be considered read-only, and must be copied before they are -// modified. -// -// Typical usage: -// -// var itr Iterator = ... -// defer itr.Close() -// -// for ; itr.Valid(); itr.Next() { -// k, v := itr.Key(); itr.Value() -// ... -// } -// -// if err := itr.Error(); err != nil { -// ... -// } +// Callers must make sure the iterator is valid before calling any methods on it, +// otherwise these methods will panic. type Iterator interface { // Domain returns the start (inclusive) and end (exclusive) limits of the iterator. - // CONTRACT: start, end readonly []byte Domain() (start, end []byte) // Valid returns whether the current iterator is valid. Once invalid, the Iterator remains @@ -67,12 +50,13 @@ type Iterator interface { Next() // Key returns the key at the current position. Panics if the iterator is invalid. - // CONTRACT: key readonly []byte - Key() (key []byte) + // Note, the key returned should be a copy and thus safe for modification. + Key() []byte - // Value returns the value at the current position. Panics if the iterator is invalid. - // CONTRACT: value readonly []byte - Value() (value []byte) + // Value returns the value at the current position. Panics if the iterator is + // invalid. + // Note, the value returned should be a copy and thus safe for modification. + Value() []byte // Error returns the last error encountered by the iterator, if any. Error() error @@ -80,3 +64,15 @@ type Iterator interface { // Close closes the iterator, releasing any allocated resources. Close() error } + +// IteratorCreator defines an interface for creating forward and reverse iterators. +type IteratorCreator interface { + // Iterator creates a new iterator for the given store name and domain, where + // domain is defined by [start, end). Note, both start and end are optional. + Iterator(storeKey string, start, end []byte) (Iterator, error) + + // ReverseIterator creates a new reverse iterator for the given store name + // and domain, where domain is defined by [start, end). Note, both start and + // end are optional. + ReverseIterator(storeKey string, start, end []byte) (Iterator, error) +} diff --git a/store/database.go b/store/database.go index 992dd3db46..2df28059a4 100644 --- a/store/database.go +++ b/store/database.go @@ -3,6 +3,7 @@ package store import ( "io" + corestore "cosmossdk.io/core/store" ics23 "github.com/cosmos/ics23/go" ) @@ -38,7 +39,7 @@ type Writer interface { type Database interface { Reader Writer - IteratorCreator + corestore.IteratorCreator io.Closer } @@ -50,8 +51,8 @@ type VersionedDatabase interface { GetLatestVersion() (uint64, error) SetLatestVersion(version uint64) error - Iterator(storeKey string, version uint64, start, end []byte) (Iterator, error) - ReverseIterator(storeKey string, version uint64, start, end []byte) (Iterator, error) + Iterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) + ReverseIterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) ApplyChangeset(version uint64, cs *Changeset) error diff --git a/store/go.mod b/store/go.mod index 1a1d22508e..2f657cb222 100644 --- a/store/go.mod +++ b/store/go.mod @@ -68,3 +68,5 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect gotest.tools/v3 v3.5.1 // indirect ) + +replace cosmossdk.io/core => ../core diff --git a/store/iterator.go b/store/iterator.go deleted file mode 100644 index fa49b715be..0000000000 --- a/store/iterator.go +++ /dev/null @@ -1,38 +0,0 @@ -package store - -// Iterator defines an interface for iterating over a domain of key/value pairs. -type Iterator interface { - // Domain returns the start (inclusive) and end (exclusive) limits of the iterator. - Domain() ([]byte, []byte) - - // Valid returns if the iterator is currently valid. - Valid() bool - - // Error returns any accumulated error. Error() should be called after all - // key/value pairs have been exhausted, i.e. after Next() has returned false. - Error() error - - // Key returns the key of the current key/value pair, or nil if done. - Key() []byte - - // Value returns the value of the current key/value pair, or nil if done. - Value() []byte - - // Next moves the iterator to the next key/value pair. - Next() bool - - // Close releases associated resources. It should NOT be idempotent. It must - // only be called once and any call after may panic. - Close() -} - -// IteratorCreator defines an interface for creating forward and reverse iterators. -type IteratorCreator interface { - // Iterator creates a new iterator for the given store name and domain, where - // domain is defined by [start, end). Note, both start and end are optional. - Iterator(storeKey string, start, end []byte) (Iterator, error) - // ReverseIterator creates a new reverse iterator for the given store name - // and domain, where domain is defined by [start, end). Note, both start and - // end are optional. - ReverseIterator(storeKey string, start, end []byte) (Iterator, error) -} diff --git a/store/kv/branch/iterator.go b/store/kv/branch/iterator.go index 02f3e3c0f6..5fc952f5f5 100644 --- a/store/kv/branch/iterator.go +++ b/store/kv/branch/iterator.go @@ -3,10 +3,11 @@ package branch import ( "slices" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" ) -var _ store.Iterator = (*iterator)(nil) +var _ corestore.Iterator = (*iterator)(nil) // iterator walks over both the KVStore's changeset, i.e. dirty writes, and the // parent iterator, which can either be another KVStore or the SS backend, at the @@ -16,7 +17,7 @@ var _ store.Iterator = (*iterator)(nil) // iterator. This is because when an iterator is created, it takes a current // snapshot of the changeset. type iterator struct { - parentItr store.Iterator + parentItr corestore.Iterator start []byte end []byte key []byte @@ -41,21 +42,21 @@ func (itr *iterator) Value() []byte { return slices.Clone(itr.value) } -func (itr *iterator) Close() { +func (itr *iterator) Close() error { itr.key = nil itr.value = nil itr.keys = nil itr.values = nil - itr.parentItr.Close() + return itr.parentItr.Close() } -func (itr *iterator) Next() bool { +func (itr *iterator) Next() { for { switch { case itr.exhausted && len(itr.keys) == 0: // exhausted both itr.key = nil itr.value = nil - return false + return case itr.exhausted: // exhausted parent iterator but not store (dirty writes) iterator nextKey := itr.keys[0] @@ -72,15 +73,17 @@ func (itr *iterator) Next() bool { if nextValue.Value != nil { itr.key = []byte(nextKey) itr.value = nextValue.Value - return true + return } case len(itr.keys) == 0: // exhausted store (dirty writes) iterator but not parent iterator itr.key = itr.parentItr.Key() itr.value = itr.parentItr.Value() - itr.exhausted = !itr.parentItr.Next() - return true + itr.parentItr.Next() + itr.exhausted = !itr.parentItr.Valid() + + return default: // parent iterator is not exhausted and we have store (dirty writes) remaining dirtyKey := itr.keys[0] @@ -102,14 +105,17 @@ func (itr *iterator) Next() bool { if dirtyVal.Value != nil { itr.key = []byte(dirtyKey) itr.value = dirtyVal.Value - return true + return } case (!itr.reverse && parentKeyStr < dirtyKey) || (itr.reverse && parentKeyStr > dirtyKey): // parent's key should come before dirty key itr.key = parentKey itr.value = itr.parentItr.Value() - itr.exhausted = !itr.parentItr.Next() - return true + + itr.parentItr.Next() + itr.exhausted = !itr.parentItr.Valid() + + return default: // pop off key @@ -120,12 +126,13 @@ func (itr *iterator) Next() bool { itr.values[0].Value = nil itr.values = itr.values[1:] - itr.exhausted = !itr.parentItr.Next() + itr.parentItr.Next() + itr.exhausted = !itr.parentItr.Valid() if dirtyVal.Value != nil { itr.key = []byte(dirtyKey) itr.value = dirtyVal.Value - return true + return } } } diff --git a/store/kv/branch/store.go b/store/kv/branch/store.go index aaefcf0487..98416a9655 100644 --- a/store/kv/branch/store.go +++ b/store/kv/branch/store.go @@ -8,6 +8,7 @@ import ( "golang.org/x/exp/maps" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" "cosmossdk.io/store/v2/kv/trace" ) @@ -212,11 +213,11 @@ func (s *Store) Write() { // Note, writes that happen on the KVStore over an iterator will not affect the // iterator. This is because when an iterator is created, it takes a current // snapshot of the changeset. -func (s *Store) Iterator(start, end []byte) store.Iterator { +func (s *Store) Iterator(start, end []byte) corestore.Iterator { s.mu.Lock() defer s.mu.Unlock() - var parentItr store.Iterator + var parentItr corestore.Iterator if s.parent != nil { parentItr = s.parent.Iterator(start, end) } else { @@ -238,11 +239,11 @@ func (s *Store) Iterator(start, end []byte) store.Iterator { // Note, writes that happen on the KVStore over an iterator will not affect the // iterator. This is because when an iterator is created, it takes a current // snapshot of the changeset. -func (s *Store) ReverseIterator(start, end []byte) store.Iterator { +func (s *Store) ReverseIterator(start, end []byte) corestore.Iterator { s.mu.Lock() defer s.mu.Unlock() - var parentItr store.Iterator + var parentItr corestore.Iterator if s.parent != nil { parentItr = s.parent.ReverseIterator(start, end) } else { @@ -256,7 +257,7 @@ func (s *Store) ReverseIterator(start, end []byte) store.Iterator { return s.newIterator(parentItr, start, end, true) } -func (s *Store) newIterator(parentItr store.Iterator, start, end []byte, reverse bool) *iterator { +func (s *Store) newIterator(parentItr corestore.Iterator, start, end []byte, reverse bool) *iterator { startStr := string(start) endStr := string(end) @@ -305,7 +306,7 @@ func (s *Store) newIterator(parentItr store.Iterator, start, end []byte, reverse } // call Next() to move the iterator to the first key/value entry - _ = itr.Next() + itr.Next() return itr } diff --git a/store/kv/branch/store_test.go b/store/kv/branch/store_test.go index 424e396e0b..0d3ffffd3e 100644 --- a/store/kv/branch/store_test.go +++ b/store/kv/branch/store_test.go @@ -157,7 +157,6 @@ func (s *StoreTestSuite) TestIterator_NoWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -178,7 +177,6 @@ func (s *StoreTestSuite) TestIterator_NoWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -199,7 +197,6 @@ func (s *StoreTestSuite) TestIterator_NoWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -220,7 +217,6 @@ func (s *StoreTestSuite) TestIterator_NoWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) } @@ -264,7 +260,6 @@ func (s *StoreTestSuite) TestIterator_DirtyWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -290,7 +285,6 @@ func (s *StoreTestSuite) TestIterator_DirtyWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -316,7 +310,6 @@ func (s *StoreTestSuite) TestIterator_DirtyWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -342,7 +335,6 @@ func (s *StoreTestSuite) TestIterator_DirtyWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) } @@ -366,7 +358,6 @@ func (s *StoreTestSuite) TestReverseIterator_NoWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -388,7 +379,6 @@ func (s *StoreTestSuite) TestReverseIterator_NoWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -410,7 +400,6 @@ func (s *StoreTestSuite) TestReverseIterator_NoWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -432,7 +421,6 @@ func (s *StoreTestSuite) TestReverseIterator_NoWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) } @@ -477,7 +465,6 @@ func (s *StoreTestSuite) TestReverseIterator_DirtyWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -504,7 +491,6 @@ func (s *StoreTestSuite) TestReverseIterator_DirtyWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -531,7 +517,6 @@ func (s *StoreTestSuite) TestReverseIterator_DirtyWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -558,7 +543,6 @@ func (s *StoreTestSuite) TestReverseIterator_DirtyWrites() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) } diff --git a/store/kv/mem/iterator.go b/store/kv/mem/iterator.go index e6d58437ca..da7cdc61af 100644 --- a/store/kv/mem/iterator.go +++ b/store/kv/mem/iterator.go @@ -6,10 +6,11 @@ import ( "github.com/tidwall/btree" "golang.org/x/exp/slices" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" ) -var _ store.Iterator = (*iterator)(nil) +var _ corestore.Iterator = (*iterator)(nil) type iterator struct { treeItr btree.IterG[store.KVPair] @@ -19,7 +20,7 @@ type iterator struct { valid bool } -func newIterator(tree *btree.BTreeG[store.KVPair], start, end []byte, reverse bool) store.Iterator { +func newIterator(tree *btree.BTreeG[store.KVPair], start, end []byte, reverse bool) corestore.Iterator { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { panic(store.ErrKeyEmpty) } @@ -83,9 +84,9 @@ func (itr *iterator) Value() []byte { return slices.Clone(itr.treeItr.Item().Value) } -func (itr *iterator) Next() bool { +func (itr *iterator) Next() { if !itr.valid { - return false + return } if !itr.reverse { @@ -98,11 +99,12 @@ func (itr *iterator) Next() bool { itr.valid = itr.keyInRange(itr.Key()) } - return itr.valid + return } -func (itr *iterator) Close() { +func (itr *iterator) Close() error { itr.treeItr.Release() + return nil } func (itr *iterator) Error() error { diff --git a/store/kv/mem/store.go b/store/kv/mem/store.go index eca7de48d7..4e3a1dbb8e 100644 --- a/store/kv/mem/store.go +++ b/store/kv/mem/store.go @@ -5,6 +5,7 @@ import ( "github.com/tidwall/btree" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" ) @@ -92,10 +93,10 @@ func (s *Store) Reset(_ uint64) error { return nil } -func (s *Store) Iterator(start, end []byte) store.Iterator { +func (s *Store) Iterator(start, end []byte) corestore.Iterator { return newIterator(s.tree, start, end, false) } -func (s *Store) ReverseIterator(start, end []byte) store.Iterator { +func (s *Store) ReverseIterator(start, end []byte) corestore.Iterator { return newIterator(s.tree, start, end, true) } diff --git a/store/kv/mem/store_test.go b/store/kv/mem/store_test.go index 656cf53194..3eeea2bd85 100644 --- a/store/kv/mem/store_test.go +++ b/store/kv/mem/store_test.go @@ -92,7 +92,6 @@ func (s *StoreTestSuite) TestIterator() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -113,7 +112,6 @@ func (s *StoreTestSuite) TestIterator() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -134,7 +132,7 @@ func (s *StoreTestSuite) TestIterator() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) + s.Require().False(itr.Valid()) }) @@ -155,7 +153,6 @@ func (s *StoreTestSuite) TestIterator() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) } @@ -185,7 +182,6 @@ func (s *StoreTestSuite) TestReverseIterator() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -208,7 +204,6 @@ func (s *StoreTestSuite) TestReverseIterator() { // seek past domain, which should make the iterator invalid and produce an error s.Require().False(itr.Valid()) - s.Require().False(itr.Next()) }) // reverse iterator with with a start and end domain @@ -229,7 +224,6 @@ func (s *StoreTestSuite) TestReverseIterator() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) @@ -251,7 +245,6 @@ func (s *StoreTestSuite) TestReverseIterator() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) }) } diff --git a/store/kv/trace/iterator.go b/store/kv/trace/iterator.go index 98de8b2d1c..a01d8567b0 100644 --- a/store/kv/trace/iterator.go +++ b/store/kv/trace/iterator.go @@ -3,18 +3,19 @@ package trace import ( "io" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" ) -var _ store.Iterator = (*iterator)(nil) +var _ corestore.Iterator = (*iterator)(nil) type iterator struct { - parent store.Iterator + parent corestore.Iterator writer io.Writer context store.TraceContext } -func newIterator(w io.Writer, parent store.Iterator, tc store.TraceContext) store.Iterator { +func newIterator(w io.Writer, parent corestore.Iterator, tc store.TraceContext) corestore.Iterator { return &iterator{ parent: parent, writer: w, @@ -30,16 +31,16 @@ func (itr *iterator) Valid() bool { return itr.parent.Valid() } -func (itr *iterator) Next() bool { - return itr.parent.Next() +func (itr *iterator) Next() { + itr.parent.Next() } func (itr *iterator) Error() error { return itr.parent.Error() } -func (itr *iterator) Close() { - itr.parent.Close() +func (itr *iterator) Close() error { + return itr.parent.Close() } func (itr *iterator) Key() []byte { diff --git a/store/kv/trace/store.go b/store/kv/trace/store.go index 6dcecfa0f6..278f4b8e7d 100644 --- a/store/kv/trace/store.go +++ b/store/kv/trace/store.go @@ -8,6 +8,7 @@ import ( "github.com/cockroachdb/errors" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" ) @@ -98,11 +99,11 @@ func (s *Store) BranchWithTrace(_ io.Writer, _ store.TraceContext) store.Branche panic(fmt.Sprintf("cannot call BranchWithTrace() on %T", s)) } -func (s *Store) Iterator(start, end []byte) store.Iterator { +func (s *Store) Iterator(start, end []byte) corestore.Iterator { return newIterator(s.writer, s.parent.Iterator(start, end), s.context) } -func (s *Store) ReverseIterator(start, end []byte) store.Iterator { +func (s *Store) ReverseIterator(start, end []byte) corestore.Iterator { return newIterator(s.writer, s.parent.ReverseIterator(start, end), s.context) } diff --git a/store/kv/trace/store_test.go b/store/kv/trace/store_test.go index f1cf1db494..6aab8a90ba 100644 --- a/store/kv/trace/store_test.go +++ b/store/kv/trace/store_test.go @@ -203,7 +203,6 @@ func TestTestTraceKVStoreIterator(t *testing.T) { } require.False(t, iterator.Valid()) - require.False(t, iterator.Next()) } func TestTestTraceKVStoreReverseIterator(t *testing.T) { @@ -258,7 +257,6 @@ func TestTestTraceKVStoreReverseIterator(t *testing.T) { } require.False(t, iterator.Valid()) - require.False(t, iterator.Next()) } func TestTraceKVStoreGetStoreType(t *testing.T) { diff --git a/store/storage/database.go b/store/storage/database.go index 884981f613..05a7fedcf1 100644 --- a/store/storage/database.go +++ b/store/storage/database.go @@ -3,6 +3,7 @@ package storage import ( "io" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" ) @@ -16,8 +17,8 @@ type Database interface { GetLatestVersion() (uint64, error) SetLatestVersion(version uint64) error - Iterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) - ReverseIterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) + Iterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) + ReverseIterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) Prune(version uint64) error diff --git a/store/storage/pebbledb/db.go b/store/storage/pebbledb/db.go index 449656bf53..5a62c6b712 100644 --- a/store/storage/pebbledb/db.go +++ b/store/storage/pebbledb/db.go @@ -10,6 +10,7 @@ import ( "github.com/cockroachdb/pebble" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" "cosmossdk.io/store/v2/storage" ) @@ -262,7 +263,7 @@ func (db *Database) Prune(version uint64) error { return db.setPruneHeight(version) } -func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) { +func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { return nil, store.ErrKeyEmpty } @@ -283,7 +284,7 @@ func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) return newPebbleDBIterator(itr, storePrefix(storeKey), start, end, version, db.earliestVersion, false), nil } -func (db *Database) ReverseIterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) { +func (db *Database) ReverseIterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { return nil, store.ErrKeyEmpty } diff --git a/store/storage/pebbledb/iterator.go b/store/storage/pebbledb/iterator.go index aa9ff04bc0..10b03bbe11 100644 --- a/store/storage/pebbledb/iterator.go +++ b/store/storage/pebbledb/iterator.go @@ -7,20 +7,17 @@ import ( "github.com/cockroachdb/pebble" - "cosmossdk.io/store/v2" + corestore "cosmossdk.io/core/store" ) -var _ store.Iterator = (*iterator)(nil) +var _ corestore.Iterator = (*iterator)(nil) // iterator implements the store.Iterator interface. It wraps a PebbleDB iterator // with added MVCC key handling logic. The iterator will iterate over the key space // in the provided domain for a given version. If a key has been written at the // provided version, that key/value pair will be iterated over. Otherwise, the // latest version for that key/value pair will be iterated over s.t. it's less -// than the provided version. Note: -// -// - The start key must not be empty. -// - Currently, reverse iteration is NOT supported. +// than the provided version. type iterator struct { source *pebble.Iterator prefix, start, end []byte @@ -76,7 +73,7 @@ func newPebbleDBIterator(src *pebble.Iterator, prefix, mvccStart, mvccEnd []byte // The cursor might now be pointing at a key/value pair that is tombstoned. // If so, we must move the cursor. if itr.valid && itr.cursorTombstoned() { - itr.valid = itr.Next() + itr.Next() } return itr @@ -115,7 +112,7 @@ func (itr *iterator) Value() []byte { return slices.Clone(val) } -func (itr *iterator) Next() bool { +func (itr *iterator) Next() { var next bool if itr.reverse { currKey, _, ok := SplitMVCCKey(itr.source.Key()) @@ -142,12 +139,12 @@ func (itr *iterator) Next() bool { // XXX: This should not happen as that would indicate we have a malformed // MVCC key. itr.valid = false - return itr.valid + return } if !bytes.HasPrefix(nextKey, itr.prefix) { // the next key must have itr.prefix as the prefix itr.valid = false - return itr.valid + return } // Move the iterator to the closest version to the desired version, so we @@ -157,14 +154,14 @@ func (itr *iterator) Next() bool { // The cursor might now be pointing at a key/value pair that is tombstoned. // If so, we must move the cursor. if itr.valid && itr.cursorTombstoned() { - itr.valid = itr.Next() + itr.Next() } - return itr.valid + return } itr.valid = false - return itr.valid + return } func (itr *iterator) Valid() bool { @@ -195,10 +192,12 @@ func (itr *iterator) Error() error { return itr.source.Error() } -func (itr *iterator) Close() { - _ = itr.source.Close() +func (itr *iterator) Close() error { + err := itr.source.Close() itr.source = nil itr.valid = false + + return err } func (itr *iterator) assertIsValid() { diff --git a/store/storage/rocksdb/db.go b/store/storage/rocksdb/db.go index 6ebe3c89dd..b2525e3d8e 100644 --- a/store/storage/rocksdb/db.go +++ b/store/storage/rocksdb/db.go @@ -11,6 +11,7 @@ import ( "github.com/linxGnu/grocksdb" "golang.org/x/exp/slices" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" "cosmossdk.io/store/v2/storage" "cosmossdk.io/store/v2/storage/util" @@ -162,7 +163,7 @@ func (db *Database) Prune(version uint64) error { return nil } -func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) { +func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { return nil, store.ErrKeyEmpty } @@ -178,7 +179,7 @@ func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) return newRocksDBIterator(itr, prefix, start, end, false), nil } -func (db *Database) ReverseIterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) { +func (db *Database) ReverseIterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { return nil, store.ErrKeyEmpty } diff --git a/store/storage/rocksdb/db_test.go b/store/storage/rocksdb/db_test.go index 8788bfe632..e3542fe91b 100644 --- a/store/storage/rocksdb/db_test.go +++ b/store/storage/rocksdb/db_test.go @@ -62,7 +62,6 @@ func TestDatabase_ReverseIterator(t *testing.T) { require.NoError(t, iter.Error()) // seek past domain, which should make the iterator invalid and produce an error - require.False(t, iter.Next()) require.False(t, iter.Valid()) // reverse iterator with with a start and end domain @@ -83,7 +82,6 @@ func TestDatabase_ReverseIterator(t *testing.T) { require.NoError(t, iter2.Error()) // seek past domain, which should make the iterator invalid and produce an error - require.False(t, iter2.Next()) require.False(t, iter2.Valid()) // start must be <= end diff --git a/store/storage/rocksdb/iterator.go b/store/storage/rocksdb/iterator.go index 80d255dce7..e960b0277a 100644 --- a/store/storage/rocksdb/iterator.go +++ b/store/storage/rocksdb/iterator.go @@ -8,10 +8,10 @@ import ( "github.com/linxGnu/grocksdb" - "cosmossdk.io/store/v2" + corestore "cosmossdk.io/core/store" ) -var _ store.Iterator = (*iterator)(nil) +var _ corestore.Iterator = (*iterator)(nil) type iterator struct { source *grocksdb.Iterator @@ -124,9 +124,9 @@ func (itr *iterator) Value() []byte { return copyAndFreeSlice(itr.source.Value()) } -func (itr iterator) Next() bool { +func (itr iterator) Next() { if itr.invalid { - return false + return } if itr.reverse { @@ -135,17 +135,19 @@ func (itr iterator) Next() bool { itr.source.Next() } - return itr.Valid() + return } func (itr *iterator) Error() error { return itr.source.Err() } -func (itr *iterator) Close() { +func (itr *iterator) Close() error { itr.source.Close() itr.source = nil itr.invalid = true + + return nil } func (itr *iterator) assertIsValid() { diff --git a/store/storage/sqlite/db.go b/store/storage/sqlite/db.go index 05fde45b03..5d6b6138f2 100644 --- a/store/storage/sqlite/db.go +++ b/store/storage/sqlite/db.go @@ -10,6 +10,7 @@ import ( _ "github.com/mattn/go-sqlite3" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" "cosmossdk.io/store/v2/storage" ) @@ -59,7 +60,7 @@ func New(dataDir string) (*Database, error) { stmt := ` CREATE TABLE IF NOT EXISTS state_storage ( - id integer not null primary key, + id integer not null primary key, store_key varchar not null, key varchar not null, value varchar not null, @@ -214,7 +215,7 @@ func (db *Database) Prune(version uint64) error { return nil } -func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) { +func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { return nil, store.ErrKeyEmpty } @@ -226,7 +227,7 @@ func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) return newIterator(db, storeKey, version, start, end, false) } -func (db *Database) ReverseIterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) { +func (db *Database) ReverseIterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) { if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) { return nil, store.ErrKeyEmpty } diff --git a/store/storage/sqlite/db_test.go b/store/storage/sqlite/db_test.go index deff98ec43..890a55a0d1 100644 --- a/store/storage/sqlite/db_test.go +++ b/store/storage/sqlite/db_test.go @@ -61,7 +61,6 @@ func TestDatabase_ReverseIterator(t *testing.T) { require.NoError(t, iter.Error()) // seek past domain, which should make the iterator invalid and produce an error - require.False(t, iter.Next()) require.False(t, iter.Valid()) // reverse iterator with with a start and end domain @@ -82,7 +81,6 @@ func TestDatabase_ReverseIterator(t *testing.T) { require.NoError(t, iter2.Error()) // seek past domain, which should make the iterator invalid and produce an error - require.False(t, iter2.Next()) require.False(t, iter2.Valid()) // start must be <= end diff --git a/store/storage/sqlite/iterator.go b/store/storage/sqlite/iterator.go index 01ef6f85d6..4549c478fa 100644 --- a/store/storage/sqlite/iterator.go +++ b/store/storage/sqlite/iterator.go @@ -8,10 +8,10 @@ import ( "golang.org/x/exp/slices" - "cosmossdk.io/store/v2" + corestore "cosmossdk.io/core/store" ) -var _ store.Iterator = (*iterator)(nil) +var _ corestore.Iterator = (*iterator)(nil) type iterator struct { statement *sql.Stmt @@ -100,14 +100,16 @@ func newIterator(db *Database, storeKey string, targetVersion uint64, start, end return itr, nil } -func (itr *iterator) Close() { +func (itr *iterator) Close() (err error) { if itr.statement != nil { - _ = itr.statement.Close() + err = itr.statement.Close() } itr.valid = false itr.statement = nil itr.rows = nil + + return err } // Domain returns the domain of the iterator. The caller must not modify the @@ -143,14 +145,14 @@ func (itr *iterator) Valid() bool { return true } -func (itr *iterator) Next() bool { +func (itr *iterator) Next() { if itr.rows.Next() { itr.parseRow() - return itr.Valid() + return } itr.valid = false - return itr.valid + return } func (itr *iterator) Error() error { diff --git a/store/storage/storage_test_suite.go b/store/storage/storage_test_suite.go index 46551b2c3e..7c7ea03e0c 100644 --- a/store/storage/storage_test_suite.go +++ b/store/storage/storage_test_suite.go @@ -265,7 +265,6 @@ func (s *StorageTestSuite) TestDatabase_Iterator() { s.Require().NoError(itr.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr.Next()) s.Require().False(itr.Valid()) } @@ -288,7 +287,6 @@ func (s *StorageTestSuite) TestDatabase_Iterator() { s.Require().NoError(itr2.Error()) // seek past domain, which should make the iterator invalid and produce an error - s.Require().False(itr2.Next()) s.Require().False(itr2.Valid()) } diff --git a/store/storage/store.go b/store/storage/store.go index 3a6fed65db..a386bfe677 100644 --- a/store/storage/store.go +++ b/store/storage/store.go @@ -3,6 +3,7 @@ package storage import ( "fmt" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2" "cosmossdk.io/store/v2/snapshots" ) @@ -74,12 +75,12 @@ func (ss *StorageStore) SetLatestVersion(version uint64) error { } // Iterator returns an iterator over the specified domain and prefix. -func (ss *StorageStore) Iterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) { +func (ss *StorageStore) Iterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) { return ss.db.Iterator(storeKey, version, start, end) } // ReverseIterator returns an iterator over the specified domain and prefix in reverse. -func (ss *StorageStore) ReverseIterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) { +func (ss *StorageStore) ReverseIterator(storeKey string, version uint64, start, end []byte) (corestore.Iterator, error) { return ss.db.ReverseIterator(storeKey, version, start, end) } diff --git a/store/store.go b/store/store.go index 8bbeda7985..a682da8429 100644 --- a/store/store.go +++ b/store/store.go @@ -4,6 +4,7 @@ import ( "io" coreheader "cosmossdk.io/core/header" + corestore "cosmossdk.io/core/store" "cosmossdk.io/store/v2/metrics" ) @@ -148,11 +149,11 @@ type KVStore interface { // // CONTRACT: No writes may happen within a domain while an iterator exists over // it, with the exception of a branched/cached KVStore. - Iterator(start, end []byte) Iterator + Iterator(start, end []byte) corestore.Iterator // ReverseIterator creates a new reverse Iterator over the domain [start, end). // It has the some properties and contracts as Iterator. - ReverseIterator(start, end []byte) Iterator + ReverseIterator(start, end []byte) corestore.Iterator } // BranchedKVStore defines an interface for a branched a KVStore. It extends KVStore