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: adding test for RegisterInterchainAccount & adding check to rel… #552

Merged
merged 5 commits into from
Nov 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions modules/apps/27-interchain-accounts/host/keeper/account.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package keeper

import (
sdk "github.com/cosmos/cosmos-sdk/types"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"

"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types"
)

// RegisterInterchainAccount attempts to create a new account using the provided address and stores it in state keyed by the provided port identifier
// If an account for the provided address already exists this function returns early (no-op)
func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, accAddr sdk.AccAddress, controllerPortID string) {
if acc := k.accountKeeper.GetAccount(ctx, accAddr); acc != nil {
return
}

interchainAccount := types.NewInterchainAccount(
authtypes.NewBaseAccountWithAddress(accAddr),
controllerPortID,
)

k.accountKeeper.NewAccount(ctx, interchainAccount)
k.accountKeeper.SetAccount(ctx, interchainAccount)

k.SetInterchainAccountAddress(ctx, controllerPortID, interchainAccount.Address)
}
33 changes: 33 additions & 0 deletions modules/apps/27-interchain-accounts/host/keeper/account_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package keeper_test

import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/v2/modules/apps/27-interchain-accounts/types"
ibctesting "github.com/cosmos/ibc-go/v2/testing"
)

func (suite *KeeperTestSuite) TestRegisterInterchainAccount() {
suite.SetupTest()

path := NewICAPath(suite.chainA, suite.chainB)
suite.coordinator.SetupConnections(path)

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

portID, err := types.GeneratePortID(TestOwnerAddress, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
suite.Require().NoError(err)

// Get the address of the interchain account stored in state during handshake step
storedAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), portID)
suite.Require().True(found)

icaAddr, err := sdk.AccAddressFromBech32(storedAddr)
suite.Require().NoError(err)

// Check if account is created
interchainAccount := suite.chainB.GetSimApp().AccountKeeper.GetAccount(suite.chainB.GetContext(), icaAddr)
suite.Require().Equal(interchainAccount.GetAddress().String(), storedAddr)
}
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,6 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() {
} else {
suite.Require().Error(err)
}

})
}
}
Expand Down
18 changes: 0 additions & 18 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"
"github.com/tendermint/tendermint/libs/log"
Expand Down Expand Up @@ -59,23 +58,6 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", fmt.Sprintf("x/%s-%s", host.ModuleName, types.ModuleName))
}

// RegisterInterchainAccount attempts to create a new account using the provided address and stores it in state keyed by the provided port identifier
// If an account for the provided address already exists this function returns early (no-op)
func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, accAddr sdk.AccAddress, controllerPortID string) {
if acc := k.accountKeeper.GetAccount(ctx, accAddr); acc != nil {
return
}

interchainAccount := types.NewInterchainAccount(
authtypes.NewBaseAccountWithAddress(accAddr),
controllerPortID,
)

k.accountKeeper.NewAccount(ctx, interchainAccount)
k.accountKeeper.SetAccount(ctx, interchainAccount)
k.SetInterchainAccountAddress(ctx, controllerPortID, interchainAccount.Address)
}

// BindPort stores the provided portID and binds to it, returning the associated capability
func (k Keeper) BindPort(ctx sdk.Context, portID string) *capabilitytypes.Capability {
store := ctx.KVStore(k.storeKey)
Expand Down
14 changes: 14 additions & 0 deletions modules/apps/27-interchain-accounts/host/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,20 @@ func (suite *KeeperTestSuite) TestOnRecvPacket() {
err := SetupICAPath(path, TestOwnerAddress)
suite.Require().NoError(err)

portID, err := types.GeneratePortID(TestOwnerAddress, ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
suite.Require().NoError(err)

// Get the address of the interchain account stored in state during handshake step
storedAddr, found := suite.chainB.GetSimApp().ICAHostKeeper.GetInterchainAccountAddress(suite.chainB.GetContext(), portID)
suite.Require().True(found)

icaAddr, err := sdk.AccAddressFromBech32(storedAddr)
suite.Require().NoError(err)

// Check if account is created
interchainAccount := suite.chainB.GetSimApp().AccountKeeper.GetAccount(suite.chainB.GetContext(), icaAddr)
Copy link
Member

Choose a reason for hiding this comment

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

Is this only needed in OnRecvPacket? Should we be doing this elsewhere as well? If we already have an explicit test in account_test.go then is this neceessary to check in this test?

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 is the primary place that we're actually using the ICA and the critical flow to test IMO. It's not completely necessary as we have the explicit test in account_test.go but I think it's good to leave it in for sanity tbh.

suite.Require().Equal(interchainAccount.GetAddress().String(), storedAddr)

suite.fundICAWallet(suite.chainB.GetContext(), path.EndpointA.ChannelConfig.PortID, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10000))))

tc.malleate() // malleate mutates test data
Expand Down