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

feat: implementing SubmitTx gRPC endpoint #2147

Merged
merged 11 commits into from
Sep 1, 2022
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Features

* (apps/27-interchain-accounts) [\#2147](https://github.com/cosmos/ibc-go/pull/2147) Adding a `SubmitTx` gRPC endpoint for the ICS27 Controller module which allows owners of interchain accounts to submit transactions. This replaces the previously existing need for authentication modules to implement this standard functionality.

### Bug Fixes

* (makefile) [\#1785](https://github.com/cosmos/ibc-go/pull/1785) Fetch the correct versions of protocol buffers dependencies from tendermint, cosmos-sdk, and ics23.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,12 @@ import (
"context"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

"github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types"
channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v5/modules/core/24-host"
)

var _ types.MsgServer = Keeper{}
Expand All @@ -32,5 +35,27 @@ func (k Keeper) RegisterAccount(goCtx context.Context, msg *types.MsgRegisterAcc

// SubmitTx defines a rpc handler for MsgSubmitTx
func (k Keeper) SubmitTx(goCtx context.Context, msg *types.MsgSubmitTx) (*types.MsgSubmitTxResponse, error) {
return &types.MsgSubmitTxResponse{}, nil
ctx := sdk.UnwrapSDKContext(goCtx)

portID, err := icatypes.NewControllerPortID(msg.Owner)
if err != nil {
return nil, err
}

channelID, found := k.GetActiveChannelID(ctx, msg.ConnectionId, portID)
if !found {
return nil, sdkerrors.Wrapf(icatypes.ErrActiveChannelNotFound, "failed to retrieve active channel for port %s", portID)
}
Comment on lines +45 to +48
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 be taking in the channelID as the message argument? What's the UX benefit here? I'm thinking about a future where we remove the active channel mapping

cc @AdityaSripal

Copy link
Member

Choose a reason for hiding this comment

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

I suppose we could, but i think in that future we will break APIs anyway. So not sure its worth optimizing for that now


chanCap, found := k.scopedKeeper.GetCapability(ctx, host.ChannelCapabilityPath(portID, channelID))
if !found {
return nil, sdkerrors.Wrap(channeltypes.ErrChannelCapabilityNotFound, "module does not own channel capability")
}

seq, err := k.SendTx(ctx, chanCap, msg.ConnectionId, portID, msg.PacketData, msg.TimeoutTimestamp)
if err != nil {
return nil, err
}

return &types.MsgSubmitTxResponse{Sequence: seq}, nil
}
Original file line number Diff line number Diff line change
@@ -1,17 +1,24 @@
package keeper_test

import (
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
sdktypes "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"

icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
"github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
controllertypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types"
clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types"
host "github.com/cosmos/ibc-go/v5/modules/core/24-host"
ibctesting "github.com/cosmos/ibc-go/v5/testing"
)

func (suite *KeeperTestSuite) TestRegisterAccount() {
var (
msg *icatypes.MsgRegisterAccount
msg *controllertypes.MsgRegisterAccount
expectedChannelID = "channel-0"
)

Expand Down Expand Up @@ -63,7 +70,7 @@ func (suite *KeeperTestSuite) TestRegisterAccount() {
path := NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)

msg = icatypes.NewMsgRegisterAccount(ibctesting.FirstConnectionID, ibctesting.TestAccAddress, "")
msg = controllertypes.NewMsgRegisterAccount(ibctesting.FirstConnectionID, ibctesting.TestAccAddress, "")

tc.malleate()

Expand All @@ -85,3 +92,106 @@ func (suite *KeeperTestSuite) TestRegisterAccount() {
}
}
}

func (suite *KeeperTestSuite) TestSubmitTx() {
var (
path *ibctesting.Path
msg *controllertypes.MsgSubmitTx
)

testCases := []struct {
name string
malleate func()
expPass bool
}{
{
"success", func() {
},
true,
},
{
"failure - owner address is empty", func() {
msg.Owner = ""
},
false,
},
{
"failure - active channel does not exist for connection ID", func() {
msg.Owner = TestOwnerAddress
msg.ConnectionId = "connection-100"
},
false,
},
{
"failure - active channel does not exist for port ID", func() {
msg.Owner = TestAccAddress.String()
},
false,
},
{
"failure - controller module does not own capability for this channel", func() {
msg.Owner = TestAccAddress.String()
portID, err := icatypes.NewControllerPortID(msg.Owner)
suite.Require().NoError(err)

// set the active channel with the incorrect portID in order to reach the capability check
suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ConnectionID, portID, path.EndpointA.ChannelID)
},
false,
},
}

for _, tc := range testCases {
tc := tc

suite.Run(tc.name, func() {
suite.SetupTest()

owner := TestOwnerAddress
path = NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)

err := SetupICAPath(path, owner)
suite.Require().NoError(err)

portID, err := icatypes.NewControllerPortID(TestOwnerAddress)
suite.Require().NoError(err)

// get the address of the interchain account stored in state during handshake step
interchainAccountAddr, found := suite.chainA.GetSimApp().ICAControllerKeeper.GetInterchainAccountAddress(suite.chainA.GetContext(), path.EndpointA.ConnectionID, portID)
suite.Require().True(found)

// create bank transfer message that will execute on the host chain
icaMsg := &banktypes.MsgSend{
FromAddress: interchainAccountAddr,
ToAddress: suite.chainB.SenderAccount.GetAddress().String(),
Amount: sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(100))),
}

data, err := icatypes.SerializeCosmosTx(suite.chainA.Codec, []sdk.Msg{icaMsg})
suite.Require().NoError(err)

packetData := icatypes.InterchainAccountPacketData{
Type: icatypes.EXECUTE_TX,
Data: data,
Memo: "memo",
}

timeoutTimestamp := uint64(suite.chainA.GetContext().BlockTime().Add(time.Minute).UnixNano())
connectionID := path.EndpointA.ConnectionID

msg = types.NewMsgSubmitTx(owner, connectionID, clienttypes.ZeroHeight(), timeoutTimestamp, packetData)

tc.malleate() // malleate mutates test data
res, err := suite.chainA.GetSimApp().ICAControllerKeeper.SubmitTx(sdk.WrapSDKContext(suite.chainA.GetContext()), msg)

if tc.expPass {
suite.Require().NoError(err)
suite.Require().NotNil(res)
} else {
suite.Require().Error(err)
suite.Require().Nil(res)
}
})
}
}