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

refactor: always set ics27 ports regardless of whether the capability exists #4640

Merged
merged 12 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
102 changes: 97 additions & 5 deletions e2e/tests/upgrades/genesis_test.go
Copy link
Contributor Author

Choose a reason for hiding this comment

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

copy/pasted from ics27 tests

Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,24 @@ import (
"testing"
"time"

"github.com/cosmos/gogoproto/proto"
"github.com/strangelove-ventures/interchaintest/v8"
cosmos "github.com/strangelove-ventures/interchaintest/v8/chain/cosmos"
"github.com/strangelove-ventures/interchaintest/v8/ibc"
test "github.com/strangelove-ventures/interchaintest/v8/testutil"
"github.com/stretchr/testify/suite"
"go.uber.org/zap"

sdkmath "cosmossdk.io/math"

sdk "github.com/cosmos/cosmos-sdk/types"
banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"

"github.com/cosmos/ibc-go/e2e/testsuite"
"github.com/cosmos/ibc-go/e2e/testvalues"
controllertypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types"
icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
ibctesting "github.com/cosmos/ibc-go/v8/testing"
)

func TestGenesisTestSuite(t *testing.T) {
Expand Down Expand Up @@ -55,24 +65,40 @@ func (s *GenesisTestSuite) TestIBCGenesis() {

s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks")

t.Run("native IBC token transfer from chainA to chainB, sender is source of tokens", func(t *testing.T) {
t.Run("ics20: native IBC token transfer from chainA to chainB, sender is source of tokens", func(t *testing.T) {
transferTxResp := s.Transfer(ctx, chainA, chainAWallet, channelA.PortID, channelA.ChannelID, testvalues.DefaultTransferAmount(chainADenom), chainAAddress, chainBAddress, s.GetTimeoutHeight(ctx, chainB), 0, "")
s.AssertTxSuccess(transferTxResp)
})

t.Run("tokens are escrowed", func(t *testing.T) {
t.Run("ics20: tokens are escrowed", func(t *testing.T) {
actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet)
s.Require().NoError(err)

expected := testvalues.StartingTokenAmount - testvalues.IBCTransferAmount
s.Require().Equal(expected, actualBalance)
})

// setup 2 accounts: controller account on chain A, a second chain B account.
// host account will be created when the ICA is registered
controllerAccount := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
controllerAddress := controllerAccount.FormattedAddress()
chainBAccount := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount)
var hostAccount string

t.Run("ics27: broadcast MsgRegisterInterchainAccount", func(t *testing.T) {
// explicitly set the version string because we don't want to use incentivized channels.
version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version)

txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
s.AssertTxSuccess(txResp)
})

t.Run("start relayer", func(t *testing.T) {
s.StartRelayer(relayer)
})

t.Run("packets are relayed", func(t *testing.T) {
t.Run("ics20: packets are relayed", func(t *testing.T) {
s.AssertPacketRelayed(ctx, chainA, channelA.PortID, channelA.ChannelID, 1)

actualBalance, err := chainB.GetBalance(ctx, chainBAddress, chainBIBCToken.IBCDenom())
Expand All @@ -82,25 +108,91 @@ func (s *GenesisTestSuite) TestIBCGenesis() {
s.Require().Equal(expected, actualBalance.Int64())
})

t.Run("ics27: verify interchain account", func(t *testing.T) {
var err error
hostAccount, err = s.QueryInterchainAccount(ctx, chainA, controllerAddress, ibctesting.FirstConnectionID)
s.Require().NoError(err)
s.Require().NotZero(len(hostAccount))

channels, err := relayer.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
s.Require().NoError(err)
s.Require().Equal(len(channels), 2)
})

s.Require().NoError(test.WaitForBlocks(ctx, 10, chainA, chainB), "failed to wait for blocks")

t.Run("Halt chain and export genesis", func(t *testing.T) {
s.HaltChainAndExportGenesis(ctx, chainA, relayer, int64(haltHeight))
})

t.Run("native IBC token transfer from chainA to chainB, sender is source of tokens", func(t *testing.T) {
t.Run("ics20: native IBC token transfer from chainA to chainB, sender is source of tokens", func(t *testing.T) {
transferTxResp := s.Transfer(ctx, chainA, chainAWallet, channelA.PortID, channelA.ChannelID, testvalues.DefaultTransferAmount(chainADenom), chainAAddress, chainBAddress, s.GetTimeoutHeight(ctx, chainB), 0, "")
s.AssertTxSuccess(transferTxResp)
})

t.Run("tokens are escrowed", func(t *testing.T) {
t.Run("ics20: tokens are escrowed", func(t *testing.T) {
actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet)
s.Require().NoError(err)

expected := testvalues.StartingTokenAmount - 2*testvalues.IBCTransferAmount
s.Require().Equal(expected, actualBalance)
})

t.Run("ics27: interchain account executes a bank transfer on behalf of the corresponding owner account", func(t *testing.T) {
t.Run("fund interchain account wallet", func(t *testing.T) {
// fund the host account so it has some $$ to send
err := chainB.SendFunds(ctx, interchaintest.FaucetAccountKeyName, ibc.WalletAmount{
Address: hostAccount,
Amount: sdkmath.NewInt(testvalues.StartingTokenAmount),
Denom: chainB.Config().Denom,
})
s.Require().NoError(err)
})

t.Run("broadcast MsgSendTx", func(t *testing.T) {
// assemble bank transfer message from host account to user account on host chain
msgSend := &banktypes.MsgSend{
FromAddress: hostAccount,
ToAddress: chainBAccount.FormattedAddress(),
Amount: sdk.NewCoins(testvalues.DefaultTransferAmount(chainB.Config().Denom)),
}

cdc := testsuite.Codec()
bz, err := icatypes.SerializeCosmosTx(cdc, []proto.Message{msgSend}, icatypes.EncodingProtobuf)
s.Require().NoError(err)

packetData := icatypes.InterchainAccountPacketData{
Type: icatypes.EXECUTE_TX,
Data: bz,
Memo: "e2e",
}

msgSendTx := controllertypes.NewMsgSendTx(controllerAccount.FormattedAddress(), ibctesting.FirstConnectionID, uint64(time.Hour.Nanoseconds()), packetData)

resp := s.BroadcastMessages(
ctx,
chainA,
controllerAccount,
msgSendTx,
)

s.AssertTxSuccess(resp)

s.Require().NoError(test.WaitForBlocks(ctx, 10, chainA, chainB))
})

t.Run("verify tokens transferred", func(t *testing.T) {
balance, err := chainB.GetBalance(ctx, chainBAccount.FormattedAddress(), chainB.Config().Denom)
s.Require().NoError(err)

_, err = chainB.GetBalance(ctx, hostAccount, chainB.Config().Denom)
s.Require().NoError(err)

expected := testvalues.IBCTransferAmount + testvalues.StartingTokenAmount
s.Require().Equal(expected, balance.Int64())
})
})

s.Require().NoError(test.WaitForBlocks(ctx, 5, chainA, chainB), "failed to wait for blocks")
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ func (k Keeper) registerInterchainAccount(ctx sdk.Context, connectionID, portID,
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)
k.setPort(ctx, portID)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ideally gets removed #4638 in the future

capability := k.portKeeper.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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,15 @@ import (
// InitGenesis initializes the interchain accounts controller application state from a provided genesis state
func InitGenesis(ctx sdk.Context, keeper Keeper, state genesistypes.ControllerGenesisState) {
for _, portID := range state.Ports {
keeper.setPort(ctx, portID)

// generate port capability if it does not already exist
if !keeper.hasCapability(ctx, portID) {
capability := keeper.BindPort(ctx, portID)
// use the port keeper to generate a new capability
capability := keeper.portKeeper.BindPort(ctx, portID)

// use the controller scoped keeper to claim the port capability

if err := keeper.ClaimCapability(ctx, capability, host.PortPath(portID)); err != nil {
panic(fmt.Sprintf("could not claim port capability: %v", err))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,10 @@ func (k Keeper) GetAllPorts(ctx sdk.Context) []string {
return ports
}

// BindPort stores the provided portID and binds to it, returning the associated capability
func (k Keeper) BindPort(ctx sdk.Context, portID string) *capabilitytypes.Capability {
// setPort sets the provided portID in state
func (k Keeper) setPort(ctx sdk.Context, portID string) {
store := ctx.KVStore(k.storeKey)
store.Set(icatypes.KeyPort(portID), []byte{0x01})

return k.portKeeper.BindPort(ctx, portID)
}

// hasCapability checks if the interchain account controller module owns the port capability for the desired port
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,14 @@ import (

// InitGenesis initializes the interchain accounts host application state from a provided genesis state
func InitGenesis(ctx sdk.Context, keeper Keeper, state genesistypes.HostGenesisState) {
keeper.setPort(ctx, state.Port)

// generate port capability if it does not already exist
if !keeper.hasCapability(ctx, state.Port) {
capability := keeper.BindPort(ctx, state.Port)
// use the port keeper to generate a new capability
capability := keeper.portKeeper.BindPort(ctx, state.Port)

// use the host scoped keeper to claim the port capability
if err := keeper.ClaimCapability(ctx, capability, host.PortPath(state.Port)); err != nil {
panic(fmt.Sprintf("could not claim port capability: %v", err))
}
Expand Down
6 changes: 2 additions & 4 deletions modules/apps/27-interchain-accounts/host/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,10 @@ func (Keeper) Logger(ctx sdk.Context) log.Logger {
return ctx.Logger().With("module", fmt.Sprintf("x/%s-%s", exported.ModuleName, icatypes.ModuleName))
}

// BindPort stores the provided portID and binds to it, returning the associated capability
func (k Keeper) BindPort(ctx sdk.Context, portID string) *capabilitytypes.Capability {
// setPort sets the provided portID in state.
func (k Keeper) setPort(ctx sdk.Context, portID string) {
store := ctx.KVStore(k.storeKey)
store.Set(icatypes.KeyPort(portID), []byte{0x01})

return k.portKeeper.BindPort(ctx, portID)
}

// hasCapability checks if the interchain account host module owns the port capability for the desired port
Expand Down
14 changes: 7 additions & 7 deletions modules/apps/27-interchain-accounts/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ import (
"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/simulation"
"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
ibchost "github.com/cosmos/ibc-go/v8/modules/core/24-host"
)

var (
Expand Down Expand Up @@ -123,19 +122,20 @@ func NewAppModule(controllerKeeper *controllerkeeper.Keeper, hostKeeper *hostkee
// called once and as an alternative to InitGenesis.
func (am AppModule) InitModule(ctx sdk.Context, controllerParams controllertypes.Params, hostParams hosttypes.Params) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

if am.controllerKeeper != nil {
am.controllerKeeper.SetParams(ctx, controllerParams)
controllerkeeper.InitGenesis(ctx, *am.controllerKeeper, genesistypes.ControllerGenesisState{
Params: controllerParams,
})
}

if am.hostKeeper != nil {
if err := hostParams.Validate(); err != nil {
panic(fmt.Sprintf("could not set ica host params at initialization: %v", err))
}
am.hostKeeper.SetParams(ctx, hostParams)

capability := am.hostKeeper.BindPort(ctx, types.HostPortID)
if err := am.hostKeeper.ClaimCapability(ctx, capability, ibchost.PortPath(types.HostPortID)); err != nil {
panic(fmt.Sprintf("could not claim port capability: %v", err))
}
hostkeeper.InitGenesis(ctx, *am.hostKeeper, genesistypes.HostGenesisState{
Params: hostParams,
Port: types.HostPortID,
})
}
}

Expand Down
Loading