Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

R4R: Store Refactor 2 #3395

Merged
merged 7 commits into from
Feb 5, 2019
Merged

R4R: Store Refactor 2 #3395

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4ac087a
invert dependency
mossid Jan 5, 2019
54e82df
rm /types import
mossid Jan 25, 2019
33ad665
add test for errors
mossid Jan 25, 2019
65b5b19
rm remaining stypes
mossid Feb 4, 2019
e3924a8
deduplicate errors
mossid Feb 5, 2019
67f9143
fix test
mossid Feb 5, 2019
14a33ed
fix lint
mossid Feb 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions store/errors/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
package errors

import (
sdk "github.com/cosmos/cosmos-sdk/types"
)

// nolint - reexport
const (
CodeOK = sdk.CodeOK
CodeInternal = sdk.CodeInternal
CodeTxDecode = sdk.CodeTxDecode
CodeUnknownRequest = sdk.CodeUnknownRequest

CodespaceRoot = sdk.CodespaceRoot
)

// nolint - reexport
type Error = sdk.Error

// nolint - reexport
func ErrInternal(msg string) Error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not just use the errors in the types/ package?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is, this is just a wrapper

return sdk.ErrInternal(msg)
}
func ErrTxDecode(msg string) Error {
return sdk.ErrTxDecode(msg)
}
func ErrUnknownRequest(msg string) Error {
return sdk.ErrUnknownRequest(msg)
}
28 changes: 14 additions & 14 deletions store/gaskv/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ import (

"github.com/cosmos/cosmos-sdk/store/dbadapter"
"github.com/cosmos/cosmos-sdk/store/gaskv"
stypes "github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/store/types"

"github.com/stretchr/testify/require"
)

