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

chore: correctly set/delete active channels #463

Merged
15 changes: 11 additions & 4 deletions modules/apps/27-interchain-accounts/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ func NewIBCModule(k keeper.Keeper, app porttypes.IBCModule) IBCModule {
}
}

// Implement IBCModule callbacks
// OnChanOpenInit implements the IBCModule interface
func (im IBCModule) OnChanOpenInit(
ctx sdk.Context,
order channeltypes.Order,
Expand All @@ -43,6 +43,7 @@ func (im IBCModule) OnChanOpenInit(
return im.keeper.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version)
}

// OnChanOpenTry implements the IBCModule interface
func (im IBCModule) OnChanOpenTry(
ctx sdk.Context,
order channeltypes.Order,
Expand All @@ -57,6 +58,7 @@ func (im IBCModule) OnChanOpenTry(
return im.keeper.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, chanCap, counterparty, version, counterpartyVersion)
}

// OnChanOpenAck implements the IBCModule interface
func (im IBCModule) OnChanOpenAck(
ctx sdk.Context,
portID,
Expand All @@ -66,6 +68,7 @@ func (im IBCModule) OnChanOpenAck(
return im.keeper.OnChanOpenAck(ctx, portID, channelID, counterpartyVersion)
}

// OnChanOpenConfirm implements the IBCModule interface
func (im IBCModule) OnChanOpenConfirm(
ctx sdk.Context,
portID,
Expand All @@ -74,6 +77,7 @@ func (im IBCModule) OnChanOpenConfirm(
return im.keeper.OnChanOpenConfirm(ctx, portID, channelID)
}

// OnChanCloseInit implements the IBCModule interface
func (im IBCModule) OnChanCloseInit(
ctx sdk.Context,
portID,
Expand All @@ -83,14 +87,16 @@ func (im IBCModule) OnChanCloseInit(
return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "user cannot close channel")
}

// OnChanCloseConfirm implements the IBCModule interface
func (im IBCModule) OnChanCloseConfirm(
ctx sdk.Context,
portID,
channelID string,
) error {
return nil
return im.keeper.OnChanCloseConfirm(ctx, portID, channelID)
}

// OnRecvPacket implements the IBCModule interface
func (im IBCModule) OnRecvPacket(
ctx sdk.Context,
packet channeltypes.Packet,
Expand All @@ -116,6 +122,7 @@ func (im IBCModule) OnRecvPacket(
return ack
}

// OnAcknowledgementPacket implements the IBCModule interface
func (im IBCModule) OnAcknowledgementPacket(
ctx sdk.Context,
packet channeltypes.Packet,
Expand All @@ -139,13 +146,13 @@ func (im IBCModule) OnAcknowledgementPacket(
return nil
}

// OnTimeoutPacket implements the IBCModule interface
func (im IBCModule) OnTimeoutPacket(
ctx sdk.Context,
packet channeltypes.Packet,
_ sdk.AccAddress,
) error {
// TODO
return nil
return im.keeper.OnTimeoutPacket(ctx, packet)
}

// NegotiateAppVersion implements the IBCModule interface
Expand Down
19 changes: 18 additions & 1 deletion modules/apps/27-interchain-accounts/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,12 +164,29 @@ func (k Keeper) OnChanOpenAck(
return nil
}

// Set active channel
// OnChanOpenConfirm completes the handshake process by setting the active channel in state on the host chain
//
// Host Chain
func (k Keeper) OnChanOpenConfirm(
ctx sdk.Context,
portID,
channelID string,
) error {

k.SetActiveChannel(ctx, portID, channelID)

return nil
}

// OnChanCloseConfirm removes the active channel stored in state
func (k Keeper) OnChanCloseConfirm(
ctx sdk.Context,
portID,
channelID string,
) error {

k.DeleteActiveChannel(ctx, portID)

return nil
}

Expand Down
44 changes: 44 additions & 0 deletions modules/apps/27-interchain-accounts/keeper/handshake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -428,3 +428,47 @@ func (suite *KeeperTestSuite) TestOnChanOpenConfirm() {
})
}
}

func (suite *KeeperTestSuite) TestOnChanCloseConfirm() {
var (
path *ibctesting.Path
)

testCases := []struct {
name string
malleate func()
expPass bool
}{

{
"success", func() {}, true,
},
}

for _, tc := range testCases {
suite.Run(tc.name, func() {
suite.SetupTest() // reset
path = NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)

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

tc.malleate() // explicitly change fields in channel and testChannel

err = suite.chainB.GetSimApp().ICAKeeper.OnChanCloseConfirm(suite.chainB.GetContext(),
path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)

activeChannel, found := suite.chainB.GetSimApp().ICAKeeper.GetActiveChannel(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)

if tc.expPass {
suite.Require().NoError(err)
suite.Require().False(found)
suite.Require().Empty(activeChannel)
} else {
suite.Require().Error(err)
}

})
}
}
6 changes: 6 additions & 0 deletions modules/apps/27-interchain-accounts/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,12 @@ func (k Keeper) SetActiveChannel(ctx sdk.Context, portID, channelID string) {
store.Set(types.KeyActiveChannel(portID), []byte(channelID))
}

