feat: add configurable limits for receiver and ICS-20 memo fields (#1377

)

* Add ics20 memo limit

* Add max receiver size and ics-20 memo limit configuration

This change introduces new configuration, MaxReceiverSize & ICS20MemoLimit, in the relayer, path processor, and the global config. It updates the existing functions and methods to utilize this new configuration property for managing and validating receiver and memo sizes in IBC related tasks.

* Add WriteConfig function and update method names

Added a new function WriteConfig in relayertest/system.go for testing. Also, updated method names in relayer.go to start with an uppercase letter for public visibility. Besides, upgraded Docker image version in localhost_client_test.go from v8.0.0-beta.1 to v8.0.0.

* test: add memo and receiver limit test

Created the Memo and Receiver Limit test within the interchain test suite. This test evaluates the functionality of IBC transfers with a memo field length exceeding configured limit, as well as receiver field exceeding the limit. It also evaluates a successful IBC transfer for completeness.

* test: update test for memo and receiver limit

Adjusted the test case 'TestMemoAndReceiverLimit' to focus more on the functionality of sending transfers with a memo and receiver limit configured. This reduces the complexity of the test case, and it now focuses more closely on the actual purpose of the method. The general flow of the test has been maintained, but some unnecessary operations have been removed.

* fix: properly unmarshal ics-20 packet data & remove debug print statements

This commit removes various debug print statements from the code, making it cleaner and easier to read. Additionally, it updates the way the 'transfertypes.ModuleCdc.Unmarshal' function is used to 'transfertypes.ModuleCdc.UnmarshalJSON', simplifying the overall structure of the code.

* test: expand test users and IBC transfers in memo/receiver limit test

The test for the memo receiver limit now supports four test users instead of two, allowing for more complex transfer scenarios. This change requires increased checks for account balances, as well as additional IBC transfers. This expansion helps ensure more comprehensive testing coverage and overall system validation.

---------

Co-authored-by: Andrew Gouin <andrew@gouin.io>

cmd/config.go

