refactor: remove the ability to take a fee for each forwarded packet (#…

…202)

* refactor: remove the ability to take a fee that is sent to the community pool

* fix: mocks

* fix: remove WithFee unit test (no longer used due to fee refactor)

* fix(e2e): upgrade test

---------

Co-authored-by: Reece Williams <reecepbcups@gmail.com>

middleware/packet-forward-middleware/Makefile

@@ -142,9 +142,8 @@ cosmovisor:
 .PHONY: build build-linux build-simd-all build-simd-linux cosmovisor
 
 mocks: $(MOCKS_DIR)
-	go install go.uber.org/mock/mockgen@v0.2.0
+	go install go.uber.org/mock/mockgen@v0.4.0
 	mockgen -package=mock -destination=./test/mock/transfer_keeper.go $(GOMOD)/packetforward/types TransferKeeper
-	mockgen -package=mock -destination=./test/mock/distribution_keeper.go $(GOMOD)/packetforward/types DistributionKeeper
 	mockgen -package=mock -destination=./test/mock/bank_keeper.go $(GOMOD)/packetforward/types BankKeeper
 	mockgen -package=mock -destination=./test/mock/channel_keeper.go $(GOMOD)/packetforward/types ChannelKeeper
 	mockgen -package=mock -destination=./test/mock/ics4_wrapper.go github.com/cosmos/ibc-go/v8/modules/core/05-port/types ICS4Wrapper

middleware/packet-forward-middleware/e2e/forward_timeout_test.go

