Skip to content
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

Add GRPc Query for Upgrade #3491

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
995c1f1
added validateProposedUpgradeFields function and CheckIsOpen helper f…
colin-axner Apr 12, 2023
f8f6ad4
added unit test for CheckIsOpen
chatton Apr 13, 2023
808ec70
adding unit tests for ValidateProposedUpgradeFields
chatton Apr 13, 2023
8dd0c44
fix linter
chatton Apr 13, 2023
64878ae
update to use errorsmod instead of sdk errors
chatton Apr 13, 2023
51bb7a7
added upgrade verification function and unit tests
chatton Apr 13, 2023
f203df9
improved variable naming
chatton Apr 13, 2023
837e3c9
Merge branch '04-channel-upgrades' into cian/issue#3445-add-verifiabl…
chatton Apr 13, 2023
5f9ad67
re-arranged order of test functions
chatton Apr 13, 2023
7bdf33c
Merge branch 'cian/issue#3445-add-verifiable-upgrade-type-and-validat…
chatton Apr 13, 2023
13812eb
refactor chan open init
chatton Apr 13, 2023
2f7d0fc
removed commented out code
chatton Apr 13, 2023
28779f5
rename ValidateProposedUpgradeFields to ValidateUpgradeFields
chatton Apr 18, 2023
08af922
merge 04-channel-upgrades
chatton Apr 18, 2023
458b171
merge 04-channel-upgrades branch
chatton Apr 18, 2023
93db50e
refactoring to use new upgrade type
chatton Apr 18, 2023
b83849a
removed unused functions and tests
chatton Apr 18, 2023
f29e2f8
remove unnecessary upgrade prefix
chatton Apr 18, 2023
09ddf84
addressing PR feedback
chatton Apr 18, 2023
8f36c16
fix linter
chatton Apr 18, 2023
0b36176
fixed logging messages
crodriguezvega Apr 19, 2023
dfd9be3
wip: pr comments
chatton Apr 19, 2023
f7eb3dd
Merge branch 'cian/issue#3447-modify-upgradeinit-to-use-new-upgrade-t…
chatton Apr 19, 2023
c631e06
pr feedback: use connection id instead of channel id
chatton Apr 19, 2023
b8b5ca4
pr feedback: refactor MsgChannelUpgradeInitResponse to contain Upgrad…
chatton Apr 19, 2023
0e8092b
pr feedback: adjusting error messages and test case names
chatton Apr 19, 2023
d18ca78
adding grpc proto and implementation
chatton Apr 19, 2023
63c6acd
Merge branch 'cian/issue#3447-modify-upgradeinit-to-use-new-upgrade-t…
chatton Apr 19, 2023
4f4bff2
fixing test for Upgrade query grpc
chatton Apr 19, 2023
fe9e8ef
adding cli command for upgrade
chatton Apr 19, 2023
f35cfbd
re-added upgrade sequence endpoint
chatton Apr 19, 2023
4f7109e
corrected docstrings
chatton Apr 19, 2023
816f930
fix linter
chatton Apr 19, 2023
d5eb9b3
fix linter
chatton Apr 19, 2023
7ed8706
merge 04-channel-upgrades
chatton Apr 19, 2023
ddf721b
corrected naming
chatton Apr 19, 2023
086e065
pr feedback
chatton Apr 20, 2023
8684782
fix merge conflicts
chatton Apr 24, 2023
4c3b627
pr feedback
chatton Apr 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion modules/core/04-channel/client/cli/cli.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func GetQueryCmd() *cobra.Command {
GetCmdQueryUnreceivedAcks(),
GetCmdQueryNextSequenceReceive(),
GetCmdQueryUpgradeError(),
// TODO: next sequence Send ?
GetCmdQueryUpgrade(),
)

return queryCmd
Expand Down
34 changes: 34 additions & 0 deletions modules/core/04-channel/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -489,3 +489,37 @@ func GetCmdQueryUpgradeError() *cobra.Command {

return cmd
}

