From b6c4ac51b4380dd5fdfb8fff9d3da33b44171f80 Mon Sep 17 00:00:00 2001 From: Charly Date: Fri, 12 Jan 2024 19:45:54 +0100 Subject: [PATCH 1/5] add test --- e2e/tests/interchain_accounts/base_test.go | 122 +++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/e2e/tests/interchain_accounts/base_test.go b/e2e/tests/interchain_accounts/base_test.go index 1425e512787..26ef1e02759 100644 --- a/e2e/tests/interchain_accounts/base_test.go +++ b/e2e/tests/interchain_accounts/base_test.go @@ -17,6 +17,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + govtypes "github.com/cosmos/cosmos-sdk/x/gov/types" "github.com/cosmos/ibc-go/e2e/testsuite" "github.com/cosmos/ibc-go/e2e/testvalues" @@ -406,3 +407,124 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop s.Require().Equal(expected, balance.Int64()) }) } + +func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterUpgradingOrdertoUnordered() { + t := s.T() + ctx := context.TODO() + + // setup relayers and connection-0 between two chains + // channel-0 is a transfer channel but it will not be used in this test case + relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil) + chainA, chainB := s.GetChains() + + // 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 ( + portID string + hostAccount string + + initialChannelID = "channel-1" + ) + + t.Run("register interchain account", func(t *testing.T) { + var err error + // explicitly set the version string because we don't want to use incentivized channels. + version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) + msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version) + s.RegisterInterchainAccount(ctx, chainA, controllerAccount, msgRegisterInterchainAccount) + portID, err = icatypes.NewControllerPortID(controllerAddress) + s.Require().NoError(err) + }) + + t.Run("start relayer", func(t *testing.T) { + s.StartRelayer(relayer) + }) + + t.Run("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)) + + _, err = s.QueryChannel(ctx, chainA, portID, initialChannelID) + s.Require().NoError(err) + }) + + t.Run("fund interchain account wallet", func(t *testing.T) { + // fund the host account 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(controllerAddress, ibctesting.FirstConnectionID, uint64(5*time.Minute), packetData) + + resp := s.BroadcastMessages( + ctx, + chainA, + controllerAccount, + msgSendTx, + ) + + s.AssertTxSuccess(resp) + + // time for the packet to be relayed + s.Require().NoError(test.WaitForBlocks(ctx, 5, chainA, chainB)) + }) + + t.Run("verify tokens transferred", func(t *testing.T) { + balance, err := s.QueryBalance(ctx, chainB, chainBAccount.FormattedAddress(), chainB.Config().Denom) + s.Require().NoError(err) + + expected := testvalues.IBCTransferAmount + testvalues.StartingTokenAmount + s.Require().Equal(expected, balance.Int64()) + }) + + channel, err := s.QueryChannel(ctx, chainA, portID, initialChannelID) + s.Require().NoError(err) + // upgrade the channel ordering to UNORDERED + upgradeFields := channeltypes.NewUpgradeFields(channeltypes.UNORDERED, channel.ConnectionHops, channel.Version) + + t.Run("execute gov proposal to initiate channel upgrade", func(t *testing.T) { + govModuleAddress, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainA) + s.Require().NoError(err) + s.Require().NotNil(govModuleAddress) + + msg := channeltypes.NewMsgChannelUpgradeInit(portID, initialChannelID, upgradeFields, govModuleAddress.String()) + s.ExecuteAndPassGovV1Proposal(ctx, msg, chainA, controllerAccount) + }) + + s.Require().NoError(test.WaitForBlocks(ctx, 10, chainA, chainB), "failed to wait for blocks") + + t.Run("verify channel A upgraded and is now unordered", func(t *testing.T) { + channel, err := s.QueryChannel(ctx, chainA, portID, initialChannelID) + s.Require().NoError(err) + s.Require().Equal(channeltypes.UNORDERED, channel.Ordering, "the channel was not in an expected state") + }) +} From 684be170d912dc18b3515ff0faf5b993140de17f Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 16 Jan 2024 12:11:46 +0100 Subject: [PATCH 2/5] passing basic logic --- e2e/tests/interchain_accounts/base_test.go | 120 +++++++++--------- .../controller/ibc_middleware_test.go | 4 +- .../host/ibc_module_test.go | 2 +- 3 files changed, 65 insertions(+), 61 deletions(-) diff --git a/e2e/tests/interchain_accounts/base_test.go b/e2e/tests/interchain_accounts/base_test.go index 26ef1e02759..66e20ceacfc 100644 --- a/e2e/tests/interchain_accounts/base_test.go +++ b/e2e/tests/interchain_accounts/base_test.go @@ -415,13 +415,13 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterUpgr // setup relayers and connection-0 between two chains // channel-0 is a transfer channel but it will not be used in this test case relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil) - chainA, chainB := s.GetChains() + chainA, _ := s.GetChains() // 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) + // chainBAccount := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) var ( portID string @@ -454,57 +454,57 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterUpgr s.Require().NoError(err) }) - t.Run("fund interchain account wallet", func(t *testing.T) { - // fund the host account 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(controllerAddress, ibctesting.FirstConnectionID, uint64(5*time.Minute), packetData) - - resp := s.BroadcastMessages( - ctx, - chainA, - controllerAccount, - msgSendTx, - ) - - s.AssertTxSuccess(resp) - - // time for the packet to be relayed - s.Require().NoError(test.WaitForBlocks(ctx, 5, chainA, chainB)) - }) - - t.Run("verify tokens transferred", func(t *testing.T) { - balance, err := s.QueryBalance(ctx, chainB, chainBAccount.FormattedAddress(), chainB.Config().Denom) - s.Require().NoError(err) - - expected := testvalues.IBCTransferAmount + testvalues.StartingTokenAmount - s.Require().Equal(expected, balance.Int64()) - }) + // t.Run("fund interchain account wallet", func(t *testing.T) { + // // fund the host account 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(controllerAddress, ibctesting.FirstConnectionID, uint64(5*time.Minute), packetData) + + // resp := s.BroadcastMessages( + // ctx, + // chainA, + // controllerAccount, + // msgSendTx, + // ) + + // s.AssertTxSuccess(resp) + + // // time for the packet to be relayed + // s.Require().NoError(test.WaitForBlocks(ctx, 5, chainA, chainB)) + // }) + + // t.Run("verify tokens transferred", func(t *testing.T) { + // balance, err := s.QueryBalance(ctx, chainB, chainBAccount.FormattedAddress(), chainB.Config().Denom) + // s.Require().NoError(err) + + // expected := testvalues.IBCTransferAmount + testvalues.StartingTokenAmount + // s.Require().Equal(expected, balance.Int64()) + // }) channel, err := s.QueryChannel(ctx, chainA, portID, initialChannelID) s.Require().NoError(err) @@ -520,11 +520,15 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterUpgr s.ExecuteAndPassGovV1Proposal(ctx, msg, chainA, controllerAccount) }) - s.Require().NoError(test.WaitForBlocks(ctx, 10, chainA, chainB), "failed to wait for blocks") - t.Run("verify channel A upgraded and is now unordered", func(t *testing.T) { - channel, err := s.QueryChannel(ctx, chainA, portID, initialChannelID) - s.Require().NoError(err) - s.Require().Equal(channeltypes.UNORDERED, channel.Ordering, "the channel was not in an expected state") + var channel channeltypes.Channel + waitErr := test.WaitForCondition(time.Minute*10, time.Second*5, func() (bool, error) { + channel, err = s.QueryChannel(ctx, chainA, portID, initialChannelID) + if err != nil { + return false, err + } + return channel.Ordering == channeltypes.UNORDERED, nil + }) + s.Require().NoErrorf(waitErr, "channel was not upgraded: expected %s got %s", channeltypes.UNORDERED, channel.Ordering) }) } diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index b55598fa44e..a13a2c722d0 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -843,7 +843,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() { suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - channeltypes.ORDERED, + channeltypes.UNORDERED, []string{path.EndpointA.ConnectionID}, version, ) @@ -1016,7 +1016,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - channeltypes.ORDERED, + channeltypes.UNORDERED, []string{path.EndpointA.ConnectionID}, counterpartyVersion, ) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 3fb78abeefb..398e02731d7 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -189,7 +189,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { counterparty := channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) channel = &channeltypes.Channel{ State: channeltypes.TRYOPEN, - Ordering: channeltypes.ORDERED, + Ordering: channeltypes.UNORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointB.ConnectionID}, Version: path.EndpointB.ChannelConfig.Version, From 1985e438f094eb36606e1e5698455f666dee789d Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 16 Jan 2024 12:28:19 +0100 Subject: [PATCH 3/5] add order to account creation --- e2e/tests/interchain_accounts/base_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/e2e/tests/interchain_accounts/base_test.go b/e2e/tests/interchain_accounts/base_test.go index 0f677747599..dd0243b1a8c 100644 --- a/e2e/tests/interchain_accounts/base_test.go +++ b/e2e/tests/interchain_accounts/base_test.go @@ -434,7 +434,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterUpgr var err error // explicitly set the version string because we don't want to use incentivized channels. version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) - msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version) + msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED) s.RegisterInterchainAccount(ctx, chainA, controllerAccount, msgRegisterInterchainAccount) portID, err = icatypes.NewControllerPortID(controllerAddress) s.Require().NoError(err) @@ -508,6 +508,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterUpgr channel, err := s.QueryChannel(ctx, chainA, portID, initialChannelID) s.Require().NoError(err) + // upgrade the channel ordering to UNORDERED upgradeFields := channeltypes.NewUpgradeFields(channeltypes.UNORDERED, channel.ConnectionHops, channel.Version) From b0b3880917f19b5da1090b11d2827f9168786f56 Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 16 Jan 2024 12:49:18 +0100 Subject: [PATCH 4/5] pr review --- e2e/tests/interchain_accounts/base_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/interchain_accounts/base_test.go b/e2e/tests/interchain_accounts/base_test.go index dd0243b1a8c..19e94f0a3ec 100644 --- a/e2e/tests/interchain_accounts/base_test.go +++ b/e2e/tests/interchain_accounts/base_test.go @@ -523,7 +523,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterUpgr t.Run("verify channel A upgraded and is now unordered", func(t *testing.T) { var channel channeltypes.Channel - waitErr := test.WaitForCondition(time.Minute*10, time.Second*5, func() (bool, error) { + waitErr := test.WaitForCondition(time.Minute*2, time.Second*5, func() (bool, error) { channel, err = s.QueryChannel(ctx, chainA, portID, initialChannelID) if err != nil { return false, err From 039c598d410ce97a1952db652869b1ad35b6bb0e Mon Sep 17 00:00:00 2001 From: Charly Date: Tue, 16 Jan 2024 16:57:42 +0100 Subject: [PATCH 5/5] split up test cases --- .../controller/ibc_middleware_test.go | 31 ++++++++++++++----- .../host/ibc_module_test.go | 9 ++++-- 2 files changed, 30 insertions(+), 10 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 7a294134410..597cc4f2921 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -762,9 +762,10 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() { func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() { var ( - path *ibctesting.Path - isNilApp bool - version string + path *ibctesting.Path + isNilApp bool + version string + channelOrder channeltypes.Order ) testCases := []struct { @@ -773,7 +774,12 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() { expError error }{ { - "success", func() {}, nil, + "success w/ ORDERED channel", func() {}, nil, + }, + { + "success w/ UNORDERED channel", func() { + channelOrder = channeltypes.UNORDERED + }, nil, }, { "success: nil underlying app", @@ -839,11 +845,13 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() { cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper) } + channelOrder = channeltypes.ORDERED + version, err = cbs.OnChanUpgradeInit( suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - channeltypes.UNORDERED, + channelOrder, []string{path.EndpointA.ConnectionID}, version, ) @@ -957,6 +965,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { path *ibctesting.Path isNilApp bool counterpartyVersion string + channelOrder channeltypes.Order ) testCases := []struct { @@ -964,8 +973,12 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { malleate func() }{ { - "success", - func() {}, + "success w/ ORDERED channel", func() {}, + }, + { + "success w/ UNORDERED channel", func() { + channelOrder = channeltypes.UNORDERED + }, }, { "success: nil app", @@ -1012,11 +1025,13 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper) } + channelOrder = channeltypes.ORDERED + cbs.OnChanUpgradeOpen( suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, - channeltypes.UNORDERED, + channelOrder, []string{path.EndpointA.ConnectionID}, counterpartyVersion, ) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 398e02731d7..574e15e2453 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -141,7 +141,12 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { expPass bool }{ { - "success", func() {}, true, + "success w/ ORDERED channel", func() {}, true, + }, + { + "success w/ UNORDERED channel", func() { + channel.Ordering = channeltypes.UNORDERED + }, true, }, { "account address generation is block dependent", func() { @@ -189,7 +194,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { counterparty := channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) channel = &channeltypes.Channel{ State: channeltypes.TRYOPEN, - Ordering: channeltypes.UNORDERED, + Ordering: channeltypes.ORDERED, Counterparty: counterparty, ConnectionHops: []string{path.EndpointB.ConnectionID}, Version: path.EndpointB.ChannelConfig.Version,