@@ -304,7 +304,7 @@ func TestTimeoutOnForward(t *testing.T) {
 	_, err = testutil.PollForAck(ctx, chainA, chainAHeight, chainAHeight+30, transferTx.Packet)
 	require.NoError(t, err)
 
-	err = testutil.WaitForBlocks(ctx, 5, chainA)
+	err = testutil.WaitForBlocks(ctx, 10, chainA)
 	require.NoError(t, err)
 
 	// Assert balances are updated to reflect tokens now being on ChainD
@@ -411,4 +411,143 @@ func TestTimeoutOnForward(t *testing.T) {
 	require.True(t, firstHopEscrowBalance.Equal(transferAmount))
 	require.True(t, secondHopEscrowBalance.Equal(transferAmount))
 	require.True(t, thirdHopEscrowBalance.Equal(transferAmount))
+
+	// ---
+
+	// Compose IBC tx that will go from ChainD -> ChainC -> ChainB -> ChainA and succeed.
+	transfer = ibc.WalletAmount{
+		Address: userC.FormattedAddress(),
+		Denom:   thirdHopDenom,
+		Amount:  transferAmount,
+	}
+
+	secondHopMetadata = &PacketMetadata{
+		Forward: &ForwardMetadata{
+			Receiver: userA.FormattedAddress(),
+			Channel:  baChan.ChannelID,
+			Port:     baChan.PortID,
+		},
+	}
+	nextBz, err = json.Marshal(secondHopMetadata)
+	require.NoError(t, err)
+	next = string(nextBz)
+
+	firstHopMetadata = &PacketMetadata{
+		Forward: &ForwardMetadata{
+			Receiver: userB.FormattedAddress(),
+			Channel:  cbChan.ChannelID,
+			Port:     cbChan.PortID,
+			Next:     &next,
+		},
+	}
+
+	memo, err = json.Marshal(firstHopMetadata)
+	require.NoError(t, err)
+
+	chainDHeight, err = chainD.Height(ctx)
+	require.NoError(t, err)
+
+	transferTx, err = chainD.SendIBCTransfer(ctx, dcChan.ChannelID, userD.KeyName(), transfer, ibc.TransferOptions{Memo: string(memo)})
+	require.NoError(t, err)
+
+	_, err = testutil.PollForAck(ctx, chainD, chainDHeight, chainDHeight+25, transferTx.Packet)
+	require.NoError(t, err)
+
+	err = testutil.WaitForBlocks(ctx, 5, chainD)
+	require.NoError(t, err)
+
+	// Assert balances to ensure timeout happened and user funds are still present on ChainD
+	chainABalance, err = chainA.GetBalance(ctx, userA.FormattedAddress(), chainA.Config().Denom)
+	require.NoError(t, err)
+
+	chainBBalance, err = chainB.GetBalance(ctx, userB.FormattedAddress(), firstHopIBCDenom)
+	require.NoError(t, err)
+
+	chainCBalance, err = chainC.GetBalance(ctx, userC.FormattedAddress(), secondHopIBCDenom)
+	require.NoError(t, err)
+
+	chainDBalance, err = chainD.GetBalance(ctx, userD.FormattedAddress(), thirdHopIBCDenom)
+	require.NoError(t, err)
+
+	require.True(t, chainABalance.Equal(initBal))
+	require.True(t, chainBBalance.Equal(zeroBal))
+	require.True(t, chainCBalance.Equal(zeroBal))
+	require.True(t, chainDBalance.Equal(zeroBal))
+
+	firstHopEscrowBalance, err = chainA.GetBalance(ctx, firstHopEscrowAccount, chainA.Config().Denom)
+	require.NoError(t, err)
+
+	secondHopEscrowBalance, err = chainB.GetBalance(ctx, secondHopEscrowAccount, firstHopIBCDenom)
+	require.NoError(t, err)
+
+	thirdHopEscrowBalance, err = chainC.GetBalance(ctx, thirdHopEscrowAccount, secondHopIBCDenom)
+	require.NoError(t, err)
+
+	require.True(t, firstHopEscrowBalance.Equal(zeroBal))
+	require.True(t, secondHopEscrowBalance.Equal(zeroBal))
+	require.True(t, thirdHopEscrowBalance.Equal(zeroBal))
+
+	// ----- 2
+
+	// Compose IBC tx that will go from ChainD -> ChainC -> ChainB -> ChainA and succeed.
+	transfer = ibc.WalletAmount{
+		Address: userB.FormattedAddress(),
+		Denom:   chainA.Config().Denom,
+		Amount:  transferAmount,
+	}
+
+	firstHopMetadata = &PacketMetadata{
+		Forward: &ForwardMetadata{
+			Receiver: userA.FormattedAddress(),
+			Channel:  baChan.ChannelID,
+			Port:     baChan.PortID,
+			Timeout:  1 * time.Second,
+		},
+	}
+
+	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+25, transferTx.Packet)
+	require.NoError(t, err)
+
+	err = testutil.WaitForBlocks(ctx, 5, chainA)
+	require.NoError(t, err)
+
+	// Assert balances to ensure timeout happened and user funds are still present on ChainD
+	chainABalance, err = chainA.GetBalance(ctx, userA.FormattedAddress(), chainA.Config().Denom)
+	require.NoError(t, err)
+
+	chainBBalance, err = chainB.GetBalance(ctx, userB.FormattedAddress(), firstHopIBCDenom)
+	require.NoError(t, err)
+
+	chainCBalance, err = chainC.GetBalance(ctx, userC.FormattedAddress(), secondHopIBCDenom)
+	require.NoError(t, err)
+
+	chainDBalance, err = chainD.GetBalance(ctx, userD.FormattedAddress(), thirdHopIBCDenom)
+	require.NoError(t, err)
+
+	require.True(t, chainABalance.Equal(initBal))
+	require.True(t, chainBBalance.Equal(zeroBal))
+	require.True(t, chainCBalance.Equal(zeroBal))
+	require.True(t, chainDBalance.Equal(zeroBal))
+
+	firstHopEscrowBalance, err = chainA.GetBalance(ctx, firstHopEscrowAccount, chainA.Config().Denom)
+	require.NoError(t, err)
+
+	secondHopEscrowBalance, err = chainB.GetBalance(ctx, secondHopEscrowAccount, firstHopIBCDenom)
+	require.NoError(t, err)
+
+	thirdHopEscrowBalance, err = chainC.GetBalance(ctx, thirdHopEscrowAccount, secondHopIBCDenom)
+	require.NoError(t, err)
+
+	require.True(t, firstHopEscrowBalance.Equal(zeroBal))
+	require.True(t, secondHopEscrowBalance.Equal(zeroBal))
+	require.True(t, thirdHopEscrowBalance.Equal(zeroBal))
 }

middleware/packet-forward-middleware/e2e/upgrade_test.go

@@ -2,7 +2,6 @@ package e2e
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"testing"
 	"time"
