Cleanup MultiWriterAppStore (#1609)

enlight · web-flow · commit 0ea1e1e1e27a · 2020-01-30T16:43:19.000+07:00
MultiWriterAppStore was used to migrate EVM state keys from app.db to evm.db, since all clusters have now been migrated this functionality is no longer necessary, and is in fact incompatible with the changes introduced in PR #1532. This set of changes cleans out most of the obsolete code from that store: - Eliminate writes to the EvmStore from MultiWriterAppStore. - Eliminate Get/Has/Set/Delete from EvmStore. - Eliminate use of AppStore.SaveEVMStateToIAVL config setting.
diff --git a/app_test.go b/app_test.go
@@ -61,7 +61,7 @@ func mockMultiWriterStore(flushInterval int64) (*store.MultiWriterAppStore, erro
 	}
 	memDb, _ = db.LoadMemDB()
 	evmStore := store.NewEvmStore(memDb, 100, 0)
-	multiWriterStore, err := store.NewMultiWriterAppStore(iavlStore, evmStore, false)
+	multiWriterStore, err := store.NewMultiWriterAppStore(iavlStore, evmStore)
 	if err != nil {
 		return nil, err
 	}
diff --git a/cmd/loom/db/evm.go b/cmd/loom/db/evm.go
@@ -53,7 +53,7 @@ func newDumpEVMStateFromEvmDB() *cobra.Command {
 
 			fmt.Printf("version: %d, root: %x\n", version, root)
 
-			srcStateDB := gstate.NewDatabase(store.NewLoomEthDB(evmStore))
+			srcStateDB := gstate.NewDatabase(store.NewLoomEthDB(evmDB))
 			srcStateDBTrie, err := srcStateDB.OpenTrie(evmRoot)
 			if err != nil {
 				fmt.Printf("cannot open trie, %s\n", evmRoot.Hex())
diff --git a/cmd/loom/loom.go b/cmd/loom/loom.go
@@ -646,7 +646,7 @@ func loadAppStore(
 		if err != nil {
 			return nil, nil, err
 		}
-		appStore, err = store.NewMultiWriterAppStore(iavlStore, evmStore, cfg.AppStore.SaveEVMStateToIAVL)
+		appStore, err = store.NewMultiWriterAppStore(iavlStore, evmStore)
 		if err != nil {
 			return nil, nil, err
 		}
diff --git a/config/config.go b/config/config.go
@@ -747,9 +747,6 @@ AppStore:
   PruneInterval: {{ .AppStore.PruneInterval }}
   # Number of versions to prune at a time.
   PruneBatchSize: {{ .AppStore.PruneBatchSize }}
-  # If true the app store will write EVM state to both IAVLStore and EvmStore
-  # This config works with AppStore Version 3 (MultiWriterAppStore) only
-  SaveEVMStateToIAVL: {{ .AppStore.SaveEVMStateToIAVL }}
   # Specifies the number of IAVL tree versions that should be kept in memory before writing a new
   # version to disk.
   # If set to zero every version will be written to disk unless overridden via the on-chain config.
diff --git a/e2e/chainconfig-loom.yaml b/e2e/chainconfig-loom.yaml
@@ -6,6 +6,5 @@ ContractLogLevel: "debug"
 LoomLogLevel: "debug"
 AppStore:
   Version: 3
-  SaveEVMStateToIAVL: false
 CachingStore:
   CachingEnabled: true
diff --git a/e2e/chainconfig-routine-loom.yaml b/e2e/chainconfig-routine-loom.yaml
@@ -8,6 +8,5 @@ ContractLogLevel: "debug"
 LoomLogLevel: "debug"
 AppStore:
   Version: 3
-  SaveEVMStateToIAVL: false
 CachingStore:
   CachingEnabled: true
diff --git a/e2e/loom-3-loom.yaml b/e2e/loom-3-loom.yaml
@@ -6,7 +6,6 @@ ContractLogLevel: "debug"
 LoomLogLevel: "debug"
 AppStore:
   Version: 3
-  SaveEVMStateToIAVL: false
 CachingStoreConfig:
   CachingEnabled: true
 
diff --git a/e2e/loom-4-loom.yaml b/e2e/loom-4-loom.yaml
@@ -6,7 +6,6 @@ ContractLogLevel: "debug"
 LoomLogLevel: "debug"
 AppStore:
   Version: 3
-  SaveEVMStateToIAVL: false
 CachingStoreConfig:
   CachingEnabled: true
 
diff --git a/e2e/loom-5-loom.yaml b/e2e/loom-5-loom.yaml
@@ -7,6 +7,5 @@ ContractLogLevel: "debug"
 LoomLogLevel: "debug"
 AppStore:
   Version: 3
-  SaveEVMStateToIAVL: false
 CachingStoreConfig:
   CachingEnabled: true
diff --git a/store/config.go b/store/config.go
@@ -12,8 +12,7 @@ type AppStoreConfig struct {
 	PruneInterval int64
 	// Number of versions to prune at a time.
 	PruneBatchSize int64
-	// If true the app store will write EVM state to both IAVLStore and EvmStore
-	// This config works with AppStore Version 3 (MultiWriterAppStore) only
+	// Obsolete and no longer used
 	SaveEVMStateToIAVL bool
 	// Specifies the number of IAVL tree versions that should be kept in memory before writing a new
 	// version to disk.
diff --git a/store/evmstore.go b/store/evmstore.go
@@ -3,25 +3,24 @@ package store
 import (
 	"bytes"
 	"encoding/binary"
-	"sort"
 	"time"
 
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/trie"
 	"github.com/go-kit/kit/metrics"
 	kitprometheus "github.com/go-kit/kit/metrics/prometheus"
 	lru "github.com/hashicorp/golang-lru"
-	"github.com/loomnetwork/go-loom/plugin"
 	"github.com/loomnetwork/go-loom/util"
 	"github.com/loomnetwork/loomchain/db"
 	"github.com/pkg/errors"
 	stdprometheus "github.com/prometheus/client_golang/prometheus"
-	dbm "github.com/tendermint/tendermint/libs/db"
 )
 
 var (
 	defaultRoot = []byte{1}
 	rootHashKey = util.PrefixKey(vmPrefix, rootKey)
+	// Prefix for versioned Patricia roots
+	evmRootPrefix = []byte("evmroot")
 
 	commitDuration metrics.Histogram
 )
@@ -75,82 +74,11 @@ func NewEvmStore(evmDB db.DBWrapper, numCachedRoots int, flushInterval int64) *E
 		rootCache:     rootCache,
 		flushInterval: flushInterval,
 	}
-	ethDB := NewLoomEthDB(evmStore)
+	ethDB := NewLoomEthDB(evmDB)
 	evmStore.trieDB = trie.NewDatabase(ethDB)
 	return evmStore
 }
 
-func (s *EvmStore) NewBatch() dbm.Batch {
-	return s.evmDB.NewBatch()
-}
-
-// Range iterates in-order over the keys in the store prefixed by the given prefix.
-// TODO (VM): This needs a proper review, other than tests there is no code that really makes use of
-//            this function, only place it's called is from MultiWriterAppStore.Range but only when
-//            iterating over the "vm" prefix - which no code currently does.
-// NOTE: This version of EvmStore supports Range(nil)
-func (s *EvmStore) Range(prefix []byte) plugin.RangeData {
-	rangeCacheKeys := []string{}
-	rangeCache := make(map[string][]byte)
-
-	// Add records from evm.db to range cache
-	iter := s.evmDB.Iterator(prefix, nil)
-	defer iter.Close()
-	for ; iter.Valid(); iter.Next() {
-		key := string(iter.Key())
-		value := iter.Value()
-		if util.HasPrefix([]byte(key), prefix) || len(prefix) == 0 {
-			rangeCache[key] = value
-			rangeCacheKeys = append(rangeCacheKeys, key)
-		}
-	}
-
-	// Make Range return root hash (vmvmroot) from EvmStore.rootHash
-	if _, exist := rangeCache[string(rootHashKey)]; exist {
-		rangeCache[string(rootHashKey)] = s.rootHash
-	}
-
-	ret := make(plugin.RangeData, 0)
-	// Sorting makes RangeData deterministic
-	sort.Strings(rangeCacheKeys)
-	for _, key := range rangeCacheKeys {
-		var unprefixedKey []byte
-		var err error
-		if len(prefix) > 0 {
-			unprefixedKey, err = util.UnprefixKey([]byte(key), prefix)
-			if err != nil {
-				continue
-			}
-		} else {
-			unprefixedKey = []byte(key)
-		}
-		re := &plugin.RangeEntry{
-			Key:   unprefixedKey,
-			Value: rangeCache[key],
-		}
-		ret = append(ret, re)
-	}
-	return ret
-}
-
-// TODO: Range/Has/Get/Delete/Set are probably only called from the MultiWriterAppStore which
-//       doesn't need to do so anymore, remove these functions when MultiWriterAppStore is cleaned up.
-func (s *EvmStore) Has(key []byte) bool {
-	return s.evmDB.Has(key)
-}
-
-func (s *EvmStore) Get(key []byte) []byte {
-	return s.evmDB.Get(key)
-}
-
-func (s *EvmStore) Delete(key []byte) {
-	s.evmDB.Delete(key)
-}
-
-func (s *EvmStore) Set(key, val []byte) {
-	s.evmDB.Set(key, val)
-}
-
 // Commit may persist the changes made to the store since the last commit to the underlying DB.
 // The specified version is associated with the current root, which is returned by this function.
 // Whether or not changes are actually flushed to the DB depends on the flush interval, which can
@@ -272,47 +200,3 @@ func (s *EvmStore) GetRootAt(version int64) ([]byte, int64) {
 	}
 	return s.getLastSavedRoot(version)
 }
-
-// TODO: Get rid of this function. EvmStore does not provide snapshot anymore but EVMState does.
-func (s *EvmStore) GetSnapshot(version int64) *EvmStoreSnapshot {
-	root, _ := s.GetRootAt(version)
-	return NewEvmStoreSnapshot(s.evmDB.GetSnapshot(), root)
-}
-
-// TODO: Get rid of EvmStoreSnapshot. EvmStore does not provide snapshot anymore but EVMState does.
-func NewEvmStoreSnapshot(snapshot db.Snapshot, rootHash []byte) *EvmStoreSnapshot {
-	return &EvmStoreSnapshot{
-		Snapshot: snapshot,
-		rootHash: rootHash,
-	}
-}
-
-type EvmStoreSnapshot struct {
-	db.Snapshot
-	rootHash []byte
-}
-
-func (s *EvmStoreSnapshot) Get(key []byte) []byte {
-	if bytes.Equal(key, rootHashKey) {
-		return s.rootHash
-	}
-	return s.Snapshot.Get(key)
-}
-
-func (s *EvmStoreSnapshot) Has(key []byte) bool {
-	// snapshot always has a root hash
-	// nil or empty root hash is considered valid root hash
-	if bytes.Equal(key, rootHashKey) {
-		return true
-	}
-	return s.Snapshot.Has(key)
-}
-
-func remove(keys []string, key string) []string {
-	for i, value := range keys {
-		if value == key {
-			return append(keys[:i], keys[i+1:]...)
-		}
-	}
-	return keys
-}
diff --git a/store/evmstore_test.go b/store/evmstore_test.go
@@ -2,10 +2,8 @@ package store
 
 import (
 	"bytes"
-	"fmt"
 	"testing"
 
-	"github.com/loomnetwork/go-loom/util"
 	"github.com/loomnetwork/loomchain/db"
 	"github.com/stretchr/testify/suite"
 )
@@ -20,118 +18,6 @@ func (t *EvmStoreTestSuite) SetupTest() {
 func TestEvmStoreTestSuite(t *testing.T) {
 	suite.Run(t, new(EvmStoreTestSuite))
 }
-func (t *EvmStoreTestSuite) TestEvmStoreRangeAndCommit() {
-	require := t.Require()
-	evmDb, err := db.LoadMemDB()
-	require.NoError(err)
-	evmStore := NewEvmStore(evmDb, 100, 0)
-	for i := 0; i <= 100; i++ {
-		key := []byte(fmt.Sprintf("Key%d", i))
-		evmStore.Set(key, key)
-	}
-	evmStore.Set([]byte("hellovm"), []byte("world"))
-	evmStore.Set([]byte("hellovm"), []byte("world3"))
-	evmStore.Set([]byte("hello1"), []byte("world1"))
-	evmStore.Set([]byte("hello2"), []byte("world2"))
-	evmStore.Set([]byte("hello3"), []byte("world3"))
-	evmStore.Set([]byte("hello3"), []byte("world3"))
-	evmStore.Delete([]byte("hello2"))
-	dataRange := evmStore.Range(nil)
-	require.Equal(104, len(dataRange))
-	require.Equal(false, evmStore.Has([]byte("hello2")))
-	evmStore.Commit(1, 0)
-	evmStore.Set([]byte("SSSSS"), []byte("SSSSS"))
-	evmStore.Set([]byte("vvvvv"), []byte("vvv"))
-	dataRange = evmStore.Range(nil)
-	require.Equal(106+1, len(dataRange)) // +1 default evm root key
-	evmStore.Commit(2, 0)
-	evmStore.Set([]byte("SSSSS"), []byte("S1"))
-	ret := evmStore.Get([]byte("SSSSS"))
-	require.Equal(0, bytes.Compare(ret, []byte("S1")))
-	evmStore.Delete([]byte("SSSSS"))
-	evmStore.Delete([]byte("hello1"))
-	dataRange = evmStore.Range(nil)
-	require.Equal(104+1, len(dataRange)) // +1 default evm root key
-	evmStore.Commit(3, 0)
-	evmStore.Delete([]byte("SSSSS"))
-	evmStore.Delete([]byte("hello1"))
-	dataRange = evmStore.Range(nil)
-	require.Equal(104+1, len(dataRange)) // +1 default evm root key
-}
-
-func (t *EvmStoreTestSuite) TestEvmStoreBasicMethods() {
-	require := t.Require()
-	// Test Get|Set|Has|Delete methods
-	evmDb, err := db.LoadMemDB()
-	require.NoError(err)
-	evmStore := NewEvmStore(evmDb, 100, 0)
-	key1 := []byte("hello")
-	key2 := []byte("hello2")
-	value1 := []byte("world")
-	value2 := []byte("world2")
-	value3 := []byte("This is a new value")
-	evmStore.Set(key1, value1)
-	evmStore.Set(key2, value2)
-	result := evmStore.Get(key1)
-	require.Equal(0, bytes.Compare(value1, result))
-	evmStore.Set(key1, value3)
-	result = evmStore.Get(key1)
-	require.Equal(0, bytes.Compare(value3, result))
-	has := evmStore.Has(key1)
-	require.Equal(true, has)
-	evmStore.Delete(key1)
-	has = evmStore.Has(key1)
-	require.Equal(false, has)
-	result = evmStore.Get(key1)
-	fmt.Println(result)
-	require.Equal(0, len(result))
-}
-
-func (t *EvmStoreTestSuite) TestEvmStoreRangePrefix() {
-	require := t.Require()
-	// Test Range Prefix
-	evmDb, err := db.LoadMemDB()
-	require.NoError(err)
-	evmStore := NewEvmStore(evmDb, 100, 0)
-	for i := 0; i <= 100; i++ {
-		key := []byte(fmt.Sprintf("Key%d", i))
-		evmStore.Set(key, key)
-	}
-	for i := 0; i <= 100; i++ {
-		key := []byte(fmt.Sprintf("vv%dKey", i))
-		evmStore.Set(key, key)
-	}
-	dataRange := evmStore.Range(nil)
-	require.Equal(202, len(dataRange))
-
-	dataRange = evmStore.Range([]byte("Key"))
-	require.Equal(0, len(dataRange))
-
-	for i := 0; i <= 100; i++ {
-		key := util.PrefixKey([]byte("Key"), []byte(fmt.Sprintf("%d", i)))
-		evmStore.Set(key, key)
-		key = util.PrefixKey([]byte("vv"), []byte(fmt.Sprintf("%d", i)))
-		evmStore.Set(key, key)
-	}
-
-	dataRange = evmStore.Range([]byte("Key"))
-	require.Equal(101, len(dataRange))
-
-	dataRange = evmStore.Range([]byte("vv"))
-	require.Equal(101, len(dataRange))
-
-	evmStore.Commit(1, 0)
-	dataRange = evmStore.Range([]byte("Key"))
-	require.Equal(101, len(dataRange))
-
-	dataRange = evmStore.Range([]byte("vv"))
-	require.Equal(101, len(dataRange))
-
-	evmStore.Commit(2, 0)
-	evmStore.Delete(util.PrefixKey([]byte("vv"), []byte(fmt.Sprintf("%d", 10))))
-	dataRange = evmStore.Range([]byte("vv"))
-	require.Equal(100, len(dataRange))
-}
 
 func (t *EvmStoreTestSuite) TestLoadVersionEvmStore() {
 	require := t.Require()
diff --git a/store/loomethdb.go b/store/loomethdb.go
diff --git a/store/multi_writer_app_store.go b/store/multi_writer_app_store.go
diff --git a/store/multi_writer_app_store_test.go b/store/multi_writer_app_store_test.go

Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ func mockMultiWriterStore(flushInterval int64) (*store.MultiWriterAppStore, erro`
`61`	`61`	`}`
`62`	`62`	`memDb, _ = db.LoadMemDB()`
`63`	`63`	`evmStore := store.NewEvmStore(memDb, 100, 0)`
`64`		`- multiWriterStore, err := store.NewMultiWriterAppStore(iavlStore, evmStore, false)`
	`64`	`+ multiWriterStore, err := store.NewMultiWriterAppStore(iavlStore, evmStore)`
`65`	`65`	`if err != nil {`
`66`	`66`	`return nil, err`
`67`	`67`	`}`
Original file line number	Diff line number	Diff line change
`@@ -646,7 +646,7 @@ func loadAppStore(`
`646`	`646`	`if err != nil {`
`647`	`647`	`return nil, nil, err`
`648`	`648`	`}`
`649`		`- appStore, err = store.NewMultiWriterAppStore(iavlStore, evmStore, cfg.AppStore.SaveEVMStateToIAVL)`
	`649`	`+ appStore, err = store.NewMultiWriterAppStore(iavlStore, evmStore)`
`650`	`650`	`if err != nil {`
`651`	`651`	`return nil, nil, err`
`652`	`652`	`}`