-
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: Judge if trust-node predefined before create certifier add verification for getBlock, queryTx and getValidators #2210
Changes from 19 commits
71625ed
0681648
f254d28
d8a951d
bed593d
e8ef342
582886b
61c6d33
4146cb2
f075e25
9e1bd1a
68655ce
5cc7088
e1b15c6
bf6b445
ca840e6
abf1a8a
253db7f
a63ea1c
7a7c6df
8dcc536
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 |
---|---|---|
|
@@ -177,7 +177,7 @@ func TestBlock(t *testing.T) { | |
|
||
// -- | ||
|
||
res, body = Request(t, port, "GET", "/blocks/1", nil) | ||
res, body = Request(t, port, "GET", "/blocks/2", 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. Why did this change? 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 have added a comment about the reason above. When gaia-lite is launched, it will fetch the latest height as its trust basement. Then it can only verify blocks or transactions in later height. 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. OK, we should update gaia lite so that you can easily start it with a separate root-of-trust. 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. Ref #2323 |
||
require.Equal(t, http.StatusOK, res.StatusCode, body) | ||
|
||
err = wire.Cdc.UnmarshalJSON([]byte(body), &resultBlock) | ||
|
@@ -210,7 +210,7 @@ func TestValidators(t *testing.T) { | |
|
||
// -- | ||
|
||
res, body = Request(t, port, "GET", "/validatorsets/1", nil) | ||
res, body = Request(t, port, "GET", "/validatorsets/2", nil) | ||
require.Equal(t, http.StatusOK, res.StatusCode, body) | ||
|
||
err = cdc.UnmarshalJSON([]byte(body), &resultVals) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,14 +3,11 @@ package tx | |
import ( | ||
"encoding/hex" | ||
"fmt" | ||
"net/http" | ||
"strconv" | ||
|
||
"github.com/tendermint/tendermint/libs/common" | ||
"net/http" | ||
|
||
"github.com/gorilla/mux" | ||
"github.com/spf13/cobra" | ||
"github.com/spf13/viper" | ||
abci "github.com/tendermint/tendermint/abci/types" | ||
ctypes "github.com/tendermint/tendermint/rpc/core/types" | ||
|
||
|
@@ -30,11 +27,10 @@ func QueryTxCmd(cdc *wire.Codec) *cobra.Command { | |
RunE: func(cmd *cobra.Command, args []string) error { | ||
// find the key to look up the account | ||
hashHexStr := args[0] | ||
trustNode := viper.GetBool(client.FlagTrustNode) | ||
|
||
cliCtx := context.NewCLIContext().WithCodec(cdc) | ||
|
||
output, err := queryTx(cdc, cliCtx, hashHexStr, trustNode) | ||
output, err := queryTx(cdc, cliCtx, hashHexStr) | ||
if err != nil { | ||
return err | ||
} | ||
|
@@ -45,13 +41,12 @@ func QueryTxCmd(cdc *wire.Codec) *cobra.Command { | |
} | ||
|
||
cmd.Flags().StringP(client.FlagNode, "n", "tcp://localhost:26657", "Node to connect to") | ||
|
||
// TODO: change this to false when we can | ||
cmd.Flags().Bool(client.FlagTrustNode, true, "Don't verify proofs for responses") | ||
cmd.Flags().Bool(client.FlagTrustNode, false, "Trust connected full node (don't verify proofs for responses)") | ||
cmd.Flags().String(client.FlagChainID, "", "Chain ID of Tendermint node") | ||
return cmd | ||
} | ||
|
||
func queryTx(cdc *wire.Codec, cliCtx context.CLIContext, hashHexStr string, trustNode bool) ([]byte, error) { | ||
func queryTx(cdc *wire.Codec, cliCtx context.CLIContext, hashHexStr string) ([]byte, error) { | ||
hash, err := hex.DecodeString(hashHexStr) | ||
if err != nil { | ||
return nil, err | ||
|
@@ -62,21 +57,32 @@ func queryTx(cdc *wire.Codec, cliCtx context.CLIContext, hashHexStr string, trus | |
return nil, err | ||
} | ||
|
||
res, err := node.Tx(hash, !trustNode) | ||
res, err := node.Tx(hash, !cliCtx.TrustNode) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
info, err := formatTxResult(cdc, res) | ||
info, err := formatTxResult(cdc, cliCtx, res) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return wire.MarshalJSONIndent(cdc, info) | ||
} | ||
|
||
func formatTxResult(cdc *wire.Codec, res *ctypes.ResultTx) (Info, error) { | ||
// TODO: verify the proof if requested | ||
func formatTxResult(cdc *wire.Codec, cliCtx context.CLIContext, res *ctypes.ResultTx) (Info, error) { | ||
if !cliCtx.TrustNode { | ||
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. This logic is fine but I think it's confusing to put it in a function called 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 added the verification logic here because I noticed the TODO.
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'm sorry for misunderstanding. I have changed the code move the tx verification out of tx format. |
||
check, err := cliCtx.Certify(res.Height) | ||
if err != nil { | ||
return Info{}, err | ||
} | ||
|
||
err = res.Proof.Validate(check.Header.DataHash) | ||
if err != nil { | ||
return Info{}, err | ||
} | ||
} | ||
|
||
tx, err := parseTx(cdc, res.Tx) | ||
if err != nil { | ||
return Info{}, err | ||
|
@@ -116,13 +122,8 @@ func QueryTxRequestHandlerFn(cdc *wire.Codec, cliCtx context.CLIContext) http.Ha | |
return func(w http.ResponseWriter, r *http.Request) { | ||
vars := mux.Vars(r) | ||
hashHexStr := vars["hash"] | ||
trustNode, err := strconv.ParseBool(r.FormValue("trust_node")) | ||
// trustNode defaults to true | ||
if err != nil { | ||
trustNode = true | ||
} | ||
|
||
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 remove this because if rest-server is stared with trust-node option, then we can't verify tx proof even if the request requires proof. 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. OK 👍 |
||
output, err := queryTx(cdc, cliCtx, hashHexStr, trustNode) | ||
output, err := queryTx(cdc, cliCtx, hashHexStr) | ||
if err != nil { | ||
w.WriteHeader(500) | ||
w.Write([]byte(err.Error())) | ||
|
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.
Ref #2322