// GetCmdQueryUpgrade defines the command to query for the upgrade associated with a port and channel id
func GetCmdQueryUpgrade() *cobra.Command {
cmd := &cobra.Command{
Use: "upgrade [port-id] [channel-id]",
Short: "Query the upgrade",
Long: "Query the upgrade for a given channel",
Example: fmt.Sprintf(
"%s query %s %s upgrade [port-id] [channel-id]", version.AppName, ibcexported.ModuleName, types.SubModuleName,
),
Args: cobra.ExactArgs(2),
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientQueryContext(cmd)
if err != nil {
return err
}
portID := args[0]
channelID := args[1]
prove, _ := cmd.Flags().GetBool(flags.FlagProve)
colin-axner marked this conversation as resolved.
Show resolved Hide resolved

upgrade, err := utils.QueryUpgrade(clientCtx, portID, channelID, prove)
if err != nil {
return err
}

clientCtx = clientCtx.WithHeight(int64(upgrade.ProofHeight.RevisionHeight))
return clientCtx.PrintProto(upgrade)
},
}
cmd.Flags().Bool(flags.FlagProve, false, "show proofs for the query results")
flags.AddQueryFlagsToCmd(cmd)

return cmd
}
43 changes: 43 additions & 0 deletions modules/core/04-channel/client/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,25 @@ func QueryUpgradeError(
return queryClient.UpgradeError(context.Background(), req)
}

// QueryUpgrade returns the upgrade.
// If prove is true, it performs an ABCI store query in order to retrieve the merkle proof. Otherwise,
// it uses the gRPC query client.
func QueryUpgrade(
clientCtx client.Context, portID, channelID string, prove bool,
) (*types.QueryUpgradeResponse, error) {
if prove {
return queryUpgradeABCI(clientCtx, portID, channelID)
}

queryClient := types.NewQueryClient(clientCtx)
req := &types.QueryUpgradeRequest{
PortId: portID,
ChannelId: channelID,
}

return queryClient.Upgrade(context.Background(), req)
}

// queryUpgradeErrorABCI queries the upgrade error from the store.
func queryUpgradeErrorABCI(clientCtx client.Context, portID, channelID string) (*types.QueryUpgradeErrorResponse, error) {
key := host.ChannelUpgradeErrorKey(portID, channelID)
Expand All @@ -237,6 +256,30 @@ func queryUpgradeErrorABCI(clientCtx client.Context, portID, channelID string) (
return types.NewQueryUpgradeErrorResponse(receipt, proofBz, proofHeight), nil
}

// queryUpgradeABCI queries the upgrade from the store.
func queryUpgradeABCI(clientCtx client.Context, portID, channelID string) (*types.QueryUpgradeResponse, error) {
key := host.ChannelUpgradeKey(portID, channelID)

value, proofBz, proofHeight, err := ibcclient.QueryTendermintProof(clientCtx, key)
if err != nil {
return nil, err
}

// check if upgrade exists
if len(value) == 0 {
return nil, errorsmod.Wrapf(types.ErrUpgradeErrorNotFound, "portID (%s), channelID (%s)", portID, channelID)
}

cdc := codec.NewProtoCodec(clientCtx.InterfaceRegistry)

var upgrade types.Upgrade
if err := cdc.Unmarshal(value, &upgrade); err != nil {
return nil, err
}

return types.NewQueryUpgradeResponse(upgrade, proofBz, proofHeight), nil
}

// QueryPacketCommitment returns a packet commitment.
// If prove is true, it performs an ABCI store query in order to retrieve the merkle proof. Otherwise,
// it uses the gRPC query client.
Expand Down
23 changes: 23 additions & 0 deletions modules/core/04-channel/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,29 @@ func (q Keeper) UpgradeErrorReceipt(c context.Context, req *types.QueryUpgradeEr
return types.NewQueryUpgradeErrorResponse(receipt, nil, selfHeight), nil
}

// Upgrade implements the Query/UpgradeSequence gRPC method
func (q Keeper) Upgrade(c context.Context, req *types.QueryUpgradeRequest) (*types.QueryUpgradeResponse, error) {
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
if req == nil {
return nil, status.Error(codes.InvalidArgument, "empty request")
}

if err := validategRPCRequest(req.PortId, req.ChannelId); err != nil {
return nil, err
}

ctx := sdk.UnwrapSDKContext(c)
upgrade, found := q.GetUpgrade(ctx, req.PortId, req.ChannelId)
if !found {
return nil, status.Error(
codes.NotFound,
errorsmod.Wrapf(types.ErrUpgradeNotFound, "port-id: %s, channel-id %s", req.PortId, req.ChannelId).Error(),
)
}

selfHeight := clienttypes.GetSelfHeight(ctx)
return types.NewQueryUpgradeResponse(upgrade, nil, selfHeight), nil
}

func validategRPCRequest(portID, channelID string) error {
if err := host.PortIdentifierValidator(portID); err != nil {
return status.Error(codes.InvalidArgument, err.Error())
Expand Down
103 changes: 103 additions & 0 deletions modules/core/04-channel/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import (
clienttypes "github.com/cosmos/ibc-go/v7/modules/core/02-client/types"
connectiontypes "github.com/cosmos/ibc-go/v7/modules/core/03-connection/types"
"github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
"github.com/cosmos/ibc-go/v7/modules/core/exported"
ibctesting "github.com/cosmos/ibc-go/v7/testing"
"github.com/cosmos/ibc-go/v7/testing/mock"
)

const doesnotexist = "doesnotexist"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is unrelated to this PR i guess, but this const is a bit random here if we are just using string values in all the rest of the tests

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diff would be quite large if I changed all of these here

Expand Down Expand Up @@ -1539,3 +1541,104 @@ func (suite *KeeperTestSuite) TestQueryUpgradeError() {
})
}
}

