From 291c730d2aee27ef4205a60e90935e57d4db69aa Mon Sep 17 00:00:00 2001 From: "mergify[bot]" <37929162+mergify[bot]@users.noreply.github.com> Date: Thu, 9 Jun 2022 11:49:28 +0200 Subject: [PATCH] chore: improve DenomTrace grpc (backport #1342) (#1500) * chore: improve DenomTrace grpc (#1342) * change DenomTrace grpc * update CHANGELOG.md * minor * minor * minor * minor * minor * minor * Update modules/apps/transfer/keeper/grpc_query_test.go Co-authored-by: Carlos Rodriguez * Update modules/apps/transfer/keeper/grpc_query_test.go Co-authored-by: Carlos Rodriguez * Update CHANGELOG.md Co-authored-by: Damian Nolan * use TrimPrefix() in DenomTrace() * update migration doc Co-authored-by: Carlos Rodriguez Co-authored-by: Damian Nolan (cherry picked from commit 23e7e7dcfbe6303e83fe1f6e3d984ade97fcbbd9) # Conflicts: # CHANGELOG.md # docs/ibc/proto-docs.md # docs/migrations/v3-to-v4.md # modules/apps/transfer/client/cli/query.go * fix conflict Co-authored-by: khanh <50263489+catShaark@users.noreply.github.com> Co-authored-by: crodriguezvega --- CHANGELOG.md | 2 +- docs/client/swagger-ui/swagger.yaml | 4 ++- docs/ibc/proto-docs.md | 2 +- docs/migrations/v3-to-v4.md | 31 +++++++++++++++++++ modules/apps/transfer/client/cli/query.go | 8 ++--- modules/apps/transfer/keeper/grpc_query.go | 6 ++-- .../apps/transfer/keeper/grpc_query_test.go | 31 +++++++++++++------ modules/apps/transfer/types/query.pb.go | 2 +- .../ibc/applications/transfer/v1/query.proto | 2 +- 9 files changed, 68 insertions(+), 20 deletions(-) create mode 100644 docs/migrations/v3-to-v4.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a0e3a8998e..f68c36e41b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,7 +46,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements -* (modules/core/04-channel) [\#1130](https://github.com/cosmos/ibc-go/pull/1130) Call `packet.GetSequence()` rather than passing func in `WriteAcknowledgement` log output +* (transfer) [\#1342](https://github.com/cosmos/ibc-go/pull/1342) `DenomTrace` grpc now takes in either an `ibc denom` or a `hash` instead of only accepting a `hash`. * (modules/core/keeper) [\#1284](https://github.com/cosmos/ibc-go/pull/1284) Add sanity check for the keepers passed into `ibckeeper.NewKeeper`. `ibckeeper.NewKeeper` now panics if any of the keepers passed in is empty. * (transfer) [\#1414](https://github.com/cosmos/ibc-go/pull/1414) Emitting Sender address from `fungible_token_packet` events in `OnRecvPacket` and `OnAcknowledgementPacket`. * (modules/core/04-channel) [\#1464](https://github.com/cosmos/ibc-go/pull/1464) Emit a channel close event when an ordered channel is closed. diff --git a/docs/client/swagger-ui/swagger.yaml b/docs/client/swagger-ui/swagger.yaml index 5feb43dd2a5..46ed1f188b8 100644 --- a/docs/client/swagger-ui/swagger.yaml +++ b/docs/client/swagger-ui/swagger.yaml @@ -235,7 +235,9 @@ paths: format: byte parameters: - name: hash - description: hash (in hex format) of the denomination trace information. + description: >- + hash (in hex format) or denom (full denom with ibc prefix) of the + denomination trace information. in: path required: true type: string diff --git a/docs/ibc/proto-docs.md b/docs/ibc/proto-docs.md index b77f0b03620..c8973da070c 100644 --- a/docs/ibc/proto-docs.md +++ b/docs/ibc/proto-docs.md @@ -389,7 +389,7 @@ method | Field | Type | Label | Description | | ----- | ---- | ----- | ----------- | -| `hash` | [string](#string) | | hash (in hex format) of the denomination trace information. | +| `hash` | [string](#string) | | hash (in hex format) or denom (full denom with ibc prefix) of the denomination trace information. | diff --git a/docs/migrations/v3-to-v4.md b/docs/migrations/v3-to-v4.md new file mode 100644 index 00000000000..5f684581ab2 --- /dev/null +++ b/docs/migrations/v3-to-v4.md @@ -0,0 +1,31 @@ +# Migrating from ibc-go v3 to v4 + +This document is intended to highlight significant changes which may require more information than presented in the CHANGELOG. +Any changes that must be done by a user of ibc-go should be documented here. + +There are four sections based on the four potential user groups of this document: +- Chains +- IBC Apps +- Relayers +- IBC Light Clients + +**Note:** ibc-go supports golang semantic versioning and therefore all imports must be updated to bump the version number on major releases. +```go +github.com/cosmos/ibc-go/v3 -> github.com/cosmos/ibc-go/v4 +``` + +No genesis or in-place migrations required when upgrading from v1 or v2 of ibc-go. + +## Chains + +### IS04 - Channel + +The `WriteAcknowledgement` API now takes the `exported.Acknowledgement` type instead of passing in the acknowledgement byte array directly. +This is an API breaking change and as such IBC application developers will have to update any calls to `WriteAcknowledgement`. + +The `OnChanOpenInit` application callback has been modified. +The return signature now includes the application version as detailed in the latest IBC [spec changes](https://github.com/cosmos/ibc/pull/629). + +## Relayers + +When using the `DenomTrace` gRPC, the full IBC denomination with the `ibc/` prefix may now be passed in. diff --git a/modules/apps/transfer/client/cli/query.go b/modules/apps/transfer/client/cli/query.go index 3ad85f77759..2b664706eb9 100644 --- a/modules/apps/transfer/client/cli/query.go +++ b/modules/apps/transfer/client/cli/query.go @@ -11,12 +11,12 @@ import ( "github.com/cosmos/ibc-go/v2/modules/apps/transfer/types" ) -// GetCmdQueryDenomTrace defines the command to query a a denomination trace from a given hash. +// GetCmdQueryDenomTrace defines the command to query a a denomination trace from a given trace hash or ibc denom. func GetCmdQueryDenomTrace() *cobra.Command { cmd := &cobra.Command{ - Use: "denom-trace [hash]", - Short: "Query the denom trace info from a given trace hash", - Long: "Query the denom trace info from a given trace hash", + Use: "denom-trace [hash/denom]", + Short: "Query the denom trace info from a given trace hash or ibc denom", + Long: "Query the denom trace info from a given trace hash or ibc denom", Example: fmt.Sprintf("%s query ibc-transfer denom-trace 27A6394C3F9FF9C9DCF5DFFADF9BB5FE9A37C7E92B006199894CF1824DF9AC7C", version.AppName), Args: cobra.ExactArgs(1), RunE: func(cmd *cobra.Command, args []string) error { diff --git a/modules/apps/transfer/keeper/grpc_query.go b/modules/apps/transfer/keeper/grpc_query.go index 0b66acb3350..dddca4a24f3 100644 --- a/modules/apps/transfer/keeper/grpc_query.go +++ b/modules/apps/transfer/keeper/grpc_query.go @@ -3,6 +3,7 @@ package keeper import ( "context" "fmt" + "strings" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -22,9 +23,10 @@ func (q Keeper) DenomTrace(c context.Context, req *types.QueryDenomTraceRequest) return nil, status.Error(codes.InvalidArgument, "empty request") } - hash, err := types.ParseHexHash(req.Hash) + hash, err := types.ParseHexHash(strings.TrimPrefix(req.Hash, "ibc/")) + if err != nil { - return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("invalid denom trace hash %s, %s", req.Hash, err)) + return nil, status.Error(codes.InvalidArgument, fmt.Sprintf("invalid denom trace hash: %s, error: %s", hash.String(), err)) } ctx := sdk.UnwrapSDKContext(c) diff --git a/modules/apps/transfer/keeper/grpc_query_test.go b/modules/apps/transfer/keeper/grpc_query_test.go index 4aec8a653f3..ece18be1b06 100644 --- a/modules/apps/transfer/keeper/grpc_query_test.go +++ b/modules/apps/transfer/keeper/grpc_query_test.go @@ -20,37 +20,50 @@ func (suite *KeeperTestSuite) TestQueryDenomTrace() { expPass bool }{ { - "invalid hex hash", + "success: correct ibc denom", func() { + expTrace.Path = "transfer/channelToA/transfer/channelToB" + expTrace.BaseDenom = "uatom" + suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace(suite.chainA.GetContext(), expTrace) + req = &types.QueryDenomTraceRequest{ - Hash: "!@#!@#!", + Hash: expTrace.IBCDenom(), } }, - false, + true, }, { - "not found denom trace", + "success: correct hex hash", func() { expTrace.Path = "transfer/channelToA/transfer/channelToB" expTrace.BaseDenom = "uatom" + suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace(suite.chainA.GetContext(), expTrace) + req = &types.QueryDenomTraceRequest{ Hash: expTrace.Hash().String(), } }, + true, + }, + { + "failure: invalid hash", + func() { + req = &types.QueryDenomTraceRequest{ + Hash: "!@#!@#!", + } + }, false, }, { - "success", + "failure: not found denom trace", func() { expTrace.Path = "transfer/channelToA/transfer/channelToB" expTrace.BaseDenom = "uatom" - suite.chainA.GetSimApp().TransferKeeper.SetDenomTrace(suite.chainA.GetContext(), expTrace) - req = &types.QueryDenomTraceRequest{ - Hash: expTrace.Hash().String(), + Hash: expTrace.IBCDenom(), } }, - true, + false, }, } diff --git a/modules/apps/transfer/types/query.pb.go b/modules/apps/transfer/types/query.pb.go index 5e2cecd4cdf..c8e9f954687 100644 --- a/modules/apps/transfer/types/query.pb.go +++ b/modules/apps/transfer/types/query.pb.go @@ -33,7 +33,7 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // QueryDenomTraceRequest is the request type for the Query/DenomTrace RPC // method type QueryDenomTraceRequest struct { - // hash (in hex format) of the denomination trace information. + // hash (in hex format) or denom (full denom with ibc prefix) of the denomination trace information. Hash string `protobuf:"bytes,1,opt,name=hash,proto3" json:"hash,omitempty"` } diff --git a/proto/ibc/applications/transfer/v1/query.proto b/proto/ibc/applications/transfer/v1/query.proto index 39d8b0f1c9b..14b436a8718 100644 --- a/proto/ibc/applications/transfer/v1/query.proto +++ b/proto/ibc/applications/transfer/v1/query.proto @@ -35,7 +35,7 @@ service Query { // QueryDenomTraceRequest is the request type for the Query/DenomTrace RPC // method message QueryDenomTraceRequest { - // hash (in hex format) of the denomination trace information. + // hash (in hex format) or denom (full denom with ibc prefix) of the denomination trace information. string hash = 1; }