chore(store/v2): API tweaks and cleanup

store/batch.go

@@ -15,14 +15,3 @@ type Batch interface {
 	// Reset resets the batch.
 	Reset()
 }
-
-// Batcher defines an interface for creating batches with a backing data store.
-type Batcher interface {
-	// NewBatch creates a write-only database that buffers changes to the underlying
-	// db until a final write is called.
-	NewBatch() (Batch, error)
-}
-
-type VersionedBatcher interface {
-	NewBatch(version uint64) (Batch, error)
-}

store/branchkv/store.go

@@ -224,7 +224,7 @@ func (s *Store) Iterator(start, end []byte) store.Iterator {
 		parentItr = s.parent.Iterator(start, end)
 	} else {
 		var err error
-		parentItr, err = s.storage.NewIterator(s.storeKey, s.version, start, end)
+		parentItr, err = s.storage.Iterator(s.storeKey, s.version, start, end)
 		if err != nil {
 			panic(err)
 		}
@@ -250,7 +250,7 @@ func (s *Store) ReverseIterator(start, end []byte) store.Iterator {
 		parentItr = s.parent.ReverseIterator(start, end)
 	} else {
 		var err error
-		parentItr, err = s.storage.NewReverseIterator(s.storeKey, s.version, start, end)
+		parentItr, err = s.storage.ReverseIterator(s.storeKey, s.version, start, end)
 		if err != nil {
 			panic(err)
 		}

store/branchkv/store_test.go

@@ -28,18 +28,15 @@ func (s *StoreTestSuite) SetupTest() {
 	storage, err := sqlite.New(s.T().TempDir())
 	s.Require().NoError(err)
 
-	batch, err := storage.NewBatch(1)
-	s.Require().NoError(err)
-
+	cs := new(store.Changeset)
 	for i := 0; i < 100; i++ {
 		key := fmt.Sprintf("key%03d", i) // key000, key001, ..., key099
 		val := fmt.Sprintf("val%03d", i) // val000, val001, ..., val099
-		err = batch.Set(storeKey, []byte(key), []byte(val))
-		s.Require().NoError(err)
+
+		cs.AddKVPair(store.KVPair{StoreKey: storeKey, Key: []byte(key), Value: []byte(val)})
 	}
 
-	err = batch.Write()
-	s.Require().NoError(err)
+	s.Require().NoError(storage.ApplyChangeset(1, cs))
 
 	kvStore, err := branchkv.New(storeKey, storage)
 	s.Require().NoError(err)

store/database.go

@@ -4,13 +4,6 @@ import (
 	"io"
 )
 
-// VersionedReader extends the Reader interface by making all reads versioned.
-type VersionedReader interface {
-	Has(storeKey string, version uint64, key []byte) (bool, error)
-	Get(storeKey string, version uint64, key []byte) ([]byte, error)
-	GetLatestVersion() (uint64, error)
-}
-
 // Reader wraps the Has and Get method of a backing data store.
 type Reader interface {
 	// Has retrieves if a key is present in the key-value data store.
@@ -25,13 +18,6 @@ type Reader interface {
 	Get(storeKey string, key []byte) ([]byte, error)
 }
 
-// VersionedWriter extends the Writer interface by making all writes versioned.
-type VersionedWriter interface {
-	Set(storeKey string, version uint64, key, value []byte) error
-	Delete(storeKey string, version uint64, key []byte) error
-	SetLatestVersion(version uint64) error
-}
-
 // Writer wraps the Set method of a backing data store.
 type Writer interface {
 	// Set inserts the given value into the key-value data store.
@@ -45,32 +31,27 @@ type Writer interface {
 	Delete(storeKey string, key []byte) error
 }
 
-// VersionedReaderWriter combines the VersionedReader and VersionedWriter interfaces.
-type VersionedReaderWriter interface {
-	VersionedReader
-	VersionedWriter
-}
-
-// ReaderWriter combines the Reader and Writer interfaces.
-type ReaderWriter interface {
-	Reader
-	Writer
-}
-
 // Database contains all the methods required to allow handling different
 // key-value data stores backing the database.
 type Database interface {
-	ReaderWriter
+	Reader
+	Writer
 	IteratorCreator
 	io.Closer
 }
 
-// VersionedDatabase extends the Database interface by making all reads and writes
-// versioned.
+// VersionedDatabase defines an API for a versioned database that allows reads,
+// writes, iteration and commitment over a series of versions.
 type VersionedDatabase interface {
-	VersionedReaderWriter
-	VersionedIteratorCreator
-	VersionedBatcher
+	Has(storeKey string, version uint64, key []byte) (bool, error)
+	Get(storeKey string, version uint64, key []byte) ([]byte, error)
+	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)
+
+	ApplyChangeset(version uint64, cs *Changeset) error
 
 	// Prune attempts to prune all versions up to and including the provided
 	// version argument. The operation should be idempotent. An error should be
@@ -82,6 +63,7 @@ type VersionedDatabase interface {
 	io.Closer
 }
 
+// Committer defines a contract for committing state.
 type Committer interface {
 	Commit() error
 }

store/iterator.go

@@ -1,6 +1,6 @@
 package store
 
-// Iterator ...
+// 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)
@@ -26,18 +26,13 @@ type Iterator interface {
 	Close()
 }
 
-// IteratorCreator ...
+// IteratorCreator defines an interface for creating forward and reverse iterators.
 type IteratorCreator interface {
-	// NewIterator creates a new iterator for the given store name and domain, where
+	// 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.
-	NewIterator(storeKey string, start, end []byte) (Iterator, error)
-	// NewReverseIterator creates a new reverse iterator for the given store name
+	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.
-	NewReverseIterator(storeKey string, start, end []byte) (Iterator, error)
-}
-
-type VersionedIteratorCreator interface {
-	NewIterator(storeKey string, version uint64, start, end []byte) (Iterator, error)
-	NewReverseIterator(storeKey string, version uint64, start, end []byte) (Iterator, error)
+	ReverseIterator(storeKey string, start, end []byte) (Iterator, error)
 }

store/root/store.go

@@ -217,10 +217,10 @@ func (s *Store) Commit() ([]byte, error) {
 		s.logger.Debug("commit header and version mismatch", "header_height", s.commitHeader.GetHeight(), "version", version)
 	}
 
-	changeSet := s.rootKVStore.GetChangeset()
+	changeset := s.rootKVStore.GetChangeset()
 
 	// commit SS
-	if err := s.commitSS(version, changeSet); err != nil {
+	if err := s.stateStore.ApplyChangeset(version, changeset); err != nil {
 		return nil, fmt.Errorf("failed to commit SS: %w", err)
 	}
 
@@ -308,30 +308,3 @@ func (s *Store) commitSC() error {
 
 	return nil
 }
-
-// commitSS flushes all accumulated writes to the SS backend via a single batch.
-// Note, this is a synchronous operation. It returns an error if the batch write
-// fails.
-//
-// TODO: Commit writes to SS backend asynchronously.
-// Ref: https://github.com/cosmos/cosmos-sdk/issues/17314
-func (s *Store) commitSS(version uint64, cs *store.Changeset) error {
-	batch, err := s.stateStore.NewBatch(version)
-	if err != nil {
-		return err
-	}
-
-	for _, pair := range cs.Pairs {
-		if pair.Value == nil {
-			if err := batch.Delete(pair.StoreKey, pair.Key); err != nil {
-				return err
-			}
-		} else {
-			if err := batch.Set(pair.StoreKey, pair.Key, pair.Value); err != nil {
-				return err
-			}
-		}
-	}
-
-	return batch.Write()
-}

store/storage/pebbledb/db.go

@@ -126,39 +126,27 @@ func (db *Database) Get(storeKey string, targetVersion uint64, key []byte) ([]by
 	return nil, nil
 }
 
-func (db *Database) Set(storeKey string, version uint64, key, value []byte) error {
-	b, err := db.NewBatch(version)
+func (db *Database) ApplyChangeset(version uint64, cs *store.Changeset) error {
+	b, err := NewBatch(db.storage, version)
 	if err != nil {
 		return err
 	}
 
-	if err := b.Set(storeKey, key, value); err != nil {
-		return err
-	}
-
-	return b.Write()
-}
-
-// Delete marks the key as deleted. The key will not be retrievable at or before
-// the provided version. However, the key is still persisted in the underlying
-// database engine as it's value is marked with a non-zero tombestone.
-func (db *Database) Delete(storeKey string, version uint64, key []byte) error {
-	b, err := db.NewBatch(version)
-	if err != nil {
-		return err
-	}
-
-	if err := b.Delete(storeKey, key); err != nil {
-		return err
+	for _, kvPair := range cs.Pairs {
+		if kvPair.Value == nil {
+			if err := b.Delete(kvPair.StoreKey, kvPair.Key); err != nil {
+				return err
+			}
+		} else {
+			if err := b.Set(kvPair.StoreKey, kvPair.Key, kvPair.Value); err != nil {
+				return err
+			}
+		}
 	}
 
 	return b.Write()
 }
 
-func (db *Database) NewBatch(version uint64) (store.Batch, error) {
-	return NewBatch(db.storage, version)
-}
-
 // Prune for the PebbleDB SS backend is currently not supported. It seems the only
 // reliable way to prune is to iterate over the desired domain and either manually
 // tombstone or delete. Either way, the operation would be timely.
@@ -168,7 +156,7 @@ func (db *Database) Prune(version uint64) error {
 	panic("not implemented!")
 }
 
-func (db *Database) NewIterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) {
+func (db *Database) Iterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) {
 	if (start != nil && len(start) == 0) || (end != nil && len(end) == 0) {
 		return nil, store.ErrKeyEmpty
 	}
@@ -192,7 +180,7 @@ func (db *Database) NewIterator(storeKey string, version uint64, start, end []by
 	return newPebbleDBIterator(itr, storePrefix(storeKey), start, end, version), nil
 }
 
-func (db *Database) NewReverseIterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) {
+func (db *Database) ReverseIterator(storeKey string, version uint64, start, end []byte) (store.Iterator, error) {
 	panic("not implemented!")
 }
 

store/storage/pebbledb/db_test.go

@@ -32,5 +32,5 @@ func TestDatabase_ReverseIterator(t *testing.T) {
 	require.NoError(t, err)
 	defer db.Close()
 
-	require.Panics(t, func() { _, _ = db.NewReverseIterator(storeKey1, 1, []byte("key000"), nil) })
+	require.Panics(t, func() { _, _ = db.ReverseIterator(storeKey1, 1, []byte("key000"), nil) })
 }