R4R: Store Refactor 1

PENDING.md

@@ -49,6 +49,7 @@ IMPROVEMENTS
   * [\#3420](https://github.com/cosmos/cosmos-sdk/issues/3420) Added maximum length to governance proposal descriptions and titles
 
 * SDK
+  * [\#2986](https://github.com/cosmos/cosmos-sdk/pull/2986) Store Refactor
   * \#3435 Test that store implementations do not allow nil values
 
 * Tendermint

baseapp/baseapp.go

@@ -114,7 +114,7 @@ func (app *BaseApp) Name() string {
 // SetCommitMultiStoreTracer sets the store tracer on the BaseApp's underlying
 // CommitMultiStore.
 func (app *BaseApp) SetCommitMultiStoreTracer(w io.Writer) {
-	app.cms.WithTracer(w)
+	app.cms.SetTracer(w)
 }
 
 // Mount IAVL or DB stores to the provided keys in the BaseApp multistore
@@ -483,8 +483,7 @@ func handleQueryCustom(app *BaseApp, path []string, req abci.RequestQuery) (res
 // BeginBlock implements the ABCI application interface.
 func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeginBlock) {
 	if app.cms.TracingEnabled() {
-		app.cms.ResetTraceContext()
-		app.cms.WithTracingContext(sdk.TraceContext(
+		app.cms.SetTracingContext(sdk.TraceContext(
 			map[string]interface{}{"blockHeight": req.Header.Height},
 		))
 	}
@@ -679,7 +678,7 @@ func (app *BaseApp) cacheTxContext(ctx sdk.Context, txBytes []byte) (
 	// TODO: https://github.com/cosmos/cosmos-sdk/issues/2824
 	msCache := ms.CacheMultiStore()
 	if msCache.TracingEnabled() {
-		msCache = msCache.WithTracingContext(
+		msCache = msCache.SetTracingContext(
 			sdk.TraceContext(
 				map[string]interface{}{
 					"txHash": fmt.Sprintf("%X", tmhash.Sum(txBytes)),
@@ -813,7 +812,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 // EndBlock implements the ABCI application interface.
 func (app *BaseApp) EndBlock(req abci.RequestEndBlock) (res abci.ResponseEndBlock) {
 	if app.deliverState.ms.TracingEnabled() {
-		app.deliverState.ms = app.deliverState.ms.ResetTraceContext().(sdk.CacheMultiStore)
+		app.deliverState.ms = app.deliverState.ms.SetTracingContext(nil).(sdk.CacheMultiStore)
 	}
 
 	if app.endBlocker != nil {

baseapp/baseapp_test.go

@@ -7,7 +7,7 @@ import (
 	"os"
 	"testing"
 
-	"github.com/cosmos/cosmos-sdk/store"
+	store "github.com/cosmos/cosmos-sdk/store/types"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"

client/context/query.go

@@ -10,15 +10,14 @@ import (
 
 	"strings"
 
+	"github.com/cosmos/cosmos-sdk/store/rootmulti"
 	abci "github.com/tendermint/tendermint/abci/types"
 	"github.com/tendermint/tendermint/crypto/merkle"
 	cmn "github.com/tendermint/tendermint/libs/common"
 	tmliteErr "github.com/tendermint/tendermint/lite/errors"
 	tmliteProxy "github.com/tendermint/tendermint/lite/proxy"
 	rpcclient "github.com/tendermint/tendermint/rpc/client"
 	tmtypes "github.com/tendermint/tendermint/types"
-
-	"github.com/cosmos/cosmos-sdk/store"
 )
 
 // GetNode returns an RPC client. If the context's client is not defined, an
@@ -207,7 +206,7 @@ func (ctx CLIContext) verifyProof(queryPath string, resp abci.ResponseQuery) err
 	}
 
 	// TODO: Instead of reconstructing, stash on CLIContext field?
-	prt := store.DefaultProofRuntime()
+	prt := rootmulti.DefaultProofRuntime()
 
 	// TODO: Better convention for path?
 	storeName, err := parseQueryStorePath(queryPath)
@@ -247,7 +246,7 @@ func isQueryStoreWithProof(path string) bool {
 		return false
 	case paths[0] != "store":
 		return false
-	case store.RequireProof("/" + paths[2]):
+	case rootmulti.RequireProof("/" + paths[2]):
 		return true
 	}
 

cmd/gaia/cmd/gaiad/main.go

@@ -60,7 +60,7 @@ func main() {
 func newApp(logger log.Logger, db dbm.DB, traceStore io.Writer) abci.Application {
 	return app.NewGaiaApp(
 		logger, db, traceStore, true,
-		baseapp.SetPruning(store.NewPruningOptions(viper.GetString("pruning"))),
+		baseapp.SetPruning(store.NewPruningOptionsFromString(viper.GetString("pruning"))),
 		baseapp.SetMinGasPrices(viper.GetString(server.FlagMinGasPrices)),
 	)
 }

cmd/gaia/cmd/gaiadebug/hack.go

@@ -50,7 +50,7 @@ func runHackCmd(cmd *cobra.Command, args []string) error {
 		fmt.Println(err)
 		os.Exit(1)
 	}
-	app := NewGaiaApp(logger, db, baseapp.SetPruning(store.NewPruningOptions(viper.GetString("pruning"))))
+	app := NewGaiaApp(logger, db, baseapp.SetPruning(store.NewPruningOptionsFromString(viper.GetString("pruning"))))
 
 	// print some info
 	id := app.LastCommitID()

server/mock/store.go

@@ -26,19 +26,15 @@ func (ms multiStore) CacheWrapWithTrace(_ io.Writer, _ sdk.TraceContext) sdk.Cac
 	panic("not implemented")
 }
 
-func (ms multiStore) ResetTraceContext() sdk.MultiStore {
-	panic("not implemented")
-}
-
 func (ms multiStore) TracingEnabled() bool {
 	panic("not implemented")
 }
 
-func (ms multiStore) WithTracingContext(tc sdk.TraceContext) sdk.MultiStore {
+func (ms multiStore) SetTracingContext(tc sdk.TraceContext) sdk.MultiStore {
 	panic("not implemented")
 }
 
-func (ms multiStore) WithTracer(w io.Writer) sdk.MultiStore {
+func (ms multiStore) SetTracer(w io.Writer) sdk.MultiStore {
 	panic("not implemented")
 }
 

store/memiterator.go → store/cachekv/memiterator.go

@@ -1,4 +1,4 @@
-package store
+package cachekv
 
 import (
 	"bytes"

store/cachemergeiterator.go → store/cachekv/mergeiterator.go

@@ -1,7 +1,9 @@
-package store
+package cachekv
 
 import (
 	"bytes"
+
+	"github.com/cosmos/cosmos-sdk/store/types"
 )
 
 // cacheMergeIterator merges a parent Iterator and a cache Iterator.
@@ -12,14 +14,14 @@ import (
 //
 // TODO: Optimize by memoizing.
 type cacheMergeIterator struct {
-	parent    Iterator
-	cache     Iterator
+	parent    types.Iterator
+	cache     types.Iterator
 	ascending bool
 }
 
-var _ Iterator = (*cacheMergeIterator)(nil)
+var _ types.Iterator = (*cacheMergeIterator)(nil)
 
-func newCacheMergeIterator(parent, cache Iterator, ascending bool) *cacheMergeIterator {
+func newCacheMergeIterator(parent, cache types.Iterator, ascending bool) *cacheMergeIterator {
 	iter := &cacheMergeIterator{
 		parent:    parent,
 		cache:     cache,

store/cachekv/store.go

@@ -0,0 +1,196 @@
+package cachekv
+
+import (
+	"bytes"
+	"io"
+	"sort"
+	"sync"
+
+	cmn "github.com/tendermint/tendermint/libs/common"
+	dbm "github.com/tendermint/tendermint/libs/db"
+
+	"github.com/cosmos/cosmos-sdk/store/types"
+
+	"github.com/cosmos/cosmos-sdk/store/tracekv"
+)
+
+// If value is nil but deleted is false, it means the parent doesn't have the
+// key.  (No need to delete upon Write())
+type cValue struct {
+	value   []byte
+	deleted bool
+	dirty   bool
+}
+
+// Store wraps an in-memory cache around an underlying types.KVStore.
+type Store struct {
+	mtx    sync.Mutex
+	cache  map[string]cValue
+	parent types.KVStore
+}
+
+var _ types.CacheKVStore = (*Store)(nil)
+
+// nolint
+func NewStore(parent types.KVStore) *Store {
+	return &Store{
+		cache:  make(map[string]cValue),
+		parent: parent,
+	}
+}
+
+// Implements Store.
+func (store *Store) GetStoreType() types.StoreType {
+	return store.parent.GetStoreType()
+}
+
+// Implements types.KVStore.
+func (store *Store) Get(key []byte) (value []byte) {
+	store.mtx.Lock()
+	defer store.mtx.Unlock()
+	types.AssertValidKey(key)
+
+	cacheValue, ok := store.cache[string(key)]
+	if !ok {
+		value = store.parent.Get(key)
+		store.setCacheValue(key, value, false, false)
+	} else {
+		value = cacheValue.value
+	}
+
+	return value
+}
+
+// Implements types.KVStore.
+func (store *Store) Set(key []byte, value []byte) {
+	store.mtx.Lock()
+	defer store.mtx.Unlock()
+	types.AssertValidKey(key)
+	types.AssertValidValue(value)
+
+	store.setCacheValue(key, value, false, true)
+}
+
+// Implements types.KVStore.
+func (store *Store) Has(key []byte) bool {
+	value := store.Get(key)
+	return value != nil
+}
+
+// Implements types.KVStore.
+func (store *Store) Delete(key []byte) {
+	store.mtx.Lock()
+	defer store.mtx.Unlock()
+	types.AssertValidKey(key)
+
+	store.setCacheValue(key, nil, true, true)
+}
+
+// Implements Cachetypes.KVStore.
+func (store *Store) Write() {
+	store.mtx.Lock()
+	defer store.mtx.Unlock()
+
+	// We need a copy of all of the keys.
+	// Not the best, but probably not a bottleneck depending.
+	keys := make([]string, 0, len(store.cache))
+	for key, dbValue := range store.cache {
+		if dbValue.dirty {
+			keys = append(keys, key)
+		}
+	}
+
+	sort.Strings(keys)
+
+	// TODO: Consider allowing usage of Batch, which would allow the write to
+	// at least happen atomically.
+	for _, key := range keys {
+		cacheValue := store.cache[key]
+		if cacheValue.deleted {
+			store.parent.Delete([]byte(key))
+		} else if cacheValue.value == nil {
+			// Skip, it already doesn't exist in parent.
+		} else {
+			store.parent.Set([]byte(key), cacheValue.value)
+		}
+	}
+
+	// Clear the cache
+	store.cache = make(map[string]cValue)
+}
+
+//----------------------------------------
+// To cache-wrap this Store further.
+
+// Implements CacheWrapper.
+func (store *Store) CacheWrap() types.CacheWrap {
+	return NewStore(store)
+}
+
+// CacheWrapWithTrace implements the CacheWrapper interface.
+func (store *Store) CacheWrapWithTrace(w io.Writer, tc types.TraceContext) types.CacheWrap {
+	return NewStore(tracekv.NewStore(store, w, tc))
+}
+
+//----------------------------------------
+// Iteration
+
+// Implements types.KVStore.
+func (store *Store) Iterator(start, end []byte) types.Iterator {
+	return store.iterator(start, end, true)
+}
+
+// Implements types.KVStore.
+func (store *Store) ReverseIterator(start, end []byte) types.Iterator {
+	return store.iterator(start, end, false)
+}
+
+func (store *Store) iterator(start, end []byte, ascending bool) types.Iterator {
+	var parent, cache types.Iterator
+
+	if ascending {
+		parent = store.parent.Iterator(start, end)
+	} else {
+		parent = store.parent.ReverseIterator(start, end)
+	}
+
+	items := store.dirtyItems(start, end, ascending)
+	cache = newMemIterator(start, end, items)
+
+	return newCacheMergeIterator(parent, cache, ascending)
+}
+
+// Constructs a slice of dirty items, to use w/ memIterator.
+func (store *Store) dirtyItems(start, end []byte, ascending bool) []cmn.KVPair {
+	items := make([]cmn.KVPair, 0)
+
+	for key, cacheValue := range store.cache {
+		if !cacheValue.dirty {
+			continue
+		}
+		if dbm.IsKeyInDomain([]byte(key), start, end) {
+			items = append(items, cmn.KVPair{Key: []byte(key), Value: cacheValue.value})
+		}
+	}
+
+	sort.Slice(items, func(i, j int) bool {
+		if ascending {
+			return bytes.Compare(items[i].Key, items[j].Key) < 0
+		}
+		return bytes.Compare(items[i].Key, items[j].Key) > 0
+	})
+
+	return items
+}
+
+//----------------------------------------
+// etc
+
+// Only entrypoint to mutate store.cache.
+func (store *Store) setCacheValue(key, value []byte, deleted bool, dirty bool) {
+	store.cache[string(key)] = cValue{
+		value:   value,
+		deleted: deleted,
+		dirty:   dirty,
+	}
+}