test: account/keeper tests for ICA (#420)

* test: adding tests for account type

* tests: adding test for keeper & account

* fix: updating channel closing capabilities

* fix: updating to use test library instead of hardcoded values

* fix: updating error handling for account

* test: adding test for account string comparison

* fix: updating marshal yaml

Author: seantking

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

@@ -56,16 +56,3 @@ func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, accAddr sdk.AccAddres
 	k.accountKeeper.SetAccount(ctx, interchainAccount)
 	k.SetInterchainAccountAddress(ctx, portID, interchainAccount.Address)
 }
-
-func (k Keeper) GetInterchainAccount(ctx sdk.Context, addr sdk.AccAddress) (types.InterchainAccount, error) {
-	acc := k.accountKeeper.GetAccount(ctx, addr)
-	if acc == nil {
-		return types.InterchainAccount{}, sdkerrors.Wrap(types.ErrInterchainAccountNotFound, "there is no account")
-	}
-
-	interchainAccount, ok := acc.(*types.InterchainAccount)
-	if !ok {
-		return types.InterchainAccount{}, sdkerrors.Wrap(types.ErrInterchainAccountNotFound, "account is not an interchain account")
-	}
-	return *interchainAccount, nil
-}

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

@@ -17,7 +17,6 @@ func (suite *KeeperTestSuite) TestInitInterchainAccount() {
 		malleate func()
 		expPass  bool
 	}{
-
 		{
 			"success", func() {}, true,
 		},
@@ -31,6 +30,11 @@ func (suite *KeeperTestSuite) TestInitInterchainAccount() {
 		   			}, false,
 		   		},
 		*/