// DeleteActiveChannel removes the active channel keyed by the provided portID stored in state
func (k Keeper) DeleteActiveChannel(ctx sdk.Context, portID string) {
store := ctx.KVStore(k.storeKey)
store.Delete(types.KeyActiveChannel(portID))
}

// IsActiveChannel returns true if there exists an active channel for the provided portID, otherwise false
func (k Keeper) IsActiveChannel(ctx sdk.Context, portID string) bool {
_, ok := k.GetActiveChannel(ctx, portID)
Expand Down
3 changes: 1 addition & 2 deletions modules/apps/27-interchain-accounts/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ func (suite *KeeperTestSuite) SetupICAPath(path *ibctesting.Path, owner string)
return err
}

if err := suite.chainB.GetSimApp().ICAKeeper.OnChanOpenConfirm(suite.chainA.GetContext(),
path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID); err != nil {
if err := path.EndpointB.ChanOpenConfirm(); err != nil {
return err
}

Expand Down
8 changes: 4 additions & 4 deletions modules/apps/27-interchain-accounts/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,10 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac
}
}

func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet, data types.InterchainAccountPacketData) error {
if k.hook != nil {
k.hook.OnTxFailed(ctx, packet.SourcePort, packet.SourceChannel, packet.Data, data.Data)
}
// OnTimeoutPacket removes the active channel associated with the provided packet, the underlying channel end is closed
// due to the semantics of ORDERED channels
func (k Keeper) OnTimeoutPacket(ctx sdk.Context, packet channeltypes.Packet) error {
k.DeleteActiveChannel(ctx, packet.SourcePort)

return nil
}
42 changes: 42 additions & 0 deletions modules/apps/27-interchain-accounts/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,45 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
})
}
}

func (suite *KeeperTestSuite) TestOnTimeoutPacket() {
var (
path *ibctesting.Path
)

testCases := []struct {
name string
malleate func()
expPass bool
}{
{
"success", func() {}, true,
},
}

for _, tc := range testCases {
suite.Run(tc.name, func() {
suite.SetupTest() // reset
path = NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)

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

tc.malleate() // malleate mutates test data

packet := channeltypes.NewPacket([]byte{}, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, clienttypes.NewHeight(0, 100), 0)
err = suite.chainA.GetSimApp().ICAKeeper.OnTimeoutPacket(suite.chainA.GetContext(), packet)

channel, found := suite.chainA.GetSimApp().ICAKeeper.GetActiveChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)

if tc.expPass {
suite.Require().NoError(err)
suite.Require().Empty(channel)
suite.Require().False(found)
} else {
suite.Require().Error(err)
}
})
}
}