@@ -16,8 +15,6 @@ import (
 	"github.com/strangelove-ventures/interchaintest/v8/ibc"
 	"github.com/strangelove-ventures/interchaintest/v8/testutil"
 	"github.com/stretchr/testify/require"
-
-	packetforwardtypes "github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v8/packetforward/types"
 )
 
 const (
@@ -100,20 +97,6 @@ func CosmosChainUpgradeTest(t *testing.T, chainName, upgradeRepo, upgradeDockerT
 
 	ValidatorVoting(t, ctx, chain, proposalID, height, haltHeight)
 	UpgradeNodes(t, ctx, chain, client, haltHeight, upgradeRepo, upgradeDockerTag)
-
-	// Validate the PFM subspace -> keeper migration was successful.
-	cmd := []string{
-		chain.Config().Bin, "q", "packetforward", "params", "--output=json", "--node", chain.GetRPCAddress(),
-	}
-	stdout, _, err := chain.Exec(ctx, cmd, nil)
-	fmt.Println("stdout", string(stdout))
-	require.NoError(t, err, "error fetching pfm params")
-
-	var params packetforwardtypes.Params
-	err = json.Unmarshal(stdout, &params)
-	require.NoError(t, err, "error unmarshalling pfm params")
-	t.Logf("params: %+v", params)
-	require.Equal(t, sdkmath.LegacyNewDec(0), params.FeePercentage, "fee percentage not equal to expected value")
 }
 
 func SubmitUpgradeProposal(t *testing.T, ctx context.Context, chain *cosmos.CosmosChain, user ibc.Wallet, upgradeName string, haltHeight uint64) string {

middleware/packet-forward-middleware/go.mod

@@ -1,5 +1,7 @@
 go 1.21
 
+toolchain go1.22.5
+
 module github.com/cosmos/ibc-apps/middleware/packet-forward-middleware/v8
 
 require (
@@ -17,12 +19,10 @@ require (
 	cosmossdk.io/x/upgrade v0.1.1
 	github.com/cometbft/cometbft v0.38.6
 	github.com/cosmos/cosmos-db v1.0.2
-	github.com/cosmos/cosmos-proto v1.0.0-beta.4
 	github.com/cosmos/cosmos-sdk v0.50.5
 	github.com/cosmos/gogoproto v1.4.11
 	github.com/cosmos/ibc-go/modules/capability v1.0.0
 	github.com/cosmos/ibc-go/v8 v8.1.1
-	github.com/golang/protobuf v1.5.4
 	github.com/gorilla/mux v1.8.1
 	github.com/grpc-ecosystem/grpc-gateway v1.16.0
 	github.com/hashicorp/go-metrics v0.5.2
@@ -33,8 +33,6 @@ require (
 	github.com/spf13/viper v1.18.2
 	github.com/stretchr/testify v1.9.0
 	go.uber.org/mock v0.4.0
-	google.golang.org/genproto/googleapis/api v0.0.0-20240205150955-31a09d347014
-	google.golang.org/grpc v1.62.0
 )
 
 require (
@@ -69,6 +67,7 @@ require (
 	github.com/cockroachdb/tokenbucket v0.0.0-20230807174530-cc333fc44b06 // indirect
 	github.com/cometbft/cometbft-db v0.9.1 // indirect
 	github.com/cosmos/btcutil v1.0.5 // indirect
+	github.com/cosmos/cosmos-proto v1.0.0-beta.4 // indirect
 	github.com/cosmos/go-bip39 v1.0.0 // indirect
 	github.com/cosmos/gogogateway v1.2.0 // indirect
 	github.com/cosmos/iavl v1.0.1 // indirect
@@ -101,6 +100,7 @@ require (
 	github.com/golang/glog v1.2.0 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/mock v1.6.0 // indirect
+	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/golang/snappy v0.0.4 // indirect
 	github.com/google/btree v1.1.2 // indirect
 	github.com/google/go-cmp v0.6.0 // indirect
@@ -192,7 +192,9 @@ require (
 	google.golang.org/api v0.162.0 // indirect
 	google.golang.org/appengine v1.6.8 // indirect
 	google.golang.org/genproto v0.0.0-20240213162025-012b6fc9bca9 // indirect
+	google.golang.org/genproto/googleapis/api v0.0.0-20240205150955-31a09d347014 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240221002015-b0ce06bbee7c // indirect
+	google.golang.org/grpc v1.62.0 // indirect
 	google.golang.org/protobuf v1.33.0 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/yaml.v2 v2.4.0 // indirect

middleware/packet-forward-middleware/packetforward/keeper/genesis.go

@@ -8,10 +8,6 @@ import (
 
 // InitGenesis
 func (k Keeper) InitGenesis(ctx sdk.Context, state types.GenesisState) {
-	if err := k.SetParams(ctx, state.Params); err != nil {
-		panic(err)
-	}
-
 	// Initialize store refund path for forwarded packets in genesis state that have not yet been acked.
 	store := ctx.KVStore(k.storeKey)
 	for key, value := range state.InFlightPackets {
@@ -34,5 +30,5 @@ func (k Keeper) ExportGenesis(ctx sdk.Context) *types.GenesisState {
 		k.cdc.MustUnmarshal(itr.Value(), &inFlightPacket)
 		inFlightPackets[string(itr.Key())] = inFlightPacket
 	}
-	return &types.GenesisState{Params: k.GetParams(ctx), InFlightPackets: inFlightPackets}
+	return &types.GenesisState{InFlightPackets: inFlightPackets}
 }