+		{
+			"fails to generate port-id", func() {
+				owner = ""
+			}, false,
+		},
 		{
 			"MsgChanOpenInit fails - channel is already active", func() {
 				portID, err := types.GeneratePortID(owner, path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)

modules/apps/27-interchain-accounts/keeper/handshake.go

@@ -130,23 +130,3 @@ func (k Keeper) OnChanOpenConfirm(
 ) error {
 	return nil
 }
-
-// May want to use these for re-opening a channel when it is closed
-//// OnChanCloseInit implements the IBCModule interface
-//func (am AppModule) OnChanCloseInit(
-//	ctx sdk.Context,
-//	portID,
-//	channelID string,
-//) error {
-//	// Disallow user-initiated channel closing for transfer channels
-//	return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "user cannot close channel")
-//}
-
-//// OnChanCloseConfirm implements the IBCModule interface
-//func (am AppModule) OnChanCloseConfirm(
-//	ctx sdk.Context,
-//	portID,
-//	channelID string,
-//) error {
-//	return nil
-//}

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

@@ -129,6 +129,20 @@ func (suite *KeeperTestSuite) TestGetInterchainAccountAddress() {
 	suite.Require().Empty(retrievedAddr)
 }
 
+func (suite *KeeperTestSuite) TestIsActiveChannel() {
+	suite.SetupTest() // reset
+	path := NewICAPath(suite.chainA, suite.chainB)
+	owner := TestOwnerAddress
+	suite.coordinator.SetupConnections(path)
+
+	err := suite.SetupICAPath(path, owner)
+	suite.Require().NoError(err)
+	portID := path.EndpointA.ChannelConfig.PortID
+
+	isActive := suite.chainA.GetSimApp().ICAKeeper.IsActiveChannel(suite.chainA.GetContext(), portID)
+	suite.Require().Equal(isActive, true)
+}
+
 func (suite *KeeperTestSuite) TestSetInterchainAccountAddress() {
 	expectedAddr, portID := "address", "port"
 	suite.chainA.GetSimApp().ICAKeeper.SetInterchainAccountAddress(suite.chainA.GetContext(), portID, expectedAddr)

modules/apps/27-interchain-accounts/module.go

@@ -187,15 +187,15 @@ func (am AppModule) OnChanCloseInit(
 	channelID string,
 ) error {
 	// Disallow user-initiated channel closing for interchain account channels
-	return nil
+	return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "user cannot close channel")
 }
 
 func (am AppModule) OnChanCloseConfirm(
 	ctx sdk.Context,
 	portID,
 	channelID string,
 ) error {
-	return sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "user cannot close channel")
+	return nil
 }
 
 func (am AppModule) OnRecvPacket(

modules/apps/27-interchain-accounts/types/account.go

@@ -10,7 +10,7 @@ import (
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/tendermint/tendermint/crypto/tmhash"
-	yaml "gopkg.in/yaml.v2"
+	"gopkg.in/yaml.v2"
 
 	connectiontypes "github.com/cosmos/ibc-go/v2/modules/core/03-connection/types"
 )
@@ -66,40 +66,44 @@ func NewInterchainAccount(ba *authtypes.BaseAccount, accountOwner string) *Inter
 }
 
 // SetPubKey - Implements AccountI
-func (InterchainAccount) SetPubKey(pubKey crypto.PubKey) error {
-	return fmt.Errorf("not supported for interchain accounts")
+func (ia InterchainAccount) SetPubKey(pubKey crypto.PubKey) error {
+	return sdkerrors.Wrap(ErrUnsupported, "cannot set public key for interchain account")
 }
 
 // SetSequence - Implements AccountI
-func (InterchainAccount) SetSequence(seq uint64) error {
-	return fmt.Errorf("not supported for interchain accounts")
+func (ia InterchainAccount) SetSequence(seq uint64) error {
+	return sdkerrors.Wrap(ErrUnsupported, "cannot set sequence number for interchain account")
 }
 
 func (ia InterchainAccount) Validate() error {
+	if strings.TrimSpace(ia.AccountOwner) == "" {
+		return sdkerrors.Wrap(ErrInvalidAccountAddress, "AccountOwner cannot be empty")
+	}
+
 	return ia.BaseAccount.Validate()
 }
 
-type ibcAccountPretty struct {
+type InterchainAccountPretty struct {
 	Address       sdk.AccAddress `json:"address" yaml:"address"`
 	PubKey        string         `json:"public_key" yaml:"public_key"`
 	AccountNumber uint64         `json:"account_number" yaml:"account_number"`
 	Sequence      uint64         `json:"sequence" yaml:"sequence"`
-	AccountOwner  string         `json:"address" yaml:"account_owner"`
+	AccountOwner  string         `json:"account_owner" yaml:"account_owner"`
 }
 
 func (ia InterchainAccount) String() string {
 	out, _ := ia.MarshalYAML()
-	return out.(string)
+	return string(out)
 }
 
-// MarshalYAML returns the YAML representation of a InterchainAccount.
-func (ia InterchainAccount) MarshalYAML() (interface{}, error) {
+// MarshalYAML returns the YAML representation of an InterchainAccount
+func (ia InterchainAccount) MarshalYAML() ([]byte, error) {
 	accAddr, err := sdk.AccAddressFromBech32(ia.Address)
 	if err != nil {
 		return nil, err
 	}
 
-	bs, err := yaml.Marshal(ibcAccountPretty{
+	bz, err := yaml.Marshal(InterchainAccountPretty{
 		Address:       accAddr,
 		PubKey:        "",
 		AccountNumber: ia.AccountNumber,
@@ -111,28 +115,34 @@ func (ia InterchainAccount) MarshalYAML() (interface{}, error) {
 		return nil, err
 	}
 
-	return string(bs), nil
+	return bz, nil
 }
 
-// MarshalJSON returns the JSON representation of a InterchainAccount.
+// MarshalJSON returns the JSON representation of an InterchainAccount.
 func (ia InterchainAccount) MarshalJSON() ([]byte, error) {
 	accAddr, err := sdk.AccAddressFromBech32(ia.Address)
 	if err != nil {
 		return nil, err
 	}
 
-	return json.Marshal(ibcAccountPretty{
+	bz, err := json.Marshal(InterchainAccountPretty{
 		Address:       accAddr,
 		PubKey:        "",
 		AccountNumber: ia.AccountNumber,
 		Sequence:      ia.Sequence,
 		AccountOwner:  ia.AccountOwner,
 	})
+
+	if err != nil {
+		return nil, err
+	}
+
+	return bz, nil
 }
 
 // UnmarshalJSON unmarshals raw JSON bytes into a ModuleAccount.
 func (ia *InterchainAccount) UnmarshalJSON(bz []byte) error {
-	var alias ibcAccountPretty
+	var alias InterchainAccountPretty
 	if err := json.Unmarshal(bz, &alias); err != nil {
 		return err
 	}

modules/apps/27-interchain-accounts/types/account_test.go

@@ -1,11 +1,15 @@
 package types_test
 
 import (
+	"encoding/json"
 	"fmt"
 	"testing"
 
+	"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	"github.com/stretchr/testify/suite"
+	"gopkg.in/yaml.v2"
 
 	"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types"
 	ibctesting "github.com/cosmos/ibc-go/v2/testing"
@@ -117,3 +121,87 @@ func (suite *TypesTestSuite) TestGeneratePortID() {
 		})
 	}
 }
+
+func (suite *TypesTestSuite) TestInterchainAccount() {
+	pubkey := secp256k1.GenPrivKey().PubKey()
+	addr := sdk.AccAddress(pubkey.Address())
+	baseAcc := authtypes.NewBaseAccountWithAddress(addr)
+	interchainAcc := types.NewInterchainAccount(baseAcc, TestOwnerAddress)
+
+	// should fail when trying to set the public key or sequence of an interchain account
+	err := interchainAcc.SetPubKey(pubkey)
+	suite.Require().Error(err)
+	err = interchainAcc.SetSequence(1)
+	suite.Require().Error(err)
+}
+
+func (suite *TypesTestSuite) TestGenesisAccountValidate() {
+	pubkey := secp256k1.GenPrivKey().PubKey()
+	addr := sdk.AccAddress(pubkey.Address())
+	baseAcc := authtypes.NewBaseAccountWithAddress(addr)
+	pubkey = secp256k1.GenPrivKey().PubKey()
+	ownerAddr := sdk.AccAddress(pubkey.Address())
+
+	testCases := []struct {
+		name    string
+		acc     authtypes.GenesisAccount
+		expPass bool
+	}{
+		{
+			"success",
+			types.NewInterchainAccount(baseAcc, ownerAddr.String()),
+			true,
+		},
+		{
+			"interchain account with empty AccountOwner field",
+			types.NewInterchainAccount(baseAcc, ""),
+			false,
+		},
+	}
+
+	for _, tc := range testCases {
+		err := tc.acc.Validate()
+
+		if tc.expPass {
+			suite.Require().NoError(err)
+		} else {
+			suite.Require().Error(err)
+		}
+	}
+}
+
+func (suite *TypesTestSuite) TestInterchainAccountMarshalYAML() {
+	addr := suite.chainA.SenderAccount.GetAddress()
+	ba := authtypes.NewBaseAccountWithAddress(addr)
+
+	interchainAcc := types.NewInterchainAccount(ba, suite.chainB.SenderAccount.GetAddress().String())
+	bz, err := yaml.Marshal(types.InterchainAccountPretty{
+		Address:       addr,
+		PubKey:        "",
+		AccountNumber: interchainAcc.AccountNumber,
+		Sequence:      interchainAcc.Sequence,
+		AccountOwner:  interchainAcc.AccountOwner,
+	})
+	suite.Require().NoError(err)
+
+	bz1, err := interchainAcc.MarshalYAML()
+	suite.Require().Equal(string(bz), string(bz1))
+}
+
+func (suite *TypesTestSuite) TestInterchainAccountJSON() {
+	addr := suite.chainA.SenderAccount.GetAddress()
+	ba := authtypes.NewBaseAccountWithAddress(addr)
+
+	interchainAcc := types.NewInterchainAccount(ba, suite.chainB.SenderAccount.GetAddress().String())
+
+	bz, err := json.Marshal(interchainAcc)
+	suite.Require().NoError(err)
+
+	bz1, err := interchainAcc.MarshalJSON()
+	suite.Require().NoError(err)
+	suite.Require().Equal(string(bz), string(bz1))
+
+	var a types.InterchainAccount
+	suite.Require().NoError(json.Unmarshal(bz, &a))
+	suite.Require().Equal(a.String(), interchainAcc.String())
+}

modules/apps/27-interchain-accounts/types/errors.go

@@ -16,4 +16,5 @@ var (
 	ErrActiveChannelNotFound       = sdkerrors.Register(ModuleName, 10, "no active channel for this owner")
 	ErrInvalidVersion              = sdkerrors.Register(ModuleName, 11, "invalid interchain accounts version")
 	ErrInvalidAccountAddress       = sdkerrors.Register(ModuleName, 12, "invalid account address")
+	ErrUnsupported                 = sdkerrors.Register(ModuleName, 13, "interchain account does not support this action")
 )