diff --git a/.github/workflows/packet-forward-middleware.yml b/.github/workflows/packet-forward-middleware.yml index 91f0f726..c57150f6 100644 --- a/.github/workflows/packet-forward-middleware.yml +++ b/.github/workflows/packet-forward-middleware.yml @@ -22,7 +22,7 @@ jobs: - uses: actions/checkout@v3 - name: golangci-lint - uses: golangci/golangci-lint-action@v2 + uses: golangci/golangci-lint-action@v6 with: version: ${{ env.LINT_VERSION }} working-directory: ${{ env.WORKING_DIRECTORY }} @@ -64,7 +64,7 @@ jobs: outputs: type=docker,dest=${{ env.TAR_PATH }} - name: Upload artifact - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: ${{ env.IMAGE_NAME }} path: ${{ env.TAR_PATH }} @@ -94,7 +94,7 @@ jobs: - uses: actions/checkout@v4 - name: Download Tarball Artifact - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: ${{ env.IMAGE_NAME }} path: /tmp @@ -102,7 +102,7 @@ jobs: - name: Load Docker Image run: | docker image load -i ${{ env.TAR_PATH }} - docker image load -i testing/previous_images/pfm_7_0_1.tar + docker image load -i testing/previous_images/pfm_8_1_0.tar docker image ls -a - name: Run Test diff --git a/middleware/packet-forward-middleware/Dockerfile b/middleware/packet-forward-middleware/Dockerfile index 93ef54ef..679c2cd8 100644 --- a/middleware/packet-forward-middleware/Dockerfile +++ b/middleware/packet-forward-middleware/Dockerfile @@ -1,7 +1,11 @@ # docker build . -t pfm:local # docker run --rm -it pfm:local q ibc-router +<<<<<<< HEAD FROM golang:1.22-alpine3.20 as builder +======= +FROM golang:1.21-alpine3.18 AS builder +>>>>>>> 515bdca (fix: don't unescrow tokens that are moved between escrow accounts (#230)) RUN set -eux; apk add --no-cache git libusb-dev linux-headers gcc musl-dev make; diff --git a/middleware/packet-forward-middleware/e2e/packet_forward_test.go b/middleware/packet-forward-middleware/e2e/packet_forward_test.go index 5522a708..2b99aab4 100644 --- a/middleware/packet-forward-middleware/e2e/packet_forward_test.go +++ b/middleware/packet-forward-middleware/e2e/packet_forward_test.go @@ -3,6 +3,7 @@ package e2e import ( "context" "encoding/json" + "fmt" "testing" "time" @@ -17,6 +18,8 @@ import ( "github.com/strangelove-ventures/interchaintest/v7/testutil" "github.com/stretchr/testify/require" "go.uber.org/zap/zaptest" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" ) type PacketMetadata struct { @@ -633,6 +636,47 @@ func TestPacketForwardMiddleware(t *testing.T) { require.True(t, baEscrowBalance.Equal(transferAmount)) require.True(t, bcEscrowBalance.Equal(zeroBal)) require.True(t, cdEscrowBalance.Equal(zeroBal)) + + conn, err := grpc.Dial(chainB.GetHostGRPCAddress(), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + defer conn.Close() + + queryClient := transfertypes.NewQueryClient(conn) + + req := &transfertypes.QueryTotalEscrowForDenomRequest{Denom: chainB.Config().Denom} + res, err := queryClient.TotalEscrowForDenom(ctx, req) + require.NoError(t, err) + + // assert reported total escrow balance does not change + require.True(t, baEscrowBalance.Equal(res.Amount.Amount), fmt.Sprintf("expected B->A escrow amount %s to equal reported B total escrow %s", baEscrowBalance.String(), res.Amount.Amount.String())) + + // verify that failing another A->B->C->D works + chainAHeight, err = chainA.Height(ctx) + require.NoError(t, err) + + transferTx, err = chainA.SendIBCTransfer(ctx, abChan.ChannelID, userA.KeyName(), transfer, ibc.TransferOptions{Memo: string(memo)}) + require.NoError(t, err) + _, err = testutil.PollForAck(ctx, chainA, chainAHeight, chainAHeight+30, transferTx.Packet) + require.NoError(t, err) + err = testutil.WaitForBlocks(ctx, waitBlocks, chainA) + require.NoError(t, err) + + // verify that unwinding from A to B works + aToBTransfer := ibc.WalletAmount{ + Address: userB.FormattedAddress(), + Denom: baIBCDenom, + Amount: transferAmount, + } + + chainAHeight, err = chainA.Height(ctx) + require.NoError(t, err) + + transferTx, err = chainA.SendIBCTransfer(ctx, abChan.ChannelID, userA.KeyName(), aToBTransfer, ibc.TransferOptions{}) + require.NoError(t, err) + _, err = testutil.PollForAck(ctx, chainA, chainAHeight, chainAHeight+10, transferTx.Packet) + require.NoError(t, err) + err = testutil.WaitForBlocks(ctx, waitBlocks, chainA) + require.NoError(t, err) }) t.Run("forward a->b->a", func(t *testing.T) { diff --git a/middleware/packet-forward-middleware/e2e/upgrade_test.go b/middleware/packet-forward-middleware/e2e/upgrade_test.go index 46e4fb90..112ff521 100644 --- a/middleware/packet-forward-middleware/e2e/upgrade_test.go +++ b/middleware/packet-forward-middleware/e2e/upgrade_test.go @@ -2,25 +2,44 @@ package e2e import ( "context" + "encoding/json" "fmt" "testing" "time" +<<<<<<< HEAD upgradetypes "github.com/cosmos/cosmos-sdk/x/upgrade/types" +======= + "cosmossdk.io/math" + upgradetypes "cosmossdk.io/x/upgrade/types" +>>>>>>> 515bdca (fix: don't unescrow tokens that are moved between escrow accounts (#230)) cosmosproto "github.com/cosmos/gogoproto/proto" + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" "github.com/docker/docker/client" +<<<<<<< HEAD "github.com/strangelove-ventures/interchaintest/v7" "github.com/strangelove-ventures/interchaintest/v7/chain/cosmos" "github.com/strangelove-ventures/interchaintest/v7/ibc" "github.com/strangelove-ventures/interchaintest/v7/testutil" +======= + "github.com/strangelove-ventures/interchaintest/v8" + "github.com/strangelove-ventures/interchaintest/v8/chain/cosmos" + "github.com/strangelove-ventures/interchaintest/v8/ibc" + "github.com/strangelove-ventures/interchaintest/v8/relayer" + "github.com/strangelove-ventures/interchaintest/v8/testreporter" + "github.com/strangelove-ventures/interchaintest/v8/testutil" +>>>>>>> 515bdca (fix: don't unescrow tokens that are moved between escrow accounts (#230)) "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" ) const ( chainName = "simapp" - upgradeName = "v2" // x/params migration + upgradeName = "v3" // escrow state and balance re-sync upgrade - haltHeightDelta = uint64(9) // will propose upgrade this many blocks in the future + haltHeightDelta = uint64(20) // will propose upgrade this many blocks in the future blocksAfterUpgrade = uint64(7) VotingPeriod = "15s" @@ -29,10 +48,10 @@ const ( var ( // baseChain is the current version of the chain that will be upgraded from - // docker image load -i ../prev_builds/pfm_7_0_1.tar + // docker image load -i ../prev_builds/pfm_8_1_0.tar baseChain = ibc.DockerImage{ Repository: "pfm", - Version: "v7.0.1", + Version: "v8.1.0", UidGid: "1025:1025", } @@ -52,8 +71,6 @@ func CosmosChainUpgradeTest(t *testing.T, chainName, upgradeRepo, upgradeDockerT t.Skip("skipping in short mode") } - t.Parallel() - previousVersionGenesis := []cosmos.GenesisKV{ { Key: "app_state.gov.params.voting_period", @@ -69,33 +86,330 @@ func CosmosChainUpgradeTest(t *testing.T, chainName, upgradeRepo, upgradeDockerT }, } - // Upgrade default to use the base chain image - cfg := DefaultConfig - cfg.ModifyGenesis = cosmos.ModifyGenesis(previousVersionGenesis) - cfg.Images = []ibc.DockerImage{baseChain} - - numVals, numNodes := 2, 0 - chains := interchaintest.CreateChainWithConfig(t, numVals, numNodes, chainName, upgradeDockerTag, cfg) - chain := chains[0].(*cosmos.CosmosChain) - - ctx, ic, client, _ := interchaintest.BuildInitialChain(t, chains, false) - t.Cleanup(func() { - ic.Close() + var ( + ctx = context.Background() + client, network = interchaintest.DockerSetup(t) + rep = testreporter.NewNopReporter() + eRep = rep.RelayerExecReporter(t) + chainIdA, chainIdB, chainIdC, chainIdD = "chain-1", "chain-2", "chain-3", "chain-4" + waitBlocks = 3 + ) + + cfgA := DefaultConfig + cfgA.ChainID = chainIdA + + // ChainB is the chain that will be upgraded + cfgB := DefaultConfig + cfgB.ModifyGenesis = cosmos.ModifyGenesis(previousVersionGenesis) + cfgB.Images = []ibc.DockerImage{baseChain} + cfgB.ChainID = chainIdB + + cfgC := DefaultConfig + cfgC.ChainID = chainIdC + + cfgD := DefaultConfig + cfgD.ChainID = chainIdD + + numValsPrimary, numNodes := 2, 0 + numVals := 1 + + cf := interchaintest.NewBuiltinChainFactory(zaptest.NewLogger(t), []*interchaintest.ChainSpec{ + {Name: "pfm", ChainConfig: cfgA, NumFullNodes: &numNodes, NumValidators: &numVals}, + {Name: "pfm", ChainConfig: cfgB, NumFullNodes: &numNodes, NumValidators: &numValsPrimary}, + {Name: "pfm", ChainConfig: cfgC, NumFullNodes: &numNodes, NumValidators: &numVals}, + {Name: "pfm", ChainConfig: cfgD, NumFullNodes: &numNodes, NumValidators: &numVals}, }) +<<<<<<< HEAD const userFunds = int64(10_000_000_000) users := interchaintest.GetAndFundTestUsers(t, ctx, t.Name(), userFunds, chain) chainUser := users[0] +======= + chains, err := cf.Chains(t.Name()) + require.NoError(t, err) +>>>>>>> 515bdca (fix: don't unescrow tokens that are moved between escrow accounts (#230)) + + chainA, chainB, chainC, chainD := chains[0].(*cosmos.CosmosChain), chains[1].(*cosmos.CosmosChain), chains[2].(*cosmos.CosmosChain), chains[3].(*cosmos.CosmosChain) + + r := interchaintest.NewBuiltinRelayerFactory( + ibc.CosmosRly, + zaptest.NewLogger(t), + relayer.DockerImage(&DefaultRelayer), + relayer.StartupFlags("--processor", "events", "--block-history", "100"), + ).Build(t, client, network) + + const pathAB = "ab" + const pathBC = "bc" + const pathCD = "cd" + + ic := interchaintest.NewInterchain(). + AddChain(chainA). + AddChain(chainB). + AddChain(chainC). + AddChain(chainD). + AddRelayer(r, "relayer"). + AddLink(interchaintest.InterchainLink{ + Chain1: chainA, + Chain2: chainB, + Relayer: r, + Path: pathAB, + }). + AddLink(interchaintest.InterchainLink{ + Chain1: chainB, + Chain2: chainC, + Relayer: r, + Path: pathBC, + }). + AddLink(interchaintest.InterchainLink{ + Chain1: chainC, + Chain2: chainD, + Relayer: r, + Path: pathCD, + }) + + require.NoError(t, ic.Build(ctx, eRep, interchaintest.InterchainBuildOptions{ + TestName: t.Name(), + Client: client, + NetworkID: network, + BlockDatabaseFile: interchaintest.DefaultBlockDatabaseFilepath(), + + SkipPathCreation: false, + })) + t.Cleanup(func() { + _ = ic.Close() + }) - // upgrade - height, err := chain.Height(ctx) + initBal := math.NewInt(10_000_000_000) + users := interchaintest.GetAndFundTestUsers(t, ctx, t.Name(), initBal, chainA, chainB, chainC, chainD) + + // ------------------------------------------------------------------------- + // IBC setup + + abChan, err := ibc.GetTransferChannel(ctx, r, eRep, chainIdA, chainIdB) + require.NoError(t, err) + + baChan := abChan.Counterparty + + cbChan, err := ibc.GetTransferChannel(ctx, r, eRep, chainIdC, chainIdB) + require.NoError(t, err) + + bcChan := cbChan.Counterparty + + dcChan, err := ibc.GetTransferChannel(ctx, r, eRep, chainIdD, chainIdC) + require.NoError(t, err) + + cdChan := dcChan.Counterparty + + // Start the relayer on both paths + err = r.StartRelayer(ctx, eRep, pathAB, pathBC, pathCD) + require.NoError(t, err) + + t.Cleanup( + func() { + err := r.StopRelayer(ctx, eRep) + if err != nil { + t.Logf("an error occured while stopping the relayer: %s", err) + } + }, + ) + + // Get original account balances + userA, userB, userC, userD := users[0], users[1], users[2], users[3] + + // Compose the prefixed denoms and ibc denom for asserting balances + // firstHopDenom := transfertypes.GetPrefixedDenom(baChan.PortID, baChan.ChannelID, chainA.Config().Denom) + // secondHopDenom := transfertypes.GetPrefixedDenom(cbChan.PortID, cbChan.ChannelID, firstHopDenom) + // thirdHopDenom := transfertypes.GetPrefixedDenom(dcChan.PortID, dcChan.ChannelID, secondHopDenom) + + // firstHopDenomTrace := transfertypes.ParseDenomTrace(firstHopDenom) + // secondHopDenomTrace := transfertypes.ParseDenomTrace(secondHopDenom) + // thirdHopDenomTrace := transfertypes.ParseDenomTrace(thirdHopDenom) + + // firstHopIBCDenom := firstHopDenomTrace.IBCDenom() + // secondHopIBCDenom := secondHopDenomTrace.IBCDenom() + // thirdHopIBCDenom := thirdHopDenomTrace.IBCDenom() + + // firstHopEscrowAccount := sdk.MustBech32ifyAddressBytes(chainA.Config().Bech32Prefix, transfertypes.GetEscrowAddress(abChan.PortID, abChan.ChannelID)) + // secondHopEscrowAccount := sdk.MustBech32ifyAddressBytes(chainB.Config().Bech32Prefix, transfertypes.GetEscrowAddress(bcChan.PortID, bcChan.ChannelID)) + // thirdHopEscrowAccount := sdk.MustBech32ifyAddressBytes(chainC.Config().Bech32Prefix, transfertypes.GetEscrowAddress(cdChan.PortID, abChan.ChannelID)) + + zeroBal := math.ZeroInt() + transferAmount := math.NewInt(100_000) + + // ------------------------------------------------------------------------- + // Same as ./packet_forward_test.go "multi-hop through native chain ack error refund" + // to reproduce the invalid state + + // send normal IBC transfer from B->A to get funds in IBC denom, then do multihop A->B(native)->C->D + // this lets us test the burn from escrow account on chain C and the escrow to escrow transfer on chain B. + + // Compose the prefixed denoms and ibc denom for asserting balances + baDenom := transfertypes.GetPrefixedDenom(abChan.PortID, abChan.ChannelID, chainB.Config().Denom) + bcDenom := transfertypes.GetPrefixedDenom(cbChan.PortID, cbChan.ChannelID, chainB.Config().Denom) + cdDenom := transfertypes.GetPrefixedDenom(dcChan.PortID, dcChan.ChannelID, bcDenom) + + baDenomTrace := transfertypes.ParseDenomTrace(baDenom) + bcDenomTrace := transfertypes.ParseDenomTrace(bcDenom) + cdDenomTrace := transfertypes.ParseDenomTrace(cdDenom) + + baIBCDenom := baDenomTrace.IBCDenom() + bcIBCDenom := bcDenomTrace.IBCDenom() + cdIBCDenom := cdDenomTrace.IBCDenom() + + transfer := ibc.WalletAmount{ + Address: userA.FormattedAddress(), + Denom: chainB.Config().Denom, + Amount: transferAmount, + } + + chainBHeight, err := chainB.Height(ctx) + require.NoError(t, err) + + transferTx, err := chainB.SendIBCTransfer(ctx, baChan.ChannelID, userB.KeyName(), transfer, ibc.TransferOptions{}) + require.NoError(t, err) + _, err = testutil.PollForAck(ctx, chainB, chainBHeight, chainBHeight+10, transferTx.Packet) + require.NoError(t, err) + err = testutil.WaitForBlocks(ctx, waitBlocks, chainB) + require.NoError(t, err) + + // assert balance for user controlled wallet + chainABalance, err := chainA.GetBalance(ctx, userA.FormattedAddress(), baIBCDenom) + require.NoError(t, err) + + baEscrowBalance, err := chainB.GetBalance(ctx, transfertypes.GetEscrowAddress(baChan.PortID, baChan.ChannelID).String(), chainB.Config().Denom) + require.NoError(t, err) + + require.True(t, chainABalance.Equal(transferAmount)) + require.True(t, baEscrowBalance.Equal(transferAmount)) + + // Send a malformed packet with invalid receiver address from Chain A->Chain B->Chain C->Chain D + // This should succeed in the first hop and second hop, then fail to make the third hop. + // Funds should be refunded to Chain B and then to Chain A via acknowledgements with errors. + transfer = ibc.WalletAmount{ + Address: userB.FormattedAddress(), + Denom: baIBCDenom, + Amount: transferAmount, + } + + secondHopMetadata := &PacketMetadata{ + Forward: &ForwardMetadata{ + Receiver: "xyz1t8eh66t2w5k67kwurmn5gqhtq6d2ja0vp7jmmq", // malformed receiver address on chain D + Channel: cdChan.ChannelID, + Port: cdChan.PortID, + }, + } + + nextBz, err := json.Marshal(secondHopMetadata) + require.NoError(t, err) + + next := string(nextBz) + + firstHopMetadata := &PacketMetadata{ + Forward: &ForwardMetadata{ + Receiver: userC.FormattedAddress(), + Channel: bcChan.ChannelID, + Port: bcChan.PortID, + Next: &next, + }, + } + + memo, err := json.Marshal(firstHopMetadata) + require.NoError(t, err) + + chainAHeight, err := chainA.Height(ctx) + require.NoError(t, err) + + transferTx, err = chainA.SendIBCTransfer(ctx, abChan.ChannelID, userA.KeyName(), transfer, ibc.TransferOptions{Memo: string(memo)}) + require.NoError(t, err) + _, err = testutil.PollForAck(ctx, chainA, chainAHeight, chainAHeight+30, transferTx.Packet) + require.NoError(t, err) + err = testutil.WaitForBlocks(ctx, waitBlocks, chainA) + require.NoError(t, err) + + // assert balances for user controlled wallets + chainDBalance, err := chainD.GetBalance(ctx, userD.FormattedAddress(), cdIBCDenom) + require.NoError(t, err) + + chainCBalance, err := chainC.GetBalance(ctx, userC.FormattedAddress(), bcIBCDenom) + require.NoError(t, err) + + chainBBalance, err := chainB.GetBalance(ctx, userB.FormattedAddress(), chainB.Config().Denom) + require.NoError(t, err) + + chainABalance, err = chainA.GetBalance(ctx, userA.FormattedAddress(), baIBCDenom) + require.NoError(t, err) + + require.True(t, chainABalance.Equal(transferAmount)) + require.True(t, chainBBalance.Equal(initBal.Sub(transferAmount))) + require.True(t, chainCBalance.Equal(zeroBal)) + require.True(t, chainDBalance.Equal(zeroBal)) + + // assert balances for IBC escrow accounts + bcEscrowBalance, err := chainB.GetBalance(ctx, transfertypes.GetEscrowAddress(bcChan.PortID, bcChan.ChannelID).String(), chainB.Config().Denom) + require.NoError(t, err) + + baEscrowBalance, err = chainB.GetBalance(ctx, transfertypes.GetEscrowAddress(baChan.PortID, baChan.ChannelID).String(), chainB.Config().Denom) + require.NoError(t, err) + + require.True(t, baEscrowBalance.Equal(transferAmount)) + require.True(t, bcEscrowBalance.Equal(zeroBal)) + + conn, err := grpc.Dial(chainB.GetHostGRPCAddress(), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + defer conn.Close() + + queryClient := transfertypes.NewQueryClient(conn) + + req := &transfertypes.QueryTotalEscrowForDenomRequest{Denom: chainB.Config().Denom} + res, err := queryClient.TotalEscrowForDenom(ctx, req) + require.NoError(t, err) + + // assert the WRONG escrow state before the upgrade + require.Falsef(t, + baEscrowBalance.Equal(res.Amount.Amount), + "before upgrade: expected B->A escrow amount %s to NOT equal reported B total escrow %s", + baEscrowBalance.String(), res.Amount.Amount.String(), + ) + + // Assert baEscrowBalance was still decreased by the transfer + require.True(t, baEscrowBalance.Equal(transferAmount)) + + t.Logf("before: baEscrowBalance: %s", baEscrowBalance.String()) + t.Logf("before: total escrow for denom: %s", res.Amount.Amount.String()) + + // ------------------------------------------------------------------------- + // ChainB upgrade, as it is the chain with the escrow state that will be corrected + height, err := chainB.Height(ctx) require.NoError(t, err, "error fetching height before submit upgrade proposal") haltHeight := height + haltHeightDelta - proposalID := SubmitUpgradeProposal(t, ctx, chain, chainUser, upgradeName, haltHeight) + proposalID := SubmitUpgradeProposal(t, ctx, chainB, userB, upgradeName, haltHeight) + + ValidatorVoting(t, ctx, chainB, proposalID, height, haltHeight) + UpgradeNodes(t, ctx, chainB, client, haltHeight, upgradeRepo, upgradeDockerTag) + + // Re-create client + conn, err = grpc.Dial(chainB.GetHostGRPCAddress(), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + defer conn.Close() - ValidatorVoting(t, ctx, chain, proposalID, height, haltHeight) - UpgradeNodes(t, ctx, chain, client, haltHeight, upgradeRepo, upgradeDockerTag) + queryClient = transfertypes.NewQueryClient(conn) + + // Validate escrow state after migration matches escrow account balances + reqAfter := &transfertypes.QueryTotalEscrowForDenomRequest{Denom: chainB.Config().Denom} + totalEscrowAfter, err := queryClient.TotalEscrowForDenom(ctx, reqAfter) + require.NoError(t, err) + + // assert the CORRECT escrow state after the upgrade + require.Truef( + t, + baEscrowBalance.Equal(totalEscrowAfter.Amount.Amount), + "after upgrade: expected B->A escrow amount %s to equal reported B total escrow %s in bank", + baEscrowBalance.String(), totalEscrowAfter.Amount.Amount.String(), + ) + + t.Logf("after: baEscrowBalance: %s", baEscrowBalance.String()) + t.Logf("after: total escrow for denom: %s", totalEscrowAfter.Amount.Amount.String()) } func SubmitUpgradeProposal(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain, user ibc.Wallet, upgradeName string, haltHeight uint64) string { @@ -165,9 +479,12 @@ func ValidatorVoting(t *testing.T, ctx context.Context, chain *cosmos.CosmosChai // this should timeout due to chain halt at upgrade height. _ = testutil.WaitForBlocks(timeoutCtx, int(haltHeight-height), chain) + // Wait another 10 seconds after chain should have halted to ensure it is halted + time.Sleep(time.Second * 10) + height, err = chain.Height(ctx) require.NoError(t, err, "error fetching height after chain should have halted") // make sure that chain is halted - require.Equal(t, haltHeight, height, "height is not equal to halt height") + require.Equal(t, haltHeight, height, "height is not equal to halt height, chain did not halt") } diff --git a/middleware/packet-forward-middleware/packetforward/keeper/keeper.go b/middleware/packet-forward-middleware/packetforward/keeper/keeper.go index 072e3d15..7faf23c4 100644 --- a/middleware/packet-forward-middleware/packetforward/keeper/keeper.go +++ b/middleware/packet-forward-middleware/packetforward/keeper/keeper.go @@ -261,11 +261,9 @@ func (k *Keeper) WriteAcknowledgementForForwardedPacket( // to burn. panic(fmt.Sprintf("cannot burn coins after a successful send from escrow account to module account: %v", err)) } - } - // We move funds from the escrowAddress in both cases, - // update the total escrow amount for the denom. - k.unescrowToken(ctx, token) + k.unescrowToken(ctx, token) + } } else { // Funds in the escrow account were burned, // so on a timeout or acknowledgement error we need to mint the funds back to the escrow account. diff --git a/middleware/packet-forward-middleware/packetforward/keeper/migrator.go b/middleware/packet-forward-middleware/packetforward/keeper/migrator.go new file mode 100644 index 00000000..bfd6e54f --- /dev/null +++ b/middleware/packet-forward-middleware/packetforward/keeper/migrator.go @@ -0,0 +1,28 @@ +package keeper + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + v3 "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v8/packetforward/migrations/v3" +) + +// Migrator is a struct for handling in-place state migrations. +type Migrator struct { + keeper *Keeper +} + +func NewMigrator(k *Keeper) Migrator { + return Migrator{ + keeper: k, + } +} + +// Migrate2to3 migrates the module state from the consensus version 2 to +// version 3 +func (m Migrator) Migrate2to3(ctx sdk.Context) error { + return v3.Migrate( + ctx, + m.keeper.bankKeeper, + m.keeper.channelKeeper, + m.keeper.transferKeeper, + ) +} diff --git a/middleware/packet-forward-middleware/packetforward/migrations/v3/migrate.go b/middleware/packet-forward-middleware/packetforward/migrations/v3/migrate.go new file mode 100644 index 00000000..498ee3e2 --- /dev/null +++ b/middleware/packet-forward-middleware/packetforward/migrations/v3/migrate.go @@ -0,0 +1,55 @@ +package v3 + +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v8/packetforward/types" + + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" +) + +// Migrate migrates the x/packetforward module state from the consensus version +// 2 to version 3 +func Migrate( + ctx sdk.Context, + bankKeeper types.BankKeeper, + channelKeeper types.ChannelKeeper, + transferKeeper types.TransferKeeper, +) error { + logger := ctx.Logger() + + expectedTotalEscrowed := sdk.Coins{} + + // 1. Iterate over all IBC transfer channels + portID := transferKeeper.GetPort(ctx) + transferChannels := channelKeeper.GetAllChannelsWithPortPrefix(ctx, portID) + for _, channel := range transferChannels { + // 2. For each channel, get the escrow address and corresponding bank balance + escrowAddress := transfertypes.GetEscrowAddress(portID, channel.ChannelId) + bankBalances := bankKeeper.GetAllBalances(ctx, escrowAddress) + + // 3. Aggregate the bank balances to calculate the expected total escrowed + expectedTotalEscrowed = expectedTotalEscrowed.Add(bankBalances...) + } + + logger.Info( + "Calculated expected total escrowed from escrow account bank balances", + "num channels", len(transferChannels), + "bank total escrowed", expectedTotalEscrowed, + ) + + // 4. Set the total escrowed for each denom + for _, totalEscrowCoin := range expectedTotalEscrowed { + prevDenomEscrow := transferKeeper.GetTotalEscrowForDenom(ctx, totalEscrowCoin.Denom) + + transferKeeper.SetTotalEscrowForDenom(ctx, totalEscrowCoin) + + logger.Info( + "Corrected total escrow for denom to match escrow account bank balances", + "denom", totalEscrowCoin.Denom, + "previous escrow", prevDenomEscrow, + "new escrow", totalEscrowCoin, + ) + } + + return nil +} diff --git a/middleware/packet-forward-middleware/packetforward/migrations/v3/migrate_test.go b/middleware/packet-forward-middleware/packetforward/migrations/v3/migrate_test.go new file mode 100644 index 00000000..0952b495 --- /dev/null +++ b/middleware/packet-forward-middleware/packetforward/migrations/v3/migrate_test.go @@ -0,0 +1,176 @@ +package v3_test + +import ( + "testing" + + "cosmossdk.io/log" + "cosmossdk.io/store" + "cosmossdk.io/store/metrics" + v3 "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v8/packetforward/migrations/v3" + "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v8/test/mock" + transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + tmproto "github.com/cometbft/cometbft/proto/tendermint/types" + dbm "github.com/cosmos/cosmos-db" + sdk "github.com/cosmos/cosmos-sdk/types" +) + +func TestMigrate(t *testing.T) { + ctrl := gomock.NewController(t) + + channelKeeper := mock.NewMockChannelKeeper(ctrl) + bankKeeper := mock.NewMockBankKeeper(ctrl) + transferKeeper := mock.NewMockTransferKeeper(ctrl) + + logger := log.NewTestLogger(t) + logger.Debug("initializing test setup") + + db := dbm.NewMemDB() + stateStore := store.NewCommitMultiStore(db, log.NewNopLogger(), metrics.NewNoOpMetrics()) + + ctx := sdk.NewContext(stateStore, tmproto.Header{}, false, logger) + + // Test addresses + escrowChannels := []string{ + "channel-0", + "channel-1", + "channel-2", + } + + // Test various scenarios of ibc transfer module escrow state that match + // or do not match the bank balances. The provided bank balances are both + // the values are used to correct the escrow state in the migration and also + // to verify the correctness of the new escrow state afterwards. + tests := []struct { + name string + giveTransferEscrowState map[string]sdk.Coin // denom -> escrow amount + bankBalances map[string]sdk.Coins // escrow address -> bank balance + }{ + { + name: "empty channels", + giveTransferEscrowState: map[string]sdk.Coin{}, + bankBalances: map[string]sdk.Coins{}, + }, + { + name: "balanced - 1 channel", + giveTransferEscrowState: map[string]sdk.Coin{ + "meow": sdk.NewInt64Coin("meow", 100), + }, + bankBalances: map[string]sdk.Coins{ + escrowChannels[0]: sdk.NewCoins(sdk.NewInt64Coin("meow", 100)), + }, + }, + { + name: "balanced - multi channel", + giveTransferEscrowState: map[string]sdk.Coin{ + "meow": sdk.NewInt64Coin("meow", 100), + "woof": sdk.NewInt64Coin("woof", 200), + }, + bankBalances: map[string]sdk.Coins{ + escrowChannels[0]: sdk.NewCoins(sdk.NewInt64Coin("meow", 100)), + escrowChannels[1]: sdk.NewCoins(sdk.NewInt64Coin("woof", 200)), + }, + }, + { + name: "underbalanced escrow state - single channel", + giveTransferEscrowState: map[string]sdk.Coin{ + "meow": sdk.NewInt64Coin("meow", 80), + }, + bankBalances: map[string]sdk.Coins{ + escrowChannels[0]: sdk.NewCoins(sdk.NewInt64Coin("meow", 100)), + }, + }, + { + name: "underbalanced escrow state - multi channel", + giveTransferEscrowState: map[string]sdk.Coin{ + "meow": sdk.NewInt64Coin("meow", 80), + "woof": sdk.NewInt64Coin("woof", 180), + }, + bankBalances: map[string]sdk.Coins{ + escrowChannels[0]: sdk.NewCoins(sdk.NewInt64Coin("meow", 100)), + escrowChannels[1]: sdk.NewCoins(sdk.NewInt64Coin("woof", 200)), + }, + }, + // Escrow module state shouldn't be overbalanced, but we test it here + // to ensure the migration correctly updates the escrow state even in + // this case. + { + name: "overbalanced escrow state - single channel", + giveTransferEscrowState: map[string]sdk.Coin{ + "meow": sdk.NewInt64Coin("meow", 120), + }, + bankBalances: map[string]sdk.Coins{ + escrowChannels[0]: sdk.NewCoins(sdk.NewInt64Coin("meow", 100)), + }, + }, + { + name: "overbalanced escrow state - multi channel", + giveTransferEscrowState: map[string]sdk.Coin{ + "meow": sdk.NewInt64Coin("meow", 120), + "woof": sdk.NewInt64Coin("woof", 230), + }, + bankBalances: map[string]sdk.Coins{ + escrowChannels[0]: sdk.NewCoins(sdk.NewInt64Coin("meow", 100)), + escrowChannels[1]: sdk.NewCoins(sdk.NewInt64Coin("woof", 200)), + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + expectedTotalEscrowed := sdk.NewCoins() + + // Initialize mock calls + // 1. Fetch each escrow bank balances for the EXPECTED state values. + + portID := "transfer" + channels := []channeltypes.IdentifiedChannel{} + for _, channel := range escrowChannels { + channels = append(channels, channeltypes.NewIdentifiedChannel(portID, channel, channeltypes.Channel{})) + } + + transferKeeper.EXPECT().GetPort(ctx).Return(portID).Times(1) + channelKeeper.EXPECT(). + GetAllChannelsWithPortPrefix(ctx, portID). + Return(channels). + Times(1) + + // 2. Aggregate the bank balances to calculate the expected total escrowed + for _, escrowChannel := range escrowChannels { + expectedBalance := tt.bankBalances[escrowChannel] + + escrowAddress := transfertypes.GetEscrowAddress(portID, escrowChannel) + + bankKeeper.EXPECT(). + GetAllBalances(ctx, escrowAddress). + Return(expectedBalance). + Times(1) + + // Aggregate escrow balances + expectedTotalEscrowed = expectedTotalEscrowed.Add(expectedBalance...) + } + + // 3. Update the escrow state in the transfer keeper, for each denom + // from the aggregated escrow balances. + for _, escrowCoin := range expectedTotalEscrowed { + transferKeeper.EXPECT(). + GetTotalEscrowForDenom(ctx, escrowCoin.Denom). + Return(sdk.Coin{}). + Times(1) + + transferKeeper.EXPECT(). + SetTotalEscrowForDenom( + ctx, + escrowCoin, + ). + Times(1) + } + + err := v3.Migrate(ctx, bankKeeper, channelKeeper, transferKeeper) + require.NoError(t, err) + }) + } +} diff --git a/middleware/packet-forward-middleware/packetforward/module.go b/middleware/packet-forward-middleware/packetforward/module.go index 76bb2b7c..13e07bc1 100644 --- a/middleware/packet-forward-middleware/packetforward/module.go +++ b/middleware/packet-forward-middleware/packetforward/module.go @@ -102,6 +102,10 @@ func (AppModule) QuerierRoute() string { // RegisterServices registers module services. func (am AppModule) RegisterServices(cfg module.Configurator) { + m := keeper.NewMigrator(am.keeper) + if err := cfg.RegisterMigration(types.ModuleName, 2, m.Migrate2to3); err != nil { + panic(fmt.Sprintf("failed to migrate x/%s from version 2 to 3: %v", types.ModuleName, err)) + } } // InitGenesis performs genesis initialization for the packetforward module. It returns @@ -127,7 +131,7 @@ func (am AppModule) ExportGenesis(ctx sdk.Context, cdc codec.JSONCodec) json.Raw } // ConsensusVersion implements AppModule/ConsensusVersion. -func (AppModule) ConsensusVersion() uint64 { return 2 } +func (AppModule) ConsensusVersion() uint64 { return 3 } // BeginBlock implements the AppModule interface func (am AppModule) BeginBlock(_ sdk.Context, _ abci.RequestBeginBlock) {} diff --git a/middleware/packet-forward-middleware/packetforward/types/expected_keepers.go b/middleware/packet-forward-middleware/packetforward/types/expected_keepers.go index 4e98bb5c..a237b858 100644 --- a/middleware/packet-forward-middleware/packetforward/types/expected_keepers.go +++ b/middleware/packet-forward-middleware/packetforward/types/expected_keepers.go @@ -16,6 +16,9 @@ type TransferKeeper interface { DenomPathFromHash(ctx sdk.Context, denom string) (string, error) GetTotalEscrowForDenom(ctx sdk.Context, denom string) sdk.Coin SetTotalEscrowForDenom(ctx sdk.Context, coin sdk.Coin) + + // Only used for v3 migration + GetPort(ctx sdk.Context) string } // ChannelKeeper defines the expected IBC channel keeper @@ -24,13 +27,27 @@ type ChannelKeeper interface { GetPacketCommitment(ctx sdk.Context, portID, channelID string, sequence uint64) []byte GetNextSequenceSend(ctx sdk.Context, portID, channelID string) (uint64, bool) LookupModuleByChannel(ctx sdk.Context, portID, channelID string) (string, *capabilitytypes.Capability, error) + + // Only used for v3 migration + GetAllChannelsWithPortPrefix(ctx sdk.Context, portPrefix string) []channeltypes.IdentifiedChannel } // BankKeeper defines the expected bank keeper type BankKeeper interface { +<<<<<<< HEAD SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error SendCoinsFromModuleToAccount(ctx sdk.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error SendCoinsFromAccountToModule(ctx sdk.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error MintCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error BurnCoins(ctx sdk.Context, moduleName string, amt sdk.Coins) error +======= + SendCoins(ctx context.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) error + SendCoinsFromModuleToAccount(ctx context.Context, senderModule string, recipientAddr sdk.AccAddress, amt sdk.Coins) error + SendCoinsFromAccountToModule(ctx context.Context, senderAddr sdk.AccAddress, recipientModule string, amt sdk.Coins) error + MintCoins(ctx context.Context, moduleName string, amt sdk.Coins) error + BurnCoins(ctx context.Context, moduleName string, amt sdk.Coins) error + + // Only used for v3 migration + GetAllBalances(ctx context.Context, addr sdk.AccAddress) sdk.Coins +>>>>>>> 515bdca (fix: don't unescrow tokens that are moved between escrow accounts (#230)) } diff --git a/middleware/packet-forward-middleware/test/mock/bank_keeper.go b/middleware/packet-forward-middleware/test/mock/bank_keeper.go index 048981f2..15186d63 100644 --- a/middleware/packet-forward-middleware/test/mock/bank_keeper.go +++ b/middleware/packet-forward-middleware/test/mock/bank_keeper.go @@ -48,6 +48,20 @@ func (mr *MockBankKeeperMockRecorder) BurnCoins(arg0, arg1, arg2 any) *gomock.Ca return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BurnCoins", reflect.TypeOf((*MockBankKeeper)(nil).BurnCoins), arg0, arg1, arg2) } +// GetAllBalances mocks base method. +func (m *MockBankKeeper) GetAllBalances(arg0 context.Context, arg1 types.AccAddress) types.Coins { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAllBalances", arg0, arg1) + ret0, _ := ret[0].(types.Coins) + return ret0 +} + +// GetAllBalances indicates an expected call of GetAllBalances. +func (mr *MockBankKeeperMockRecorder) GetAllBalances(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllBalances", reflect.TypeOf((*MockBankKeeper)(nil).GetAllBalances), arg0, arg1) +} + // MintCoins mocks base method. func (m *MockBankKeeper) MintCoins(arg0 types.Context, arg1 string, arg2 types.Coins) error { m.ctrl.T.Helper() diff --git a/middleware/packet-forward-middleware/test/mock/channel_keeper.go b/middleware/packet-forward-middleware/test/mock/channel_keeper.go index 49cb9483..b02a4394 100644 --- a/middleware/packet-forward-middleware/test/mock/channel_keeper.go +++ b/middleware/packet-forward-middleware/test/mock/channel_keeper.go @@ -41,6 +41,20 @@ func (m *MockChannelKeeper) EXPECT() *MockChannelKeeperMockRecorder { return m.recorder } +// GetAllChannelsWithPortPrefix mocks base method. +func (m *MockChannelKeeper) GetAllChannelsWithPortPrefix(arg0 types.Context, arg1 string) []types1.IdentifiedChannel { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAllChannelsWithPortPrefix", arg0, arg1) + ret0, _ := ret[0].([]types1.IdentifiedChannel) + return ret0 +} + +// GetAllChannelsWithPortPrefix indicates an expected call of GetAllChannelsWithPortPrefix. +func (mr *MockChannelKeeperMockRecorder) GetAllChannelsWithPortPrefix(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAllChannelsWithPortPrefix", reflect.TypeOf((*MockChannelKeeper)(nil).GetAllChannelsWithPortPrefix), arg0, arg1) +} + // GetChannel mocks base method. func (m *MockChannelKeeper) GetChannel(arg0 types.Context, arg1, arg2 string) (types1.Channel, bool) { m.ctrl.T.Helper() diff --git a/middleware/packet-forward-middleware/test/mock/transfer_keeper.go b/middleware/packet-forward-middleware/test/mock/transfer_keeper.go index d4b93881..a1ad8762 100644 --- a/middleware/packet-forward-middleware/test/mock/transfer_keeper.go +++ b/middleware/packet-forward-middleware/test/mock/transfer_keeper.go @@ -56,6 +56,20 @@ func (mr *MockTransferKeeperMockRecorder) DenomPathFromHash(arg0, arg1 any) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DenomPathFromHash", reflect.TypeOf((*MockTransferKeeper)(nil).DenomPathFromHash), arg0, arg1) } +// GetPort mocks base method. +func (m *MockTransferKeeper) GetPort(arg0 types.Context) string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetPort", arg0) + ret0, _ := ret[0].(string) + return ret0 +} + +// GetPort indicates an expected call of GetPort. +func (mr *MockTransferKeeperMockRecorder) GetPort(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetPort", reflect.TypeOf((*MockTransferKeeper)(nil).GetPort), arg0) +} + // GetTotalEscrowForDenom mocks base method. func (m *MockTransferKeeper) GetTotalEscrowForDenom(arg0 types.Context, arg1 string) types.Coin { m.ctrl.T.Helper() diff --git a/middleware/packet-forward-middleware/testing/previous_images/README.md b/middleware/packet-forward-middleware/testing/previous_images/README.md index 7b52cc2c..97e55ad9 100644 --- a/middleware/packet-forward-middleware/testing/previous_images/README.md +++ b/middleware/packet-forward-middleware/testing/previous_images/README.md @@ -2,8 +2,8 @@ This section is used for upgrades e2e test. -v7.0.1 +v8.1.0 - -- docker build . -t pfm:v7.0.1 -- docker save pfm:v7.0.1 > pfm_7_0_1.tar +- docker build . -t pfm:v8.1.0 +- docker save pfm:v8.1.0 > pfm_8_1_0.tar diff --git a/middleware/packet-forward-middleware/testing/previous_images/pfm_8_1_0.tar b/middleware/packet-forward-middleware/testing/previous_images/pfm_8_1_0.tar new file mode 100644 index 00000000..c5905aba Binary files /dev/null and b/middleware/packet-forward-middleware/testing/previous_images/pfm_8_1_0.tar differ diff --git a/middleware/packet-forward-middleware/testing/simapp/app.go b/middleware/packet-forward-middleware/testing/simapp/app.go index 4520f3c5..27b006a6 100644 --- a/middleware/packet-forward-middleware/testing/simapp/app.go +++ b/middleware/packet-forward-middleware/testing/simapp/app.go @@ -2,6 +2,7 @@ package simapp import ( "encoding/json" + "fmt" "io" "net/http" "os" @@ -529,16 +530,65 @@ func NewSimApp( transfer.NewAppModule(app.TransferKeeper), ) +<<<<<<< HEAD +======= + // BasicModuleManager defines the module BasicManager is in charge of setting up basic, + // non-dependant module elements, such as codec registration and genesis verification. + // By default it is composed of all the module from the module manager. + // Additionally, app module basics can be overwritten by passing them as argument. + app.BasicModuleManager = module.NewBasicManagerFromManager( + app.mm, + map[string]module.AppModuleBasic{ + genutiltypes.ModuleName: genutil.NewAppModuleBasic(genutiltypes.DefaultMessageValidator), + govtypes.ModuleName: gov.NewAppModuleBasic( + []govclient.ProposalHandler{ + paramsclient.ProposalHandler, + }, + ), + }) + app.BasicModuleManager.RegisterLegacyAminoCodec(legacyAmino) + app.BasicModuleManager.RegisterInterfaces(interfaceRegistry) + + // NOTE: upgrade module is required to be prioritized + app.mm.SetOrderPreBlockers( + upgradetypes.ModuleName, + ) + +>>>>>>> 515bdca (fix: don't unescrow tokens that are moved between escrow accounts (#230)) // During begin block slashing happens after distr.BeginBlocker so that // there is nothing left over in the validator fee pool, so as to keep the // CanWithdrawInvariant invariant. // NOTE: staking module is required if HistoricalEntries param > 0 // NOTE: capability module's beginblocker must come before any modules using capabilities (e.g. IBC) app.mm.SetOrderBeginBlockers( +<<<<<<< HEAD upgradetypes.ModuleName, capabilitytypes.ModuleName, minttypes.ModuleName, distrtypes.ModuleName, slashingtypes.ModuleName, evidencetypes.ModuleName, stakingtypes.ModuleName, ibcexported.ModuleName, packetforwardtypes.ModuleName, ibctransfertypes.StoreKey, authtypes.ModuleName, banktypes.ModuleName, govtypes.ModuleName, crisistypes.ModuleName, genutiltypes.ModuleName, authz.ModuleName, feegrant.ModuleName, paramstypes.ModuleName, vestingtypes.ModuleName, group.ModuleName, consensusparamtypes.ModuleName, +======= + capabilitytypes.ModuleName, + minttypes.ModuleName, + distrtypes.ModuleName, + slashingtypes.ModuleName, + evidencetypes.ModuleName, + stakingtypes.ModuleName, + ibcexported.ModuleName, + packetforwardtypes.ModuleName, + ibctransfertypes.StoreKey, + authtypes.ModuleName, + banktypes.ModuleName, + govtypes.ModuleName, + crisistypes.ModuleName, + genutiltypes.ModuleName, + authz.ModuleName, + feegrant.ModuleName, + paramstypes.ModuleName, + vestingtypes.ModuleName, + ibcfeetypes.ModuleName, + group.ModuleName, + consensusparamtypes.ModuleName, +>>>>>>> 515bdca (fix: don't unescrow tokens that are moved between escrow accounts (#230)) ) app.mm.SetOrderEndBlockers( crisistypes.ModuleName, govtypes.ModuleName, stakingtypes.ModuleName, ibcexported.ModuleName, packetforwardtypes.ModuleName, ibctransfertypes.StoreKey, @@ -598,6 +648,7 @@ func NewSimApp( app.setupUpgradeStoreLoaders() // initialize BaseApp + app.SetPreBlocker(app.PreBlocker) app.SetInitChainer(app.InitChainer) app.SetBeginBlocker(app.BeginBlocker) anteHandler, err := NewAnteHandler( @@ -635,6 +686,11 @@ func NewSimApp( // Name returns the name of the App func (app *SimApp) Name() string { return app.BaseApp.Name() } +// PreBlocker application updates every pre block +func (app *SimApp) PreBlocker(ctx sdk.Context, _ *abci.RequestFinalizeBlock) (*sdk.ResponsePreBlock, error) { + return app.mm.PreBlock(ctx) +} + // BeginBlocker application updates every begin block func (app *SimApp) BeginBlocker(ctx sdk.Context, req abci.RequestBeginBlock) abci.ResponseBeginBlock { return app.mm.BeginBlock(ctx, req) @@ -758,32 +814,27 @@ func (app *SimApp) SimulationManager() *module.SimulationManager { return app.sm } +// setupUpgradeHandlers sets up all the upgrade handlers for the application. func (app *SimApp) setupUpgradeHandlers() { app.UpgradeKeeper.SetUpgradeHandler( - upgrades.V2, - upgrades.CreateV2UpgradeHandler(app.mm, app.configurator, app.ParamsKeeper, app.ConsensusParamsKeeper, app.PacketForwardKeeper), + upgrades.V3, + upgrades.CreateV3UpgradeHandler(app.mm, app.configurator), ) } // setupUpgradeStoreLoaders sets all necessary store loaders required by upgrades. func (app *SimApp) setupUpgradeStoreLoaders() { - // upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk() - // if err != nil { - // tmos.Exit(fmt.Sprintf("failed to read upgrade info from disk %s", err)) - // } + upgradeInfo, err := app.UpgradeKeeper.ReadUpgradeInfoFromDisk() + if err != nil { + tmos.Exit(fmt.Sprintf("failed to read upgrade info from disk %s", err)) + } - // // Future: if we want to fix the module name, we can do it here. - // if upgradeInfo.Name == upgrades.V2 && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) { - // storeUpgrades := storetypes.StoreUpgrades{ - // Renamed: []storetypes.StoreRename{{ - // OldKey: "packetfoward", // previous misspelling - // NewKey: packetforwardtypes.ModuleName, - // }}, - // } - - // // configure store loader that checks if version == upgradeHeight and applies store upgrades - // app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades)) - // } + if upgradeInfo.Name == upgrades.V3 && !app.UpgradeKeeper.IsSkipHeight(upgradeInfo.Height) { + storeUpgrades := storetypes.StoreUpgrades{} + + // configure store loader that checks if version == upgradeHeight and applies store upgrades + app.SetStoreLoader(upgradetypes.UpgradeStoreLoader(upgradeInfo.Height, &storeUpgrades)) + } } // RegisterAPIRoutes registers all application module routes with the provided diff --git a/middleware/packet-forward-middleware/testing/simapp/upgrades/upgrades.go b/middleware/packet-forward-middleware/testing/simapp/upgrades/upgrades.go index d73fd08d..74fc990c 100644 --- a/middleware/packet-forward-middleware/testing/simapp/upgrades/upgrades.go +++ b/middleware/packet-forward-middleware/testing/simapp/upgrades/upgrades.go @@ -13,6 +13,7 @@ import ( const ( V2 = "v2" + V3 = "v3" ) // CreateDefaultUpgradeHandler creates a simple migration upgrade handler. @@ -47,3 +48,17 @@ func CreateV2UpgradeHandler( return versionMap, err } } + +func CreateV3UpgradeHandler( + mm *module.Manager, + cfg module.Configurator, +) upgradetypes.UpgradeHandler { + return func(ctx context.Context, _ upgradetypes.Plan, fromVM module.VersionMap) (module.VersionMap, error) { + versionMap, err := mm.RunMigrations(ctx, cfg, fromVM) + if err != nil { + return nil, err + } + + return versionMap, err + } +}