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.

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

R4R: Store Accessor Types #4514

Closed
wants to merge 43 commits into from
Closed
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
9375649
add mapping
mossid Jun 7, 2019
16eea68
rm unused mapping/*, rm interfaces
mossid Jun 11, 2019
f73215e
rm unused code
mossid Jun 11, 2019
cf0b8f2
mv mapping -> state, rm x/ibc
mossid Jun 11, 2019
e25f652
Merge branch 'master' of github.com:cosmos/cosmos-sdk into joon/store…
mossid Jun 17, 2019
2b17a39
rm GetIfExists
mossid Jun 25, 2019
dc84e1e
add key
mossid Jun 25, 2019
19d7155
rm custom encoding/decoding in enum/bool
mossid Jun 26, 2019
6764bc4
Merge branch 'master' of github.com:cosmos/cosmos-sdk into joon/store…
mossid Jun 29, 2019
8affaa7
fix lint
mossid Jun 29, 2019
a7898b4
rm tests
mossid Jul 7, 2019
3939bd4
Merge branch 'master' of github.com:cosmos/cosmos-sdk into joon/store…
mossid Jul 9, 2019
f609249
add docs in progre
mossid Jul 24, 2019
7da69c1
add comments
mossid Jul 25, 2019
600445a
add comments
mossid Jul 25, 2019
94e2cdc
Merge branch 'master' of github.com:cosmos/cosmos-sdk into joon/store…
mossid Jul 26, 2019
8d9e779
add comments
mossid Jul 26, 2019
92d0fb4
fix comment
mossid Jul 26, 2019
5d913b7
recover IntEncoding scheme for integer
mossid Jul 29, 2019
699b0ba
add uint tests, don't use codec in custom types
mossid Jul 29, 2019
97edd89
Merge branch 'master' of github.com:cosmos/cosmos-sdk into joon/store…
mossid Jul 30, 2019
24e621a
add query
mossid Jul 31, 2019
971a642
update query.go
mossid Jul 31, 2019
fcad487
add Query to boolean.go
mossid Aug 1, 2019
6de0ed5
fix key
mossid Aug 1, 2019
6dc912f
godoc cleanup
alexanderbez Aug 7, 2019
63c6f3c
godoc cleanup
alexanderbez Aug 7, 2019
b6441c0
godoc cleanup
alexanderbez Aug 7, 2019
4d723ae
godoc cleanup
alexanderbez Aug 7, 2019
244d2c2
godoc cleanup
alexanderbez Aug 7, 2019
7bf4d57
merge from ics04 branch
mossid Aug 15, 2019
7471bdf
Merge branch 'joon/store-mapping' of github.com:cosmos/cosmos-sdk int…
mossid Aug 15, 2019
04543f8
Merge branch 'master' of github.com:cosmos/cosmos-sdk into joon/store…
mossid Sep 2, 2019
10d1c46
Apply suggestions from code review
mossid Sep 10, 2019
09e6600
applying review in progress
mossid Sep 11, 2019
6f5f633
Merge branch 'joon/store-mapping' of github.com:cosmos/cosmos-sdk int…
mossid Sep 11, 2019
4116bb7
apply review - make querier interface
mossid Sep 11, 2019
8040c81
Merge branch 'master' of github.com:cosmos/cosmos-sdk into joon/store…
mossid Sep 17, 2019
828badd
revise querier interface to work both on cli & store
mossid Sep 18, 2019
21bd8d0
Store accessor upstream changes (#5119)
fedekunze Oct 1, 2019
9b3b120
Merge branch 'master' into joon/store-mapping
fedekunze Oct 1, 2019
abd9fec
Apply suggestions from code review
mossid Oct 1, 2019
a6dadef
Merge branch 'master' into joon/store-mapping
fedekunze Oct 4, 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
47 changes: 33 additions & 14 deletions client/context/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,12 @@ func (ctx CLIContext) QueryStore(key cmn.HexBytes, storeName string) ([]byte, in
return ctx.queryStore(key, storeName, "key")
}

// QueryABCI performs a query to a Tendermint node with the provide RequestQuery.
// It returns the ResultQuery obtained from the query.
func (ctx CLIContext) QueryABCI(req abci.RequestQuery) (abci.ResponseQuery, error) {
return ctx.queryABCI(req)
}

// QuerySubspace performs a query to a Tendermint node with the provided
// store name and subspace. It returns key value pair and height of the query
// upon success or an error if the query fails.
Expand All @@ -72,21 +78,18 @@ func (ctx CLIContext) GetFromName() string {
return ctx.FromName
}

// query performs a query to a Tendermint node with the provided store name
// and path. It returns the result and height of the query upon success
// or an error if the query fails.
func (ctx CLIContext) query(path string, key cmn.HexBytes) (res []byte, height int64, err error) {
func (ctx CLIContext) queryABCI(req abci.RequestQuery) (resp abci.ResponseQuery, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

unnecessary leading newline (from whitespace)

node, err := ctx.GetNode()
if err != nil {
return res, height, err
return resp, err
}

// When a client did not provide a query height, manually query for it so it can
// be injected downstream into responses.
if ctx.Height == 0 {
status, err := node.Status()
if err != nil {
return res, height, err
return resp, err
}
ctx = ctx.WithHeight(status.SyncInfo.LatestBlockHeight)
}
Expand All @@ -96,24 +99,40 @@ func (ctx CLIContext) query(path string, key cmn.HexBytes) (res []byte, height i
Prove: !ctx.TrustNode,
}

result, err := node.ABCIQueryWithOptions(path, key, opts)
result, err := node.ABCIQueryWithOptions(req.Path, req.Data, opts)
if err != nil {
return res, height, err
return
}

resp := result.Response
resp = result.Response
if !resp.IsOK() {
return res, height, errors.New(resp.Log)
err = errors.New(resp.Log)
return
}

// data from trusted node or subspace query doesn't need verification
if ctx.TrustNode || !isQueryStoreWithProof(path) {
return resp.Value, resp.Height, nil
if ctx.TrustNode || !isQueryStoreWithProof(req.Path) {
return resp, nil
}

err = ctx.verifyProof(path, resp)
err = ctx.verifyProof(req.Path, resp)
if err != nil {
return res, height, err
return
}

return
Copy link
Contributor

Choose a reason for hiding this comment

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

naked return in func queryABCI with 44 lines of code (from nakedret)

}

// query performs a query to a Tendermint node with the provided store name
// and path. It returns the result and height of the query upon success
// or an error if the query fails.
func (ctx CLIContext) query(path string, key cmn.HexBytes) (res []byte, height int64, err error) {
resp, err := ctx.queryABCI(abci.RequestQuery{
Path: path,
Data: key,
})
if err != nil {
return
}

return resp.Value, resp.Height, nil
Expand Down
32 changes: 32 additions & 0 deletions store/state/boolean.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package state

// Boolean is a bool typed wrapper for Value.
//
// false <-> []byte{0x00}
// true <-> []byte{0x01}
type Boolean struct {
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
Value
}

func (v Value) Boolean() Boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

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

All these functions should be moved to value.go

func (v Value) Type() Type {

return Boolean{v}
}

// Get decodes and returns the stored boolean value if it exists. It will panic
// if the value exists but is not boolean type.
func (v Boolean) Get(ctx Context) (res bool) {
v.Value.Get(ctx, &res)
return
}

// GetSafe decodes and returns the stored boolean value. It will return an error
// if the value does not exist or not boolean.
func (v Boolean) GetSafe(ctx Context) (res bool, err error) {
err = v.Value.GetSafe(ctx, &res)
return
}

// Set encodes and sets the boolean argument to the state.
func (v Boolean) Set(ctx Context, value bool) {
v.Value.Set(ctx, value)
}
54 changes: 54 additions & 0 deletions store/state/enum.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package state

// Enum is a byte typed wrapper for Value.
// x <-> []byte{x}
type Enum struct {
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
Value
}

func (v Value) Enum() Enum {
return Enum{v}
}

// Get decodes and returns the stored byte value if it exists. It will panic if
// the value exists but is not byte type.
func (v Enum) Get(ctx Context) (res byte) {
return v.Value.GetRaw(ctx)[0]
}

// GetSafe decodes and returns the stored byte value. It will returns an error
// if the value does not exists or not byte.
func (v Enum) GetSafe(ctx Context) (res byte, err error) {
bz := v.Value.GetRaw(ctx)
if bz == nil {
return res, ErrEmptyValue()
}
return bz[0], nil // TODO: check length
Copy link
Contributor

Choose a reason for hiding this comment

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

We should take care of this TODO if possible now.

}

// Set encodes and sets the byte argument to the state.
func (v Enum) Set(ctx Context, value byte) {
v.Value.SetRaw(ctx, []byte{value})
}

// Incr increments the stored value, and returns the updated value.
func (v Enum) Incr(ctx Context) (res byte) {
res = v.Get(ctx) + 1
v.Set(ctx, res)
return
}

// Transit checks whether the stored value matching with the "from" argument.
// If it matches, it stores the "to" argument to the state and returns true.
func (v Enum) Transit(ctx Context, from, to byte) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this used for?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand the name either

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Transit is used for moving the stored state from from to to. Mostly used for the handshaking process in connection and channel. Named Enum because it is essentially a byte and considered as an enum type. Can be changed.

if v.Get(ctx) != from {
return false
}
v.Set(ctx, to)
return true
}

func (v Enum) Query(ctx CLIContext) (res byte, proof *Proof, err error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

  • Missing godoc
  • Instead of having a dependency on CLIContext, we should instead provide an interface as an argument:
type ABCIQuerier interface {
     QueryABCI(req abci.RequestQuery) (abci.ResponseQuery, error)
}
Suggested change
func (v Enum) Query(ctx CLIContext) (res byte, proof *Proof, err error) {
func (v Enum) Query(q ABCIQuerier) (res byte, proof *Proof, err error) {

value, proof, err := v.Value.QueryRaw(ctx)
return value[0], proof, err
}
56 changes: 56 additions & 0 deletions store/state/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package state

import (
"fmt"
)

fedekunze marked this conversation as resolved.
Show resolved Hide resolved
type GetSafeErrorType byte
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
type GetSafeErrorType byte
type SafeErrorType byte


const (
ErrTypeEmptyValue GetSafeErrorType = iota
ErrTypeUnmarshal
)

func (ty GetSafeErrorType) Format(msg string) (res string) {
switch ty {
case ErrTypeEmptyValue:
res = fmt.Sprintf("Empty Value found")
case ErrTypeUnmarshal:
res = fmt.Sprintf("Error while unmarshal")
default:
panic("Unknown error type")
}

if msg != "" {
res = fmt.Sprintf("%s: %s", res, msg)
}

return
}

type GetSafeError struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
type GetSafeError struct {
type SafeError struct {

Copy link
Collaborator

Choose a reason for hiding this comment

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

I really don't understand why we need an error wrapper type like this one. What happens if GetSafe just returns an error? Why do we want to wrap it? All those answers should be specified on a comment here

ty GetSafeErrorType
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
ty GetSafeErrorType
typeOf SafeErrorType

inner error
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
inner error
err error

}

var _ error = (*GetSafeError)(nil) // TODO: sdk.Error
Copy link
Contributor

Choose a reason for hiding this comment

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

??? on TODO


func (err *GetSafeError) Error() string {
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably doesn't need to operate on a pointer receiver.

if err.inner == nil {
return err.ty.Format("")
}
return err.ty.Format(err.inner.Error())
}

func ErrEmptyValue() *GetSafeError {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to return pointers?

return &GetSafeError{
ty: ErrTypeEmptyValue,
}
}

func ErrUnmarshal(err error) *GetSafeError {
return &GetSafeError{
ty: ErrTypeUnmarshal,
inner: err,
}
}
120 changes: 120 additions & 0 deletions store/state/indexer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package state

import (
"encoding/binary"
"fmt"
"strconv"
)

// IntEncoding is an enum type defining the integer serialization scheme.
// All encoding schemes preserves order.
type IntEncoding byte
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved

const (
// Dec is human readable decimal encoding scheme.
// Has fixed length of 20 bytes.
Dec IntEncoding = iota
// Hex is human readable hexadecimal encoding scheme
// Has fixed length of 16 bytes.
Hex
// Bin is machine readable big endian encoding scheme
// Has fixed length of 8 bytes
Bin
)

// Indexer is a integer typed key wrapper for Mapping. Except for the type
// checking, it does not alter the behaviour. All keys are encoded depending on
// the IntEncoding.
type Indexer struct {
m Mapping
mossid marked this conversation as resolved.
Show resolved Hide resolved

enc IntEncoding
mossid marked this conversation as resolved.
Show resolved Hide resolved
}

// NewIndexer constructs the Indexer with a predetermined prefix and IntEncoding.
func NewIndexer(m Mapping, enc IntEncoding) Indexer {
return Indexer{
m: m,
enc: enc,
}
}

// EncodeInt provides order preserving integer encoding function.
func EncodeInt(index uint64, enc IntEncoding) (res []byte) {
switch enc {
case Dec:
// return decimal number index, 20-length 0 padded
return []byte(fmt.Sprintf("%020d", index))

case Hex:
// return hexadecimal number index, 20-length 0 padded
return []byte(fmt.Sprintf("%016x", index))

case Bin:
// return bigendian encoded number index, 8-length
res = make([]byte, 8)
binary.BigEndian.PutUint64(res, index)
return

default:
panic("invalid IntEncoding")
}
}

// DecodeInt provides integer decoding function, inversion of EncodeInt.
func DecodeInt(bz []byte, enc IntEncoding) (res uint64, err error) {
switch enc {
case Dec:
return strconv.ParseUint(string(bz), 10, 64)

case Hex:
return strconv.ParseUint(string(bz), 16, 64)

case Bin:
return binary.BigEndian.Uint64(bz), nil

default:
panic("invalid IntEncoding")
}
}

// Value returns the Value corresponding to the provided index.
func (ix Indexer) Value(index uint64) Value {
return ix.m.Value(EncodeInt(index, ix.enc))
}

// Get decodes and sets the stored value to the pointer if it exists. It will
// panic if the value exists but not unmarshalable.
func (ix Indexer) Get(ctx Context, index uint64, ptr interface{}) {
ix.Value(index).Get(ctx, ptr)
}

// GetSafe decodes and sets the stored value to the pointer. It will return an
// error if the value does not exist or unmarshalable.
func (ix Indexer) GetSafe(ctx Context, index uint64, ptr interface{}) error {
return ix.Value(index).GetSafe(ctx, ptr)
}

// Set encodes and sets the argument to the state.
func (ix Indexer) Set(ctx Context, index uint64, o interface{}) {
ix.Value(index).Set(ctx, o)
}

// Has returns true if the stored value is not nil.
func (ix Indexer) Has(ctx Context, index uint64) bool {
return ix.Value(index).Exists(ctx)
}

// Delete removes the stored value.
func (ix Indexer) Delete(ctx Context, index uint64) {
ix.Value(index).Delete(ctx)
}

// Prefix returns a new Indexer with the updated prefix
func (ix Indexer) Prefix(prefix []byte) Indexer {
return Indexer{
m: ix.m.Prefix(prefix),
Copy link
Contributor

Choose a reason for hiding this comment

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

ix.m undefined (type Indexer has no field or method m) (from typecheck)


enc: ix.enc,
Copy link
Contributor

Choose a reason for hiding this comment

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

ix.enc undefined (type Indexer has no field or method enc) (from typecheck)

}
}
Loading