diff --git a/modules/apps/27-interchain-accounts/keeper/account_test.go b/modules/apps/27-interchain-accounts/keeper/account_test.go index 3d5a1bb31af..fceea5eb48b 100644 --- a/modules/apps/27-interchain-accounts/keeper/account_test.go +++ b/modules/apps/27-interchain-accounts/keeper/account_test.go @@ -39,7 +39,7 @@ func (suite *KeeperTestSuite) TestInitInterchainAccount() { func() { portID, err := types.GeneratePortID(owner, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) suite.Require().NoError(err) - suite.chainA.GetSimApp().ICAKeeper.SetActiveChannel(suite.chainA.GetContext(), portID, path.EndpointA.ChannelID) + suite.chainA.GetSimApp().ICAKeeper.SetActiveChannelID(suite.chainA.GetContext(), portID, path.EndpointA.ChannelID) }, false, }, diff --git a/modules/apps/27-interchain-accounts/keeper/handshake.go b/modules/apps/27-interchain-accounts/keeper/handshake.go index 6513b65758a..7481898fe4b 100644 --- a/modules/apps/27-interchain-accounts/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/keeper/handshake.go @@ -57,9 +57,9 @@ func (k Keeper) OnChanOpenInit( return sdkerrors.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.VersionPrefix, version) } - existingChannelID, found := k.GetActiveChannel(ctx, portID) + activeChannelID, found := k.GetActiveChannelID(ctx, portID) if found { - return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "existing active channel %s for portID %s", existingChannelID, portID) + return sdkerrors.Wrapf(porttypes.ErrInvalidPort, "existing active channel %s for portID %s", activeChannelID, portID) } // Claim channel capability passed back by IBC module @@ -152,7 +152,7 @@ func (k Keeper) OnChanOpenAck( return sdkerrors.Wrap(err, "counterparty version validation failed") } - k.SetActiveChannel(ctx, portID, channelID) + k.SetActiveChannelID(ctx, portID, channelID) accAddr, err := types.ParseAddressFromVersion(counterpartyVersion) if err != nil { @@ -173,7 +173,7 @@ func (k Keeper) OnChanOpenConfirm( channelID string, ) error { - k.SetActiveChannel(ctx, portID, channelID) + k.SetActiveChannelID(ctx, portID, channelID) return nil } @@ -185,7 +185,7 @@ func (k Keeper) OnChanCloseConfirm( channelID string, ) error { - k.DeleteActiveChannel(ctx, portID) + k.DeleteActiveChannelID(ctx, portID) return nil } diff --git a/modules/apps/27-interchain-accounts/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/keeper/handshake_test.go index 06eb217ecb5..e81a5781cd5 100644 --- a/modules/apps/27-interchain-accounts/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/keeper/handshake_test.go @@ -99,7 +99,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { { "channel is already active", func() { - suite.chainA.GetSimApp().ICAKeeper.SetActiveChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + suite.chainA.GetSimApp().ICAKeeper.SetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) }, false, }, @@ -325,7 +325,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { func (suite *KeeperTestSuite) TestOnChanOpenAck() { var ( path *ibctesting.Path - expectedChannel string + expectedChannelID string counterpartyVersion string ) @@ -339,7 +339,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() { }, { "invalid counterparty version", func() { - expectedChannel = "" + expectedChannelID = "" counterpartyVersion = "version" }, false, }, @@ -359,7 +359,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() { err = path.EndpointB.ChanOpenTry() suite.Require().NoError(err) - expectedChannel = path.EndpointA.ChannelID + expectedChannelID = path.EndpointA.ChannelID tc.malleate() // explicitly change fields in channel and testChannel @@ -367,9 +367,9 @@ func (suite *KeeperTestSuite) TestOnChanOpenAck() { path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, counterpartyVersion, ) - activeChannel, _ := suite.chainA.GetSimApp().ICAKeeper.GetActiveChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) + activeChannelID, _ := suite.chainA.GetSimApp().ICAKeeper.GetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) - suite.Require().Equal(activeChannel, expectedChannel) + suite.Require().Equal(activeChannelID, expectedChannelID) if tc.expPass { suite.Require().NoError(err) @@ -459,12 +459,12 @@ func (suite *KeeperTestSuite) TestOnChanCloseConfirm() { 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) + activeChannelID, found := suite.chainB.GetSimApp().ICAKeeper.GetActiveChannelID(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID) if tc.expPass { suite.Require().NoError(err) suite.Require().False(found) - suite.Require().Empty(activeChannel) + suite.Require().Empty(activeChannelID) } else { suite.Require().Error(err) } diff --git a/modules/apps/27-interchain-accounts/keeper/keeper.go b/modules/apps/27-interchain-accounts/keeper/keeper.go index d059f5a4ca8..467fbc519dc 100644 --- a/modules/apps/27-interchain-accounts/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/keeper/keeper.go @@ -99,10 +99,10 @@ func (k Keeper) ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability return k.scopedKeeper.ClaimCapability(ctx, cap, name) } -// GetActiveChannel retrieves the active channelID from the store keyed by the provided portID -func (k Keeper) GetActiveChannel(ctx sdk.Context, portId string) (string, bool) { +// GetActiveChannelID retrieves the active channelID from the store keyed by the provided portID +func (k Keeper) GetActiveChannelID(ctx sdk.Context, portID string) (string, bool) { store := ctx.KVStore(k.storeKey) - key := types.KeyActiveChannel(portId) + key := types.KeyActiveChannel(portID) if !store.Has(key) { return "", false @@ -111,21 +111,21 @@ func (k Keeper) GetActiveChannel(ctx sdk.Context, portId string) (string, bool) return string(store.Get(key)), true } -// SetActiveChannel stores the active channelID, keyed by the provided portID -func (k Keeper) SetActiveChannel(ctx sdk.Context, portID, channelID string) { +// SetActiveChannelID stores the active channelID, keyed by the provided portID +func (k Keeper) SetActiveChannelID(ctx sdk.Context, portID, channelID string) { store := ctx.KVStore(k.storeKey) 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) { +// DeleteActiveChannelID removes the active channel keyed by the provided portID stored in state +func (k Keeper) DeleteActiveChannelID(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) + _, ok := k.GetActiveChannelID(ctx, portID) return ok } diff --git a/modules/apps/27-interchain-accounts/keeper/relay.go b/modules/apps/27-interchain-accounts/keeper/relay.go index 3fbb0d7b10c..2e3a896858a 100644 --- a/modules/apps/27-interchain-accounts/keeper/relay.go +++ b/modules/apps/27-interchain-accounts/keeper/relay.go @@ -14,20 +14,20 @@ import ( // manage helper module access to owner addresses they do not have capabilities for func (k Keeper) TrySendTx(ctx sdk.Context, portID string, icaPacketData types.InterchainAccountPacketData) (uint64, error) { // Check for the active channel - activeChannelId, found := k.GetActiveChannel(ctx, portID) + activeChannelID, found := k.GetActiveChannelID(ctx, portID) if !found { - return 0, types.ErrActiveChannelNotFound + return 0, sdkerrors.Wrapf(types.ErrActiveChannelNotFound, "failed to retrieve active channel for port %s", portID) } - sourceChannelEnd, found := k.channelKeeper.GetChannel(ctx, portID, activeChannelId) + sourceChannelEnd, found := k.channelKeeper.GetChannel(ctx, portID, activeChannelID) if !found { - return 0, sdkerrors.Wrap(channeltypes.ErrChannelNotFound, activeChannelId) + return 0, sdkerrors.Wrap(channeltypes.ErrChannelNotFound, activeChannelID) } destinationPort := sourceChannelEnd.GetCounterparty().GetPortID() destinationChannel := sourceChannelEnd.GetCounterparty().GetChannelID() - return k.createOutgoingPacket(ctx, portID, activeChannelId, destinationPort, destinationChannel, icaPacketData) + return k.createOutgoingPacket(ctx, portID, activeChannelID, destinationPort, destinationChannel, icaPacketData) } func (k Keeper) createOutgoingPacket( @@ -50,7 +50,7 @@ func (k Keeper) createOutgoingPacket( // get the next sequence sequence, found := k.channelKeeper.GetNextSequenceSend(ctx, sourcePort, sourceChannel) if !found { - return 0, channeltypes.ErrSequenceSendNotFound + return 0, sdkerrors.Wrapf(channeltypes.ErrSequenceSendNotFound, "failed to retrieve next sequence send for channel %s on port %s", sourceChannel, sourcePort) } // timeoutTimestamp is set to be a max number here so that we never recieve a timeout @@ -147,7 +147,8 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) error var data types.InterchainAccountPacketData if err := types.ModuleCdc.UnmarshalJSON(packet.GetData(), &data); err != nil { - return sdkerrors.Wrapf(types.ErrUnknownPacketData, "cannot unmarshal ICS-27 interchain account packet data") + // UnmarshalJSON errors are indeterminate and therefore are not wrapped and included in failed acks + return sdkerrors.Wrapf(types.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain account packet data") } switch data.Type { @@ -164,7 +165,7 @@ func (k Keeper) OnRecvPacket(ctx sdk.Context, packet channeltypes.Packet) error return nil default: - return types.ErrUnknownPacketData + return types.ErrUnknownDataType } } @@ -175,7 +176,7 @@ func (k Keeper) OnAcknowledgementPacket(ctx sdk.Context, packet channeltypes.Pac // 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) + k.DeleteActiveChannelID(ctx, packet.SourcePort) return nil } diff --git a/modules/apps/27-interchain-accounts/keeper/relay_test.go b/modules/apps/27-interchain-accounts/keeper/relay_test.go index e40512dd265..106af7ccd96 100644 --- a/modules/apps/27-interchain-accounts/keeper/relay_test.go +++ b/modules/apps/27-interchain-accounts/keeper/relay_test.go @@ -9,7 +9,6 @@ import ( "github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types" clienttypes "github.com/cosmos/ibc-go/v2/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v2/modules/core/04-channel/types" - ibctesting "github.com/cosmos/ibc-go/v2/testing" ) @@ -52,7 +51,7 @@ func (suite *KeeperTestSuite) TestTrySendTx() { }, { "channel does not exist", func() { - suite.chainA.GetSimApp().ICAKeeper.SetActiveChannel(suite.chainA.GetContext(), portID, "channel-100") + suite.chainA.GetSimApp().ICAKeeper.SetActiveChannelID(suite.chainA.GetContext(), portID, "channel-100") }, false, }, { @@ -255,11 +254,11 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacket() { 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) + activeChannelID, found := suite.chainA.GetSimApp().ICAKeeper.GetActiveChannelID(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) if tc.expPass { suite.Require().NoError(err) - suite.Require().Empty(channel) + suite.Require().Empty(activeChannelID) suite.Require().False(found) } else { suite.Require().Error(err) diff --git a/modules/apps/27-interchain-accounts/types/errors.go b/modules/apps/27-interchain-accounts/types/errors.go index 77787281d1a..7b4bfe5d8d1 100644 --- a/modules/apps/27-interchain-accounts/types/errors.go +++ b/modules/apps/27-interchain-accounts/types/errors.go @@ -5,7 +5,7 @@ import ( ) var ( - ErrUnknownPacketData = sdkerrors.Register(ModuleName, 2, "unknown packet data") + ErrUnknownDataType = sdkerrors.Register(ModuleName, 2, "unknown data type") ErrAccountAlreadyExist = sdkerrors.Register(ModuleName, 3, "account already exist") ErrPortAlreadyBound = sdkerrors.Register(ModuleName, 4, "port is already bound") ErrUnsupportedChain = sdkerrors.Register(ModuleName, 5, "unsupported chain")