From a0fc86f42c77b78e04299ab047008c45ea6789c4 Mon Sep 17 00:00:00 2001
From: Joe Bowman <joe@ingenuity.build>
Date: Fri, 10 Mar 2023 13:45:14 +0000
Subject: [PATCH] remove unused CoinFromRequestKey method; add tests for
 DenomFromRequestKey; handle case where key is invalid and contains no denom

---
 utils/coins.go                               | 13 ++--
 utils/coins_test.go                          | 74 ++++++++++++++++++++
 x/interchainstaking/keeper/callbacks_test.go | 12 +---
 3 files changed, 79 insertions(+), 20 deletions(-)
 create mode 100644 utils/coins_test.go

diff --git a/utils/coins.go b/utils/coins.go
index 131bb82c7..3e691309e 100644
--- a/utils/coins.go
+++ b/utils/coins.go
@@ -7,15 +7,6 @@ import (
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 )
 
-// coinFromRequestKey parses
-func CoinFromRequestKey(query []byte, accAddr sdk.AccAddress) (sdk.Coin, error) {
-	denom, err := DenomFromRequestKey(query, accAddr)
-	if err != nil {
-		return sdk.Coin{}, err
-	}
-	return sdk.NewCoin(denom, sdk.ZeroInt()), nil
-}
-
 func DenomFromRequestKey(query []byte, accAddr sdk.AccAddress) (string, error) {
 	balancesStore := query[1:]
 	gotAccAddress, denom, err := banktypes.AddressAndDenomFromBalancesStore(balancesStore)
@@ -23,6 +14,10 @@ func DenomFromRequestKey(query []byte, accAddr sdk.AccAddress) (string, error) {
 		return "", err
 	}
 
+	if len(denom) == 0 {
+		return "", fmt.Errorf("key contained no denom")
+	}
+
 	if !gotAccAddress.Equals(accAddr) {
 		return "", fmt.Errorf("account mismatch; expected %s, got %s", accAddr.String(), gotAccAddress.String())
 	}
diff --git a/utils/coins_test.go b/utils/coins_test.go
new file mode 100644
index 000000000..e77f1bb22
--- /dev/null
+++ b/utils/coins_test.go
@@ -0,0 +1,74 @@
+package utils_test
+
+import (
+	"testing"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	"github.com/stretchr/testify/require"
+
+	utils "github.com/ingenuity-build/quicksilver/utils"
+)
+
+func TestDenomFromRequestKey(t *testing.T) {
+	cases := []struct {
+		name string
+		fn   func() (sdk.AccAddress, string, []byte)
+		err  string
+	}{
+		{
+			"valid",
+			func() (sdk.AccAddress, string, []byte) {
+				accAddr := utils.GenerateAccAddressForTest()
+				prefix := banktypes.CreateAccountBalancesPrefix(accAddr.Bytes())
+				key := append(prefix, []byte("denom")...)
+				return accAddr, "denom", key
+			},
+			"",
+		},
+		{
+			"invalid - address mismatch",
+			func() (sdk.AccAddress, string, []byte) {
+				keyAddr, err := utils.AccAddressFromBech32("cosmos135rd8ft0dyq8fv3w3hhmaa55qu3pe668j99qh67mg747ew4ad03qsgq8vh", "cosmos")
+				require.NoError(t, err)
+				checkAddr, err := utils.AccAddressFromBech32("cosmos1ent5eg0xn3pskf3fhdw8mky88ry7t4kx628ru3pzp4nqjp6eufusphlldy", "cosmos")
+				require.NoError(t, err)
+				prefix := banktypes.CreateAccountBalancesPrefix(keyAddr.Bytes())
+				key := append(prefix, []byte("denom")...)
+				return checkAddr, "denom", key
+			},
+			"account mismatch; expected cosmos135rd8ft0dyq8fv3w3hhmaa55qu3pe668j99qh67mg747ew4ad03qsgq8vh, got cosmos1ent5eg0xn3pskf3fhdw8mky88ry7t4kx628ru3pzp4nqjp6eufusphlldy",
+		},
+		{
+			"invalid - empty address",
+			func() (sdk.AccAddress, string, []byte) {
+				accAddr := sdk.AccAddress{}
+				prefix := banktypes.CreateAccountBalancesPrefix(accAddr.Bytes())
+				key := append(prefix, []byte("denom")...)
+				return accAddr, "denom", key
+			},
+			"invalid key",
+		},
+		{
+			"invalid - empty denom",
+			func() (sdk.AccAddress, string, []byte) {
+				accAddr := utils.GenerateAccAddressForTest()
+				prefix := banktypes.CreateAccountBalancesPrefix(accAddr.Bytes())
+				key := append(prefix, []byte("")...)
+				return accAddr, "", key
+			},
+			"key contained no denom",
+		},
+	}
+
+	for _, c := range cases {
+		address, expectedDenom, key := c.fn()
+		actualDenom, error := utils.DenomFromRequestKey(key, address)
+		if len(c.err) == 0 {
+			require.NoError(t, error)
+			require.Equal(t, expectedDenom, actualDenom)
+		} else {
+			require.Errorf(t, error, c.err)
+		}
+	}
+}
diff --git a/x/interchainstaking/keeper/callbacks_test.go b/x/interchainstaking/keeper/callbacks_test.go
index 7560a6c1d..77f31f288 100644
--- a/x/interchainstaking/keeper/callbacks_test.go
+++ b/x/interchainstaking/keeper/callbacks_test.go
@@ -23,16 +23,6 @@ import (
 	icstypes "github.com/ingenuity-build/quicksilver/x/interchainstaking/types"
 )
 
-func TestCoinFromRequestKey(t *testing.T) {
-	accAddr := utils.GenerateAccAddressForTest()
-	prefix := banktypes.CreateAccountBalancesPrefix(accAddr.Bytes())
-	query := append(prefix, []byte("denom")...)
-
-	coin, err := utils.CoinFromRequestKey(query, accAddr)
-	require.NoError(t, err)
-	require.Equal(t, "denom", coin.Denom)
-}
-
 // ValSetCallback
 
 func (s *KeeperTestSuite) TestHandleValsetCallback() {
@@ -681,7 +671,7 @@ func (s *KeeperTestSuite) TestHandleValidatorCallbackJailedWithSlashing() {
 // 			s.Require().NoError(err)
 
 // 			expected := test.expected(zone.Validators)
-// 			fmt.Println(app.InterchainstakingKeeper.GetAllDelegations(ctx, &zone))
+// 			9(app.InterchainstakingKeeper.GetAllDelegations(ctx, &zone))
 // 			_, found = app.InterchainstakingKeeper.GetDelegation(ctx, &zone, expected.DelegationAddress, expected.ValidatorAddress)
 // 			s.Require().True(found)
 // 		})