From 34fa6fde56f3e1f15d68a170fead501a9f342892 Mon Sep 17 00:00:00 2001
From: Robert Zaremba <robert@zaremba.ch>
Date: Fri, 8 Mar 2024 16:39:46 +0100
Subject: [PATCH] feat(uics20 memo hook): add signer check

---
 x/uibc/uics20/memo_handler.go      | 19 +++++++++++--------
 x/uibc/uics20/memo_handler_test.go |  8 +++-----
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/x/uibc/uics20/memo_handler.go b/x/uibc/uics20/memo_handler.go
index bbde77db6f..bb717825a3 100644
--- a/x/uibc/uics20/memo_handler.go
+++ b/x/uibc/uics20/memo_handler.go
@@ -111,8 +111,9 @@ func (mh MemoHandler) execute(ctx *sdk.Context) error {
 
 // error messages used in validateMemoMsg
 var (
-	errNoSubCoins = errors.New("message must use only coins sent from the transfer")
-	errMsg0Type   = errors.New("only MsgSupply, MsgSupplyCollateral and MsgLiquidate are supported as messages[0]")
+	errWrongSigner = errors.New("msg signer doesn't match the ICS20 receiver")
+	errNoSubCoins  = errors.New("message must use only coins sent from the transfer")
+	errMsg0Type    = errors.New("only MsgSupply, MsgSupplyCollateral and MsgLiquidate are supported as messages[0]")
 	// errMsg1Type = errors.New("only MsgBorrow is supported as messages[1]")
 )
 
@@ -122,17 +123,25 @@ var (
 // - [MsgLiquidate]
 // Signer of each message (account under charged with coins), must be the receiver of the ICS20
 // transfer.
+// Must be called after onRecvPacketPrepare.
 func (mh MemoHandler) validateMemoMsg() error {
 	msgLen := len(mh.msgs)
 	if msgLen == 0 {
 		return nil
 	}
+
 	// In this release we only support 1msg, and only messages that don't create or change
 	// a borrow position
 	if msgLen > 1 {
 		return errors.New("ics20 memo with more than 1 message is not supported")
 	}
 
+	for _, msg := range mh.msgs {
+		if signers := msg.GetSigners(); len(signers) != 1 || !signers[0].Equals(mh.receiver) {
+			return errWrongSigner
+		}
+	}
+
 	var (
 		asset *sdk.Coin
 		// collateral sdk.Coin
@@ -154,12 +163,6 @@ func (mh MemoHandler) validateMemoMsg() error {
 	/**
 	   TODO: handlers v2
 
-	for _, msg := range msgs {
-		if signers := msg.GetSigners(); len(signers) != 1 || !signers[0].Equals(receiver) {
-			return errors.New(
-				"msg signer doesn't match the receiver, expected signer: " + receiver.String())
-		}
-	}
 
 	if msgLen == 1 {
 		// early return - we don't need to do more checks
diff --git a/x/uibc/uics20/memo_handler_test.go b/x/uibc/uics20/memo_handler_test.go
index 7226ede427..9e69cf2c3a 100644
--- a/x/uibc/uics20/memo_handler_test.go
+++ b/x/uibc/uics20/memo_handler_test.go
@@ -37,19 +37,17 @@ func TestValidateMemoMsg(t *testing.T) {
 	errManyMsgs := "memo with more than 1 message is not supported"
 	errNoSubCoins := errNoSubCoins.Error()
 	errMsg0type := errMsg0Type.Error()
-	// errWrongSigner := "signer doesn't match the receiver"
+	errWrongSigner := errWrongSigner.Error()
 
 	mh := MemoHandler{leverage: mocks.NewLvgNoopMsgSrv()}
 	tcs := []struct {
 		msgs   []sdk.Msg
 		errstr string
 	}{
-		/** we don't check signers in handlers v1
 		{[]sdk.Msg{ltypes.NewMsgSupply(accs.Bob, asset)}, errWrongSigner},
 		{[]sdk.Msg{ltypes.NewMsgSupplyCollateral(accs.Bob, asset)}, errWrongSigner},
-		{[]sdk.Msg{goodMsgSupplyColl,
-			ltypes.NewMsgBorrow(accs.Bob, asset)}, errWrongSigner},
-		*/
+		// {[]sdk.Msg{goodMsgSupplyColl,
+		// 	ltypes.NewMsgBorrow(accs.Bob, asset)}, errWrongSigner},
 
 		// good messages[0]
 		{[]sdk.Msg{goodMsgSupply}, ""},