e2e: v8.1 upgrade test for capital efficient escrows (cosmos#5652)

* chore: add default upgrade handler for v8.1

* chore: adding initial test structure and genesis modification

* fix: genesis modification compatibility support for channel params

* fix: use MsgSoftwareUpgrade in upgrade chain func in favour of deprecated msg

* e2e: add more test cases to e2e upgrade test

* e2e: assert escrow account balance of fee module account

* chore: remove unnecessary comments

* e2e: add workflow jobs for e2e tests v8.1

* e2e: remove EOL upgrade tests from e2e upgrades workflow

* add gov v1 messages feature releases and move fee versions constructor to testsuite

* chore: update godoc for modifyChannelGenesisAppState

---------

Co-authored-by: Carlos Rodriguez <carlos@interchain.io>

.github/workflows/e2e-upgrade.yaml

@@ -12,20 +12,6 @@ on:
 
 
 jobs:
-  upgrade-v5-hermes:
-    uses: ./.github/workflows/e2e-test-workflow-call.yml
-    with:
-      chain-image: ghcr.io/cosmos/ibc-go-simd
-      chain-binary: simd
-      chain-a-tag: v4.4.1
-      chain-b-tag: v4.4.1
-      chain-upgrade-tag: v5.1.0
-      upgrade-plan-name: "normal upgrade"
-      test-entry-point:  "TestUpgradeTestSuite"
-      test: "TestIBCChainUpgrade"
-      upload-logs: true
-      relayer-type: hermes
-
   upgrade-v7-hermes:
     uses: ./.github/workflows/e2e-test-workflow-call.yml
     with:
@@ -68,21 +54,19 @@ jobs:
       upload-logs: true
       relayer-type: hermes
 
-  upgrade-v5-rly:
+  upgrade-v8_1-hermes:
     uses: ./.github/workflows/e2e-test-workflow-call.yml
     with:
       chain-image: ghcr.io/cosmos/ibc-go-simd
       chain-binary: simd
-      chain-a-tag: v4.4.1
-      chain-b-tag: v4.4.1
-      chain-upgrade-tag: v5.1.0
-      upgrade-plan-name: "normal upgrade"
+      chain-a-tag: v8.0.0
+      chain-b-tag: v8.0.0
+      chain-upgrade-tag: pr-5652 # TODO: add git tag for v8.1.0-{prelease}
+      upgrade-plan-name: "v8.1"
       test-entry-point:  "TestUpgradeTestSuite"
-      test: "TestIBCChainUpgrade"
+      test: "TestV8ToV8_1ChainUpgrade"
       upload-logs: true
-      relayer-type: rly
-      relayer-image: ghcr.io/cosmos/relayer
-      relayer-tag: latest
+      relayer-type: hermes
 
   upgrade-v7-rly:
     uses: ./.github/workflows/e2e-test-workflow-call.yml
@@ -131,6 +115,22 @@ jobs:
       relayer-type: rly
       relayer-image: ghcr.io/cosmos/relayer
       relayer-tag: latest
+
+  upgrade-v8_1-rly:
+    uses: ./.github/workflows/e2e-test-workflow-call.yml
+    with:
+      chain-image: ghcr.io/cosmos/ibc-go-simd
+      chain-binary: simd
+      chain-a-tag: v8.0.0
+      chain-b-tag: v8.0.0
+      chain-upgrade-tag: pr-5652 # TODO: add git tag for v8.1.0-{prelease}
+      upgrade-plan-name: "v8.1"
+      test-entry-point:  "TestUpgradeTestSuite"
+      test: "TestV8ToV8_1ChainUpgrade"
+      upload-logs: true
+      relayer-type: rly
+      relayer-image: ghcr.io/cosmos/relayer
+      relayer-tag: latest    
 
   upgrade-ibcwasm-v8-hermes:
     uses: ./.github/workflows/e2e-test-workflow-call.yml

e2e/README.md

@@ -115,7 +115,7 @@ Both chains have started, but the relayer is not yet started.
 The relayer should be started as part of the test if required. See [Starting the Relayer](#starting-the-relayer)
 
 ```go
-relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
+relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.FeeMiddlewareChannelOptions())
 chainA, chainB := s.GetChains()
 ```
 
@@ -154,7 +154,7 @@ We can broadcast arbitrary messages which are signed on behalf of users created
 This example shows a multi message transaction being broadcast on chainA and signed on behalf of chainAWallet.
 
 ```go
-relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
+relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.FeeMiddlewareChannelOptions())
 chainA, chainB := s.GetChains()
 
 chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)

e2e/tests/transfer/incentivized_test.go

@@ -33,7 +33,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_AsyncSingleSender_Su
 	t := s.T()
 	ctx := context.TODO()
 
-	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
+	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.FeeMiddlewareChannelOptions())
 	chainA, chainB := s.GetChains()
 
 	var (
@@ -150,7 +150,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_InvalidReceiverAccou
 	t := s.T()
 	ctx := context.TODO()
 
-	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
+	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.FeeMiddlewareChannelOptions())
 	chainA, chainB := s.GetChains()
 
 	var (
@@ -263,7 +263,7 @@ func (s *IncentivizedTransferTestSuite) TestMultiMsg_MsgPayPacketFeeSingleSender
 	t := s.T()
 	ctx := context.TODO()
 
-	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
+	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.FeeMiddlewareChannelOptions())
 
 	chainA, chainB := s.GetChains()
 
@@ -369,7 +369,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_SingleSender_TimesOu
 	t := s.T()
 	ctx := context.TODO()
 
-	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
+	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.FeeMiddlewareChannelOptions())
 	chainA, chainB := s.GetChains()
 
 	var (
@@ -484,7 +484,7 @@ func (s *IncentivizedTransferTestSuite) TestPayPacketFeeAsync_SingleSender_NoCou
 	t := s.T()
 	ctx := context.TODO()
 
-	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
+	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.FeeMiddlewareChannelOptions())
 	chainA, _ := s.GetChains()
 
 	var (
@@ -584,7 +584,7 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_AsyncMultipleSenders
 	t := s.T()
 	ctx := context.TODO()
 
-	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, feeMiddlewareChannelOptions())
+	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.FeeMiddlewareChannelOptions())
 	chainA, chainB := s.GetChains()
 
 	var (
@@ -719,12 +719,3 @@ func (s *IncentivizedTransferTestSuite) TestMsgPayPacketFee_AsyncMultipleSenders
 		s.Require().Equal(expected2, actualBalance2)
 	})
 }
-
-// feeMiddlewareChannelOptions configures both of the chains to have fee middleware enabled.
-func feeMiddlewareChannelOptions() func(options *ibc.CreateChannelOptions) {
-	return func(opts *ibc.CreateChannelOptions) {
-		opts.Version = "{\"fee_version\":\"ics29-1\",\"app_version\":\"ics20-1\"}"
-		opts.DestPortName = "transfer"
-		opts.SourcePortName = "transfer"
-	}
-}

e2e/tests/upgrades/upgrade_test.go

@@ -19,14 +19,17 @@ import (
 	upgradetypes "cosmossdk.io/x/upgrade/types"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 
 	e2erelayer "github.com/cosmos/ibc-go/e2e/relayer"
 	"github.com/cosmos/ibc-go/e2e/testsuite"
 	"github.com/cosmos/ibc-go/e2e/testvalues"
+	feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
 	v7migrations "github.com/cosmos/ibc-go/v8/modules/core/02-client/migrations/v7"
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 	connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
+	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
 	"github.com/cosmos/ibc-go/v8/modules/core/exported"
 	solomachine "github.com/cosmos/ibc-go/v8/modules/light-clients/06-solomachine"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
@@ -59,8 +62,17 @@ func (s *UpgradeTestSuite) UpgradeChain(ctx context.Context, chain *cosmos.Cosmo
 		Info:   fmt.Sprintf("upgrade version test from %s to %s", currentVersion, upgradeVersion),
 	}
 
-	upgradeProposal := upgradetypes.NewSoftwareUpgradeProposal(fmt.Sprintf("upgrade from %s to %s", currentVersion, upgradeVersion), "upgrade chain E2E test", plan)
-	s.ExecuteAndPassGovV1Beta1Proposal(ctx, chain, wallet, upgradeProposal)
+	if testvalues.GovV1MessagesFeatureReleases.IsSupported(chain.Config().Images[0].Version) {
+		msgSoftwareUpgrade := &upgradetypes.MsgSoftwareUpgrade{
+			Plan:      plan,
+			Authority: authtypes.NewModuleAddress(govtypes.ModuleName).String(),
+		}
+
+		s.ExecuteAndPassGovV1Proposal(ctx, msgSoftwareUpgrade, chain, wallet)
+	} else {
+		upgradeProposal := upgradetypes.NewSoftwareUpgradeProposal(fmt.Sprintf("upgrade from %s to %s", currentVersion, upgradeVersion), "upgrade chain E2E test", plan)
+		s.ExecuteAndPassGovV1Beta1Proposal(ctx, chain, wallet, upgradeProposal)
+	}
 
 	height, err := chain.Height(ctx)
 	s.Require().NoError(err, "error fetching height before upgrade")
@@ -606,6 +618,132 @@ func (s *UpgradeTestSuite) TestV7ToV8ChainUpgrade() {
 	})
 }
 
+func (s *UpgradeTestSuite) TestV8ToV8_1ChainUpgrade() {
+	t := s.T()
+
+	ctx := context.Background()
+	relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, s.FeeMiddlewareChannelOptions())
+
+	chainA, chainB := s.GetChains()
+	chainADenom := chainA.Config().Denom
+
+	chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
+	chainAAddress := chainAWallet.FormattedAddress()
+
+	chainBWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount)
+	chainBAddress := chainBWallet.FormattedAddress()
+
+	s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks")
+
+	t.Run("transfer native tokens from chainA to chainB", func(t *testing.T) {
+		txResp := s.Transfer(ctx, chainA, chainAWallet, channelA.PortID, channelA.ChannelID, testvalues.DefaultTransferAmount(chainADenom), chainAAddress, chainBAddress, s.GetTimeoutHeight(ctx, chainB), 0, "")
+		s.AssertTxSuccess(txResp)
+	})
+
+	t.Run("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)
+	})
+
+	t.Run("pay packet fee", func(t *testing.T) {
+		t.Run("no packet fees in escrow", func(t *testing.T) {
+			packets, err := s.QueryIncentivizedPacketsForChannel(ctx, chainA, channelA.PortID, channelA.ChannelID)
+			s.Require().NoError(err)
+			s.Require().Empty(packets)
+		})
+
+		testFee := testvalues.DefaultFee(chainADenom)
+		packetID := channeltypes.NewPacketID(channelA.PortID, channelA.ChannelID, 1)
+		packetFee := feetypes.NewPacketFee(testFee, chainAWallet.FormattedAddress(), nil)
+
+		t.Run("pay packet fee", func(t *testing.T) {
+			txResp := s.PayPacketFeeAsync(ctx, chainA, chainAWallet, packetID, packetFee)
+			s.AssertTxSuccess(txResp)
+		})
+
+		t.Run("query incentivized packets", func(t *testing.T) {
+			packets, err := s.QueryIncentivizedPacketsForChannel(ctx, chainA, channelA.PortID, channelA.ChannelID)
+			s.Require().NoError(err)
+			s.Require().Len(packets, 1)
+			actualFee := packets[0].PacketFees[0].Fee
+
+			s.Require().True(actualFee.RecvFee.Equal(testFee.RecvFee))
+			s.Require().True(actualFee.AckFee.Equal(testFee.AckFee))
+			s.Require().True(actualFee.TimeoutFee.Equal(testFee.TimeoutFee))
+		})
+	})
+
+	t.Run("upgrade chain", func(t *testing.T) {
+		testCfg := testsuite.LoadConfig()
+		proposalWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
+		s.UpgradeChain(ctx, chainA.(*cosmos.CosmosChain), proposalWallet, testCfg.UpgradeConfig.PlanName, testCfg.ChainConfigs[0].Tag, testCfg.UpgradeConfig.Tag)
+	})
+
+	t.Run("29-fee migration partially refunds escrowed tokens", func(t *testing.T) {
+		actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet)
+		s.Require().NoError(err)
+
+		testFee := testvalues.DefaultFee(chainADenom)
+		legacyTotal := testFee.RecvFee.Add(testFee.AckFee...).Add(testFee.TimeoutFee...)
+		refundCoins := legacyTotal.Sub(testFee.Total()...) // Total() returns the denomwise max of (recvFee + ackFee, timeoutFee)
+
+		expected := testvalues.StartingTokenAmount - testvalues.IBCTransferAmount - legacyTotal.AmountOf(chainADenom).Int64() + refundCoins.AmountOf(chainADenom).Int64()
+		s.Require().Equal(expected, actualBalance)
+
+		// query incentivised packets and assert calculated values are correct
+		packets, err := s.QueryIncentivizedPacketsForChannel(ctx, chainA, channelA.PortID, channelA.ChannelID)
+		s.Require().NoError(err)
+		s.Require().Len(packets, 1)
+		actualFee := packets[0].PacketFees[0].Fee
+
+		s.Require().True(actualFee.RecvFee.Equal(testFee.RecvFee))
+		s.Require().True(actualFee.AckFee.Equal(testFee.AckFee))
+		s.Require().True(actualFee.TimeoutFee.Equal(testFee.TimeoutFee))
+
+		escrowBalance, err := s.QueryBalance(ctx, chainA, authtypes.NewModuleAddress(feetypes.ModuleName).String(), chainADenom)
+		s.Require().NoError(err)
+
+		expected = testFee.Total().AmountOf(chainADenom).Int64()
+		s.Require().Equal(expected, escrowBalance.Int64())
+	})
+
+	t.Run("start relayer", func(t *testing.T) {
+		s.StartRelayer(relayer)
+	})
+
+	chainBIBCToken := testsuite.GetIBCToken(chainADenom, channelA.Counterparty.PortID, channelA.Counterparty.ChannelID)
+
+	t.Run("packet is relayed", func(t *testing.T) {
+		s.AssertPacketRelayed(ctx, chainA, channelA.PortID, channelA.ChannelID, 1)
+
+		actualBalance, err := s.QueryBalance(ctx, chainB, chainBAddress, chainBIBCToken.IBCDenom())
+		s.Require().NoError(err)
+
+		expected := testvalues.IBCTransferAmount
+		s.Require().Equal(expected, actualBalance.Int64())
+	})
+
+	s.Require().NoError(test.WaitForBlocks(ctx, 5, chainA), "failed to wait for blocks")
+
+	t.Run("IBC token transfer from chainA to chainB, to make sure the upgrade did not break the packet flow", 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("packets are relayed", func(t *testing.T) {
+		s.AssertPacketRelayed(ctx, chainA, channelA.PortID, channelA.ChannelID, 1)
+
+		actualBalance, err := chainB.GetBalance(ctx, chainBAddress, chainBIBCToken.IBCDenom())
+		s.Require().NoError(err)
+
+		expected := testvalues.IBCTransferAmount * 2
+		s.Require().Equal(expected, actualBalance.Int64())
+	})
+}
+
 // ClientState queries the current ClientState by clientID
 func (s *UpgradeTestSuite) ClientState(ctx context.Context, chain ibc.Chain, clientID string) (*clienttypes.QueryClientStateResponse, error) {
 	queryClient := s.GetChainGRCPClients(chain).ClientQueryClient

e2e/testsuite/testconfig.go

@@ -568,6 +568,14 @@ func defaultGovv1ModifyGenesis(version string) func(ibc.ChainConfig, []byte) ([]
 			appState[ibcexported.ModuleName] = ibcGenBz
 		}
 
+		if !testvalues.ChannelParamsFeatureReleases.IsSupported(version) {
+			ibcGenBz, err := modifyChannelGenesisAppState(appState[ibcexported.ModuleName])
+			if err != nil {
+				return nil, err
+			}
+			appState[ibcexported.ModuleName] = ibcGenBz
+		}
+
 		appGenesis.AppState, err = json.Marshal(appState)
 		if err != nil {
 			return nil, err
@@ -723,3 +731,18 @@ func modifyClientGenesisAppState(ibcAppState []byte) ([]byte, error) {
 
 	return ibcGenBz, nil
 }
+
+// modifyChannelGenesisAppState takes the existing ibc app state, unmarshals it to a map and removes the `params` entry from ibc channel genesis.
+// It marshals and returns the ibc GenesisState JSON map as bytes.
+func modifyChannelGenesisAppState(ibcAppState []byte) ([]byte, error) {
+	var ibcGenesisMap map[string]interface{}
+	if err := json.Unmarshal(ibcAppState, &ibcGenesisMap); err != nil {
+		return nil, err
+	}
+
+	// be ashamed, be very ashamed
+	channelGenesis := ibcGenesisMap["channel_genesis"].(map[string]interface{})
+	delete(channelGenesis, "params")
+
+	return json.Marshal(ibcGenesisMap)
+}

e2e/testsuite/testsuite.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/cosmos/ibc-go/e2e/relayer"
 	"github.com/cosmos/ibc-go/e2e/testsuite/diagnostics"
+	feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
 	transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
 	clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
 )
@@ -399,6 +400,22 @@ func (*E2ETestSuite) TransferChannelOptions() func(options *ibc.CreateChannelOpt
 	}
 }
 
+// FeeMiddlewareChannelOptions configures both of the chains to have fee middleware enabled.
+func (s *E2ETestSuite) FeeMiddlewareChannelOptions() func(options *ibc.CreateChannelOptions) {
+	versionMetadata := feetypes.Metadata{
+		FeeVersion: feetypes.Version,
+		AppVersion: transfertypes.Version,
+	}
+	versionBytes, err := feetypes.ModuleCdc.MarshalJSON(&versionMetadata)
+	s.Require().NoError(err)
+
+	return func(opts *ibc.CreateChannelOptions) {
+		opts.Version = string(versionBytes)
+		opts.DestPortName = transfertypes.PortID
+		opts.SourcePortName = transfertypes.PortID
+	}
+}
+
 // GetTimeoutHeight returns a timeout height of 1000 blocks above the current block height.
 // This function should be used when the timeout is never expected to be reached
 func (s *E2ETestSuite) GetTimeoutHeight(ctx context.Context, chain ibc.Chain) clienttypes.Height {