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

test: account/keeper tests for ICA #420

Merged
merged 9 commits into from
Sep 24, 2021
13 changes: 0 additions & 13 deletions modules/apps/27-interchain-accounts/keeper/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't used anywhere. Some leftover code from the previous implementation I think.

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
}
6 changes: 5 additions & 1 deletion modules/apps/27-interchain-accounts/keeper/account_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ func (suite *KeeperTestSuite) TestInitInterchainAccount() {
malleate func()
expPass bool
}{

{
"success", func() {}, true,
},
Expand All @@ -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)
Expand Down
20 changes: 0 additions & 20 deletions modules/apps/27-interchain-accounts/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
//}
14 changes: 14 additions & 0 deletions modules/apps/27-interchain-accounts/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions modules/apps/27-interchain-accounts/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@colin-axner updating chan closing capabilities

}

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(
Expand Down
40 changes: 25 additions & 15 deletions modules/apps/27-interchain-accounts/types/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

Tiny nit: can we make this unexposed/private?

Address sdk.AccAddress `json:"address" yaml:"address"`
Copy link
Member

Choose a reason for hiding this comment

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

This field Address has the same json tag as AccountOwner below, maybe we should update the json tag for AccountOwner as they're both using json:"address"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Updated.

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,
Expand All @@ -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
}
Expand Down
88 changes: 88 additions & 0 deletions modules/apps/27-interchain-accounts/types/account_test.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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",
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
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))
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().Equal(a.String(), interchainAcc.String())
}
1 change: 1 addition & 0 deletions modules/apps/27-interchain-accounts/types/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)