chore: fix linter warnings (backport #3311)

.github/workflows/golangci.yml

@@ -22,5 +22,10 @@ jobs:
       - name: golangci-lint
         uses: golangci/golangci-lint-action@v3.2.0
         with:
+<<<<<<< HEAD
           version: latest
-          args: --timeout 5m
+          args: --timeout 5m
+=======
+          version: v1.52.0
+          args: --timeout 5m
+>>>>>>> 5a67efc4 (chore: fix linter warnings (#3311))

.golangci.yml

@@ -33,6 +33,15 @@ linters:
 
 issues:
   exclude-rules:
+<<<<<<< HEAD
+=======
+    - text: "unused-parameter"
+      linters:
+        - revive
+    - text: "SA1019:"
+      linters:
+        - staticcheck
+>>>>>>> 5a67efc4 (chore: fix linter warnings (#3311))
     - text: "Use of weak random number generator"
       linters:
         - gosec

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -105,11 +105,7 @@ func SetupICAPath(path *ibctesting.Path, owner string) error {
 		return err
 	}
 
-	if err := path.EndpointB.ChanOpenConfirm(); err != nil {
-		return err
-	}
-
-	return nil
+	return path.EndpointB.ChanOpenConfirm()
 }
 
 func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {

modules/apps/27-interchain-accounts/controller/keeper/account.go

@@ -52,3 +52,47 @@ func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner,
 
 	return nil
 }
+<<<<<<< HEAD
+=======
+
+// registerInterchainAccount registers an interchain account, returning the channel id of the MsgChannelOpenInitResponse
+// and an error if one occurred.
+func (k Keeper) registerInterchainAccount(ctx sdk.Context, connectionID, portID, version string) (string, error) {
+	// if there is an active channel for this portID / connectionID return an error
+	activeChannelID, found := k.GetOpenActiveChannel(ctx, connectionID, portID)
+	if found {
+		return "", errorsmod.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s on connection %s", activeChannelID, portID, connectionID)
+	}
+
+	switch {
+	case k.portKeeper.IsBound(ctx, portID) && !k.HasCapability(ctx, portID):
+		return "", errorsmod.Wrapf(icatypes.ErrPortAlreadyBound, "another module has claimed capability for and bound port with portID: %s", portID)
+	case !k.portKeeper.IsBound(ctx, portID):
+		capability := k.BindPort(ctx, portID)
+		if err := k.ClaimCapability(ctx, capability, host.PortPath(portID)); err != nil {
+			return "", errorsmod.Wrapf(err, "unable to bind to newly generated portID: %s", portID)
+		}
+	}
+
+	msg := channeltypes.NewMsgChannelOpenInit(portID, version, channeltypes.ORDERED, []string{connectionID}, icatypes.HostPortID, authtypes.NewModuleAddress(icatypes.ModuleName).String())
+	handler := k.msgRouter.Handler(msg)
+	res, err := handler(ctx, msg)
+	if err != nil {
+		return "", err
+	}
+
+	events := res.GetEvents()
+	k.Logger(ctx).Debug("emitting interchain account registration events", logging.SdkEventsToLogArguments(events))
+
+	// NOTE: The sdk msg handler creates a new EventManager, so events must be correctly propagated back to the current context
+	ctx.EventManager().EmitEvents(events)
+
+	firstMsgResponse := res.MsgResponses[0]
+	channelOpenInitResponse, ok := firstMsgResponse.GetCachedValue().(*channeltypes.MsgChannelOpenInitResponse)
+	if !ok {
+		return "", errorsmod.Wrapf(ibcerrors.ErrInvalidType, "failed to covert %T message response to %T", firstMsgResponse.GetCachedValue(), &channeltypes.MsgChannelOpenInitResponse{})
+	}
+
+	return channelOpenInitResponse.ChannelId, nil
+}
+>>>>>>> 5a67efc4 (chore: fix linter warnings (#3311))

modules/apps/27-interchain-accounts/controller/keeper/account_test.go

@@ -25,8 +25,8 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount() {
 		{
 			"port is already bound for owner but capability is claimed by another module",
 			func() {
-				cap := suite.chainA.GetSimApp().IBCKeeper.PortKeeper.BindPort(suite.chainA.GetContext(), TestPortID)
-				err := suite.chainA.GetSimApp().TransferKeeper.ClaimCapability(suite.chainA.GetContext(), cap, host.PortPath(TestPortID))
+				capability := suite.chainA.GetSimApp().IBCKeeper.PortKeeper.BindPort(suite.chainA.GetContext(), TestPortID)
+				err := suite.chainA.GetSimApp().TransferKeeper.ClaimCapability(suite.chainA.GetContext(), capability, host.PortPath(TestPortID))
 				suite.Require().NoError(err)
 			},
 			false,

modules/apps/27-interchain-accounts/controller/keeper/genesis.go

@@ -12,9 +12,15 @@ import (
 // InitGenesis initializes the interchain accounts controller application state from a provided genesis state
 func InitGenesis(ctx sdk.Context, keeper Keeper, state icatypes.ControllerGenesisState) {
 	for _, portID := range state.Ports {
+<<<<<<< HEAD
 		if !keeper.IsBound(ctx, portID) {
 			cap := keeper.BindPort(ctx, portID)
 			if err := keeper.ClaimCapability(ctx, cap, host.PortPath(portID)); err != nil {
+=======
+		if !keeper.HasCapability(ctx, portID) {
+			capability := keeper.BindPort(ctx, portID)
+			if err := keeper.ClaimCapability(ctx, capability, host.PortPath(portID)); err != nil {
+>>>>>>> 5a67efc4 (chore: fix linter warnings (#3311))
 				panic(fmt.Sprintf("could not claim port capability: %v", err))
 			}
 		}

modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go

@@ -71,11 +71,7 @@ func SetupICAPath(path *ibctesting.Path, owner string) error {
 		return err
 	}
 
-	if err := path.EndpointB.ChanOpenConfirm(); err != nil {
-		return err
-	}
-
-	return nil
+	return path.EndpointB.ChanOpenConfirm()
 }
 
 // RegisterInterchainAccount is a helper function for starting the channel handshake

modules/apps/27-interchain-accounts/controller/migrations/v6/migrations_test.go

@@ -0,0 +1,196 @@
+package v6_test
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
+	"github.com/stretchr/testify/suite"
+
+	v6 "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/migrations/v6"
+	"github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/controller/types"
+	icatypes "github.com/cosmos/ibc-go/v7/modules/apps/27-interchain-accounts/types"
+	channeltypes "github.com/cosmos/ibc-go/v7/modules/core/04-channel/types"
+	host "github.com/cosmos/ibc-go/v7/modules/core/24-host"
+	ibctesting "github.com/cosmos/ibc-go/v7/testing"
+	ibcmock "github.com/cosmos/ibc-go/v7/testing/mock"
+)
+
+type MigrationsTestSuite struct {
+	suite.Suite
+
+	chainA *ibctesting.TestChain
+	chainB *ibctesting.TestChain
+
+	coordinator *ibctesting.Coordinator
+	path        *ibctesting.Path
+}
+
+func (suite *MigrationsTestSuite) SetupTest() {
+	suite.coordinator = ibctesting.NewCoordinator(suite.T(), 2)
+
+	suite.chainA = suite.coordinator.GetChain(ibctesting.GetChainID(1))
+	suite.chainB = suite.coordinator.GetChain(ibctesting.GetChainID(2))
+
+	suite.path = ibctesting.NewPath(suite.chainA, suite.chainB)
+	suite.path.EndpointA.ChannelConfig.PortID = icatypes.HostPortID
+	suite.path.EndpointB.ChannelConfig.PortID = icatypes.HostPortID
+	suite.path.EndpointA.ChannelConfig.Order = channeltypes.ORDERED
+	suite.path.EndpointB.ChannelConfig.Order = channeltypes.ORDERED
+	suite.path.EndpointA.ChannelConfig.Version = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
+	suite.path.EndpointB.ChannelConfig.Version = icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
+}
+
+func (suite *MigrationsTestSuite) SetupPath() error {
+	if err := suite.RegisterInterchainAccount(suite.path.EndpointA, ibctesting.TestAccAddress); err != nil {
+		return err
+	}
+
+	if err := suite.path.EndpointB.ChanOpenTry(); err != nil {
+		return err
+	}
+
+	if err := suite.path.EndpointA.ChanOpenAck(); err != nil {
+		return err
+	}
+
+	return suite.path.EndpointB.ChanOpenConfirm()
+}
+
+func (suite *MigrationsTestSuite) RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) error {
+	portID, err := icatypes.NewControllerPortID(owner)
+	if err != nil {
+		return err
+	}
+
+	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
+
+	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil {
+		return err
+	}
+
+	// commit state changes for proof verification
+	endpoint.Chain.NextBlock()
+
+	// update port/channel ids
+	endpoint.ChannelID = channeltypes.FormatChannelIdentifier(channelSequence)
+	endpoint.ChannelConfig.PortID = portID
+
+	return nil
+}
+
+func TestKeeperTestSuite(t *testing.T) {
+	suite.Run(t, new(MigrationsTestSuite))
+}
+
+func (suite *MigrationsTestSuite) TestMigrateICS27ChannelCapability() {
+	suite.SetupTest()
+	suite.coordinator.SetupConnections(suite.path)
+
+	err := suite.SetupPath()
+	suite.Require().NoError(err)
+
+	// create additional capabilities to cover edge cases
+	suite.CreateMockCapabilities()
+
+	// create and claim a new capability with ibc/mock for "channel-1"
+	// note: suite.SetupPath() now claims the chanel capability using icacontroller for "channel-0"
+	capName := host.ChannelCapabilityPath(suite.path.EndpointA.ChannelConfig.PortID, channeltypes.FormatChannelIdentifier(1))
+
+	capability, err := suite.chainA.GetSimApp().ScopedIBCKeeper.NewCapability(suite.chainA.GetContext(), capName)
+	suite.Require().NoError(err)
+
+	err = suite.chainA.GetSimApp().ScopedICAMockKeeper.ClaimCapability(suite.chainA.GetContext(), capability, capName)
+	suite.Require().NoError(err)
+
+	// assert the capability is owned by the mock module
+	capability, found := suite.chainA.GetSimApp().ScopedICAMockKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().NotNil(capability)
+	suite.Require().True(found)
+
+	isAuthenticated := suite.chainA.GetSimApp().ScopedICAMockKeeper.AuthenticateCapability(suite.chainA.GetContext(), capability, capName)
+	suite.Require().True(isAuthenticated)
+
+	capability, found = suite.chainA.GetSimApp().ScopedICAControllerKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().Nil(capability)
+	suite.Require().False(found)
+
+	suite.ResetMemStore() // empty the x/capability in-memory store
+
+	err = v6.MigrateICS27ChannelCapability(
+		suite.chainA.GetContext(),
+		suite.chainA.Codec,
+		suite.chainA.GetSimApp().GetKey(capabilitytypes.StoreKey),
+		suite.chainA.GetSimApp().CapabilityKeeper,
+		ibcmock.ModuleName+types.SubModuleName,
+	)
+
+	suite.Require().NoError(err)
+
+	// assert the capability is now owned by the ICS27 controller submodule
+	capability, found = suite.chainA.GetSimApp().ScopedICAControllerKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().NotNil(capability)
+	suite.Require().True(found)
+
+	isAuthenticated = suite.chainA.GetSimApp().ScopedICAControllerKeeper.AuthenticateCapability(suite.chainA.GetContext(), capability, capName)
+	suite.Require().True(isAuthenticated)
+
+	capability, found = suite.chainA.GetSimApp().ScopedICAMockKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().Nil(capability)
+	suite.Require().False(found)
+
+	// ensure channel capability for "channel-0" is still owned by the controller
+	capName = host.ChannelCapabilityPath(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)
+	capability, found = suite.chainA.GetSimApp().ScopedICAControllerKeeper.GetCapability(suite.chainA.GetContext(), capName)
+	suite.Require().NotNil(capability)
+	suite.Require().True(found)
+
+	isAuthenticated = suite.chainA.GetSimApp().ScopedICAControllerKeeper.AuthenticateCapability(suite.chainA.GetContext(), capability, capName)
+	suite.Require().True(isAuthenticated)
+
+	suite.AssertMockCapabiltiesUnchanged()
+}
+
+// CreateMockCapabilities creates an additional two capabilities used for testing purposes:
+// 1. A capability with a single owner
+// 2. A capability with two owners, neither of which is "ibc"
+func (suite *MigrationsTestSuite) CreateMockCapabilities() {
+	capability, err := suite.chainA.GetSimApp().ScopedIBCMockKeeper.NewCapability(suite.chainA.GetContext(), "mock_one")
+	suite.Require().NoError(err)
+	suite.Require().NotNil(capability)
+
+	capability, err = suite.chainA.GetSimApp().ScopedICAMockKeeper.NewCapability(suite.chainA.GetContext(), "mock_two")
+	suite.Require().NoError(err)
+	suite.Require().NotNil(capability)
+
+	err = suite.chainA.GetSimApp().ScopedIBCMockKeeper.ClaimCapability(suite.chainA.GetContext(), capability, "mock_two")
+	suite.Require().NoError(err)
+}
+
+// AssertMockCapabiltiesUnchanged authenticates the mock capabilities created at the start of the test to ensure they remain unchanged
+func (suite *MigrationsTestSuite) AssertMockCapabiltiesUnchanged() {
+	capability, found := suite.chainA.GetSimApp().ScopedIBCMockKeeper.GetCapability(suite.chainA.GetContext(), "mock_one")
+	suite.Require().True(found)
+	suite.Require().NotNil(capability)
+
+	capability, found = suite.chainA.GetSimApp().ScopedIBCMockKeeper.GetCapability(suite.chainA.GetContext(), "mock_two")
+	suite.Require().True(found)
+	suite.Require().NotNil(capability)
+
+	isAuthenticated := suite.chainA.GetSimApp().ScopedICAMockKeeper.AuthenticateCapability(suite.chainA.GetContext(), capability, "mock_two")
+	suite.Require().True(isAuthenticated)
+}
+
+// ResetMemstore removes all existing fwd and rev capability kv pairs and deletes `KeyMemInitialised` from the x/capability memstore.
+// This effectively mocks a new chain binary being started. Migration code is run against persisted state only and allows the memstore to be reinitialised.
+func (suite *MigrationsTestSuite) ResetMemStore() {
+	memStore := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetMemKey(capabilitytypes.MemStoreKey))
+	memStore.Delete(capabilitytypes.KeyMemInitialized)
+
+	iterator := memStore.Iterator(nil, nil)
+	defer sdk.LogDeferred(suite.chainA.GetContext().Logger(), func() error { return iterator.Close() })
+
+	for ; iterator.Valid(); iterator.Next() {
+		memStore.Delete(iterator.Key())
+	}
+}

modules/apps/27-interchain-accounts/controller/types/params.go

@@ -33,11 +33,15 @@ func DefaultParams() Params {
 
 // Validate validates all controller submodule parameters
 func (p Params) Validate() error {
+<<<<<<< HEAD
 	if err := validateEnabled(p.ControllerEnabled); err != nil {
 		return err
 	}
 
 	return nil
+=======
+	return validateEnabledType(p.ControllerEnabled)
+>>>>>>> 5a67efc4 (chore: fix linter warnings (#3311))
 }
 
 // ParamSetPairs implements params.ParamSet

modules/apps/27-interchain-accounts/host/ibc_module_test.go

@@ -106,11 +106,7 @@ func SetupICAPath(path *ibctesting.Path, owner string) error {
 		return err
 	}
 
-	if err := path.EndpointB.ChanOpenConfirm(); err != nil {
-		return err
-	}
-
-	return nil
+	return path.EndpointB.ChanOpenConfirm()
 }
 
 // Test initiating a ChanOpenInit using the host chain instead of the controller chain

modules/apps/27-interchain-accounts/host/keeper/genesis.go

@@ -10,10 +10,17 @@ import (
 )
 
 // InitGenesis initializes the interchain accounts host application state from a provided genesis state
+<<<<<<< HEAD
 func InitGenesis(ctx sdk.Context, keeper Keeper, state icatypes.HostGenesisState) {
 	if !keeper.IsBound(ctx, state.Port) {
 		cap := keeper.BindPort(ctx, state.Port)
 		if err := keeper.ClaimCapability(ctx, cap, host.PortPath(state.Port)); err != nil {
+=======
+func InitGenesis(ctx sdk.Context, keeper Keeper, state genesistypes.HostGenesisState) {
+	if !keeper.HasCapability(ctx, state.Port) {
+		capability := keeper.BindPort(ctx, state.Port)
+		if err := keeper.ClaimCapability(ctx, capability, host.PortPath(state.Port)); err != nil {
+>>>>>>> 5a67efc4 (chore: fix linter warnings (#3311))
 			panic(fmt.Sprintf("could not claim port capability: %v", err))
 		}
 	}

modules/apps/27-interchain-accounts/host/keeper/keeper_test.go

@@ -71,11 +71,7 @@ func SetupICAPath(path *ibctesting.Path, owner string) error {
 		return err
 	}
 
-	if err := path.EndpointB.ChanOpenConfirm(); err != nil {
-		return err
-	}
-
-	return nil
+	return path.EndpointB.ChanOpenConfirm()
 }
 
 // RegisterInterchainAccount is a helper function for starting the channel handshake

modules/apps/27-interchain-accounts/host/types/params.go

@@ -43,11 +43,7 @@ func (p Params) Validate() error {
 		return err
 	}
 
-	if err := validateAllowlist(p.AllowMessages); err != nil {
-		return err
-	}
-
-	return nil
+	return validateAllowlist(p.AllowMessages)
 }
 
 // ParamSetPairs implements params.ParamSet