From c949d5b02b5576ba6d2064879ee9f02b33139776 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 29 Jan 2024 13:17:09 +0100 Subject: [PATCH] refactor: move ica unordered field conditionals to santize.Messages --- e2e/tests/interchain_accounts/base_test.go | 46 ++----------------- e2e/tests/interchain_accounts/gov_test.go | 10 +--- e2e/tests/interchain_accounts/groups_test.go | 10 +--- .../interchain_accounts/incentivized_test.go | 20 +------- e2e/testsuite/sanitize/messages.go | 14 ++++++ e2e/testvalues/values.go | 8 ---- 6 files changed, 23 insertions(+), 85 deletions(-) diff --git a/e2e/tests/interchain_accounts/base_test.go b/e2e/tests/interchain_accounts/base_test.go index f95419698bb..c3948dca81b 100644 --- a/e2e/tests/interchain_accounts/base_test.go +++ b/e2e/tests/interchain_accounts/base_test.go @@ -63,7 +63,6 @@ func (s *InterchainAccountsTestSuite) testMsgSendTxSuccessfulTransfer(order chan // 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() - chainAVersion := chainA.Config().Images[0].Version // setup 2 accounts: controller account on chain A, a second chain B account. // host account will be created when the ICA is registered @@ -73,16 +72,9 @@ func (s *InterchainAccountsTestSuite) testMsgSendTxSuccessfulTransfer(order chan var hostAccount string t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) { - // Must broadcast MsgRegisterInterchainAccount with default value for order field - // for those versions where MsgRegisterInterchainAccount does not have the order field - msgOrder := order - if !testvalues.UnorderedICAChannelFeatureReleases.IsSupported(chainAVersion) { - msgOrder = channeltypes.NONE - } - // 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, msgOrder) + msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, order) txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount) s.AssertTxSuccess(txResp) @@ -103,12 +95,7 @@ func (s *InterchainAccountsTestSuite) testMsgSendTxSuccessfulTransfer(order chan s.Require().Equal(len(channels), 2) icaChannel := channels[0] - // for the versions that don't support unordered channels, the ordering will default to ordered - orderKey := order - if !testvalues.UnorderedICAChannelFeatureReleases.IsSupported(chainAVersion) { - orderKey = channeltypes.ORDERED - } - s.Require().Contains(orderMapping[orderKey], icaChannel.Ordering) + s.Require().Contains(orderMapping[order], icaChannel.Ordering) }) t.Run("interchain account executes a bank transfer on behalf of the corresponding owner account", func(t *testing.T) { @@ -175,7 +162,6 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_FailedTransfer_InsufficientF // 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() - chainAVersion := chainA.Config().Images[0].Version // setup 2 accounts: controller account on chain A, a second chain B account. // host account will be created when the ICA is registered @@ -185,16 +171,9 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_FailedTransfer_InsufficientF var hostAccount string t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) { - // Must broadcast MsgRegisterInterchainAccount with default value for order field - // for those versions where MsgRegisterInterchainAccount does not have the order field - msgOrder := channeltypes.ORDERED - if !testvalues.UnorderedICAChannelFeatureReleases.IsSupported(chainAVersion) { - msgOrder = channeltypes.NONE - } - // 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, msgOrder) + msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED) txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount) s.AssertTxSuccess(txResp) @@ -273,7 +252,6 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop // 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() - chainAVersion := chainA.Config().Images[0].Version // setup 2 accounts: controller account on chain A, a second chain B account. // host account will be created when the ICA is registered @@ -290,17 +268,10 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop ) t.Run("register interchain account", func(t *testing.T) { - // Must broadcast MsgRegisterInterchainAccount with default value for order field - // for those versions where MsgRegisterInterchainAccount does not have the order field - msgOrder := channeltypes.ORDERED - if !testvalues.UnorderedICAChannelFeatureReleases.IsSupported(chainAVersion) { - msgOrder = channeltypes.NONE - } - 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, msgOrder) + msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED) s.RegisterInterchainAccount(ctx, chainA, controllerAccount, msgRegisterInterchainAccount) portID, err = icatypes.NewControllerPortID(controllerAddress) s.Require().NoError(err) @@ -396,16 +367,9 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop // re-register interchain account to reopen the channel now that it has been closed due to timeout // on an ordered channel t.Run("register interchain account", func(t *testing.T) { - // Must broadcast MsgRegisterInterchainAccount with default value for order field - // for those versions where MsgRegisterInterchainAccount does not have the order field - msgOrder := channeltypes.ORDERED - if !testvalues.UnorderedICAChannelFeatureReleases.IsSupported(chainAVersion) { - msgOrder = channeltypes.NONE - } - // 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, msgOrder) + msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED) s.RegisterInterchainAccount(ctx, chainA, controllerAccount, msgRegisterInterchainAccount) s.Require().NoError(test.WaitForBlocks(ctx, 10, chainA, chainB)) diff --git a/e2e/tests/interchain_accounts/gov_test.go b/e2e/tests/interchain_accounts/gov_test.go index f41460b6cb8..212b5d173f4 100644 --- a/e2e/tests/interchain_accounts/gov_test.go +++ b/e2e/tests/interchain_accounts/gov_test.go @@ -43,7 +43,6 @@ func (s *InterchainAccountsGovTestSuite) TestInterchainAccountsGovIntegration() // 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() - chainAVersion := chainA.Config().Images[0].Version controllerAccount := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount) chainBAccount := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) @@ -54,15 +53,8 @@ func (s *InterchainAccountsGovTestSuite) TestInterchainAccountsGovIntegration() s.Require().NotNil(govModuleAddress) t.Run("execute proposal for MsgRegisterInterchainAccount", func(t *testing.T) { - // Must broadcast MsgRegisterInterchainAccount with default value for order field - // for those versions where MsgRegisterInterchainAccount does not have the order field - msgOrder := channeltypes.ORDERED - if !testvalues.UnorderedICAChannelFeatureReleases.IsSupported(chainAVersion) { - msgOrder = channeltypes.NONE - } - version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID) - msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, govModuleAddress.String(), version, msgOrder) + msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, govModuleAddress.String(), version, channeltypes.ORDERED) s.ExecuteAndPassGovV1Proposal(ctx, msgRegisterAccount, chainA, controllerAccount) }) diff --git a/e2e/tests/interchain_accounts/groups_test.go b/e2e/tests/interchain_accounts/groups_test.go index 561cbe64578..43faca17a3f 100644 --- a/e2e/tests/interchain_accounts/groups_test.go +++ b/e2e/tests/interchain_accounts/groups_test.go @@ -90,7 +90,6 @@ func (s *InterchainAccountsGroupsTestSuite) TestInterchainAccountsGroupsIntegrat // 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() - chainAVersion := chainA.Config().Images[0].Version chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount) chainAAddress := chainAWallet.FormattedAddress() @@ -115,15 +114,8 @@ func (s *InterchainAccountsGroupsTestSuite) TestInterchainAccountsGroupsIntegrat }) t.Run("submit proposal for MsgRegisterInterchainAccount", func(t *testing.T) { - // Must broadcast MsgRegisterInterchainAccount with default value for order field - // for those versions where MsgRegisterInterchainAccount does not have the order field - msgOrder := channeltypes.ORDERED - if !testvalues.UnorderedICAChannelFeatureReleases.IsSupported(chainAVersion) { - msgOrder = channeltypes.NONE - } - groupPolicyAddr = s.QueryGroupPolicyAddress(ctx, chainA) - msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, groupPolicyAddr, icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID), msgOrder) + msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, groupPolicyAddr, icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID), channeltypes.ORDERED) msgSubmitProposal, err := grouptypes.NewMsgSubmitProposal(groupPolicyAddr, []string{chainAAddress}, []sdk.Msg{msgRegisterAccount}, DefaultMetadata, grouptypes.Exec_EXEC_UNSPECIFIED, "e2e groups proposal: for MsgRegisterInterchainAccount", "e2e groups proposal: for MsgRegisterInterchainAccount") s.Require().NoError(err) diff --git a/e2e/tests/interchain_accounts/incentivized_test.go b/e2e/tests/interchain_accounts/incentivized_test.go index cb12d5e5f82..309a8e92b8f 100644 --- a/e2e/tests/interchain_accounts/incentivized_test.go +++ b/e2e/tests/interchain_accounts/incentivized_test.go @@ -43,7 +43,6 @@ func (s *IncentivizedInterchainAccountsTestSuite) TestMsgSendTx_SuccessfulBankSe // 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() - chainAVersion := chainA.Config().Images[0].Version var ( chainADenom = chainA.Config().Denom @@ -72,15 +71,8 @@ func (s *IncentivizedInterchainAccountsTestSuite) TestMsgSendTx_SuccessfulBankSe chainBAccount := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) { - // Must broadcast MsgRegisterInterchainAccount with default value for order field - // for those versions where MsgRegisterInterchainAccount does not have the order field - msgOrder := channeltypes.ORDERED - if !testvalues.UnorderedICAChannelFeatureReleases.IsSupported(chainAVersion) { - msgOrder = channeltypes.NONE - } - version := "" // allow version to be specified by the controller chain since both chains should support incentivized channels - msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAccount.FormattedAddress(), version, msgOrder) + msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAccount.FormattedAddress(), version, channeltypes.ORDERED) txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount) s.AssertTxSuccess(txResp) @@ -229,7 +221,6 @@ func (s *IncentivizedInterchainAccountsTestSuite) TestMsgSendTx_FailedBankSend_I // 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() - chainAVersion := chainA.Config().Images[0].Version var ( chainADenom = chainA.Config().Denom @@ -258,15 +249,8 @@ func (s *IncentivizedInterchainAccountsTestSuite) TestMsgSendTx_FailedBankSend_I chainBAccount := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount) t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) { - // Must broadcast MsgRegisterInterchainAccount with default value for order field - // for those versions where MsgRegisterInterchainAccount does not have the order field - msgOrder := channeltypes.ORDERED - if !testvalues.UnorderedICAChannelFeatureReleases.IsSupported(chainAVersion) { - msgOrder = channeltypes.NONE - } - version := "" // allow version to be specified by the controller chain since both chains should support incentivized channels - msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAccount.FormattedAddress(), version, msgOrder) + msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAccount.FormattedAddress(), version, channeltypes.ORDERED) txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount) s.AssertTxSuccess(txResp) diff --git a/e2e/testsuite/sanitize/messages.go b/e2e/testsuite/sanitize/messages.go index ba1a00b47cd..481f9218ec1 100644 --- a/e2e/testsuite/sanitize/messages.go +++ b/e2e/testsuite/sanitize/messages.go @@ -5,6 +5,9 @@ import ( govtypesv1 "github.com/cosmos/cosmos-sdk/x/gov/types/v1" grouptypes "github.com/cosmos/cosmos-sdk/x/group" + icacontrollertypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" + "github.com/cosmos/ibc-go/e2e/semverutil" ) @@ -17,6 +20,13 @@ var ( govv1ProposalTitleAndSummary = semverutil.FeatureReleases{ MajorVersion: "v7", } + // icaUnorderedChannelFeatureReleases represents the releasees that support the new ordering field. + icaUnorderedChannelFeatureReleases = semverutil.FeatureReleases{ + MajorVersion: "v9", + MinorVersions: []string{ + "v8.1", + }, + } ) // Messages removes any fields that are not supported by the chain version. @@ -45,6 +55,10 @@ func removeUnknownFields(tag string, msg sdk.Msg) sdk.Msg { msg.Summary = "" } return msg + case *icacontrollertypes.MsgRegisterInterchainAccount: + if !icaUnorderedChannelFeatureReleases.IsSupported(tag) { + msg.Ordering = channeltypes.NONE + } } return msg } diff --git a/e2e/testvalues/values.go b/e2e/testvalues/values.go index 27edd2d470c..b6f750ef7f7 100644 --- a/e2e/testvalues/values.go +++ b/e2e/testvalues/values.go @@ -113,14 +113,6 @@ var GovV1MessagesFeatureReleases = semverutil.FeatureReleases{ MajorVersion: "v8", } -// UnorderedICAChannelFeatureReleases represents the releases the support for unordered ICA channels was released in. -var UnorderedICAChannelFeatureReleases = semverutil.FeatureReleases{ - MajorVersion: "v9", - MinorVersions: []string{ - "v8.1", - }, -} - // CapitalEfficientFeeEscrowFeatureReleases represents the releases the support for capital efficient fee escrow was released in. var CapitalEfficientFeeEscrowFeatureReleases = semverutil.FeatureReleases{ MajorVersion: "v9",