-
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: Tx Query return values #3642
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ import ( | |
"encoding/hex" | ||
"fmt" | ||
"net/http" | ||
"strings" | ||
|
||
"github.com/gorilla/mux" | ||
"github.com/spf13/cobra" | ||
|
@@ -26,18 +27,18 @@ func QueryTxCmd(cdc *codec.Codec) *cobra.Command { | |
Short: "Matches this txhash over all committed blocks", | ||
Args: cobra.ExactArgs(1), | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
// find the key to look up the account | ||
hashHexStr := args[0] | ||
|
||
cliCtx := context.NewCLIContext().WithCodec(cdc) | ||
|
||
output, err := queryTx(cdc, cliCtx, hashHexStr) | ||
output, err := queryTx(cdc, cliCtx, args[0]) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
fmt.Println(string(output)) | ||
return nil | ||
if output.Empty() { | ||
return fmt.Errorf("No transaction found with hash %s", args[0]) | ||
} | ||
|
||
return cliCtx.PrintOutput(output) | ||
}, | ||
} | ||
|
||
|
@@ -48,50 +49,44 @@ func QueryTxCmd(cdc *codec.Codec) *cobra.Command { | |
return cmd | ||
} | ||
|
||
func queryTx(cdc *codec.Codec, cliCtx context.CLIContext, hashHexStr string) ([]byte, error) { | ||
func queryTx(cdc *codec.Codec, cliCtx context.CLIContext, hashHexStr string) (out sdk.TxResponse, err error) { | ||
hash, err := hex.DecodeString(hashHexStr) | ||
if err != nil { | ||
return nil, err | ||
return out, err | ||
} | ||
|
||
node, err := cliCtx.GetNode() | ||
if err != nil { | ||
return nil, err | ||
return out, err | ||
} | ||
|
||
res, err := node.Tx(hash, !cliCtx.TrustNode) | ||
if err != nil { | ||
return nil, err | ||
return out, err | ||
} | ||
|
||
if !cliCtx.TrustNode { | ||
err := ValidateTxResult(cliCtx, res) | ||
if err != nil { | ||
return nil, err | ||
} | ||
if err = ValidateTxResult(cliCtx, res); !cliCtx.TrustNode && err != 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. This does not equal to what was before. Before we didn't execute 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. good catch. |
||
return out, err | ||
} | ||
|
||
info, err := formatTxResult(cdc, res) | ||
if err != nil { | ||
return nil, err | ||
if out, err = formatTxResult(cdc, res); err != nil { | ||
return out, err | ||
} | ||
|
||
if cliCtx.Indent { | ||
return cdc.MarshalJSONIndent(info, "", " ") | ||
} | ||
return cdc.MarshalJSON(info) | ||
return out, nil | ||
} | ||
|
||
// ValidateTxResult performs transaction verification | ||
func ValidateTxResult(cliCtx context.CLIContext, res *ctypes.ResultTx) error { | ||
check, err := cliCtx.Verify(res.Height) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
err = res.Proof.Validate(check.Header.DataHash) | ||
if err != nil { | ||
return err | ||
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. if cliCtx.TrustNode {
return 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. this code does this. Slightly confused what you mean here? If we trust the node, we don't want to validate the results. 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 meant instead of wrapping big chunk of code into if condition, you can return early! |
||
check, err := cliCtx.Verify(res.Height) | ||
if err != nil { | ||
return err | ||
} | ||
err = res.Proof.Validate(check.Header.DataHash) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
return nil | ||
} | ||
|
@@ -118,17 +113,26 @@ func parseTx(cdc *codec.Codec, txBytes []byte) (sdk.Tx, error) { | |
|
||
// REST | ||
|
||
// transaction query REST handler | ||
// QueryTxRequestHandlerFn transaction query REST handler | ||
func QueryTxRequestHandlerFn(cdc *codec.Codec, cliCtx context.CLIContext) http.HandlerFunc { | ||
return func(w http.ResponseWriter, r *http.Request) { | ||
vars := mux.Vars(r) | ||
hashHexStr := vars["hash"] | ||
|
||
output, err := queryTx(cdc, cliCtx, hashHexStr) | ||
if err != nil { | ||
if strings.Contains(err.Error(), hashHexStr) { | ||
rest.WriteErrorResponse(w, http.StatusNotFound, err.Error()) | ||
return | ||
} | ||
rest.WriteErrorResponse(w, http.StatusInternalServerError, err.Error()) | ||
return | ||
} | ||
|
||
if output.Empty() { | ||
rest.WriteErrorResponse(w, http.StatusNotFound, fmt.Sprintf("no transaction found with hash %s", hashHexStr)) | ||
} | ||
|
||
rest.PostProcessResponse(w, cdc, output, cliCtx.Indent) | ||
} | ||
} |
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.
why delete?
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.
Because it's only used in one place.
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.
But it add meaning! Otherwise, you'd have to guess what it is