R4R: Tx Query return values #3642
Conversation
jackzampolin
requested review from
cwgoes,
ebuchman and
rigelrozanski
as code owners
jackzampolin
changed the title
There was a problem hiding this comment.
@jackzampolin did you test this locally? It fails CI on a nil pointer exception.
|
Do we have a cli test for this? |
|
Moving back to WIP pending fixed tests and added test coverage for this |
Codecov Report
@@ Coverage Diff @@
## develop #3642 +/- ##
===========================================
- Coverage 61.32% 61.27% -0.05%
===========================================
Files 186 186
Lines 13998 14002 +4
===========================================
- Hits 8584 8580 -4
- Misses 4870 4878 +8
Partials 544 544 |
| @@ -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] | |||
There was a problem hiding this comment.
Because it's only used in one place.
There was a problem hiding this comment.
But it add meaning! Otherwise, you'd have to guess what it is
client/tx/query.go
Outdated
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if err = ValidateTxResult(cliCtx, res); !cliCtx.TrustNode && err != nil { |
There was a problem hiding this comment.
This does not equal to what was before. Before we didn't execute ValidateTxResult if TrustNode == true.
| err = res.Proof.Validate(check.Header.DataHash) | ||
| if err != nil { | ||
| return err | ||
| if !cliCtx.TrustNode { |
There was a problem hiding this comment.
if cliCtx.TrustNode {
return nil
}There was a problem hiding this comment.
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.
I meant instead of wrapping big chunk of code into if condition, you can return early!
Fixes: #3639
cc @alexanderbez @alessio