func newGasKVStore() stypes.KVStore {
meter := stypes.NewGasMeter(10000)
func newGasKVStore() types.KVStore {
meter := types.NewGasMeter(10000)
mem := dbadapter.Store{dbm.NewMemDB()}
return gaskv.NewStore(mem, meter, stypes.KVGasConfig())
return gaskv.NewStore(mem, meter, types.KVGasConfig())
}

func bz(s string) []byte { return []byte(s) }
Expand All @@ -26,20 +26,20 @@ func valFmt(i int) []byte { return bz(fmt.Sprintf("value%0.8d", i)) }

func TestGasKVStoreBasic(t *testing.T) {
mem := dbadapter.Store{dbm.NewMemDB()}
meter := stypes.NewGasMeter(10000)
st := gaskv.NewStore(mem, meter, stypes.KVGasConfig())
meter := types.NewGasMeter(10000)
st := gaskv.NewStore(mem, meter, types.KVGasConfig())
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
st.Set(keyFmt(1), valFmt(1))
require.Equal(t, valFmt(1), st.Get(keyFmt(1)))
st.Delete(keyFmt(1))
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
require.Equal(t, meter.GasConsumed(), stypes.Gas(6429))
require.Equal(t, meter.GasConsumed(), types.Gas(6429))
}

func TestGasKVStoreIterator(t *testing.T) {
mem := dbadapter.Store{dbm.NewMemDB()}
meter := stypes.NewGasMeter(10000)
st := gaskv.NewStore(mem, meter, stypes.KVGasConfig())
meter := types.NewGasMeter(10000)
st := gaskv.NewStore(mem, meter, types.KVGasConfig())
require.Empty(t, st.Get(keyFmt(1)), "Expected `key1` to be empty")
require.Empty(t, st.Get(keyFmt(2)), "Expected `key2` to be empty")
st.Set(keyFmt(1), valFmt(1))
Expand All @@ -57,20 +57,20 @@ func TestGasKVStoreIterator(t *testing.T) {
iterator.Next()
require.False(t, iterator.Valid())
require.Panics(t, iterator.Next)
require.Equal(t, meter.GasConsumed(), stypes.Gas(6987))
require.Equal(t, meter.GasConsumed(), types.Gas(6987))
}

func TestGasKVStoreOutOfGasSet(t *testing.T) {
mem := dbadapter.Store{dbm.NewMemDB()}
meter := stypes.NewGasMeter(0)
st := gaskv.NewStore(mem, meter, stypes.KVGasConfig())
meter := types.NewGasMeter(0)
st := gaskv.NewStore(mem, meter, types.KVGasConfig())
require.Panics(t, func() { st.Set(keyFmt(1), valFmt(1)) }, "Expected out-of-gas")
}

func TestGasKVStoreOutOfGasIterator(t *testing.T) {
mem := dbadapter.Store{dbm.NewMemDB()}
meter := stypes.NewGasMeter(20000)
st := gaskv.NewStore(mem, meter, stypes.KVGasConfig())
meter := types.NewGasMeter(20000)
st := gaskv.NewStore(mem, meter, types.KVGasConfig())
st.Set(keyFmt(1), valFmt(1))
iterator := st.Iterator(nil, nil)
iterator.Next()
Expand Down
17 changes: 8 additions & 9 deletions store/iavl/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ import (
cmn "github.com/tendermint/tendermint/libs/common"
dbm "github.com/tendermint/tendermint/libs/db"

stypes "github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/cosmos-sdk/store/cachekv"
"github.com/cosmos/cosmos-sdk/store/errors"
"github.com/cosmos/cosmos-sdk/store/tracekv"
"github.com/cosmos/cosmos-sdk/store/types"
)

const (
Expand Down Expand Up @@ -132,7 +131,7 @@ func (st *Store) CacheWrapWithTrace(w io.Writer, tc types.TraceContext) types.Ca

// Implements types.KVStore.
func (st *Store) Set(key, value []byte) {
stypes.AssertValidValue(value)
types.AssertValidValue(value)
st.tree.Set(key, value)
}

Expand Down Expand Up @@ -186,7 +185,7 @@ func getHeight(tree *iavl.MutableTree, req abci.RequestQuery) int64 {
func (st *Store) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
if len(req.Data) == 0 {
msg := "Query cannot be zero length"
return types.ErrTxDecode(msg).QueryResult()
return errors.ErrTxDecode(msg).QueryResult()
}

tree := st.tree
Expand Down Expand Up @@ -236,7 +235,7 @@ func (st *Store) Query(req abci.RequestQuery) (res abci.ResponseQuery) {
subspace := req.Data
res.Key = subspace

iterator := stypes.KVStorePrefixIterator(st, subspace)
iterator := types.KVStorePrefixIterator(st, subspace)
for ; iterator.Valid(); iterator.Next() {
KVs = append(KVs, types.KVPair{Key: iterator.Key(), Value: iterator.Value()})
}
Expand All @@ -246,7 +245,7 @@ func (st *Store) Query(req abci.RequestQuery) (res abci.ResponseQuery) {

default:
msg := fmt.Sprintf("Unexpected Query path: %v", req.Path)
return types.ErrUnknownRequest(msg).QueryResult()
return errors.ErrUnknownRequest(msg).QueryResult()
}

return
Expand Down Expand Up @@ -291,8 +290,8 @@ var _ types.Iterator = (*iavlIterator)(nil)
func newIAVLIterator(tree *iavl.ImmutableTree, start, end []byte, ascending bool) *iavlIterator {
iter := &iavlIterator{
tree: tree,
start: stypes.Cp(start),
end: stypes.Cp(end),
start: types.Cp(start),
end: types.Cp(end),
ascending: ascending,
iterCh: make(chan cmn.KVPair, 0), // Set capacity > 0?
quitCh: make(chan struct{}),
Expand Down
23 changes: 12 additions & 11 deletions store/iavl/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ import (
cmn "github.com/tendermint/tendermint/libs/common"
dbm "github.com/tendermint/tendermint/libs/db"

"github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/store/errors"
"github.com/cosmos/cosmos-sdk/store/types"
)

var (
Expand Down Expand Up @@ -443,7 +444,7 @@ func TestIAVLStoreQuery(t *testing.T) {

// query subspace before anything set
qres := iavlStore.Query(querySub)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Equal(t, valExpSubEmpty, qres.Value)

// set data
Expand All @@ -452,24 +453,24 @@ func TestIAVLStoreQuery(t *testing.T) {

// set data without commit, doesn't show up
qres = iavlStore.Query(query)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Nil(t, qres.Value)

// commit it, but still don't see on old version
cid = iavlStore.Commit()
qres = iavlStore.Query(query)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Nil(t, qres.Value)

// but yes on the new version
query.Height = cid.Version
qres = iavlStore.Query(query)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Equal(t, v1, qres.Value)

// and for the subspace
qres = iavlStore.Query(querySub)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Equal(t, valExpSub1, qres.Value)

// modify
Expand All @@ -478,28 +479,28 @@ func TestIAVLStoreQuery(t *testing.T) {

// query will return old values, as height is fixed
qres = iavlStore.Query(query)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Equal(t, v1, qres.Value)

// update to latest in the query and we are happy
query.Height = cid.Version
qres = iavlStore.Query(query)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Equal(t, v3, qres.Value)
query2 := abci.RequestQuery{Path: "/key", Data: k2, Height: cid.Version}

qres = iavlStore.Query(query2)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Equal(t, v2, qres.Value)
// and for the subspace
qres = iavlStore.Query(querySub)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Equal(t, valExpSub2, qres.Value)

// default (height 0) will show latest -1
query0 := abci.RequestQuery{Path: "/key", Data: k1}
qres = iavlStore.Query(query0)
require.Equal(t, uint32(types.CodeOK), qres.Code)
require.Equal(t, uint32(errors.CodeOK), qres.Code)
require.Equal(t, v1, qres.Value)
}

Expand Down
5 changes: 2 additions & 3 deletions store/rootmulti/proof_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,18 @@ package rootmulti
import (
"testing"

"github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
dbm "github.com/tendermint/tendermint/libs/db"

"github.com/cosmos/cosmos-sdk/store/iavl"
stypes "github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/store/types"
)

func TestVerifyIAVLStoreQueryProof(t *testing.T) {
// Create main tree for testing.
db := dbm.NewMemDB()
iStore, err := iavl.LoadStore(db, types.CommitID{}, stypes.PruneNothing)
iStore, err := iavl.LoadStore(db, types.CommitID{}, types.PruneNothing)
store := iStore.(*iavl.Store)
require.Nil(t, err)
store.Set([]byte("MYKEY"), []byte("MYVALUE"))
Expand Down
15 changes: 8 additions & 7 deletions store/rootmulti/store.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,11 @@ import (

"github.com/cosmos/cosmos-sdk/store/cachemulti"
"github.com/cosmos/cosmos-sdk/store/dbadapter"
"github.com/cosmos/cosmos-sdk/store/errors"
"github.com/cosmos/cosmos-sdk/store/iavl"
"github.com/cosmos/cosmos-sdk/store/tracekv"
"github.com/cosmos/cosmos-sdk/store/transient"
"github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/store/types"
)

const (
Expand Down Expand Up @@ -288,12 +289,12 @@ func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery {
store := rs.getStoreByName(storeName)
if store == nil {
msg := fmt.Sprintf("no such store: %s", storeName)
return types.ErrUnknownRequest(msg).QueryResult()
return errors.ErrUnknownRequest(msg).QueryResult()
}
queryable, ok := store.(types.Queryable)
if !ok {
msg := fmt.Sprintf("store %s doesn't support queries", storeName)
return types.ErrUnknownRequest(msg).QueryResult()
return errors.ErrUnknownRequest(msg).QueryResult()
}

// trim the path and make the query
Expand All @@ -305,12 +306,12 @@ func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery {
}

if res.Proof == nil || len(res.Proof.Ops) == 0 {
return types.ErrInternal("substore proof was nil/empty when it should never be").QueryResult()
return errors.ErrInternal("substore proof was nil/empty when it should never be").QueryResult()
}

commitInfo, errMsg := getCommitInfo(rs.db, res.Height)
if errMsg != nil {
return types.ErrInternal(errMsg.Error()).QueryResult()
return errors.ErrInternal(errMsg.Error()).QueryResult()
}

// Restore origin path and append proof op.
Expand All @@ -327,9 +328,9 @@ func (rs *Store) Query(req abci.RequestQuery) abci.ResponseQuery {
// parsePath expects a format like /<storeName>[/<subpath>]
// Must start with /, subpath may be empty
// Returns error if it doesn't start with /
func parsePath(path string) (storeName string, subpath string, err types.Error) {
func parsePath(path string) (storeName string, subpath string, err errors.Error) {
if !strings.HasPrefix(path, "/") {
err = types.ErrUnknownRequest(fmt.Sprintf("invalid path: %s", path))
err = errors.ErrUnknownRequest(fmt.Sprintf("invalid path: %s", path))
return
}

Expand Down
24 changes: 12 additions & 12 deletions store/rootmulti/store_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ import (
"github.com/tendermint/tendermint/crypto/merkle"
dbm "github.com/tendermint/tendermint/libs/db"

stypes "github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/store/errors"
"github.com/cosmos/cosmos-sdk/store/types"
)

const useDebugDB = false
Expand Down Expand Up @@ -157,37 +157,37 @@ func TestMultiStoreQuery(t *testing.T) {
// Test bad path.
query := abci.RequestQuery{Path: "/key", Data: k, Height: ver}
qres := multi.Query(query)
require.EqualValues(t, types.CodeUnknownRequest, qres.Code)
require.EqualValues(t, types.CodespaceRoot, qres.Codespace)
require.EqualValues(t, errors.CodeUnknownRequest, qres.Code)
require.EqualValues(t, errors.CodespaceRoot, qres.Codespace)

query.Path = "h897fy32890rf63296r92"
qres = multi.Query(query)
require.EqualValues(t, types.CodeUnknownRequest, qres.Code)
require.EqualValues(t, types.CodespaceRoot, qres.Codespace)
require.EqualValues(t, errors.CodeUnknownRequest, qres.Code)
require.EqualValues(t, errors.CodespaceRoot, qres.Codespace)

// Test invalid store name.
query.Path = "/garbage/key"
qres = multi.Query(query)
require.EqualValues(t, types.CodeUnknownRequest, qres.Code)
require.EqualValues(t, types.CodespaceRoot, qres.Codespace)
require.EqualValues(t, errors.CodeUnknownRequest, qres.Code)
require.EqualValues(t, errors.CodespaceRoot, qres.Codespace)

// Test valid query with data.
query.Path = "/store1/key"
qres = multi.Query(query)
require.EqualValues(t, types.CodeOK, qres.Code)
require.EqualValues(t, errors.CodeOK, qres.Code)
require.Equal(t, v, qres.Value)

// Test valid but empty query.
query.Path = "/store2/key"
query.Prove = true
qres = multi.Query(query)
require.EqualValues(t, types.CodeOK, qres.Code)
require.EqualValues(t, errors.CodeOK, qres.Code)
require.Nil(t, qres.Value)

// Test store2 data.
query.Data = k2
qres = multi.Query(query)
require.EqualValues(t, types.CodeOK, qres.Code)
require.EqualValues(t, errors.CodeOK, qres.Code)
require.Equal(t, v2, qres.Value)
}

Expand All @@ -196,7 +196,7 @@ func TestMultiStoreQuery(t *testing.T) {

func newMultiStoreWithMounts(db dbm.DB) *Store {
store := NewStore(db)
store.pruningOpts = stypes.PruneSyncable
store.pruningOpts = types.PruneSyncable
store.MountStoreWithDB(
types.NewKVStoreKey("store1"), types.StoreTypeIAVL, nil)
store.MountStoreWithDB(
Expand Down