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

Refactor lcd code according to code reviewers #73

Merged
merged 2 commits into from
Aug 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 8 additions & 8 deletions client/context/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ type CLIContext struct {
Async bool
JSON bool
PrintResponse bool
Cert tendermintLite.Certifier
ClientMgr *ClientManager
Certifier tendermintLite.Certifier
ClientManager *ClientManager
}

// NewCLIContext returns a new initialized CLIContext with parameters from the
Expand Down Expand Up @@ -117,14 +117,14 @@ func (ctx CLIContext) WithUseLedger(useLedger bool) CLIContext {
return ctx
}

// WithCert - return a copy of the context with an updated Cert
func (ctx CLIContext) WithCert(cert tendermintLite.Certifier) CLIContext {
ctx.Cert = cert
// WithCertifier - return a copy of the context with an updated Certifier
func (ctx CLIContext) WithCertifier(certifier tendermintLite.Certifier) CLIContext {
ctx.Certifier = certifier
return ctx
}

// WithClientMgr - return a copy of the context with an updated ClientMgr
func (ctx CLIContext) WithClientMgr(clientMgr *ClientManager) CLIContext {
ctx.ClientMgr = clientMgr
// WithClientManager - return a copy of the context with an updated ClientManager
func (ctx CLIContext) WithClientManager(clientManager *ClientManager) CLIContext {
ctx.ClientManager = clientManager
return ctx
}
80 changes: 45 additions & 35 deletions client/context/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,14 @@ import (
"github.com/cosmos/cosmos-sdk/wire"
"strings"
tendermintLiteProxy "github.com/tendermint/tendermint/lite/proxy"
abci "github.com/tendermint/tendermint/abci/types"
)

// GetNode returns an RPC client. If the context's client is not defined, an
// error is returned.
func (ctx CLIContext) GetNode() (rpcclient.Client, error) {
if ctx.ClientMgr != nil {
return ctx.ClientMgr.getClient(), nil
if ctx.ClientManager != nil {
return ctx.ClientManager.getClient(), nil
}
if ctx.Client == nil {
return nil, errors.New("no RPC client defined")
Expand Down Expand Up @@ -284,64 +285,73 @@ func (ctx CLIContext) ensureBroadcastTx(txBytes []byte) error {
return nil
}

// nolint: gocyclo
// query performs a query from a Tendermint node with the provided store name
// and path.
func (ctx CLIContext) query(path string, key common.HexBytes) (res []byte, err error) {
node, err := ctx.GetNode()
if err != nil {
return res, err
}

opts := rpcclient.ABCIQueryOptions{
Height: ctx.Height,
Trusted: ctx.TrustNode,
}

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

resp := result.Response
if resp.Code != uint32(0) {
return res, errors.Errorf("query failed: (%d) %s", resp.Code, resp.Log)
}

// Data from trusted node or subspace doesn't need verification
// proofVerify perform response proof verification
func (ctx CLIContext) proofVerify(path string, resp abci.ResponseQuery) error {
// Data from trusted node or subspace query doesn't need verification
if ctx.TrustNode || !isQueryStoreWithProof(path) {
return resp.Value,nil
return nil
}

// TODO: Later we consider to return error for missing valid certifier to verify data from untrusted node
if ctx.Cert == nil {
if ctx.Certifier == nil {
if ctx.Logger != nil {
io.WriteString(ctx.Logger, fmt.Sprintf("Missing valid certifier to verify data from untrusted node\n"))
}
return resp.Value, nil
return nil
}

node, err := ctx.GetNode()
// AppHash for height H is in header H+1
commit, err := tendermintLiteProxy.GetCertifiedCommit(resp.Height+1, node, ctx.Cert)
commit, err := tendermintLiteProxy.GetCertifiedCommit(resp.Height+1, node, ctx.Certifier)
if err != nil {
return nil, err
return err
}

var multiStoreProof store.MultiStoreProof
cdc := wire.NewCodec()
err = cdc.UnmarshalBinary(resp.Proof, &multiStoreProof)
if err != nil {
return res, errors.Wrap(err, "failed to unmarshalBinary rangeProof")
return errors.Wrap(err, "failed to unmarshalBinary rangeProof")
}

// Validate the substore commit hash against trusted appHash
substoreCommitHash, err := store.VerifyMultiStoreCommitInfo(multiStoreProof.StoreName, multiStoreProof.CommitIDList, commit.Header.AppHash)
if err != nil {
return nil, errors.Wrap(err, "failed in verifying the proof against appHash")
return errors.Wrap(err, "failed in verifying the proof against appHash")
}
err = store.VerifyRangeProof(resp.Key, resp.Value, substoreCommitHash, &multiStoreProof.RangeProof)
if err != nil {
return nil, errors.Wrap(err, "failed in the range proof verification")
return errors.Wrap(err, "failed in the range proof verification")
}
return nil
}

// query performs a query from a Tendermint node with the provided store name
// and path.
func (ctx CLIContext) query(path string, key common.HexBytes) (res []byte, err error) {
node, err := ctx.GetNode()
if err != nil {
return res, err
}

opts := rpcclient.ABCIQueryOptions{
Height: ctx.Height,
Trusted: ctx.TrustNode,
}

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

resp := result.Response
if resp.Code != uint32(0) {
return res, errors.Errorf("query failed: (%d) %s", resp.Code, resp.Log)
}

err = ctx.proofVerify(path, resp)
if err != nil {
return nil, err
}

return resp.Value, nil
Expand Down
46 changes: 23 additions & 23 deletions client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,21 +180,17 @@ func AddNewKeyRequestHandler(w http.ResponseWriter, r *http.Request) {
}

body, err := ioutil.ReadAll(r.Body)
err = json.Unmarshal(body, &m)
err = cdc.UnmarshalJSON(body, &m)

if err != nil {
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte(err.Error()))
return
}
if m.Name == "" {
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte("You have to specify a name for the locally stored account."))
return
}
if m.Password == "" {

if paramCheck(m) != nil {
w.WriteHeader(http.StatusBadRequest)
w.Write([]byte("You have to specify a password for the locally stored account."))
w.Write([]byte(err.Error()))
return
}

Expand Down Expand Up @@ -239,34 +235,38 @@ func AddNewKeyRequestHandler(w http.ResponseWriter, r *http.Request) {
w.Write(bz)
}

// paramCheck performs add new key parameters checking
func paramCheck(m NewKeyBody) error {
if len(m.Name) < 1 || len(m.Name) > 16 {
return fmt.Errorf("account name length should not be longer than 16")
}
for _, char := range []rune(m.Name) {
if !syntax.IsWordChar(char) {
return fmt.Errorf("account name should not contains any char beyond [_0-9A-Za-z]")
}
}
if len(m.Password) < 8 || len(m.Password) > 16 {
return fmt.Errorf("account password length should be no less than 8 and no greater than 16")
}
return nil
}

// AddNewKeyRequest is the handler of adding new key in swagger rest server
// nolint: gocyclo
func AddNewKeyRequest(gtx *gin.Context) {
var m NewKeyBody
body, err := ioutil.ReadAll(gtx.Request.Body)
if err != nil {
httputils.NewError(gtx, http.StatusBadRequest, err)
return
}
err = json.Unmarshal(body, &m)
err = cdc.UnmarshalJSON(body, &m)
if err != nil {
httputils.NewError(gtx, http.StatusBadRequest, err)
return
}

if len(m.Name) < 1 || len(m.Name) > 16 {
httputils.NewError(gtx, http.StatusBadRequest, fmt.Errorf("account name length should not be longer than 16"))
return
}
for _, char := range []rune(m.Name) {
if !syntax.IsWordChar(char) {
httputils.NewError(gtx, http.StatusBadRequest, fmt.Errorf("account name should not contains any char beyond [_0-9A-Za-z]"))
return
}
}
if len(m.Password) < 8 || len(m.Password) > 16 {
httputils.NewError(gtx, http.StatusBadRequest, fmt.Errorf("account password length should be no less than 8 and no greater than 16"))
return
if paramCheck(m) != nil {
httputils.NewError(gtx, http.StatusBadRequest, err)
}

kb, err := GetKeyBase()
Expand Down
10 changes: 8 additions & 2 deletions client/keys/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/spf13/cobra"
"github.com/gin-gonic/gin"
"github.com/cosmos/cosmos-sdk/client/httputils"
"io/ioutil"
)

func deleteKeyCommand() *cobra.Command {
Expand Down Expand Up @@ -96,10 +97,15 @@ func DeleteKeyRequestHandler(w http.ResponseWriter, r *http.Request) {
// DeleteKeyRequest is the handler of deleting specified key in swagger rest server
func DeleteKeyRequest(gtx *gin.Context) {
name := gtx.Param("name")
var kb keys.Keybase
var m DeleteKeyBody

if err := gtx.BindJSON(&m); err != nil {
body, err := ioutil.ReadAll(gtx.Request.Body)
if err != nil {
httputils.NewError(gtx, http.StatusBadRequest, err)
return
}
err = cdc.UnmarshalJSON(body, &m)
if err != nil {
httputils.NewError(gtx, http.StatusBadRequest, err)
return
}
Expand Down
7 changes: 3 additions & 4 deletions client/keys/list.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package keys

import (
"encoding/json"
"net/http"

"github.com/spf13/cobra"
Expand Down Expand Up @@ -61,7 +60,7 @@ func QueryKeysRequestHandler(w http.ResponseWriter, r *http.Request) {
w.Write([]byte(err.Error()))
return
}
output, err := json.MarshalIndent(keysOutput, "", " ")
output, err := cdc.MarshalJSONIndent(keysOutput, "", " ")
if err != nil {
w.WriteHeader(500)
w.Write([]byte(err.Error()))
Expand All @@ -70,7 +69,7 @@ func QueryKeysRequestHandler(w http.ResponseWriter, r *http.Request) {
w.Write(output)
}

// DeleteKeyRequest is the handler of listing all keys in swagger rest server
// QueryKeysRequest is the handler of listing all keys in swagger rest server
func QueryKeysRequest(gtx *gin.Context) {
kb, err := GetKeyBase()
if err != nil {
Expand All @@ -92,7 +91,7 @@ func QueryKeysRequest(gtx *gin.Context) {
httputils.NewError(gtx, http.StatusInternalServerError, err)
return
}
output, err := json.MarshalIndent(keysOutput, "", " ")
output, err := cdc.MarshalJSONIndent(keysOutput, "", " ")
if err != nil {
httputils.NewError(gtx, http.StatusInternalServerError, err)
return
Expand Down
10 changes: 8 additions & 2 deletions client/keys/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/gin-gonic/gin"
"errors"
"github.com/cosmos/cosmos-sdk/client/httputils"
"io/ioutil"
)

func updateKeyCommand() *cobra.Command {
Expand Down Expand Up @@ -100,10 +101,15 @@ func UpdateKeyRequestHandler(w http.ResponseWriter, r *http.Request) {
// UpdateKeyRequest is the handler of updating specified key in swagger rest server
func UpdateKeyRequest(gtx *gin.Context) {
name := gtx.Param("name")
var kb keys.Keybase
var m UpdateKeyBody

if err := gtx.BindJSON(&m); err != nil {
body, err := ioutil.ReadAll(gtx.Request.Body)
if err != nil {
httputils.NewError(gtx, http.StatusBadRequest, err)
return
}
err = cdc.UnmarshalJSON(body, &m)
if err != nil {
httputils.NewError(gtx, http.StatusBadRequest, err)
return
}
Expand Down
Loading