From 5f2521214f0b1309697abb64b92dcdd5a39bf35e Mon Sep 17 00:00:00 2001
From: Sean Young <sean.young@monax.io>
Date: Fri, 6 Apr 2018 13:59:07 +0100
Subject: [PATCH] Remove iteration from cache
The IterateStorage and IterateAccounts methods are broken. They return
entries accounts which have been removed, and entries which are in
the cache are returned twice (from cache and backend).
So remove iterability from the cache and solve these problems.
Signed-off-by: Sean Young <sean.young@monax.io>
---
account/state/state_cache.go | 23 +++++++++--------------
account/state/state_cache_test.go | 20 --------------------
execution/execution.go | 24 +-----------------------
3 files changed, 10 insertions(+), 57 deletions(-)
diff --git a/account/state/state_cache.go b/account/state/state_cache.go
index fd7ad33c..04107211 100644
--- a/account/state/state_cache.go
+++ b/account/state/state_cache.go
@@ -24,16 +24,15 @@ import (
)
type Cache interface {
- IterableWriter
+ Writer
Sync(state Writer) error
Reset(backend Iterable)
Flush(state IterableWriter) error
- Backend() Iterable
}
type stateCache struct {
sync.RWMutex
- backend Iterable
+ backend Reader
accounts map[acm.Address]*accountInfo
}
@@ -47,7 +46,7 @@ type accountInfo struct {
// Returns a Cache that wraps an underlying Reader to use on a cache miss, can write to an output Writer
// via Sync. Goroutine safe for concurrent access.
-func NewCache(backend Iterable) Cache {
+func NewCache(backend Reader) Cache {
return &stateCache{
backend: backend,
accounts: make(map[acm.Address]*accountInfo),
@@ -97,8 +96,8 @@ func (cache *stateCache) RemoveAccount(address acm.Address) error {
return nil
}
-// Iterates over all accounts first in cache and then in backend until consumer returns true for 'stop'
-func (cache *stateCache) IterateAccounts(consumer func(acm.Account) (stop bool)) (stopped bool, err error) {
+// Iterates over all cached accounts first in cache and then in backend until consumer returns true for 'stop'
+func (cache *stateCache) IterateCachedAccount(consumer func(acm.Account) (stop bool)) (stopped bool, err error) {
// Try cache first for early exit
cache.RLock()
for _, info := range cache.accounts {
@@ -108,7 +107,7 @@ func (cache *stateCache) IterateAccounts(consumer func(acm.Account) (stop bool))
}
}
cache.RUnlock()
- return cache.backend.IterateAccounts(consumer)
+ return false, nil
}
func (cache *stateCache) GetStorage(address acm.Address, key binary.Word256) (binary.Word256, error) {
@@ -152,8 +151,8 @@ func (cache *stateCache) SetStorage(address acm.Address, key binary.Word256, val
return nil
}
-// Iterates over all storage items first in cache and then in backend until consumer returns true for 'stop'
-func (cache *stateCache) IterateStorage(address acm.Address,
+// Iterates over all cached storage items first in cache and then in backend until consumer returns true for 'stop'
+func (cache *stateCache) IterateCachedStorage(address acm.Address,
consumer func(key, value binary.Word256) (stop bool)) (stopped bool, err error) {
accInfo, err := cache.get(address)
if err != nil {
@@ -168,7 +167,7 @@ func (cache *stateCache) IterateStorage(address acm.Address,
}
}
accInfo.RUnlock()
- return cache.backend.IterateStorage(address, consumer)
+ return false, nil
}
// Syncs changes to the backend in deterministic order. Sends storage updates before updating
@@ -234,10 +233,6 @@ func (cache *stateCache) Flush(state IterableWriter) error {
return nil
}
-func (cache *stateCache) Backend() Iterable {
- return cache.backend
-}
-
// Get the cache accountInfo item creating it if necessary
func (cache *stateCache) get(address acm.Address) (*accountInfo, error) {
cache.RLock()
diff --git a/account/state/state_cache_test.go b/account/state/state_cache_test.go
index 80801d8c..a0c5bc80 100644
--- a/account/state/state_cache_test.go
+++ b/account/state/state_cache_test.go
@@ -131,8 +131,6 @@ type testState struct {
Storage map[acm.Address]map[binary.Word256]binary.Word256
}
-var _ Iterable = &testState{}
-
func newTestState() *testState {
return &testState{
Accounts: make(map[acm.Address]acm.Account),
@@ -165,24 +163,6 @@ func word(str string) binary.Word256 {
return binary.LeftPadWord256([]byte(str))
}
-func (tsr *testState) IterateAccounts(consumer func(acm.Account) (stop bool)) (stopped bool, err error) {
- for _, acc := range tsr.Accounts {
- if consumer(acc) {
- return true, nil
- }
- }
- return false, nil
-}
-
-func (tsr *testState) IterateStorage(address acm.Address, consumer func(key, value binary.Word256) (stop bool)) (stopped bool, err error) {
- for key, value := range tsr.Storage[address] {
- if consumer(key, value) {
- return true, nil
- }
- }
- return false, nil
-}
-
func (tsr *testState) GetAccount(address acm.Address) (acm.Account, error) {
return tsr.Accounts[address], nil
}
diff --git a/execution/execution.go b/execution/execution.go
index 7b5a9ee7..58277636 100644
--- a/execution/execution.go
+++ b/execution/execution.go
@@ -37,9 +37,7 @@ import (
const GasLimit = uint64(1000000)
type BatchExecutor interface {
- state.Iterable
- state.AccountUpdater
- state.StorageSetter
+ state.Reader
// Execute transaction against block cache (i.e. block buffer)
Execute(tx txs.Tx) error
// Reset executor to underlying State
@@ -120,31 +118,11 @@ func (exe *executor) GetAccount(address acm.Address) (acm.Account, error) {
return exe.stateCache.GetAccount(address)
}
-func (exe *executor) UpdateAccount(account acm.Account) error {
- return exe.stateCache.UpdateAccount(account)
-}
-
-func (exe *executor) RemoveAccount(address acm.Address) error {
- return exe.stateCache.RemoveAccount(address)
-}
-
-func (exe *executor) IterateAccounts(consumer func(acm.Account) bool) (bool, error) {
- return exe.stateCache.IterateAccounts(consumer)
-}
-
// Storage
func (exe *executor) GetStorage(address acm.Address, key binary.Word256) (binary.Word256, error) {
return exe.stateCache.GetStorage(address, key)
}
-func (exe *executor) SetStorage(address acm.Address, key binary.Word256, value binary.Word256) error {
- return exe.stateCache.SetStorage(address, key, value)
-}
-
-func (exe *executor) IterateStorage(address acm.Address, consumer func(key, value binary.Word256) bool) (bool, error) {
- return exe.stateCache.IterateStorage(address, consumer)
-}
-
func (exe *executor) Commit() (hash []byte, err error) {
exe.Lock()
defer exe.Unlock()
--
GitLab