Skip to content

Commit

Permalink
fix: Evidence API does not decode the hash properly (#13740)
Browse files Browse the repository at this point in the history
## Description

Closes: #13444 

This PR fixes the x/evidence api query which is not decoding the evidence hash properly because of its type ([]byte). We fix it with the following steps:
1. Adds a new proto field `hash` of type `string`
2. Make the old `evidence_hash` field deprecated
3. Updates query handler 

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
likhita-809 authored Nov 4, 2022
1 parent 5a94358 commit 2bb114a
Show file tree
Hide file tree
Showing 10 changed files with 287 additions and 125 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* (x/evidence) [#13740](https://github.com/cosmos/cosmos-sdk/pull/13740) Add new proto field `hash` of type `string` to `QueryEvidenceRequest` which helps to decode the hash properly while using query API.
* (core) [#13306](https://github.com/cosmos/cosmos-sdk/pull/13306) Add a `FormatCoins` function to in `core/coins` to format sdk Coins following the Value Renderers spec.
* (math) [#13306](https://github.com/cosmos/cosmos-sdk/pull/13306) Add `FormatInt` and `FormatDec` functiosn in `math` to format integers and decimals following the Value Renderers spec.
* (x/staking) [#13122](https://github.com/cosmos/cosmos-sdk/pull/13122) Add `UnbondingCanComplete` and `PutUnbondingOnHold` to `x/staking` module.
Expand Down Expand Up @@ -107,6 +108,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### API Breaking Changes

* (x/evidence) [#13740](https://github.com/cosmos/cosmos-sdk/pull/13740) The `NewQueryEvidenceRequest` function now takes `hash` as a HEX encoded `string`.
* (server) [#13485](https://github.com/cosmos/cosmos-sdk/pull/13485) The `Application` service now requires the `RegisterNodeService` method to be implemented.
* (x/slashing, x/staking) [#13122](https://github.com/cosmos/cosmos-sdk/pull/13122) Add the infraction a validator commited type as an argument to the `Slash` keeper method.
* [#13437](https://github.com/cosmos/cosmos-sdk/pull/13437) Add a list of modules to export argument in `ExportAppStateAndValidators`.
Expand Down Expand Up @@ -162,6 +164,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Bug Fixes

* (x/evidence) [#13740](https://github.com/cosmos/cosmos-sdk/pull/13740) Fix evidence query API to decode the hash properly.
* (bank) [#13691](https://github.com/cosmos/cosmos-sdk/issues/13691) Fix unhandled error for vesting account transfers, when total vesting amount exceeds total balance.
* [#13553](https://github.com/cosmos/cosmos-sdk/pull/13553) Ensure all parameter validation for decimal types handles nil decimal values.
* [#13145](https://github.com/cosmos/cosmos-sdk/pull/13145) Fix panic when calling `String()` to a Record struct type.
Expand All @@ -180,6 +183,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Deprecated

* (x/evidence) [#13740](https://github.com/cosmos/cosmos-sdk/pull/13740) The `evidence_hash` field of `QueryEvidenceRequest` has been deprecated and now contains a new field `hash` with type `string`.
* (x/bank) [#11859](https://github.com/cosmos/cosmos-sdk/pull/11859) The Params.SendEnabled field is deprecated and unusable.
The information can now be accessed using the BankKeeper.
Setting can be done using MsgSetSendEnabled as a governance proposal.
Expand Down
204 changes: 142 additions & 62 deletions api/cosmos/evidence/v1beta1/query.pulsar.go

Large diffs are not rendered by default.

10 changes: 8 additions & 2 deletions proto/cosmos/evidence/v1beta1/query.proto
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ option go_package = "github.com/cosmos/cosmos-sdk/x/evidence/types";
service Query {
// Evidence queries evidence based on evidence hash.
rpc Evidence(QueryEvidenceRequest) returns (QueryEvidenceResponse) {
option (google.api.http).get = "/cosmos/evidence/v1beta1/evidence/{evidence_hash}";
option (google.api.http).get = "/cosmos/evidence/v1beta1/evidence/{hash}";
}

// AllEvidence queries all evidence.
Expand All @@ -24,7 +24,13 @@ service Query {
// QueryEvidenceRequest is the request type for the Query/Evidence RPC method.
message QueryEvidenceRequest {
// evidence_hash defines the hash of the requested evidence.
bytes evidence_hash = 1 [(gogoproto.casttype) = "github.com/tendermint/tendermint/libs/bytes.HexBytes"];
// Deprecated: Use hash, a HEX encoded string, instead.
bytes evidence_hash = 1 [deprecated = true, (gogoproto.casttype) = "github.com/tendermint/tendermint/libs/bytes.HexBytes"];

// hash defines the evidence hash of the requested evidence.
//
// Since: cosmos-sdk 0.47
string hash = 2;
}

// QueryEvidenceResponse is the response type for the Query/Evidence RPC method.
Expand Down
8 changes: 1 addition & 7 deletions x/evidence/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package cli

import (
"context"
"encoding/hex"
"fmt"
"strings"

Expand Down Expand Up @@ -64,14 +63,9 @@ func QueryEvidenceCmd() func(*cobra.Command, []string) error {
}

func queryEvidence(clientCtx client.Context, hash string) error {
decodedHash, err := hex.DecodeString(hash)
if err != nil {
return fmt.Errorf("invalid evidence hash: %w", err)
}

queryClient := types.NewQueryClient(clientCtx)

params := &types.QueryEvidenceRequest{EvidenceHash: decodedHash}
params := &types.QueryEvidenceRequest{Hash: hash}
res, err := queryClient.Evidence(context.Background(), params)
if err != nil {
return err
Expand Down
15 changes: 11 additions & 4 deletions x/evidence/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package keeper

import (
"context"
"encoding/hex"
"fmt"

"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
Expand All @@ -24,15 +26,20 @@ func (k Keeper) Evidence(c context.Context, req *types.QueryEvidenceRequest) (*t
return nil, status.Errorf(codes.InvalidArgument, "empty request")
}

if req.EvidenceHash == nil {
return nil, status.Errorf(codes.InvalidArgument, "invalid hash")
if req.Hash == "" {
return nil, status.Errorf(codes.InvalidArgument, "invalid request; hash is empty")
}

ctx := sdk.UnwrapSDKContext(c)

evidence, _ := k.GetEvidence(ctx, req.EvidenceHash)
decodedHash, err := hex.DecodeString(req.Hash)
if err != nil {
return nil, fmt.Errorf("invalid evidence hash: %w", err)
}

evidence, _ := k.GetEvidence(ctx, decodedHash)
if evidence == nil {
return nil, status.Errorf(codes.NotFound, "evidence %s not found", req.EvidenceHash)
return nil, status.Errorf(codes.NotFound, "evidence %s not found", req.Hash)
}

msg, ok := evidence.(proto.Message)
Expand Down
6 changes: 2 additions & 4 deletions x/evidence/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ import (
"github.com/cosmos/cosmos-sdk/types/query"
"github.com/cosmos/cosmos-sdk/x/evidence/exported"
"github.com/cosmos/cosmos-sdk/x/evidence/types"

tmbytes "github.com/tendermint/tendermint/libs/bytes"
)

func (suite *KeeperTestSuite) TestQueryEvidence() {
Expand All @@ -34,7 +32,7 @@ func (suite *KeeperTestSuite) TestQueryEvidence() {
{
"invalid request with empty evidence hash",
func() {
req = &types.QueryEvidenceRequest{EvidenceHash: tmbytes.HexBytes{}}
req = &types.QueryEvidenceRequest{Hash: ""}
},
false,
func(res *types.QueryEvidenceResponse) {},
Expand All @@ -44,7 +42,7 @@ func (suite *KeeperTestSuite) TestQueryEvidence() {
func() {
numEvidence := 100
evidence = suite.populateEvidence(suite.ctx, numEvidence)
req = types.NewQueryEvidenceRequest(evidence[0].Hash())
req = types.NewQueryEvidenceRequest(evidence[0].Hash().String())
},
true,
func(res *types.QueryEvidenceResponse) {
Expand Down
2 changes: 1 addition & 1 deletion x/evidence/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ package keeper
import (
"fmt"

storetypes "github.com/cosmos/cosmos-sdk/store/types"
tmbytes "github.com/tendermint/tendermint/libs/bytes"
"github.com/tendermint/tendermint/libs/log"

"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/prefix"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/evidence/exported"
Expand Down
6 changes: 2 additions & 4 deletions x/evidence/types/querier.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
package types

import (
tmbytes "github.com/tendermint/tendermint/libs/bytes"

query "github.com/cosmos/cosmos-sdk/types/query"
)

Expand All @@ -13,8 +11,8 @@ const (
)

// NewQueryEvidenceRequest creates a new instance of QueryEvidenceRequest.
func NewQueryEvidenceRequest(hash tmbytes.HexBytes) *QueryEvidenceRequest {
return &QueryEvidenceRequest{EvidenceHash: hash}
func NewQueryEvidenceRequest(hash string) *QueryEvidenceRequest {
return &QueryEvidenceRequest{Hash: hash}
}

// NewQueryAllEvidenceRequest creates a new instance of QueryAllEvidenceRequest.
Expand Down
121 changes: 89 additions & 32 deletions x/evidence/types/query.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 2bb114a

Please sign in to comment.