func (suite *KeeperTestSuite) TestQueryUpgrade() {
var (
req *types.QueryUpgradeRequest
expectedUpgrade types.Upgrade
)

testCases := []struct {
msg string
malleate func()
expPass bool
}{
{
"empty request",
func() {
req = nil
},
false,
},
{
"invalid port ID",
func() {
req = &types.QueryUpgradeRequest{
PortId: "",
ChannelId: "test-channel-id",
}
},
false,
},
{
"invalid channel ID",
func() {
req = &types.QueryUpgradeRequest{
PortId: "test-port-id",
ChannelId: "",
}
},
false,
},
{
"channel not found",
func() {
req = &types.QueryUpgradeRequest{
PortId: "test-port-id",
ChannelId: "test-channel-id",
}
},
false,
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we add one more test for querying an upgrade on a channel that hasn't initiated an upgrade yet?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test is actually just directly setting the upgrade in state, the upgrade isn't being set in the ChanUpgradeInit function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually we can still do this by just deleting the key 👍

{
"upgrade not found",
func() {
storeKey := suite.chainA.GetSimApp().GetKey(exported.StoreKey)
kvStore := suite.chainA.GetContext().KVStore(storeKey)
kvStore.Delete(host.ChannelUpgradeKey(req.PortId, req.ChannelId))
},
false,
},
{
"success",
func() {
},
true,
},
}

for _, tc := range testCases {
suite.Run(fmt.Sprintf("Case %s", tc.msg), func() {
suite.SetupTest() // reset
path := ibctesting.NewPath(suite.chainA, suite.chainB)
suite.coordinator.Setup(path)

expectedUpgrade = *types.NewUpgrade(
types.NewUpgradeFields(types.UNORDERED, []string{ibctesting.FirstConnectionID}, mock.Version),
types.NewUpgradeTimeout(clienttypes.ZeroHeight(), 1000000),
1,
)

suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, expectedUpgrade)

req = &types.QueryUpgradeRequest{
PortId: path.EndpointA.ChannelConfig.PortID,
ChannelId: path.EndpointA.ChannelID,
}

tc.malleate()

ctx := sdk.WrapSDKContext(suite.chainA.GetContext())

res, err := suite.chainA.QueryServer.Upgrade(ctx, req)

if tc.expPass {
suite.Require().NoError(err)
suite.Require().NotNil(res)
suite.Require().Equal(expectedUpgrade, res.Upgrade)
} else {
suite.Require().Error(err)
}
})
}
}
1 change: 1 addition & 0 deletions modules/core/04-channel/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ var (
ErrUpgradeSequenceNotFound = errorsmod.Register(SubModuleName, 28, "upgrade sequence not found")
ErrUpgradeErrorNotFound = errorsmod.Register(SubModuleName, 29, "upgrade error receipt not found")
ErrInvalidUpgrade = errorsmod.Register(SubModuleName, 30, "invalid upgrade")
ErrUpgradeNotFound = errorsmod.Register(SubModuleName, 31, "upgrade not found")
)
9 changes: 9 additions & 0 deletions modules/core/04-channel/types/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,3 +102,12 @@ func NewQueryUpgradeErrorResponse(errorReceipt ErrorReceipt, proof []byte, heigh
ProofHeight: height,
}
}

// NewQueryUpgradeResponse creates a new QueryUpgradeResponse instance
func NewQueryUpgradeResponse(upgrade Upgrade, proof []byte, height clienttypes.Height) *QueryUpgradeResponse {
return &QueryUpgradeResponse{
Upgrade: upgrade,
Proof: proof,
ProofHeight: height,
}
}
Loading