@@ -488,21 +488,24 @@ func DefaultConfig(memo string) *Config {
 
 // GlobalConfig describes any global relayer settings
 type GlobalConfig struct {
-	APIListenPort  string `yaml:"api-listen-addr" json:"api-listen-addr"`
-	Timeout        string `yaml:"timeout" json:"timeout"`
-	Memo           string `yaml:"memo" json:"memo"`
-	LightCacheSize int    `yaml:"light-cache-size" json:"light-cache-size"`
-	LogLevel       string `yaml:"log-level" json:"log-level"`
+	APIListenPort   string `yaml:"api-listen-addr" json:"api-listen-addr"`
+	Timeout         string `yaml:"timeout" json:"timeout"`
+	Memo            string `yaml:"memo" json:"memo"`
+	LightCacheSize  int    `yaml:"light-cache-size" json:"light-cache-size"`
+	LogLevel        string `yaml:"log-level" json:"log-level"`
+	ICS20MemoLimit  int    `yaml:"ics20-memo-limit" json:"ics20-memo-limit"`
+	MaxReceiverSize int    `yaml:"max-receiver-size" json:"max-receiver-size"`
 }
 
 // newDefaultGlobalConfig returns a global config with defaults set
 func newDefaultGlobalConfig(memo string) GlobalConfig {
 	return GlobalConfig{
-		APIListenPort:  ":5183",
-		Timeout:        "10s",
-		LightCacheSize: 20,
-		Memo:           memo,
-		LogLevel:       "info",
+		APIListenPort:   ":5183",
+		Timeout:         "10s",
+		LightCacheSize:  20,
+		Memo:            memo,
+		LogLevel:        "info",
+		MaxReceiverSize: 128,
 	}
 }
 

cmd/start.go

@@ -160,6 +160,8 @@ $ %s start demo-path2 --max-tx-size 10`, appName, appName, appName, appName)),
 				chains,
 				paths,
 				maxMsgLength,
+				a.config.Global.MaxReceiverSize,
+				a.config.Global.ICS20MemoLimit,
 				a.config.memo(cmd),
 				clientUpdateThresholdTime,
 				flushInterval,

cmd/tx.go

@@ -958,6 +958,8 @@ $ %s tx flush demo-path channel-0`,
 				chains,
 				paths,
 				maxMsgLength,
+				a.config.Global.MaxReceiverSize,
+				a.config.Global.ICS20MemoLimit,
 				a.config.memo(cmd),
 				0,
 				0,

interchaintest/feegrant_test.go

@@ -268,7 +268,7 @@ func TestRelayerFeeGrant(t *testing.T) {
 			//You MUST run the configure feegrant command prior to starting the relayer, otherwise it'd be like you never set it up at all (within this test)
 			//Note that Gaia supports feegrants, but Osmosis does not (x/feegrant module, or any compatible module, is not included in Osmosis SDK app modules)
 			localRelayer := r.(*Relayer)
-			res := localRelayer.sys().Run(logger, "chains", "configure", "feegrant", "basicallowance", gaia.Config().ChainID, gaiaGranterWallet.KeyName(), "--grantees", granteeCsv, "--overwrite-granter")
+			res := localRelayer.Sys().Run(logger, "chains", "configure", "feegrant", "basicallowance", gaia.Config().ChainID, gaiaGranterWallet.KeyName(), "--grantees", granteeCsv, "--overwrite-granter")
 			if res.Err != nil {
 				fmt.Printf("configure feegrant results: %s\n", res.Stdout.String())
 				t.Fatalf("failed to rly config feegrants: %v", res.Err)

interchaintest/localhost_client_test.go

@@ -78,7 +78,7 @@ func TestLocalhost_TokenTransfers(t *testing.T) {
 	numFullNodes := 0
 	image := ibc.DockerImage{
 		Repository: "ghcr.io/cosmos/ibc-go-simd",
-		Version:    "v8.0.0-beta.1",
+		Version:    "v8.0.0",
 		UidGid:     "100:1000",
 	}
 	cdc := DefaultEncoding()

interchaintest/memo_receiver_limit_test.go

@@ -0,0 +1,278 @@
+package interchaintest_test
+
+import (
+	"context"
+	"testing"
+
+	sdkmath "cosmossdk.io/math"
+	transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
+	"github.com/cosmos/relayer/v2/cmd"
+	relayertest "github.com/cosmos/relayer/v2/interchaintest"
+	"github.com/cosmos/relayer/v2/relayer/chains/cosmos"
+	"github.com/strangelove-ventures/interchaintest/v8"
+	"github.com/strangelove-ventures/interchaintest/v8/ibc"
+	"github.com/strangelove-ventures/interchaintest/v8/testreporter"
+	"github.com/strangelove-ventures/interchaintest/v8/testutil"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap/zaptest"
+	"gopkg.in/yaml.v3"
+)
+
+// TestMemoAndReceiverLimit tests the functionality of sending transfers with a memo and receiver limit configured.
+func TestMemoAndReceiverLimit(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping in short mode")
+	}
+
+	t.Parallel()
+
+	numVals := 1
+	numFullNodes := 0
+
+	image := ibc.DockerImage{
+		Repository: "ghcr.io/cosmos/ibc-go-simd",
+		Version:    "v8.0.0",
+		UidGid:     "100:1000",
+	}
+
+	cf := interchaintest.NewBuiltinChainFactory(zaptest.NewLogger(t), []*interchaintest.ChainSpec{
+		{
+			Name:          "ibc-go-simd",
+			Version:       "main",
+			NumValidators: &numVals,
+			NumFullNodes:  &numFullNodes,
+			ChainConfig: ibc.ChainConfig{
+				Type:          "cosmos",
+				Name:          "simd",
+				ChainID:       "chain-a",
+				Images:        []ibc.DockerImage{image},
+				Bin:           "simd",
+				Bech32Prefix:  "cosmos",
+				Denom:         "stake",
+				CoinType:      "118",
+				GasPrices:     "0.0stake",
+				GasAdjustment: 1.1,
+			},
+		},
+		{
+			Name:          "ibc-go-simd",
+			Version:       "main",
+			NumValidators: &numVals,
+			NumFullNodes:  &numFullNodes,
+			ChainConfig: ibc.ChainConfig{
+				Type:          "cosmos",
+				Name:          "simd",
+				ChainID:       "chain-b",
+				Images:        []ibc.DockerImage{image},
+				Bin:           "simd",
+				Bech32Prefix:  "cosmos",
+				Denom:         "stake",
+				CoinType:      "118",
+				GasPrices:     "0.0stake",
+				GasAdjustment: 1.1,
+			},
+		},
+	})
+
+	chains, err := cf.Chains(t.Name())
+	require.NoError(t, err)
+	chainA, chainB := chains[0], chains[1]
+
+	ctx := context.Background()
+	client, network := interchaintest.DockerSetup(t)
+
+	rf := relayertest.NewRelayerFactory(relayertest.RelayerConfig{InitialBlockHistory: 50})
+	r := rf.Build(t, client, network)
+
+	const pathName = "chainA-chainB"
+
+	ic := interchaintest.NewInterchain().
+		AddChain(chainA).
+		AddChain(chainB).
+		AddRelayer(r, "relayer").
+		AddLink(interchaintest.InterchainLink{
+			Chain1:  chainA,
+			Chain2:  chainB,
+			Relayer: r,
+			Path:    pathName,
+		})
+
+	rep := testreporter.NewNopReporter()
+	eRep := rep.RelayerExecReporter(t)
+
+	require.NoError(t, ic.Build(ctx, eRep, interchaintest.InterchainBuildOptions{
+		TestName:  t.Name(),
+		Client:    client,
+		NetworkID: network,
+	}))
+
+	t.Cleanup(func() {
+		_ = ic.Close()
+	})
+
+	// Create and fund user accs & assert initial balances.
+	initBal := sdkmath.NewInt(1_000_000_000_000)
+	users := interchaintest.GetAndFundTestUsers(t, ctx, t.Name(), initBal, chainA, chainB, chainA, chainB)
+
+	require.NoError(t, testutil.WaitForBlocks(ctx, 2, chainA, chainB))
+
+	userA := users[0]
+	userB := users[1]
+	userC := users[2]
+	userD := users[3]
+
+	userABal, err := chainA.GetBalance(ctx, userA.FormattedAddress(), chainA.Config().Denom)
+	require.NoError(t, err)
+	require.True(t, initBal.Equal(userABal))
+
+	userCBal, err := chainA.GetBalance(ctx, userC.FormattedAddress(), chainA.Config().Denom)
+	require.NoError(t, err)
+	require.True(t, initBal.Equal(userCBal))
+
+	userBBal, err := chainB.GetBalance(ctx, userB.FormattedAddress(), chainB.Config().Denom)
+	require.NoError(t, err)
+	require.True(t, initBal.Equal(userBBal))
+
+	userDBal, err := chainB.GetBalance(ctx, userD.FormattedAddress(), chainB.Config().Denom)
+	require.NoError(t, err)
+	require.True(t, initBal.Equal(userDBal))
+
+	// Read relayer config from disk, configure memo limit, & write config back to disk.
+	relayer := r.(*relayertest.Relayer)
+
+	cfg := relayer.Sys().MustGetConfig(t)
+
+	cfg.Global.ICS20MemoLimit = 10
+
+	cfgOutput := new(cmd.ConfigOutputWrapper)
+	cfgOutput.ProviderConfigs = cmd.ProviderConfigs{}
+	cfgOutput.Global = cfg.Global
+	cfgOutput.Paths = cfg.Paths
+
+	for _, c := range cfg.ProviderConfigs {
+		stuff := c.Value.(*cosmos.CosmosProviderConfig)
+
+		cfgOutput.ProviderConfigs[stuff.ChainID] = &cmd.ProviderConfigWrapper{
+			Type:  "cosmos",
+			Value: stuff,
+		}
+	}
+
+	cfgBz, err := yaml.Marshal(cfgOutput)
+	require.NoError(t, err)
+
+	err = relayer.Sys().WriteConfig(t, cfgBz)
+	require.NoError(t, err)
+
+	err = r.StartRelayer(ctx, eRep, pathName)
+	require.NoError(t, err)
+
+	t.Cleanup(
+		func() {
+			err := r.StopRelayer(ctx, eRep)
+			if err != nil {
+				t.Logf("an error occurred while stopping the relayer: %s", err)
+			}
+		},
+	)
+
+	// Send transfers that should succeed & assert balances.
+	channels, err := r.GetChannels(ctx, eRep, chainA.Config().ChainID)
+	require.NoError(t, err)
+	require.Equal(t, 1, len(channels))
+
+	channel := channels[0]
+
+	transferAmount := sdkmath.NewInt(1_000)
+
+	transferAB := ibc.WalletAmount{
+		Address: userB.FormattedAddress(),
+		Denom:   chainA.Config().Denom,
+		Amount:  transferAmount,
+	}
+
+	transferCD := ibc.WalletAmount{
+		Address: userD.FormattedAddress(),
+		Denom:   chainA.Config().Denom,
+		Amount:  transferAmount,
+	}
+
+	_, err = chainA.SendIBCTransfer(ctx, channel.ChannelID, userA.KeyName(), transferAB, ibc.TransferOptions{})
+	require.NoError(t, err)
+
+	_, err = chainA.SendIBCTransfer(ctx, channel.ChannelID, userC.KeyName(), transferCD, ibc.TransferOptions{})
+	require.NoError(t, err)
+
+	require.NoError(t, testutil.WaitForBlocks(ctx, 5, chainA, chainB))
+
+	// Compose the ibc denom for balance assertions on the counterparty and assert balances.
+	denom := transfertypes.GetPrefixedDenom(
+		channel.Counterparty.PortID,
+		channel.Counterparty.ChannelID,
+		chainA.Config().Denom,
+	)
+	trace := transfertypes.ParseDenomTrace(denom)
+
+	userABal, err = chainA.GetBalance(ctx, userA.FormattedAddress(), chainA.Config().Denom)
+	require.NoError(t, err)
+	require.True(t, userABal.Equal(initBal.Sub(transferAmount)))
+
+	userBBal, err = chainB.GetBalance(ctx, userB.FormattedAddress(), trace.IBCDenom())
+	require.NoError(t, err)
+	require.True(t, userBBal.Equal(transferAmount))
+
+	userCBal, err = chainA.GetBalance(ctx, userC.FormattedAddress(), chainA.Config().Denom)
+	require.NoError(t, err)
+	require.True(t, userCBal.Equal(initBal.Sub(transferAmount)))
+
+	userDBal, err = chainB.GetBalance(ctx, userD.FormattedAddress(), trace.IBCDenom())
+	require.NoError(t, err)
+	require.True(t, userDBal.Equal(transferAmount))
+
+	// Send transfer with memo that exceeds limit, ensure transfer failed and assert balances.
+	opts := ibc.TransferOptions{
+		Memo: "this memo is too long",
+		Timeout: &ibc.IBCTimeout{
+			Height: 10,
+		},
+	}
+
+	_, err = chainA.SendIBCTransfer(ctx, channel.ChannelID, userA.KeyName(), transferAB, opts)
+	require.NoError(t, err)
+
+	require.NoError(t, testutil.WaitForBlocks(ctx, 11, chainA, chainB))
+
+	userABal, err = chainA.GetBalance(ctx, userA.FormattedAddress(), chainA.Config().Denom)
+	require.NoError(t, err)
+	require.True(t, !userABal.Equal(initBal.Sub(transferAmount)))
+
+	userBBal, err = chainB.GetBalance(ctx, userB.FormattedAddress(), trace.IBCDenom())
+	require.NoError(t, err)
+	require.True(t, userBBal.Equal(transferAmount))
+
+	// Send transfer with receiver field that exceeds limit, ensure transfer failed and assert balances.
+	var junkReceiver string
+
+	for i := 0; i < 130; i++ {
+		junkReceiver += "a"
+	}
+
+	transferCD = ibc.WalletAmount{
+		Address: junkReceiver,
+		Denom:   chainA.Config().Denom,
+		Amount:  transferAmount,
+	}
+
+	_, err = chainA.SendIBCTransfer(ctx, channel.ChannelID, userC.KeyName(), transferCD, ibc.TransferOptions{})
+	require.NoError(t, err)
+
+	require.NoError(t, testutil.WaitForBlocks(ctx, 5, chainA, chainB))
+
+	userCBal, err = chainA.GetBalance(ctx, userC.FormattedAddress(), chainA.Config().Denom)
+	require.NoError(t, err)
+	require.True(t, !userCBal.Equal(initBal.Sub(transferAmount)))
+
+	userDBal, err = chainB.GetBalance(ctx, userD.FormattedAddress(), trace.IBCDenom())
+	require.NoError(t, err)
+	require.True(t, userDBal.Equal(transferAmount))
+}