-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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: Bianjie/lcd implementation #2147
Changes from 22 commits
34548db
eccb889
2047673
85f47d2
187dda4
26af936
4607ca9
7528534
473fe21
95e367a
9d944b0
f266472
6cdf21d
9409e73
8e843bc
70aa433
7eda13a
7a4b019
00c43fa
2455604
7760296
f6ebccf
986504f
9427eb5
81f294b
b6bb5ff
ff15759
5b250cc
135473c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ import ( | |
"github.com/spf13/viper" | ||
|
||
rpcclient "github.com/tendermint/tendermint/rpc/client" | ||
tendermintLite"github.com/tendermint/tendermint/lite" | ||
) | ||
|
||
const ctxAccStoreName = "acc" | ||
|
@@ -30,6 +31,8 @@ type CLIContext struct { | |
Async bool | ||
JSON bool | ||
PrintResponse bool | ||
Certifier tendermintLite.Certifier | ||
ClientManager *ClientManager | ||
} | ||
|
||
// NewCLIContext returns a new initialized CLIContext with parameters from the | ||
|
@@ -113,3 +116,15 @@ func (ctx CLIContext) WithUseLedger(useLedger bool) CLIContext { | |
ctx.UseLedger = useLedger | ||
return ctx | ||
} | ||
|
||
// WithCertifier - return a copy of the context with an updated Certifier | ||
func (ctx CLIContext) WithCertifier(certifier tendermintLite.Certifier) CLIContext { | ||
ctx.Certifier = certifier | ||
return ctx | ||
} | ||
|
||
// WithClientManager - return a copy of the context with an updated ClientManager | ||
func (ctx CLIContext) WithClientManager(clientManager *ClientManager) CLIContext { | ||
ctx.ClientManager = clientManager | ||
return ctx | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing newline. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
package context | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seems like this entire needs to be formatted correctly. Also, it should probably be renamed to |
||
|
||
import ( | ||
rpcclient "github.com/tendermint/tendermint/rpc/client" | ||
"strings" | ||
"sync" | ||
"github.com/pkg/errors" | ||
) | ||
|
||
// ClientManager is a manager of a set of rpc clients to full nodes. | ||
// This manager can do load balancing upon these rpc clients. | ||
type ClientManager struct { | ||
clients []rpcclient.Client | ||
currentIndex int | ||
mutex sync.RWMutex | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it'll suffice to just have a write only mutex here (I don't see any logic that obtains a read-lock). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean use sync.Mutex instead? |
||
} | ||
|
||
// NewClientManager create a new ClientManager | ||
func NewClientManager(nodeURIs string) (*ClientManager,error) { | ||
if nodeURIs != "" { | ||
nodeURLArray := strings.Split(nodeURIs, ",") | ||
var clients []rpcclient.Client | ||
for _, url := range nodeURLArray { | ||
client := rpcclient.NewHTTP(url, "/websocket") | ||
clients = append(clients, client) | ||
} | ||
mgr := &ClientManager{ | ||
currentIndex: 0, | ||
clients: clients, | ||
} | ||
return mgr, nil | ||
} | ||
return nil, errors.New("missing node URIs") | ||
} | ||
|
||
func (mgr *ClientManager) getClient() rpcclient.Client { | ||
mgr.mutex.Lock() | ||
defer mgr.mutex.Unlock() | ||
|
||
client := mgr.clients[mgr.currentIndex] | ||
mgr.currentIndex++ | ||
if mgr.currentIndex >= len(mgr.clients){ | ||
mgr.currentIndex = 0 | ||
} | ||
return client | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing new line. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package context | ||
|
||
import ( | ||
"testing" | ||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestLoadBalancing(t *testing.T) { | ||
nodeURIs := "10.10.10.10:26657,20.20.20.20:26657,30.30.30.30:26657" | ||
clientMgr,err := NewClientManager(nodeURIs) | ||
assert.Empty(t,err) | ||
endpoint := clientMgr.getClient() | ||
assert.NotEqual(t,endpoint,clientMgr.getClient()) | ||
clientMgr.getClient() | ||
assert.Equal(t,endpoint,clientMgr.getClient()) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,11 +14,19 @@ import ( | |
cmn "github.com/tendermint/tendermint/libs/common" | ||
rpcclient "github.com/tendermint/tendermint/rpc/client" | ||
ctypes "github.com/tendermint/tendermint/rpc/core/types" | ||
"github.com/cosmos/cosmos-sdk/store" | ||
"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.ClientManager != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can't see the reason to have a both a client manager and a client. Seems like we should only have a client manager which, in the original case, has only a single client. Thoughts? |
||
return ctx.ClientManager.getClient(), nil | ||
} | ||
if ctx.Client == nil { | ||
return nil, errors.New("no RPC client defined") | ||
} | ||
|
@@ -282,6 +290,50 @@ func (ctx CLIContext) ensureBroadcastTx(txBytes []byte) error { | |
return nil | ||
} | ||
|
||
// proofVerify perform response proof verification | ||
func (ctx CLIContext) proofVerify(path string, resp abci.ResponseQuery) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// Data from trusted node or subspace query doesn't need verification | ||
if ctx.TrustNode || !isQueryStoreWithProof(path) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the |
||
return nil | ||
} | ||
|
||
// TODO: Later we consider to return error for missing valid certifier to verify data from untrusted node | ||
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 nil | ||
} | ||
|
||
node, err := ctx.GetNode() | ||
if err != nil { | ||
return err | ||
} | ||
// AppHash for height H is in header H+1 | ||
commit, err := tendermintLiteProxy.GetCertifiedCommit(resp.Height+1, node, ctx.Certifier) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
var multiStoreProof store.MultiStoreProof | ||
cdc := wire.NewCodec() | ||
err = cdc.UnmarshalBinary(resp.Proof, &multiStoreProof) | ||
if err != nil { | ||
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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can break this call into multiple lines for legibility. |
||
if err != nil { | ||
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 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) { | ||
|
@@ -301,10 +353,15 @@ func (ctx CLIContext) query(path string, key common.HexBytes) (res []byte, err e | |
} | ||
|
||
resp := result.Response | ||
if !resp.IsOK() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry. Maybe it is a mistake in code merging. I have changed it to resp.IsOK which seems more elegant |
||
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 | ||
} | ||
|
||
|
@@ -314,3 +371,21 @@ func (ctx CLIContext) queryStore(key cmn.HexBytes, storeName, endPath string) ([ | |
path := fmt.Sprintf("/store/%s/%s", storeName, endPath) | ||
return ctx.query(path, key) | ||
} | ||
|
||
// isQueryStoreWithProof expects a format like /<queryType>/<storeName>/<subpath> | ||
// queryType can be app or store | ||
// if subpath equals to store or key, then return true | ||
func isQueryStoreWithProof(path string) (bool) { | ||
if !strings.HasPrefix(path, "/") { | ||
return false | ||
} | ||
paths := strings.SplitN(path[1:], "/", 3) | ||
if len(paths) != 3 { | ||
return false | ||
} | ||
// WARNING This should be consistent with query method in iavlstore.go | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you elaborate on this warning and why we need it? What needs to be done about it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In iavlstore.go, proof will be included in response only when the query path is /store or /key. I concern that this may be changed in iavlstore.go. If there are some changes about proof building in iavlstore.go, then we must change code here to be consistent with the changes. |
||
if paths[2] == "store" || paths[2] == "key" { | ||
return true | ||
} | ||
return false | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
package httputils | ||
|
||
import ( | ||
"github.com/gin-gonic/gin" | ||
"net/http" | ||
) | ||
|
||
// NewError create error http response | ||
func NewError(ctx *gin.Context, errCode int, err error) { | ||
errorResponse := HTTPError{ | ||
API: "2.0", | ||
Code: errCode, | ||
} | ||
if err != nil { | ||
errorResponse.ErrMsg = err.Error() | ||
} | ||
|
||
ctx.JSON(errCode, errorResponse) | ||
} | ||
|
||
// NormalResponse create normal http response | ||
func NormalResponse(ctx *gin.Context, data []byte) { | ||
ctx.Status(http.StatusOK) | ||
ctx.Writer.Write(data) | ||
} | ||
|
||
// HTTPError is http response with error | ||
type HTTPError struct { | ||
API string `json:"rest api" example:"2.0"` | ||
Code int `json:"code" example:"500"` | ||
ErrMsg string `json:"error message"` | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting error