Repository: cosmos/relayer
Pull request #665 (2022-04-04 13:33:43)

Use narrower parameters to (*RelayMsgs).Send()
And backfill some basic tests for that method.

The new, narrower RelayMsgSender struct is much simpler to mock in test
than the wide interface matching Chain. And having some basic tests in
place will make it easier to continue refactoring Send.


Patch:
diff --git a/relayer/channel.go b/relayer/channel.go
index b11c4f1..4c765d1 100644
--- a/relayer/channel.go
+++ b/relayer/channel.go
@@ -623,7 +623,7 @@ func (c *Chain) CloseChannel(ctx context.Context, dst *Chain, to time.Duration,
 			break
 		}
 
-		if closeSteps.Send(ctx, c.log, c, dst); closeSteps.Success() && closeSteps.Last {
+		if closeSteps.Send(ctx, c.log, AsRelayMsgSender(c), AsRelayMsgSender(dst)); closeSteps.Success() && closeSteps.Last {
 			srch, dsth, err := QueryLatestHeights(ctx, c, dst)
 			if err != nil {
 				return err
diff --git a/relayer/client.go b/relayer/client.go
index e3f7ae0..e0a53c0 100644
--- a/relayer/client.go
+++ b/relayer/client.go
@@ -258,7 +258,7 @@ func (c *Chain) UpdateClients(ctx context.Context, dst *Chain) (err error) {
 
 	// Send msgs to both chains
 	if clients.Ready() {
-		if clients.Send(ctx, c.log, c, dst); clients.Success() {
+		if clients.Send(ctx, c.log, AsRelayMsgSender(c), AsRelayMsgSender(dst)); clients.Success() {
 			c.log.Info(
 				"Clients updated",
 				zap.String("src_chain_id", c.ChainID()),
diff --git a/relayer/naive-strategy.go b/relayer/naive-strategy.go
index bd45407..ac3ff95 100644
--- a/relayer/naive-strategy.go
+++ b/relayer/naive-strategy.go
@@ -352,7 +352,7 @@ func RelayAcknowledgements(ctx context.Context, log *zap.Logger, src, dst *Chain
 		}
 
 		// send messages to their respective chains
-		if msgs.Send(ctx, log, src, dst); msgs.Success() {
+		if msgs.Send(ctx, log, AsRelayMsgSender(src), AsRelayMsgSender(dst)); msgs.Success() {
 			if len(msgs.Dst) > 1 {
 				dst.logPacketsRelayed(src, len(msgs.Dst)-1, srcChannel)
 			}
@@ -427,7 +427,7 @@ func RelayPackets(ctx context.Context, log *zap.Logger, src, dst *Chain, sp *Rel
 		}
 
 		// send messages to their respective chains
-		if msgs.Send(ctx, log, src, dst); msgs.Success() {
+		if msgs.Send(ctx, log, AsRelayMsgSender(src), AsRelayMsgSender(dst)); msgs.Success() {
 			if len(msgs.Dst) > 1 {
 				dst.logPacketsRelayed(src, len(msgs.Dst)-1, srcChannel)
 			}
@@ -642,7 +642,7 @@ func RelayPacket(ctx context.Context, log *zap.Logger, src, dst *Chain, sp *Rela
 	}
 
 	// send messages to their respective chains
-	if msgs.Send(ctx, log, src, dst); msgs.Success() {
+	if msgs.Send(ctx, log, AsRelayMsgSender(src), AsRelayMsgSender(dst)); msgs.Success() {
 		if len(msgs.Dst) > 1 {
 			dst.logPacketsRelayed(src, len(msgs.Dst)-1, srcChannel)
 		}
diff --git a/relayer/packet-tx.go b/relayer/packet-tx.go
index f8ac731..3d23bfa 100644
--- a/relayer/packet-tx.go
+++ b/relayer/packet-tx.go
@@ -55,7 +55,7 @@ func (c *Chain) SendTransferMsg(ctx context.Context, log *zap.Logger, dst *Chain
 		Dst: []provider.RelayerMessage{},
 	}
 
-	if txs.Send(ctx, log, c, dst); !txs.Success() {
+	if txs.Send(ctx, log, AsRelayMsgSender(c), AsRelayMsgSender(dst)); !txs.Success() {
 		return fmt.Errorf("failed to send transfer message")
 	}
 	return nil
diff --git a/relayer/relayMsgs.go b/relayer/relayMsgs.go
index 8168190..c0259df 100644
--- a/relayer/relayMsgs.go
+++ b/relayer/relayMsgs.go
@@ -87,7 +87,27 @@ func DecodeMsgs(c *Chain, msgs []string) []provider.RelayerMessage {
 	return outMsgs
 }
 
-func (r *RelayMsgs) Send(ctx context.Context, log *zap.Logger, src, dst *Chain) {
+// RelayMsgSender is a narrow subset of a Chain,
+// to simplify testing methods on RelayMsgs.
+type RelayMsgSender struct {
+	ChainID string
+
+	// SendMessages is a function matching the signature of the same method
+	// on the ChainProvider interface.
+	//
+	// Accepting this narrow subset of the interface greatly simplifies testing.
+	SendMessages func(context.Context, []provider.RelayerMessage) (*provider.RelayerTxResponse, bool, error)
+}
+
+// AsRelayMsgSender converts c to a RelayMsgSender.
+func AsRelayMsgSender(c *Chain) RelayMsgSender {
+	return RelayMsgSender{
+		ChainID:      c.ChainID(),
+		SendMessages: c.ChainProvider.SendMessages,
+	}
+}
+
+func (r *RelayMsgs) Send(ctx context.Context, log *zap.Logger, src, dst RelayMsgSender) {
 	//nolint:prealloc // can not be pre allocated
 	var (
 		msgLen, txSize uint64
@@ -109,9 +129,9 @@ func (r *RelayMsgs) Send(ctx context.Context, log *zap.Logger, src, dst *Chain)
 
 			if r.IsMaxTx(msgLen, txSize) {
 				// Submit the transactions to src chain and update its status
-				res, success, err := src.ChainProvider.SendMessages(ctx, msgs)
+				res, success, err := src.SendMessages(ctx, msgs)
 				if err != nil {
-					logFailedTx(log, src.ChainID(), res, err, msgs)
+					logFailedTx(log, src.ChainID, res, err, msgs)
 				}
 				r.Succeeded = r.Succeeded && success
 
@@ -125,9 +145,9 @@ func (r *RelayMsgs) Send(ctx context.Context, log *zap.Logger, src, dst *Chain)
 
 	// submit leftover msgs
 	if len(msgs) > 0 {
-		res, success, err := src.ChainProvider.SendMessages(ctx, msgs)
+		res, success, err := src.SendMessages(ctx, msgs)
 		if err != nil {
-			logFailedTx(log, src.ChainID(), res, err, msgs)
+			logFailedTx(log, src.ChainID, res, err, msgs)
 		}
 
 		r.Succeeded = success
@@ -149,9 +169,9 @@ func (r *RelayMsgs) Send(ctx context.Context, log *zap.Logger, src, dst *Chain)
 
 			if r.IsMaxTx(msgLen, txSize) {
 				// Submit the transaction to dst chain and update its status
-				res, success, err := dst.ChainProvider.SendMessages(ctx, msgs)
+				res, success, err := dst.SendMessages(ctx, msgs)
 				if err != nil {
-					logFailedTx(log, dst.ChainID(), res, err, msgs)
+					logFailedTx(log, dst.ChainID, res, err, msgs)
 				}
 
 				r.Succeeded = r.Succeeded && success
@@ -166,9 +186,9 @@ func (r *RelayMsgs) Send(ctx context.Context, log *zap.Logger, src, dst *Chain)
 
 	// submit leftover msgs
 	if len(msgs) > 0 {
-		res, success, err := dst.ChainProvider.SendMessages(ctx, msgs)
+		res, success, err := dst.SendMessages(ctx, msgs)
 		if err != nil {
-			logFailedTx(log, dst.ChainID(), res, err, msgs)
+			logFailedTx(log, dst.ChainID, res, err, msgs)
 		}
 
 		r.Succeeded = success


Test patch:
diff --git a/relayer/relaymsgs_test.go b/relayer/relaymsgs_test.go
new file mode 100644
index 0000000..e635934
--- /dev/null
+++ b/relayer/relaymsgs_test.go
@@ -0,0 +1,133 @@
+package relayer_test
+
+import (
+	"context"
+	"testing"
+
+	"github.com/cosmos/relayer/v2/relayer"
+	"github.com/cosmos/relayer/v2/relayer/provider"
+	"github.com/stretchr/testify/require"
+	"go.uber.org/zap/zaptest"
+)
+
+func TestRelayMsgs_IsMaxTx(t *testing.T) {
+	rm := relayer.RelayMsgs{
+		MaxTxSize:    10,
+		MaxMsgLength: 10,
+	}
+	require.True(t, rm.IsMaxTx(1, 11), "only exceeded tx size")
+	require.True(t, rm.IsMaxTx(11, 1), "only exceeded message length")
+	require.False(t, rm.IsMaxTx(4, 5), "neither exceeded")
+
+	rm = relayer.RelayMsgs{
+		MaxTxSize:    0,
+		MaxMsgLength: 10,
+	}
+	require.True(t, rm.IsMaxTx(11, 1), "exceeded set max message length")
+	require.False(t, rm.IsMaxTx(5, 100), "did not exceed set max message length")
+
+	rm = relayer.RelayMsgs{
+		MaxTxSize:    10,
+		MaxMsgLength: 0,
+	}
+	require.True(t, rm.IsMaxTx(1, 11), "exceeded set max tx size")
+	require.False(t, rm.IsMaxTx(100, 5), "did not exceed set max tx size")
+
+	rm = relayer.RelayMsgs{
+		MaxTxSize:    0,
+		MaxMsgLength: 0,
+	}
+	require.False(t, rm.IsMaxTx(9999999, 99999999), "no limits to exceed")
+}
+
+// fakeRelayerMessage is a dummy implementation of provider.RelayerMessage.
+type fakeRelayerMessage struct {
+	t, b string
+}
+
+var _ provider.RelayerMessage = fakeRelayerMessage{}
+
+func (m fakeRelayerMessage) Type() string {
+	return m.t
+}
+
+func (m fakeRelayerMessage) MsgBytes() ([]byte, error) {
+	return []byte(m.b), nil
+}
+
+func TestRelayMsgs_Send(t *testing.T) {
+	// Fixtures for test.
+	// src appends to srcSent and dst appends to dstSent.
+	var srcSent []provider.RelayerMessage
+	src := relayer.RelayMsgSender{
+		ChainID: "src",
+		SendMessages: func(ctx context.Context, msgs []provider.RelayerMessage) (*provider.RelayerTxResponse, bool, error) {
+			srcSent = append(srcSent, msgs...)
+			return nil, false, nil
+		},
+	}
+
+	var dstSent []provider.RelayerMessage
+	dst := relayer.RelayMsgSender{
+		ChainID: "dst",
+		SendMessages: func(ctx context.Context, msgs []provider.RelayerMessage) (*provider.RelayerTxResponse, bool, error) {
+			dstSent = append(dstSent, msgs...)
+			return nil, false, nil
+		},
+	}
+
+	srcMsg := fakeRelayerMessage{t: "srctype", b: "srcdata"}
+	dstMsg := fakeRelayerMessage{t: "dsttype", b: "dstdata"}
+
+	t.Run("sends in a single batch when there are no limits", func(t *testing.T) {
+		// Clear state (in case this test is ever reordered).
+		srcSent = nil
+		dstSent = nil
+
+		rm := relayer.RelayMsgs{
+			Src: []provider.RelayerMessage{srcMsg},
+			Dst: []provider.RelayerMessage{dstMsg},
+		}
+
+		rm.Send(context.Background(), zaptest.NewLogger(t), src, dst)
+
+		require.Equal(t, []provider.RelayerMessage{srcMsg}, srcSent)
+		require.Equal(t, []provider.RelayerMessage{dstMsg}, dstSent)
+	})
+
+	t.Run("sends all messages when max message length exceeded", func(t *testing.T) {
+		// Clear state from previous test.
+		srcSent = nil
+		dstSent = nil
+
+		rm := relayer.RelayMsgs{
+			Src: []provider.RelayerMessage{srcMsg, srcMsg, srcMsg},
+			Dst: []provider.RelayerMessage{dstMsg, dstMsg, dstMsg},
+
+			MaxMsgLength: 2,
+		}
+
+		rm.Send(context.Background(), zaptest.NewLogger(t), src, dst)
+
+		require.Equal(t, []provider.RelayerMessage{srcMsg, srcMsg, srcMsg}, srcSent)
+		require.Equal(t, []provider.RelayerMessage{dstMsg, dstMsg, dstMsg}, dstSent)
+	})
+
+	t.Run("sends all messages when max tx size exceeded", func(t *testing.T) {
+		// Clear state from previous test.
+		srcSent = nil
+		dstSent = nil
+
+		rm := relayer.RelayMsgs{
+			Src: []provider.RelayerMessage{srcMsg, srcMsg, srcMsg},
+			Dst: []provider.RelayerMessage{dstMsg, dstMsg, dstMsg},
+
+			MaxMsgLength: 2,
+		}
+
+		rm.Send(context.Background(), zaptest.NewLogger(t), src, dst)
+
+		require.Equal(t, []provider.RelayerMessage{srcMsg, srcMsg, srcMsg}, srcSent)
+		require.Equal(t, []provider.RelayerMessage{dstMsg, dstMsg, dstMsg}, dstSent)
+	})
+}